Bug 34596

Summary: Items in transit should not show up in the holds queue
Product: Koha Reporter: Laura Escamilla <Laura.escamilla>
Component: CirculationAssignee: Nick Clemens (kidclamp) <nick>
Status: Failed QA --- QA Contact: Martin Renvoize (ashimema) <martin.renvoize>
Severity: normal    
Priority: P5 - low CC: blawlor, bowens, carthur, chanel.wheeler, gmcharlt, karie.straube, kyle.m.hall, madamyk, martin.renvoize, mspinney, nick, rcoert, sally.healey
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=30581
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on:    
Bug Blocks: 35092    
Attachments: Bug 34596: Do not build holds queue when checkin reserve found but do if hold ignored
Bug 34596: Do not build holds queue when checkin reserve found but do if hold ignored

Description Laura Escamilla 2023-08-23 11:59:38 UTC
Libraries have reported that items are still showing up in their holds queue hours after they have been marked as "in transit". Only holds that are ready to be found should be showing up in the holds queue.
Comment 1 Laura Escamilla 2023-09-06 16:43:48 UTC

*** This bug has been marked as a duplicate of bug 32572 ***
Comment 2 Nick Clemens (kidclamp) 2023-09-13 17:07:17 UTC
This seems to be a consequence of two real time holds queue updates being triggered - once during the initial check in, before the transit is confirmed, and once after the transit/hold has been confirmed.

Currently if the two jobs attempt to update at the same time there is a collision, see bug 34596

We should try to reduce the build here to a single call, after the transfer is confirmed or rejected if possible
Comment 3 Katrin Fischer 2023-10-29 22:51:51 UTC
Any update on this one?
Comment 4 Chanel Wheeler 2024-10-09 21:09:12 UTC
We really need this as a busy consortium.
Comment 5 Nick Clemens (kidclamp) 2024-11-19 20:07:10 UTC
Created attachment 174811 [details] [review]
Bug 34596: Do not build holds queue when checkin reserve found but do if hold ignored

This patch does two things:
1:
It prevents rebulding the real time holds queue during a checkin if a reserve was found.
The reserve will either:
a - be set to waiting if at correct location and no confirmation - this is fine because we will rebuild the queue when
    filling the hold
b - trigger a popup whihc will rebuild the queue when confirming the transfer, or be ignored

2:
It adds a form to ignoring a hold so that we can rebuild the holds queue if we decided not to use the current item
to fill the found hold

To test:
0 - Enable RealTimeHoldsQueue
1 - Place several holds for different libraries on a title
2 - Checkout a copy to a patron at current library (ignore current holds)
3 - Clean up DB so we can see outcomes from next steps:
   delete from branchtransfers; delete from background_jobs; update reserves SET found=NULL where found='T';
4 - Check in item, confirm hold
5 - Check background jobs, note two rebuilds of the holds queue
6 - Check item out to previous patron, repeat 3
7 - Check in item, ignore the hold
8 - Check background jobs, note only one rebuild
9 - Apply patch, restart all
10 - Checkout item, repeat 3
11 - Checkin item, confirm hold
12 - Check background jobs, onlt one rebuild of holds queue
13 - Checkout item, repeat 3
14 - Checkin item, ignore hold
15 - Check background jobs, only one rebuild of holds queue
Comment 6 Nick Clemens (kidclamp) 2024-11-19 20:08:29 UTC
This should have a unit test, but needs testing and confirmation of approach from QA before these will be attempted
Comment 7 Brendan Lawlor 2024-12-19 19:41:46 UTC
Created attachment 175804 [details] [review]
Bug 34596: Do not build holds queue when checkin reserve found but do if hold ignored

This patch does two things:
1:
It prevents rebulding the real time holds queue during a checkin if a reserve was found.
The reserve will either:
a - be set to waiting if at correct location and no confirmation - this is fine because we will rebuild the queue when
    filling the hold
b - trigger a popup whihc will rebuild the queue when confirming the transfer, or be ignored

2:
It adds a form to ignoring a hold so that we can rebuild the holds queue if we decided not to use the current item
to fill the found hold

To test:
0 - Enable RealTimeHoldsQueue
1 - Place several holds for different libraries on a title
2 - Checkout a copy to a patron at current library (ignore current holds)
3 - Clean up DB so we can see outcomes from next steps:
   delete from branchtransfers; delete from background_jobs; update reserves SET found=NULL where found='T';
4 - Check in item, confirm hold
5 - Check background jobs, note two rebuilds of the holds queue
6 - Check item out to previous patron, repeat 3
7 - Check in item, ignore the hold
8 - Check background jobs, note only one rebuild
9 - Apply patch, restart all
10 - Checkout item, repeat 3
11 - Checkin item, confirm hold
12 - Check background jobs, onlt one rebuild of holds queue
13 - Checkout item, repeat 3
14 - Checkin item, ignore hold
15 - Check background jobs, only one rebuild of holds queue

Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Comment 8 Brendan Lawlor 2024-12-19 19:49:03 UTC
Test notes:
For step 3 I used ktd --dbshell to clean up the database
From the > prompt run:
  delete from branchtransfers; delete from background_jobs; update reserves SET found=NULL where found='T';

Then for steps 5, 8, 12 and 15 to check background jobs run:
select count(*) from background_jobs;

and I verified the count of background_jobs is 1 after the patch
Comment 9 Martin Renvoize (ashimema) 2025-01-07 15:58:05 UTC
Fancy having a go that those unit tests Nick?