Summary: | Items in transit should not show up in the holds queue | ||
---|---|---|---|
Product: | Koha | Reporter: | Laura Escamilla <Laura.escamilla> |
Component: | Circulation | Assignee: | 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
*** This bug has been marked as a duplicate of bug 32572 *** 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 Any update on this one? We really need this as a busy consortium. 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 This should have a unit test, but needs testing and confirmation of approach from QA before these will be attempted 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> 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 Fancy having a go that those unit tests Nick? (In reply to Martin Renvoize (ashimema) from comment #9) > Fancy having a go that those unit tests Nick? I haven't written the tests because I wonder if we should go about this in a different way. The current patches have AddReturn check the context of the call essentially and ignore the specific problematic case. I think it might be better to remove the queue rebuild from AddReturn and call it explicitly where needed from the scripts that call AddReturn. It will be more code, but should be more obvious. Thoughts? |