Bug 36329 - Transfer limits should respect `BranchTransferLimitsType`
Summary: Transfer limits should respect `BranchTransferLimitsType`
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: REST API (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Tomás Cohen Arazi
QA Contact:
URL:
Keywords:
Depends on: 36277
Blocks:
  Show dependency treegraph
 
Reported: 2024-03-15 11:15 UTC by Tomás Cohen Arazi
Modified: 2024-05-13 14:41 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.04,23.05.12


Attachments
Bug 36329: Add missing return codes (1.75 KB, patch)
2024-03-15 13:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36329: Miscelaneous spec fixes (3.89 KB, patch)
2024-03-15 13:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType (9.52 KB, patch)
2024-03-15 13:01 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 36329: Add missing return codes (1.79 KB, patch)
2024-03-15 21:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 36329: Miscelaneous spec fixes (3.93 KB, patch)
2024-03-15 21:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType (9.57 KB, patch)
2024-03-15 21:19 UTC, David Nind
Details | Diff | Splinter Review
Bug 36329: Add missing return codes (1.85 KB, patch)
2024-03-18 17:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36329: Miscelaneous spec fixes (3.99 KB, patch)
2024-03-18 17:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType (9.63 KB, patch)
2024-03-18 17:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 36329: (follow-up) Fix error message comparisson in testes (1.17 KB, patch)
2024-03-19 15:12 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2024-03-15 11:15:26 UTC
The endpoint allows passing both a `collection code` and an `item type` on a batch request, but the UI would never allow such combination.

I wonder if this is a design decision (i.e. supporting both in the future) or just a missing check. My intuition says it is the latter.

I'm submitting a patch just in case.

Kyle?
Comment 1 Tomás Cohen Arazi 2024-03-15 13:01:16 UTC
Created attachment 163232 [details] [review]
Bug 36329: Add missing return codes
Comment 2 Tomás Cohen Arazi 2024-03-15 13:01:19 UTC
Created attachment 163233 [details] [review]
Bug 36329: Miscelaneous spec fixes
Comment 3 Tomás Cohen Arazi 2024-03-15 13:01:21 UTC
Created attachment 163234 [details] [review]
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType

This patch adds tests for the different cases of `BranchTransferLimitsType`.
It also adds tests for the situation of the consumer sending both limit
criterias on the request.

The controller gets adjusted for this new behavior and the spec gets
documentation added about this.

Bonus: tests are added the right guidelines code, and
BranchTransferLimitsType gets mocked to avoid failures due to existing
data.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ qa
=> SUCCESS: All green, and tests pass!
3. Sign off :-D
Comment 4 David Nind 2024-03-15 21:19:51 UTC
Created attachment 163280 [details] [review]
Bug 36329: Add missing return codes

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2024-03-15 21:19:53 UTC
Created attachment 163281 [details] [review]
Bug 36329: Miscelaneous spec fixes

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2024-03-15 21:19:56 UTC
Created attachment 163282 [details] [review]
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType

This patch adds tests for the different cases of `BranchTransferLimitsType`.
It also adds tests for the situation of the consumer sending both limit
criterias on the request.

The controller gets adjusted for this new behavior and the spec gets
documentation added about this.

Bonus: tests are added the right guidelines code, and
BranchTransferLimitsType gets mocked to avoid failures due to existing
data.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ qa
=> SUCCESS: All green, and tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2024-03-15 21:21:43 UTC
Testing notes (using KTD):

1. Tests pass before and after the patches are applied: prove t/db_dependent/api/v1/transfer_limits.t
Comment 8 Kyle M Hall 2024-03-18 17:48:22 UTC
Created attachment 163365 [details] [review]
Bug 36329: Add missing return codes

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 9 Kyle M Hall 2024-03-18 17:48:29 UTC
Created attachment 163366 [details] [review]
Bug 36329: Miscelaneous spec fixes

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 10 Kyle M Hall 2024-03-18 17:48:31 UTC
Created attachment 163367 [details] [review]
Bug 36329: Make POST /transfer_limits/batch honor BranchTransferLimitsType

This patch adds tests for the different cases of `BranchTransferLimitsType`.
It also adds tests for the situation of the consumer sending both limit
criterias on the request.

The controller gets adjusted for this new behavior and the spec gets
documentation added about this.

Bonus: tests are added the right guidelines code, and
BranchTransferLimitsType gets mocked to avoid failures due to existing
data.

To test:
1. Apply this patches
2. Run:
   $ ktd --shell
  k$ qa
=> SUCCESS: All green, and tests pass!
3. Sign off :-D

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 11 Katrin Fischer 2024-03-19 09:27:55 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 12 Tomás Cohen Arazi 2024-03-19 15:12:28 UTC
Created attachment 163453 [details] [review]
Bug 36329: (follow-up) Fix error message comparisson in testes

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 13 Katrin Fischer 2024-03-21 06:27:43 UTC
Pushed the follow-up.
Comment 14 Fridolin Somers 2024-03-21 10:19:59 UTC
Pushed to 23.11.x for 23.11.04
Comment 15 Lucas Gass 2024-05-13 14:41:48 UTC
Backported to 23.05.x for upcoming 23.05.12