[Bug 40424] New: Handling query in body relies on wrong data structure
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Bug ID: 40424 Summary: Handling query in body relies on wrong data structure 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: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org CC: tomascohen@gmail.com Similar to `x-koha-request-id` handling, it relies on `extract_reserved_params` and the use of `$c->validation->output`, which is incorrect. We need tests for the feature. -- 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=40424 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |40417 Depends on| |40423 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40417 [Bug 40417] Search_rs is re-instating deleted query parameters 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 the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | Status|NEW |ASSIGNED -- 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=40424 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com, | |martin.renvoize@openfifth.c | |o.uk, | |matt.blenkinsop@openfifth.c | |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Medium patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 --- Comment #1 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184208 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184208&action=edit Bug 40424: Add missing tests for search in body -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 --- Comment #2 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184209 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184209&action=edit Bug 40424: Simplify existing code - preparation I wanted to isolate the changes for the query-in-body handling and it made sense to refactor this dupicated code at the same time. So I add this patch for tidying the area in preparation for the other part. Duplicated code gets simplified. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/ => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 --- Comment #3 from Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> --- Created attachment 184210 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184210&action=edit Bug 40424: Make query-in-body rely on `$c->req->json` This patch removes the reliance on `$c->validation->output` for the query-in-body feature. This will allow bug 40417 to move forward. It also fixes some wrong design decisions we made 5-6 years ago. This was discussed with the Mojolicious::Plugin::OpenAPI maintainer and the agreement was `$c->validation->output` was not stable nor intended to be accessed like we do. To test: 1. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/* => SUCCESS: Tests pass! 2. Apply this patchset 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Tomás Cohen Arazi (tcohen) <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Handling query in body |Handling query-in-body |relies on wrong data |relies on wrong data |structure |structure -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184208|0 |1 is obsolete| | --- Comment #4 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184218 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184218&action=edit Bug 40424: Add missing tests for search in body Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184209|0 |1 is obsolete| | --- Comment #5 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184219 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184219&action=edit Bug 40424: Simplify existing code - preparation I wanted to isolate the changes for the query-in-body handling and it made sense to refactor this dupicated code at the same time. So I add this patch for tidying the area in preparation for the other part. Duplicated code gets simplified. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/ => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184210|0 |1 is obsolete| | --- Comment #6 from Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> --- Created attachment 184220 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184220&action=edit Bug 40424: Make query-in-body rely on `$c->req->json` This patch removes the reliance on `$c->validation->output` for the query-in-body feature. This will allow bug 40417 to move forward. It also fixes some wrong design decisions we made 5-6 years ago. This was discussed with the Mojolicious::Plugin::OpenAPI maintainer and the agreement was `$c->validation->output` was not stable nor intended to be accessed like we do. To test: 1. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/* => SUCCESS: Tests pass! 2. Apply this patchset 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> 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=40424 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184218|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 184554 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184554&action=edit Bug 40424: Add missing tests for search in body Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184219|0 |1 is obsolete| | --- Comment #8 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 184555 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184555&action=edit Bug 40424: Simplify existing code - preparation I wanted to isolate the changes for the query-in-body handling and it made sense to refactor this dupicated code at the same time. So I add this patch for tidying the area in preparation for the other part. Duplicated code gets simplified. To test: 1. Apply the unit tests patch 2. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/ => SUCCESS: Tests pass! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #184220|0 |1 is obsolete| | --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 184556 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=184556&action=edit Bug 40424: Make query-in-body rely on `$c->req->json` This patch removes the reliance on `$c->validation->output` for the query-in-body feature. This will allow bug 40417 to move forward. It also fixes some wrong design decisions we made 5-6 years ago. This was discussed with the Mojolicious::Plugin::OpenAPI maintainer and the agreement was `$c->validation->output` was not stable nor intended to be accessed like we do. To test: 1. Run: $ ktd --shell k$ prove t/Koha/REST/Plugin/Query.t \ t/db_dependent/api/v1/*.t \ t/db_dependent/Koha/REST/Plugin/* => SUCCESS: Tests pass! 2. Apply this patchset 3. Repeat 1 => SUCCESS: Tests pass! 4. Sign off :-D Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Nice consistent approach here. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40424 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=40424 --- Comment #11 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=40424 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=40424 --- Comment #12 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=40424 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Needs documenting CC| |fridolin.somers@biblibre.co | |m --- Comment #13 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=40424 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Needs documenting |RESOLVED CC| |david@davidnind.com --- Comment #14 from David Nind <david@davidnind.com> --- 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=40424 Bug 40424 depends on bug 40423, which changed state. Bug 40423 Summary: Handling x-koha-request-id relies on wrong data structure https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=40423 What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org