Created attachment 150281 [details] [review] Bug 33623: Encode URL params for getAll We must encode properly URL parameters when building the REST API routes Can be tested easily using const client = APIClient.erm client.licenses.getAll({ "me.name:%ff%" })
Please make this patch part of master ASAP!
Also contains some prettier changes: yarn run prettier --trailing-comma es5 --arrow-parens avoid --write koha-tmpl/intranet-tmpl/prog/js/vue/fetch/erm-api-client.js (yes, we are missing a QA test)
(In reply to Jonathan Druart from comment #3) > Also contains some prettier changes: > yarn run prettier --trailing-comma es5 --arrow-parens avoid --write > koha-tmpl/intranet-tmpl/prog/js/vue/fetch/erm-api-client.js > > (yes, we are missing a QA test) Opened bug 33625.
Created attachment 150362 [details] [review] Bug 33623: Move ULSearchParams into http-client.js Remove unused getAll methods in erm-api-client.js Make use of new getAll query in AgreementRelationships
Created attachment 150498 [details] [review] Bug 33623: Encode URL params for getAll We must encode properly URL parameters when building the REST API routes Can be tested easily using const client = APIClient.erm client.licenses.getAll({ "me.name:%ff%" }) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 150499 [details] [review] Bug 33623: Move ULSearchParams into http-client.js Remove unused getAll methods in erm-api-client.js Make use of new getAll query in AgreementRelationships Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Great follow-up, Pedro, thanks!
Created attachment 150506 [details] [review] Bug 33623: Encode URL params for getAll We must encode properly URL parameters when building the REST API routes Can be tested easily using const client = APIClient.erm client.licenses.getAll({ "me.name:%ff%" }) Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 150507 [details] [review] Bug 33623: Move ULSearchParams into http-client.js Remove unused getAll methods in erm-api-client.js Make use of new getAll query in AgreementRelationships Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
All working as expected in testing... Signing off.
The code looks like it introduces a behaviour change to me.. but maybe I'm just not entirely close enough to it. endpoint: "agreements?" + (query || "_per_page=-1"), vs endpoint: "agreements?" + new URLSearchParams({ _per_page: -1, ...(query && { q: JSON.stringify(query) }), }), To me, that looks like before now we let the query govern the paging whereas after we're always setting _per_page=-1.
Yes you are correct, but it was not used. We should actually allow the callers to pass more options. Like embed, for a follow-up bug I need to retrieve the items with the biblio info. I wont add "embed: biblio" to items.getAll(), but allow the caller to ask for it. It should to be done when we will need it (unless I missed something).
OK, that makes sense to me.. upgrading my signoff to a PQA stamp. Thanks for the clarifications Jonathan.
Pushed to master for 23.05. Nice work everyone, thanks!
Nice work everyone! Pushed to stable for 22.11.x
Missing dependencies for 22.05.x, no backport