Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

QueryMapper parses query strings including nested params #337

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants

The QueryMapper in current release can not parse the query strings which includes nested and mixed hash and array, like {"first" => [{"two" => [{"three" => "four"}, "five"]}]}. The commit improves that.

Owner

bblimke commented Nov 23, 2013

@YanhaoYang thank you for adding support for that. I admire you managed to understand existing
code copied from addressable.

The code in query mapper which was copied from addressable was already complex,
and now it's even more complex so it's difficult to understand what it's doing :)
There is a spec so I do believe it works obviously.

It would be nice to refactor it and extract methods. With this level of nesting and 'if''s,
I hope it will just work and I won't have to maintain it. Do you think it's worth it? Makes sense?

Owner

bblimke commented Nov 23, 2013

I guess Addressable supports nested params nowadays?

@simonoff simonoff added a commit to natify/webmock that referenced this pull request Dec 12, 2013

@simonoff simonoff Fix for #337 with refactoring and more tests c0a72ad

@simonoff simonoff added a commit to natify/webmock that referenced this pull request Jun 22, 2014

@simonoff simonoff Fix for #337 with refactoring and more tests 963abd8

@bblimke bblimke closed this Sep 27, 2014

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