[Bug 24359] New: Remove items from Holds Queue when checked in
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Bug ID: 24359 Summary: Remove items from Holds Queue when checked in Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Hold requests Assignee: koha-bugs@lists.koha-community.org Reporter: joy@bywatersolutions.com QA Contact: testopia@bugs.koha-community.org CC: gmcharlt@gmail.com Currently, if an item is checked in and the hold is triggered/captured, the item will not come off the holds queue until the the cronjob runs again. This causes some confusion for multibranch systems when other branches attempt to fill holds that have already been filled. Proposal: Remove bib/items from the Holds Queue after checkin and before the next holds queue cron runs to update the holds queue. -- 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=24359 Sally <sally.healey@cheshiresharedservices.gov.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |sally.healey@cheshireshared | |services.gov.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 AspenCat Team <aspencatteam@clicweb.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |aspencatteam@clicweb.org --- Comment #1 from AspenCat Team <aspencatteam@clicweb.org> --- This would be very helpful to AspenCat. -- 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=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Assignee|koha-bugs@lists.koha-commun |agustinmoyano@theke.io |ity.org | -- 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=24359 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=24359 --- Comment #2 from Sally <sally.healey@cheshiresharedservices.gov.uk> --- This would be a fantastic addition to Koha. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Agustín Moyano <agustinmoyano@theke.io> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff Patch complexity|--- |Trivial patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 --- Comment #3 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 112641 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112641&action=edit Bug 24359: Add test -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 --- Comment #4 from Agustín Moyano <agustinmoyano@theke.io> --- Created attachment 112642 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=112642&action=edit Bug 24359: Remove hold from holds queue when captured This patch removes a hold from holds queue when captured by check in. To test: 1. check out an item from a book with multiple items for patron_1 2. place a biblio level hold for patron_2 3. perl misc/cronjobs/holds/build_holds_queue.pl CHECK => holds queue shows the placed hold 4. check in the item from step 1 and confirm hold for patron_2 SUCCESS => hold for patron_2 is no longer in holds queue 5. prove t/db_dependent/HoldsQueue.t -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Kelly McElligott <kelly@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kelly@bywatersolutions.com --- Comment #5 from Kelly McElligott <kelly@bywatersolutions.com> --- Hi, I have gone through your test plan and at step 4- after checking the item in and confirming the hold, the item does not get removed from the Holds Queue, instead, the hold stays on the Holds queue, but the item also shows waiting for the patron. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112641|0 |1 is obsolete| | --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 114405 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114405&action=edit Bug 24359: Add test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #112642|0 |1 is obsolete| | --- Comment #7 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Created attachment 114406 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=114406&action=edit Bug 24359: Remove hold from holds queue when captured This patch removes a hold from holds queue when captured by check in. To test: 1. check out an item from a book with multiple items for patron_1 2. place a biblio level hold for patron_2 3. perl misc/cronjobs/holds/build_holds_queue.pl CHECK => holds queue shows the placed hold 4. check in the item from step 1 and confirm hold for patron_2 SUCCESS => hold for patron_2 is no longer in holds queue 5. prove t/db_dependent/HoldsQueue.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com Status|Needs Signoff |Signed Off --- Comment #8 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I like this, all works as expected, signing off. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |kyle@bywatersolutions.com QA Contact|testopia@bugs.koha-communit |kyle@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Unit tests are failing: root@kohadevbox:koha(bug24359-qa)$ prove t/db_dependent/HoldsQueue.t t/db_dependent/HoldsQueue.t .. 56/57 # Failed test 'Holds queue has one element' # at t/db_dependent/HoldsQueue.t line 1712. # got: '2' # expected: '1' # Failed test 'Holds queue has no elements, even when queue was not rebuilt' # at t/db_dependent/HoldsQueue.t line 1722. # got: '1' # expected: '0' t/db_dependent/HoldsQueue.t .. 57/57 # Looks like you failed 2 tests of 2. # Failed test 'Remove holds on check-in match' # at t/db_dependent/HoldsQueue.t line 1726. # Looks like you failed 1 test of 57. t/db_dependent/HoldsQueue.t .. Dubious, test returned 1 (wstat 256, 0x100) Failed 1/57 subtests Test Summary Report ------------------- t/db_dependent/HoldsQueue.t (Wstat: 256 Tests: 57 Failed: 1) Failed test: 57 Non-zero exit status: 1 Files=1, Tests=57, 10 wallclock secs ( 0.03 usr 0.00 sys + 5.82 cusr 0.97 csys = 6.82 CPU) Result: FAIL -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=15516 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |ASSIGNED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- 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=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114405|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=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #114406|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=24359 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=24359 --- Comment #10 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117462 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117462&action=edit Bug 24359: Add test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=24359 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117463 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117463&action=edit Bug 24359: Remove hold from holds queue when captured This patch removes a hold from holds queue when captured by check in. To test: 1. check out an item from a book with multiple items for patron_1 2. place a biblio level hold for patron_2 3. perl misc/cronjobs/holds/build_holds_queue.pl CHECK => holds queue shows the placed hold 4. check in the item from step 1 and confirm hold for patron_2 SUCCESS => hold for patron_2 is no longer in holds queue 5. prove t/db_dependent/HoldsQueue.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=24359 --- Comment #12 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 117464 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117464&action=edit Bug 24359: (QA follow-up) Fix tests This patch fixes a rebase issue, and also makes sure the tests do not rely on any prior data. The query on tmp_holdsqueue is done against the actual reserve_id so no need to delete all the data from the tmp_holdsqueue and hold_fill_targets table. I also noticed the previous subtest wasn't run inside a transaction even if the code was designed to not depend on prior data as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/HoldsQueue.t => SUCCESS: Tests pass! 3. 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=24359 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- I fixed a rebase issue. I also made the tests not reset the used tables. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |27068 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=27068 [Bug 27068] HoldsQueue doesn't know how to use holds groups -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |andrew@bywatersolutions.com -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Kyle M Hall <kyle@bywatersolutions.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=24359 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #117462|0 |1 is obsolete| | Attachment #117463|0 |1 is obsolete| | Attachment #117464|0 |1 is obsolete| | --- Comment #14 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 117465 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117465&action=edit Bug 24359: Add test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=24359 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 117466 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117466&action=edit Bug 24359: Remove hold from holds queue when captured This patch removes a hold from holds queue when captured by check in. To test: 1. check out an item from a book with multiple items for patron_1 2. place a biblio level hold for patron_2 3. perl misc/cronjobs/holds/build_holds_queue.pl CHECK => holds queue shows the placed hold 4. check in the item from step 1 and confirm hold for patron_2 SUCCESS => hold for patron_2 is no longer in holds queue 5. prove t/db_dependent/HoldsQueue.t Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> 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=24359 --- Comment #16 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 117467 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=117467&action=edit Bug 24359: (QA follow-up) Fix tests This patch fixes a rebase issue, and also makes sure the tests do not rely on any prior data. The query on tmp_holdsqueue is done against the actual reserve_id so no need to delete all the data from the tmp_holdsqueue and hold_fill_targets table. I also noticed the previous subtest wasn't run inside a transaction even if the code was designed to not depend on prior data as well. To test: 1. Apply this patch 2. Run: $ kshell k$ prove t/db_dependent/HoldsQueue.t => SUCCESS: Tests pass! 3. 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=24359 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |21.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=24359 --- Comment #17 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Pushed to master for 21.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=24359 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|21.05.00 |21.05.00,20.11.04 released in| | CC| |fridolin.somers@biblibre.co | |m Status|Pushed to master |Pushed to stable --- Comment #18 from Fridolin Somers <fridolin.somers@biblibre.com> --- Small enhancement, I choose to backport. Pushed to 20.11.x for 20.11.04 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 --- Comment #19 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- While this is great, I wonder if it's not too much of a change in workflow for stable? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 --- Comment #20 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Not backporting to 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 --- Comment #21 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- (In reply to Katrin Fischer from comment #19)
While this is great, I wonder if it's not too much of a change in workflow for stable?
Discussed on IRC and deemed a good change, but release notes should be added! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |release-notes-needed -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the| |This development makes Koha release notes| |trigger an update on the | |holds queue when items are | |checked in. This way, the | |holds queue will be updated | |faster than the default 1 | |hour frequency (cronjob). | | | |Note: this doesn't trigger | |the more expensive task of | |recalculating the whole | |queue, which remains a | |cronjob-based task. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|release-notes-needed | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Fridolin Somers <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Status|Pushed to stable |RESOLVED -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24359 Nick Clemens <nick@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25891 -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org