| Summary: | Remove items from Holds Queue when checked in | ||
|---|---|---|---|
| Product: | Koha | Reporter: | Joy Nelson <1joynelson> |
| Component: | Hold requests | Assignee: | Agustín Moyano <agustinmoyano> |
| Status: | CLOSED FIXED | QA Contact: | Kyle M Hall (khall) <kyle> |
| Severity: | enhancement | ||
| Priority: | P5 - low | CC: | andrew, aspencatteam, fridolin.somers, gmcharlt, kelly, kyle, martin.renvoize, sally.healey, tomascohen |
| Version: | Main | ||
| Hardware: | All | ||
| OS: | All | ||
| See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15516 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25891 |
||
| Change sponsored?: | --- | Patch complexity: | Trivial patch |
| Documentation contact: | Documentation submission: | ||
| Text to go in the release notes: |
This development makes Koha 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.
|
Version(s) released in: |
21.05.00,20.11.04
|
| Circulation function: | |||
| Bug Depends on: | 27068 | ||
| Bug Blocks: | |||
| Attachments: |
Bug 24359: Add test
Bug 24359: Remove hold from holds queue when captured Bug 24359: Add test Bug 24359: Remove hold from holds queue when captured Bug 24359: Add test Bug 24359: Remove hold from holds queue when captured Bug 24359: (QA follow-up) Fix tests Bug 24359: Add test Bug 24359: Remove hold from holds queue when captured Bug 24359: (QA follow-up) Fix tests |
||
|
Description
Joy Nelson
2020-01-06 22:34:20 UTC
This would be very helpful to AspenCat. This would be a fantastic addition to Koha. Created attachment 112641 [details] [review] Bug 24359: Add test Created attachment 112642 [details] [review] 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 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. Created attachment 114405 [details] [review] Bug 24359: Add test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Created attachment 114406 [details] [review] 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> I like this, all works as expected, signing off. 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 Created attachment 117462 [details] [review] Bug 24359: Add test Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 117463 [details] [review] 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> Created attachment 117464 [details] [review] 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> I fixed a rebase issue. I also made the tests not reset the used tables. Created attachment 117465 [details] [review] 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> Created attachment 117466 [details] [review] 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> Created attachment 117467 [details] [review] 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> Pushed to master for 21.05, thanks to everybody involved! Small enhancement, I choose to backport. Pushed to 20.11.x for 20.11.04 While this is great, I wonder if it's not too much of a change in workflow for stable? Not backporting to 20.05 (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! |