[Bug 40417] New: Search_rs is re-instating deleted query parameters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Bug ID: 40417 Summary: Search_rs is re-instating deleted query parameters Change sponsored?: --- Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: matt.blenkinsop@openfifth.co.uk QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com Any endpoint that is calling $c->objects->search after calling $c->req->params->remove('param_name') is having that parameter reinstated to the search object -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |matt.blenkinsop@openfifth.c |ity.org |o.uk -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #1 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184155 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184155&action=edit Bug 40417: Regression test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #2 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- To reproduce, apply the regression test patch and run the test - it will fail -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184155|0 |1 is obsolete| | --- Comment #3 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184160 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184160&action=edit Bug 40417: Regression test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #4 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184161 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184161&action=edit Bug 40417: Remove 'validation' method -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |40423 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40423 [Bug 40423] Handling x-koha-request-id relies on wrong data structure -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |40424 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 [Bug 40424] Handling query in body relies on wrong data structure -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on|40423 | Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40423 [Bug 40423] Handling x-koha-request-id relies on wrong data structure -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #5 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This looks correct, but we need to deal with other things for this not to break existing features. Look at the dependencies I added. BTW. I added ``` use DDP; p($c->validation->output); p($c->req->params->to_hash); ``` at the beginning of `objects.search_rs` and run: ``` prove t/db_dependent/api/v1/query.t ``` and verified one thing I mentioned yesterday: when a query parameter is passed multiple times, the calculated structure is the same (i.e. `$args`). When it is only one, they differ. This might not be a real issue, but it is worth making sure we have tests and it doesn't break anything. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |martin.renvoize@openfifth.c | |o.uk Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- URL| |https://gitlab.com/koha-com | |munity/qa-test-tools/-/merg | |e_requests/81 --- Comment #6 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- This looks good. I added a PR on the QA tools repo so the QA script doesn't look for `$c->validation` in the test file, as we really want it there for the regression test. Please enhance the commit message. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184160|0 |1 is obsolete| | --- Comment #7 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184212&action=edit Bug 40417: Regression test prove t/db_dependent/Koha/REST/Plugin/Objects.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184161|0 |1 is obsolete| | --- Comment #8 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184213&action=edit Bug 40417: Remove 'validation' method We shouldn't use the ->validation method anymore as it isn't stable. We need to pass the params into the method accounting for any param changes made Test plan: 1) Apply the regression test patch and run the test 2) Test should fail 3) Apply this patch 4) Run test again 5) Test should pass -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 David Nind <david@davidnind.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=40417 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184212|0 |1 is obsolete| | --- Comment #9 from David Nind <david@davidnind.com> --- Created attachment 184317 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184317&action=edit Bug 40417: Regression test prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184213|0 |1 is obsolete| | --- Comment #10 from David Nind <david@davidnind.com> --- Created attachment 184318 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184318&action=edit Bug 40417: Remove 'validation' method We shouldn't use the ->validation method anymore as it isn't stable. We need to pass the params into the method accounting for any param changes made Test plan: 1) Apply the regression test patch and run the test 2) Test should fail 3) Apply this patch 4) Run test again 5) Test should pass Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Trivial patch Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184317|0 |1 is obsolete| | Attachment #184318|0 |1 is obsolete| | --- Comment #11 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184406 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184406&action=edit Bug 40417: Regression test prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #12 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184407 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184407&action=edit Bug 40417: Remove 'validation' method We shouldn't use the ->validation method anymore as it isn't stable. We need to pass the params into the method accounting for any param changes made Test plan: 1) Apply the regression test patch and run the test 2) Test should fail 3) Apply this patch 4) Run test again 5) Test should pass Signed-off-by: David Nind <david@davidnind.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184406|0 |1 is obsolete| | Attachment #184407|0 |1 is obsolete| | --- Comment #13 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184409 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184409&action=edit Bug 40417: Regression test prove t/db_dependent/Koha/REST/Plugin/Objects.t Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #14 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184410 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184410&action=edit Bug 40417: Remove 'validation' method We shouldn't use the ->validation method anymore as it isn't stable. We need to pass the params into the method accounting for any param changes made Test plan: 1) Apply the regression test patch and run the test 2) Test should fail 3) Apply this patch 4) Run test again 5) Test should pass Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #15 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- I PQA this. * Code is correct * Side effects from using ->validation covered by tests * QA script happy (with the linked PR applied locally for the QA test tools) Good catch Matthew and thanks for the patches! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.00 released in| | Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #16 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Nice work everyone! Pushed to main for 25.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Paul Derscheid <paul.derscheid@lmscloud.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|25.11.00 |25.11.00,25.05.03 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #17 from Paul Derscheid <paul.derscheid@lmscloud.de> --- Nice work everyone! Pushed to 25.05.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andreas.jonsson@kreablo.se --- Comment #18 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- *** Bug 30526 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #19 from Fridolin Somers <fridolin.somers@biblibre.com> --- Depends on Bug 40423 not in 24.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |david@davidnind.com --- Comment #20 from David Nind <david@davidnind.com> --- REST API related, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Bug 40417 depends on bug 40424, which changed state. Bug 40424 Summary: Handling query-in-body relies on wrong data structure https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Is this a bug fix or an enhancement? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|enhancement |normal --- Comment #22 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- (In reply to Katrin Fischer from comment #21)
Is this a bug fix or an enhancement?
Bugfix -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Search_rs is re-instating |search_rs is re-instating |deleted query parameters |deleted query parameters -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|search_rs is re-instating |`objects.search_rs` is |deleted query parameters |re-instating deleted query | |parameters -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org