woohoolabs / yin

The efficient and elegant JSON:API 1.1 server library for PHP
MIT License
239 stars 38 forks source link

Links for paginations should include filter and sorting parameters #70

Closed gfemorris closed 5 years ago

gfemorris commented 6 years ago

Hi, this is basically an improvement. I think it would be really cool if the sorting and filter params would automatically be included in the links of the pagination. Basically a pagination is useless if those parameters are not included. Because those values are already available inside the lib, it should be possible. For sure one can solve this in the environment, but it would be a nice improvement of your lib.

kocsismate commented 6 years ago

Hi,

This is a cool idea! I'll try to implement it as soon as I will be free from work overload... :) But I welcome any PR if you have the possibility to do it.

gfemorris commented 6 years ago

Hi,

yes i will try to do a PR if i have the time to do.. (we all have the same problems with work overload) Thanks for your reply anyway..

waghanza commented 5 years ago

@gfemorris @kocsismate I do not know it this is to handle at yin level.

As filtering is a way to extract data (not to display them) from any source (a database for example), it could be at a higher level (a bundle for symfony for example)

What do you think ?

gfemorris commented 5 years ago

@waghanza i don't think your topic is related to this one. This issue is just about the pagination links not including the filtering and sorting parameters.

For sure filtering itself should be ideally done on database level for performance reasons.

waghanza commented 5 years ago

@waghanza You talk about filter params, no ?

I think it would be really cool if the sorting and filter params would automatically be included in the links of the pagination.

gfemorris commented 5 years ago

@waghanza You can create pagination links through yin. For example in your yin document you want to have the pagination links, so you call: Links::createWithoutBaseUri()->setPagination(... Those links actually do not include filtering and sorting params automatically, so you have to add them yourself. This is what the topic is about.

waghanza commented 5 years ago

ah, ok clearer now :stuck_out_tongue:

for me it was not to be done automatically, but having a feature that can enable / disable it

what I can contribute to

waghanza commented 5 years ago

@gfemorris just to be sure, you mean using something like getting books where author's name is glen in my example https://github.com/waghanza/book-manager like with /books?filter[author.name]=glen

gfemorris commented 5 years ago

yes and if you use pagination you want to have the filters in the links: "next": "books/?filter[author.name]=glen&page[limit]=10&page[offset]=10", and not "next": "books/?page[limit]=10&page[offset]=10", if you call: Links::createWithoutBaseUri()->setPagination("books", $domainObject); it will include the pagination links but not the filter links. so you have to add them yourself like Links::createWithoutBaseUri()->setPagination("books/?filter[author.name]=glen", $domainObject); (for sure you need to add them in another way, it is just for demonstration) And because the filter and sorting params are known by yin anyway it could add them automaticaly like it does with the pagination params.

kocsismate commented 5 years ago

@gfemorris Ah, I tried to fix this issue months before, but I couldn't come up with a good solution which is easy enough... :/ Hopefully, I'll be able to revisit this in the near future... But now, I'm working on introducing support for JSON:API v1.1. :)

gfemorris commented 5 years ago

@kocsismate Anyway it is not that important just an improvement. I will probably check this again in the near future, when i start working on jsonapi projects again and then maybe provide a pull request. And great to hear that you are working on v1.1 integration.

kocsismate commented 5 years ago

Hey @gfemorris,

Finally I had the opportunity to think and work on this ticket as part of my efforts to support JSON:API 1.1 (in Yin 4.0). You can see my implementation with an example in my commit: https://github.com/woohoolabs/yin/commit/0fc64d41533f2ca9f95ac341a22f5076e23b91ff

Unline in Yin 3, the request will be directly available from the Documents and ResourceTransformers In Yin 4.0 (along with some other things), so adding this feature to the new version is more natural. Also, the proper implementation required a breaking change, so I don't plan to port it to Yin 3. :S

I hope that you'll like the feature and its implementation.