[Bug 25662] New: Create hold route does not check maxreserves syspref
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Bug ID: 25662 Summary: Create hold route does not check maxreserves syspref Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: REST API Assignee: koha-bugs@lists.koha-community.org Reporter: jonathan.druart@bugs.koha-community.org Depends on: 13903 The "maxreserves" syspref is only checked in opac/opac-reserve.pl and reserve/request.pl Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=13903 [Bug 13903] Add API routes to list, create, update, delete holds -- 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=25662 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25643 -- 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=25662 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #1 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (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? -- 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=25662 --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- It must indeed be moved to CanItemBeReserved (and I think stay in the controller as well, for multi holds). -- 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=25662 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version|unspecified |master Assignee|koha-bugs@lists.koha-commun |tomascohen@gmail.com |ity.org | -- 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=25662 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 AspenCat Team <aspencatteam@clicweb.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aspencatteam@clicweb.org -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106204 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106204&action=edit Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106205 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106205&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106206 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106206&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Patch complexity|--- |Small patch Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #6 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106212 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106212&action=edit [19.11.x] Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106213 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106213&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 106214 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106214&action=edit [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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=24466 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106204|0 |1 is obsolete| | Attachment #106205|0 |1 is obsolete| | Attachment #106206|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 106297 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106297&action=edit Bug 25662: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #10 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 106298 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106298&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #11 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 106299 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=106299&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Tomás Cohen Arazi <tomascohen@gmail.com> 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=25662 --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Let's pretend we switched - I will sign off and Kyle will count for QA ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106297|0 |1 is obsolete| | --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108940 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108940&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106298|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108941 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108941&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #106299|0 |1 is obsolete| | --- Comment #15 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 108942 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=108942&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to master Version(s)| |20.11.00 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #16 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 20.11, thanks to everybody involved! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 Aleisha Amohia <aleisha@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aleisha@catalyst.net.nz Status|Pushed to master |Pushed to stable Version(s)|20.11.00 |20.11.00, 20.05.04 released in| | --- Comment #17 from Aleisha Amohia <aleisha@catalyst.net.nz> --- backported to 20.05.x for 20.05.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25662 --- Comment #18 from Aleisha Amohia <aleisha@catalyst.net.nz> --- 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 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org