New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ngResource): Add suport for nested params #1640

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
6 participants
@cakesifu
Copy link
Contributor

cakesifu commented Dec 3, 2012

Allow ngResource to properly construct a query string for params which
have objects or arrays as values.

fix(ngResource): Add suport for nested params
Allow ngResource to properly construct a query string for params which
have objects or arrays as values.
@IgorMinar

This comment has been minimized.

Copy link

IgorMinar commented on test/ngResource/resourceSpec.js in 67b860e Dec 13, 2012

this should be a separate spec since it's not just simple url encoding but a rather complex serialization

should serialize object and array arguments into url

This comment has been minimized.

Copy link
Owner

cakesifu replied Dec 13, 2012

I agree. This should be a separate spec and it should also include some edge cases.

@IgorMinar

This comment has been minimized.

Copy link

IgorMinar commented on test/ngResource/resourceSpec.js in 67b860e Dec 13, 2012

the raw url is /Path/doh?bar[0]=e&bar[1][baz]=z&foo[c]=d

why did you pick this serialization scheme? were you inspired by rails?

in angular 1.1.1 we made it possible to similarly serialize arrays in $http calls simply as bar=value1&bar=value2 we should make both the $http and $resource apis behave the same way.

This comment has been minimized.

Copy link

IgorMinar replied Dec 13, 2012

also I'm not sure if recursive serialization is really needed (bar.baz in your case). what backend are you targeting with this change?

This comment has been minimized.

Copy link
Owner

cakesifu replied Dec 13, 2012

Yes, this is the serialization scheme used by Rails and PHP. And also probably a lot of other other backends.

bar=value1&bar=value2 does not work as expected in PHP however, the slighly longer version: bar[]=value1&bar[]=value2 works just fine.

Recursive serialization is the reason I opened this pull request. I found myself needing to send a deeply nested hash to the backend via a GET request. This is very common in Rails for GET /resource_name/index actions where you would need to send a complex and nested hash of filtering parameters to a gem like ransack.

Idealy this should work like jQuery.param method.

@marknadig

This comment has been minimized.

Copy link
Contributor

marknadig commented Dec 26, 2012

Just ran into this today and agree it would be great if it worked like jQuery.param method. I am hitting a Rails service and just trying to pass a simple array as a parameter. Note: This PR currently will encode arrays w/ index; e.g.
foo=['bar', 'baz']
is encoded as
foo[0]=bar&foo[1]=baz

parity with jQuery would be
foo[]=bar&foo[]=baz

(by supplying index - Rails parses the parameter into a hash using the index as a key rather than an array). Parity w/ jQuery would exclude the index for array.

@marknadig

This comment has been minimized.

Copy link

marknadig commented on src/ngResource/resource.js in 67b860e Dec 26, 2012

If parity w/ jQuery is desired (and I think that would be beneficial), no index is supplied for an array. ; e.g.
x = ['one', 'two']
would be encoded
x[]=one&x[]=two

@marknadig

This comment has been minimized.

Copy link
Contributor

marknadig commented Jan 28, 2013

following up - here's the issue related to $http where added ability to encode arrays that Igor referenced:
#1363

I agree keeping it consistent would be best. Actually, has me wondering - why even have this encoding logic in angular-resource's Route.url() ... perhaps a better approach would be Route.url only deals with urlParams and pass along any remaining params on to $http to keep it DRY.

@marknadig

This comment has been minimized.

Copy link
Contributor

marknadig commented Jan 28, 2013

@IgorMinar I was working on a PR to have resource delegate encoding of params to $http. The code is pretty straight forward and I think is a good change to DRY things up. However, I have one failing test related to this spec: 'should not encode @ in url params' with this comment:
//encodeURIComponent is too agressive and doesn't follow http://www.ietf.org/rfc/rfc3986.txt
//with regards to the character set (pchar) allowed in path segments
//so we need this test to make sure that we don't over-encode the params and break stuff like
//buzz api which uses @self

With the encoding of the params handled by http, this test fails which has me wondering why the fix for over aggressive encoding isn't in $http to begin with.

@Phantoms

This comment has been minimized.

Copy link

Phantoms commented Feb 11, 2013

It is a much needed (critical, i would say) feature, array serialization to x[]=one&x[]=two.

Also, not sure if related, it is now impossible to set your path to something like "?q=text&x[]=one&x[]=two" at all. It gets changed to "?q=text&x[]=two". Could not yet find the responsible piece of code, though. Happens if $location is injected.

@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

pkozlowski-opensource commented Feb 16, 2013

Since there are different back-ends and encoding schemas maybe the way to move forward would be to add a configurable strategy (on the $httpProvider-level?)

@Phantoms

This comment has been minimized.

Copy link

Phantoms commented Feb 21, 2013

Being brought up by php/rails/jquery i always assumed that "?x[]=one&x[]=two" was the only desired serialization approach. And now i'm very confused to find out that there are other approaches, entirely blocking me from using angularjs with rails backend :( I had tremendous fun learning the framework up until this point though.

I've looked into $httpProvider code briefly, but couldn't quite figure how to approach the 'configurable strategy' solution. May i ask for some tips on where to look?

Also, how does $location link into this behavior? I guess i could work around this problem if i could keep $location from mangling the incoming query string.

Would a separate issue for this be more helpful, since i'm not committing any code yet?

@marknadig

This comment has been minimized.

Copy link
Contributor

marknadig commented Feb 21, 2013

@Phantoms this PR was landed 2a21234 and should provide what you are expecting w/ array serlialization. The fix was to have $resource delegate the encoding to $http which had a separate fix for array serialization. Does that help?

@Phantoms

This comment has been minimized.

Copy link

Phantoms commented Feb 21, 2013

I'll give it a try, thanks!
Doesn't it serialize into ?x=one&x=two, though? It is not quite the same as ?x[]=one&x[]=two.

@marknadig

This comment has been minimized.

Copy link
Contributor

marknadig commented Feb 21, 2013

Andrei, it should serialize as you expect. (I'm also using rails).

@Phantoms

This comment has been minimized.

Copy link

Phantoms commented Feb 25, 2013

Thank you so much for your fix, Mark. It does indeed work as expected.
There is one more issue though, probably not directly related.
If you inject $location, and try to enter a path like ?x[]=one&x[]=two&x[]=three, it'll be changed into ?x[]=three. No clue how it works.

@ghost ghost assigned IgorMinar Apr 11, 2013

@petebacondarwin

This comment has been minimized.

Copy link
Member

petebacondarwin commented May 7, 2013

It appears that this functionality has been merged now. Can we close this PR then?

@Phantoms - you mention another issue? If this is still the case, can you create a new issue/PR for it?

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