[Bug 29562] New: CanItemBeReserved should take patron and item objects as parameters
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Bug ID: 29562 Summary: CanItemBeReserved should take patron and item objects as parameters Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: nick@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CanItemBeReserved takes a borrowernumber and an itemnumber and then fetches the objects to do calculations. Most calls to the routine already have the object, so we should be able to pass them CanBookBeReserved loops on every item, fetching the patron anew each time. Changing the parameters will increase performance. -- 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=29562 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- 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=29562 --- Comment #1 from Nick Clemens <nick@bywatersolutions.com> --- Created attachment 127966 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=127966&action=edit Bug 29562: Adjust CanItemBeReserved to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit tests -- 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=29562 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |nick@bywatersolutions.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=29562 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart+koha@gmail. | |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Severity|normal |enhancement -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |29660 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29660 [Bug 29660] reserve/request.pl should not deal with biblioitem -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com --- Comment #2 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I love this, but breaks the API and I couldn't easily find why. To reproduce the failure: 1. Run: $ kshell k$ prove t/db_dependent/api/v1/holds.t => FAIL: Tests fail with a 500. What I found using Postman: - $biblio_id is not always defined in the call to AddReserve, should be using $biblio->id instead - This were the first tests we wrote for the API, and are super weird, including the DELETE FROM reserves. Maybe is a problem with the tests themselves. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #127966|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #3 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129245 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129245&action=edit Bug 29562: Adjust CanItemBeReserved to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit 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=29562 --- Comment #4 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129246 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129246&action=edit Bug 29562: (follow-up) Fix API controller This patch fixes tests failures due to bad checks in the controller. The tests deserve to be rewritten. 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=29562 --- Comment #5 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129252 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129252&action=edit Bug 29562: Fix DecreaseLoanHighHolds.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #6 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- DecreaseLoanHighHolds.t was failing, with this patch it's now passing but I am not sure the changes are correct. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (In reply to Jonathan Druart from comment #6)
DecreaseLoanHighHolds.t was failing, with this patch it's now passing but I am not sure the changes are correct.
Ha! I missed that tests file! The patch looks correct. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129252|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129245|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129246|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #8 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129253 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129253&action=edit Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit 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=29562 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129254 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129254&action=edit Bug 29562: (follow-up) Fix API controller This patch fixes tests failures due to bad checks in the controller. The tests deserve to be rewritten. 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=29562 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 129255 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129255&action=edit Bug 29562: Fix DecreaseLoanHighHolds.t 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=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|CanItemBeReserved should |Pass objects to |take patron and item |CanItemBeReserved and |objects as parameters |checkHighHolds -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29840 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #11 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129257 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129257&action=edit Bug 29562: Test $item instead of $itemNum -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #12 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- I focused on: 1. Making sure the $item or $patron object was not modified in DB between the fetch and the CanItemBeReserved call (which could lead to wrong calculation). And haven't found any. 2. Call to CanItemBeReserved with undefined $item or $patron, that could explode. But I am not sure it would be a regression anyway. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #13 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- To be consistent we should do the same change for CanBookBeReserved IMO. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=29841 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> 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=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129253|0 |1 is obsolete| | Attachment #129254|0 |1 is obsolete| | Attachment #129255|0 |1 is obsolete| | Attachment #129257|0 |1 is obsolete| | --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129261 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129261&action=edit Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #15 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129262 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129262&action=edit Bug 29562: (follow-up) Fix API controller This patch fixes tests failures due to bad checks in the controller. The tests deserve to be rewritten. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #16 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129263 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129263&action=edit Bug 29562: Fix DecreaseLoanHighHolds.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #17 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129264 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129264&action=edit Bug 29562: Test $item instead of $itemNum Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |29058 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29058 [Bug 29058] Add option to not load existing holds table automatically -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #129261|0 |1 is obsolete| | Attachment #129262|0 |1 is obsolete| | Attachment #129263|0 |1 is obsolete| | Attachment #129264|0 |1 is obsolete| | --- Comment #18 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129926 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129926&action=edit Bug 29562: Adjust CanItemBeReserved and checkHighHolds to take objects Most of the changes here are simple, this can be read to view the changes Testing that holds can be placed via staff client, and opac, and are disallowed when expected is the best test plan, beyond running the unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #19 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129927 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129927&action=edit Bug 29562: (follow-up) Fix API controller This patch fixes tests failures due to bad checks in the controller. The tests deserve to be rewritten. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #20 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129928 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129928&action=edit Bug 29562: Fix DecreaseLoanHighHolds.t Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #21 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 129929 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=129929&action=edit Bug 29562: Test $item instead of $itemNum Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |jonathan.druart+koha@gmail. |y.org |com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m --- Comment #22 from Fridolin Somers <fridolin.somers@biblibre.com> --- Awesome work ! I add a minor typo fix $tem => $item -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #23 from Fridolin Somers <fridolin.somers@biblibre.com> --- Created attachment 130033 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=130033&action=edit Bug 29562: (follow-up) typo $tem in opac-reserve.pl -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |22.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 --- Comment #24 from Fridolin Somers <fridolin.somers@biblibre.com> --- Pushed to master for 22.05, 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=29562 Bug 29562 depends on bug 29058, which changed state. Bug 29058 Summary: Add option to not load existing holds table automatically https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29058 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29562 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41475 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41475 [Bug 41475] 500 error when placing a hold on records with multiple 773 entries -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org