The "maxreserves" syspref is only checked in opac/opac-reserve.pl and reserve/request.pl
(In reply to Jonathan Druart from comment #0) > The "maxreserves" syspref is only checked in opac/opac-reserve.pl and > reserve/request.pl Why isn't maxreserves checked in CanItemBeReserved?
It must indeed be moved to CanItemBeReserved (and I think stay in the controller as well, for multi holds).
Created attachment 106204 [details] [review] Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106205 [details] [review] Bug 25662: Make the route for holds restpect maxreserves This patch fixes the behaviour for the POST /holds route. It assumed maxreserves was checked in CanItemBeReserved which is not the case. Tests are added to check for this behaviour. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106206 [details] [review] Bug 25662: (follow-up) Add tests for the wrong patron_id added behaviour This patch adds trivial tests for the new error (400) raised when the passed patron_id doesn't exist. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106212 [details] [review] [19.11.x] Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106213 [details] [review] [19.11.x] Bug 25662: Make the route for holds restpect maxreserves This patch fixes the behaviour for the POST /holds route. It assumed maxreserves was checked in CanItemBeReserved which is not the case. Tests are added to check for this behaviour. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106214 [details] [review] [19.11.x] Bug 25662: (follow-up) Add tests for the wrong patron_id added behaviour This patch adds trivial tests for the new error (400) raised when the passed patron_id doesn't exist. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 106297 [details] [review] Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 106298 [details] [review] Bug 25662: Make the route for holds restpect maxreserves This patch fixes the behaviour for the POST /holds route. It assumed maxreserves was checked in CanItemBeReserved which is not the case. Tests are added to check for this behaviour. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Created attachment 106299 [details] [review] Bug 25662: (follow-up) Add tests for the wrong patron_id added behaviour This patch adds trivial tests for the new error (400) raised when the passed patron_id doesn't exist. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Let's pretend we switched - I will sign off and Kyle will count for QA ;)
Created attachment 108940 [details] [review] Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 108941 [details] [review] Bug 25662: Make the route for holds restpect maxreserves This patch fixes the behaviour for the POST /holds route. It assumed maxreserves was checked in CanItemBeReserved which is not the case. Tests are added to check for this behaviour. To test: 1. Apply the regression tests 2. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail! 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass! 5. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Created attachment 108942 [details] [review] Bug 25662: (follow-up) Add tests for the wrong patron_id added behaviour This patch adds trivial tests for the new error (400) raised when the passed patron_id doesn't exist. To test: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => SUCCESS: Tests pass! 2. Sign off :-D Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Pushed to master for 20.11, thanks to everybody involved!
backported to 20.05.x for 20.05.04
Tests fail on 19.11.x. Please fix and let me know when ready to re-backport. $ prove t/db_dependent/api/v1/holds.t t/db_dependent/api/v1/holds.t .. # Failed test '401 Unauthorized' # at t/db_dependent/api/v1/holds.t line 148. # got: '503' # expected: '401' # Failed test '401 Unauthorized' # at t/db_dependent/api/v1/holds.t line 150. # got: '503' # expected: '401' # Failed test '401 Unauthorized' # at t/db_dependent/api/v1/holds.t line 152. # got: '503' # expected: '401' # Failed test '401 Unauthorized' # at t/db_dependent/api/v1/holds.t line 154. # got: '503' # expected: '401' # Looks like you failed 4 tests of 8. t/db_dependent/api/v1/holds.t .. 1/9 # Failed test 'Test endpoints without authentication' # at t/db_dependent/api/v1/holds.t line 156. t/db_dependent/api/v1/holds.t .. 8/9 # Failed test '201 Created' # at t/db_dependent/api/v1/holds.t line 585. # got: '403' # expected: '201' # Failed test '201 Created' # at t/db_dependent/api/v1/holds.t line 595. # got: '403' # expected: '201' # Looks like you failed 2 tests of 7. # Failed test 'add() tests (maxreserves behaviour)' # at t/db_dependent/api/v1/holds.t line 610. # Looks like you failed 2 tests of 9. t/db_dependent/api/v1/holds.t .. Dubious, test returned 2 (wstat 512, 0x200) Failed 2/9 subtests Test Summary Report ------------------- t/db_dependent/api/v1/holds.t (Wstat: 512 Tests: 9 Failed: 2) Failed tests: 1, 9 Non-zero exit status: 2 Files=1, Tests=9, 5 wallclock secs ( 0.03 usr 0.01 sys + 4.50 cusr 0.87 csys = 5.41 CPU) Result: FAIL