| Summary: | Item-level rules not checked if both item_id and biblio_id are passed | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Nick Clemens (kidclamp) <nick> |
| Component: | REST API | Assignee: | Tomás Cohen Arazi (tcohen) <tomascohen> |
| Status: | CLOSED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P5 - low | CC: | emily.lamancusa, fridolin.somers, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=32702 | ||
| Change sponsored?: | --- | Patch complexity: | --- |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: | Version(s) released in: |
23.11.00,23.05.05,22.11.12
|
|
| Circulation function: | |||
| Attachments: |
Bug 35053: Regression tests
Bug 35053: Make sure request is checked as an item-level request if item_id passed Bug 35053: Regression tests Bug 35053: Make sure request is checked as an item-level request if item_id passed Bug 35053: Regression tests Bug 35053: Make sure request is checked as an item-level request if item_id passed |
||
Created attachment 157231 [details] [review] Bug 35053: Regression tests Created attachment 157232 [details] [review] Bug 35053: Make sure request is checked as an item-level request if item_id passed This patch makes the validation code have the `$item` variable defined when checking holdability, in the case both `item_id` and `biblio_id` params are passed. Otherwise, if the requested item is not holdable, but a biblio-level hold *could* be placed, the item-level hold is placed. This is highlighted by the regression tests. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests don't pass. A request that should be rejected is allowed because biblio-level hold is allowed. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Item-level rules are checked and thus the request rejected (code 403) 5. Sign off :-D Created attachment 157389 [details] [review] Bug 35053: Regression tests Signed-off-by: David Nind <david@davidnind.com> Created attachment 157390 [details] [review] Bug 35053: Make sure request is checked as an item-level request if item_id passed This patch makes the validation code have the `$item` variable defined when checking holdability, in the case both `item_id` and `biblio_id` params are passed. Otherwise, if the requested item is not holdable, but a biblio-level hold *could* be placed, the item-level hold is placed. This is highlighted by the regression tests. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests don't pass. A request that should be rejected is allowed because biblio-level hold is allowed. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Item-level rules are checked and thus the request rejected (code 403) 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Created attachment 157424 [details] [review] Bug 35053: Regression tests Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Created attachment 157425 [details] [review] Bug 35053: Make sure request is checked as an item-level request if item_id passed This patch makes the validation code have the `$item` variable defined when checking holdability, in the case both `item_id` and `biblio_id` params are passed. Otherwise, if the requested item is not holdable, but a biblio-level hold *could* be placed, the item-level hold is placed. This is highlighted by the regression tests. To test: 1. Apply the regression tests patch 2. Run: $ ktd --shell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests don't pass. A request that should be rejected is allowed because biblio-level hold is allowed. 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! Item-level rules are checked and thus the request rejected (code 403) 5. Sign off :-D Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Nick Clemens <nick@bywatersolutions.com> Pushed to master for 23.11. Nice work everyone, thanks! Pushed to 23.05.x for 23.05.05 Nice work everyone! Pushed to oldstable for 22.11.x *** Bug 33477 has been marked as a duplicate of this bug. *** |
In the checks we have: if ( $item_id and $biblio_id ) { .... $biblio = Koha::Biblios->find($biblio_id); ... } $item does not get assigned, then later: my $can_place_hold = $item ? C4::Reserves::CanItemBeReserved( $patron, $item ) : C4::Reserves::CanBookBeReserved( $patron_id, $biblio_id ); So while the item may not be holdable, we check if the bib is, then place the hold using the item_id