[Bug 33623] New: getAll not encoding URL params
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Bug ID: 33623 Summary: getAll not encoding URL params Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: major Priority: P5 - low Component: ERM Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com CC: jonathan.druart+koha@gmail.com, jonathan.field@ptfs-europe.com, martin.renvoize@ptfs-europe.com, matt.blenkinsop@ptfs-europe.com, pedro.amorim@ptfs-europe.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150281 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150281&action=edit 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%" }) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #2 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Please make this patch part of master ASAP! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |32474 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32474 [Bug 32474] Implement infinite scroll in vue-select -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #3 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 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) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 150362 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150362&action=edit 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 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150281|0 |1 is obsolete| | Attachment #150362|0 |1 is obsolete| | --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150498 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150498&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 150499 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150499&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #8 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Great follow-up, Pedro, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150498|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150506 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150506&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #150499|0 |1 is obsolete| | --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 150507 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=150507&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- All working as expected in testing... Signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #12 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- 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). -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #14 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- OK, that makes sense to me.. upgrading my signoff to a PQA stamp. Thanks for the clarifications Jonathan. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |23.05.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #15 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.05. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.05.00 |23.05.00,22.11.06 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 --- Comment #16 from Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> --- Nice work everyone! Pushed to stable for 22.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33623 Lucas Gass <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Resolution|--- |FIXED Status|Pushed to stable |RESOLVED --- Comment #17 from Lucas Gass <lucas@bywatersolutions.com> --- Missing dependencies for 22.05.x, no backport -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org