Description
Nick Clemens (kidclamp)
2024-02-05 18:58:14 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! 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! 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> 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! 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 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> 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> Meanwhile, here is a 2nd signoff :) And commits were split to allow testing the tests alone to confirm they fail without implementation. 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> 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> Created attachment 163177 [details] [review] Bug 35997: (QA follow-up) Number of tests Pushed for 24.05! Well done everyone, thank you! Pushed to 23.11.x for 23.11.04 |