farmOS / farmOS.js

A JavaScript library for working with farmOS data structures and interacting with farmOS servers.
MIT License
15 stars 13 forks source link

Add sort options to fetch requests #54

Closed jgaehring closed 1 year ago

jgaehring commented 2 years ago

Similar to #42.

This is not a requirement for Field Kit's current feature set, so I don't plan to implement this until after the 2.0.0 general releases, but seems like the next logical step and should be pretty easy to tack on at the end of parseFetchParams:

https://github.com/farmOS/farmOS.js/blob/d14283ee9cd34a0340215e9d10f7d468168028da/src/client/request.js#L5-L8

jgaehring commented 2 years ago

One possible reason to implement this sooner rather than later is that, according to @mstenta and @symbioquine, the sorting of paginated requests is not deterministic. To quote from our chat:

paginated API results are not ordered in a deterministic way, so you should include a sort parameter to ensure consistency [...] page 2 might be sorted differently from page 1, so you could miss/dupe results might want to just add a sort by ID always or something

This could impact how I'm chaining paginated requests currently:

https://github.com/farmOS/farmOS.js/blob/09899cb67ddbca52de68895fbdafb95d2993db7b/src/client/adapter/index.js#L90-L103

Hopefully this doesn't become an issue in Field Kit any time soon, but it will be good to look out for.

mstenta commented 2 years ago

Hopefully this doesn't become an issue in Field Kit any time soon, but it will be good to look out for.

If the results from one page to the next are not consistent, then this would be an issue for any request that is pulling in > 50 records (the number of results included in each page). Any reasonably sized operation will have > 50 assets of a given type, for example (eg: a flock of sheep). So this may be an issue right from the start.

jgaehring commented 1 year ago

Closing this, since it was resolved by #68, specicically 6429cabbdf3a4d93a13d253bea36919784e40eed.

Any reasonably sized operation will have > 50 assets of a given type, for example (eg: a flock of sheep). So this may be an issue right from the start.

I've opened farmOS/field-kit#512 to monitor this as it relates to Field Kit, but shouldn't be an issue for farmOS.js itself.