Skip to content
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

Generate query string correctly #87

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

yamaha252
Copy link
Contributor

Support generate request query strings like this: ?filter%5Bpair%5D%5B0%5D=BTCEUR&filter%5Bpair%5D%5B1%5D=BTCUSD&page=1&pageSize=10 if I put an Object to a param.

this.ordersResource.query({
      filter: {
        pair: ['BTCEUR', 'BTCUSD'],
      },
      page: page,
      pageSize: pageSize
    });
filter[pair][0]: BTCEUR
filter[pair][1]: BTCUSD
page: 1
pageSize: 10

@troyanskiy
Copy link
Owner

Thank you for the PR.

Unfortunately there is no strict standard how to sent arrays with get request and different backends expects to receive it differently.
Some expects ?param=val1&param=val2&param=val3
Some ?param=val1,val2,val3
Some ?param[]=val1&param[]=val2&param[]=val3

That PR bings breaking changes to users, how already uses current method to send arrays/objects with get. Do not close the PR, I will think around how to make it more generic.

@troyanskiy troyanskiy merged commit 40b0f33 into troyanskiy:master Mar 8, 2017
@troyanskiy
Copy link
Owner

I have published v 1.12.1
Please check change log.
To enable your way of mapping get params please set somewhere at app initialisation ResourceGlobalConfig.getParamsMappingType = TGetParamsMappingType.Bracket

@jeremypele
Copy link
Contributor

IMO, the good option would have been to implement the jquery.params way of doing

In that case for the example

filter[pair][]: BTCEUR
filter[pair][]: BTCUSD
filter: {
   nested: [{ a: 1, b: [2, 3] }]
}


filter[nested][0][a]: 1
filter[nested][0][b][]: 2
filter[nested][0][b][]: 3

I believe that it is the 'most standard' way of doing.

@troyanskiy could we imagine having a new TGetParamsMappingType that extends that? I have no problem writing a PR for that, just want to check with you if it worth it now that the TGetParamsMappingType.Bracket is defined?

@troyanskiy
Copy link
Owner

Hello @jeremypele
Personally, I never used that TGetParamsMappingType.Bracket. People needed that (how many I have no idea), they ask or write's PRs.

And PR is welcome.

Thank you!

@jeremypele
Copy link
Contributor

Ok, I'll do that then :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants