Bug 40424

Summary: Handling query-in-body relies on wrong data structure
Product: Koha Reporter: Tomás Cohen Arazi (tcohen) <tomascohen>
Component: REST APIAssignee: Tomás Cohen Arazi (tcohen) <tomascohen>
Status: Pushed to main --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: enhancement    
Priority: P5 - low CC: jonathan.druart, martin.renvoize, matt.blenkinsop, tomascohen
Version: Main   
Hardware: All   
OS: All   
GIT URL: Change sponsored?: ---
Patch complexity: Medium patch Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
25.11.00
Circulation function:
Bug Depends on: 40423    
Bug Blocks: 40417    
Attachments: Bug 40424: Add missing tests for search in body
Bug 40424: Simplify existing code - preparation
Bug 40424: Make query-in-body rely on `$c->req->json`
Bug 40424: Add missing tests for search in body
Bug 40424: Simplify existing code - preparation
Bug 40424: Make query-in-body rely on `$c->req->json`
Bug 40424: Add missing tests for search in body
Bug 40424: Simplify existing code - preparation
Bug 40424: Make query-in-body rely on `$c->req->json`

Description Tomás Cohen Arazi (tcohen) 2025-07-17 03:00:39 UTC
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.
Comment 1 Tomás Cohen Arazi (tcohen) 2025-07-17 15:01:27 UTC
Created attachment 184208 [details] [review]
Bug 40424: Add missing tests for search in body
Comment 2 Tomás Cohen Arazi (tcohen) 2025-07-17 15:01:30 UTC
Created attachment 184209 [details] [review]
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
Comment 3 Tomás Cohen Arazi (tcohen) 2025-07-17 15:01:33 UTC
Created attachment 184210 [details] [review]
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
Comment 4 Matt Blenkinsop 2025-07-17 16:04:14 UTC
Created attachment 184218 [details] [review]
Bug 40424: Add missing tests for search in body

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk>
Comment 5 Matt Blenkinsop 2025-07-17 16:04:17 UTC
Created attachment 184219 [details] [review]
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>
Comment 6 Matt Blenkinsop 2025-07-17 16:04:19 UTC
Created attachment 184220 [details] [review]
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>
Comment 7 Martin Renvoize (ashimema) 2025-07-23 14:29:23 UTC
Created attachment 184554 [details] [review]
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>
Comment 8 Martin Renvoize (ashimema) 2025-07-23 14:29:25 UTC
Created attachment 184555 [details] [review]
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>
Comment 9 Martin Renvoize (ashimema) 2025-07-23 14:29:28 UTC
Created attachment 184556 [details] [review]
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>
Comment 10 Martin Renvoize (ashimema) 2025-07-23 14:30:03 UTC
Nice consistent approach here.
Comment 11 Lucas Gass (lukeg) 2025-07-23 22:43:27 UTC
Nice work everyone!

Pushed to main for 25.11