Skip to content

#800 correct handling parameter value empty array#801

Merged
iMacTia merged 1 commit intolostisland:masterfrom
senid231:800-correct-handling-parameter-value-empty-array
May 23, 2018
Merged

#800 correct handling parameter value empty array#801
iMacTia merged 1 commit intolostisland:masterfrom
senid231:800-correct-handling-parameter-value-empty-array

Conversation

@senid231
Copy link
Contributor

@senid231 senid231 commented May 23, 2018

Description

correct handling of parameters when value is an empty array
fix #800

Todos

List any remaining work that needs to be done, i.e:

  • Tests
  • Documentation (not sure where it should be documented)

Additional Notes

Optional section
will fix JsonApiClient/json_api_client#278

buffer << "#{to_query.call(new_parent, val)}&"
end
return buffer.chop
elsif value.is_a?(Array) && value.size == 0
Copy link
Member

@olleolleolle olleolleolle May 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps the Array#empty? method is even clearer to read? For both the !value.empty? and for the value.empty? case.

Copy link
Member

@iMacTia iMacTia May 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go a step further and merge this 2 branches:

...
elsif value.is_a?(Array)
  return "#{parent}%5B%5D" if value.empty? # This is the only new line
  buffer = ""
  value.each_with_index do |val, i|
    new_parent = "#{parent}%5B%5D"
     buffer << "#{to_query.call(new_parent, val)}&"
  end
  return buffer.chop
...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice catch
fixed

Copy link
Member

@olleolleolle olleolleolle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool change, thanks for the contextual link to the other issue.

@iMacTia iMacTia added the bug label May 23, 2018
buffer << "#{to_query.call(new_parent, val)}&"
end
return buffer.chop
elsif value.is_a?(Array) && value.size == 0
Copy link
Member

@iMacTia iMacTia May 23, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd go a step further and merge this 2 branches:

...
elsif value.is_a?(Array)
  return "#{parent}%5B%5D" if value.empty? # This is the only new line
  buffer = ""
  value.each_with_index do |val, i|
    new_parent = "#{parent}%5B%5D"
     buffer << "#{to_query.call(new_parent, val)}&"
  end
  return buffer.chop
...

@senid231 senid231 force-pushed the 800-correct-handling-parameter-value-empty-array branch from 8898a87 to 2602e3b Compare May 23, 2018 14:26
@iMacTia
Copy link
Member

iMacTia commented May 23, 2018

Nice optimisation, looks good now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

empty array parameter value encodes incorrectly Filter parameters set to [] are discarded

3 participants