Bug 35997 - Cancelling a hold should remove the hold from the queue
Summary: Cancelling a hold should remove the hold from the queue
Status: Pushed to stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-02-05 18:58 UTC by Nick Clemens (kidclamp)
Modified: 2024-03-18 09:16 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.05.00,23.11.04


Attachments
Bug 35997: Cancelling a hold should remove the hold from the queue (3.90 KB, patch)
2024-02-05 19:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35997: Cancelling a hold should remove the hold from the queue (3.94 KB, patch)
2024-02-05 20:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 35997: Cancelling a hold should remove the hold from the queue (3.99 KB, patch)
2024-02-06 18:53 UTC, Brendan Lawlor
Details | Diff | Splinter Review
Bug 35997: Add tests (2.65 KB, patch)
2024-02-19 04:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35997: Cancelling a hold should remove the hold from the queue (1.75 KB, patch)
2024-02-19 04:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 35997: Add tests (2.40 KB, patch)
2024-03-15 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35997: Cancelling a hold should remove the hold from the queue (1.79 KB, patch)
2024-03-15 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 35997: (QA follow-up) Number of tests (699 bytes, patch)
2024-03-15 09:00 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens (kidclamp) 2024-02-05 18:58:14 UTC
From bug 35357 - a cancelled hold remains in the hold queue until rebuilt
Comment 1 Kyle M Hall 2024-02-05 19:48:53 UTC
Created attachment 161766 [details] [review]
Bug 35997: Cancelling a hold should remove the hold from the queue

Test plan:
1) Place a hold for a record for a patron
2) Build the holds queue
3) Note the holds queue contains a match for that patron and record
4) Cancel the holds queue
5) Reload the holds queue page, note the match for  that patron and
   record is gone!
Comment 2 Kyle M Hall 2024-02-05 20:00:37 UTC
Created attachment 161767 [details] [review]
Bug 35997: Cancelling a hold should remove the hold from the queue

Test plan:
1) Place a hold for a record for a patron
2) Build the holds queue
3) Note the holds queue contains a match for that patron and record
4) Cancel the holds queue
5) Reload the holds queue page, note the match for  that patron and
   record is gone!
Comment 3 Brendan Lawlor 2024-02-06 18:53:37 UTC
Created attachment 161794 [details] [review]
Bug 35997: Cancelling a hold should remove the hold from the queue

Test plan:
1) Place a hold for a record for a patron
2) Build the holds queue
3) Note the holds queue contains a match for that patron and record
4) Cancel the holds queue
5) Reload the holds queue page, note the match for  that patron and
   record is gone!

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 4 Brendan Lawlor 2024-02-06 18:59:12 UTC
Testing notes

1. Placed holds

2. Find the script and run it:
git grep build_holds_queue
cd misc/cronjobs/holds/
perl build_holds_queue.pl

3. Check http://localhost:8081/cgi-bin/koha/circ/view_holdsqueue.pl
4. Cancel holds and refresh the page

The holds were removed from the queue!
Comment 5 Victor Grousset/tuxayo 2024-02-19 04:02:09 UTC
Is the join on borrowernumber, biblionumber[1], item_level_request, holdingbranch[1] needed?
`itemnumber` is not the primary key of tmp_holdsqueue so is it possible that twice the same itemnumber would be in the table?

Even with 5 record level holds for the same patron on a record that has 4 item there is no duplication of itemnumber tmp_holdsqueue.

Neither with multiple item level holds on the same item for different patrons.

[1] these are 100% sure to be redundant. itemnumber implies biblionumber and holdingbranch
Comment 6 Victor Grousset/tuxayo 2024-02-19 04:02:33 UTC
Created attachment 162257 [details] [review]
Bug 35997: Add tests

Test plan: see next patch

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Victor Grousset/tuxayo 2024-02-19 04:02:36 UTC
Created attachment 162258 [details] [review]
Bug 35997: Cancelling a hold should remove the hold from the queue

Test plan:
1) Place a hold for a record for a patron
2) Build the holds queue
3) Note the holds queue contains a match for that patron and record
4) Cancel the holds queue
5) Reload the holds queue page, note the match for  that patron and
   record is gone!

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 8 Victor Grousset/tuxayo 2024-02-19 04:15:20 UTC
Meanwhile, here is a 2nd signoff :)
And commits were split to allow testing the tests alone to confirm they fail without implementation.
Comment 9 Marcel de Rooy 2024-03-15 09:00:36 UTC
Created attachment 163175 [details] [review]
Bug 35997: Add tests

Test plan: see next patch

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 10 Marcel de Rooy 2024-03-15 09:00:38 UTC
Created attachment 163176 [details] [review]
Bug 35997: Cancelling a hold should remove the hold from the queue

Test plan:
1) Place a hold for a record for a patron
2) Build the holds queue
3) Note the holds queue contains a match for that patron and record
4) Cancel the holds queue
5) Reload the holds queue page, note the match for  that patron and
   record is gone!

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 11 Marcel de Rooy 2024-03-15 09:00:41 UTC
Created attachment 163177 [details] [review]
Bug 35997: (QA follow-up) Number of tests
Comment 12 Katrin Fischer 2024-03-15 14:40:20 UTC
Pushed for 24.05!

Well done everyone, thank you!
Comment 13 Fridolin Somers 2024-03-18 09:16:09 UTC
Pushed to 23.11.x for 23.11.04