Bug 34656

Summary: CartToShelf should not trigger RealTimeHoldsQueue
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Architecture, internals, and plumbingAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Emily Lamancusa (emlam) <emily.lamancusa>
Severity: normal    
Priority: P5 - low CC: emily.lamancusa, fridolin.somers, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34678
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.11
Circulation function:
Attachments: Bug 34656: Unit test
Bug 34656: Do not update real Time Holds Queue when moving from cart to shelf
Bug 34656: Unit test
Bug 34656: Do not update real Time Holds Queue when moving from cart to shelf
Bug 34656: (QA follow-up) perltidy
Bug 34656: Unit test
Bug 34656: Do not update real Time Holds Queue when moving from cart to shelf
Bug 34656: (QA follow-up) perltidy

Description Nick Clemens (kidclamp) 2023-08-30 13:26:52 UTC
The routine only changes the location of the item, this does not affect holdability and does not need to update the queue.

This is causing issues when the cron is adding background jobs and is followed by the holds queue builder and the two processes can end up running the queue for a bib at the same time and causing clashes in the tables
Comment 1 Nick Clemens (kidclamp) 2023-08-30 13:52:30 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2023-08-30 13:52:32 UTC Comment hidden (obsolete)
Comment 3 Emily Lamancusa (emlam) 2023-08-31 14:46:29 UTC
Created attachment 155011 [details] [review]
Bug 34656: Unit test

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 4 Emily Lamancusa (emlam) 2023-08-31 14:46:31 UTC
Created attachment 155012 [details] [review]
Bug 34656: Do not update real Time Holds Queue when moving from cart to shelf

To test:
1 - Enable RealTimeHoldsQueue system preference
2 - Set UpdateItemLocationOnCheckin to _ALL_: CART
3 - Check in an item
4 - Check the background jobs - the RTHQ is updated
5 - perl -e 'use C4::Items; C4::Items::CartToShelf(##);' -- substitute the itemnumber from above
6 - Check the background jobs - no new update to RTHQ
7 - Confirm item was set back to correct permanent_location

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Comment 5 Emily Lamancusa (emlam) 2023-08-31 14:53:30 UTC
Created attachment 155013 [details] [review]
Bug 34656: (QA follow-up) perltidy
Comment 6 Emily Lamancusa (emlam) 2023-08-31 15:13:14 UTC
Testing notes:
For step 3, you need to check out and then check in the item in order to generate RTHQ updates - an update won't trigger on check-in if the item wasn't checked out in the first place.
Comment 7 Emily Lamancusa (emlam) 2023-08-31 16:26:33 UTC
Also - if the holds queue background jobs generated by CartToShelf can cause issues by clashing with the holds queue builder, wouldn't *anything* that causes a real-time holds queue update potentially cause the same issue? I agree with eliminating unnecessary background jobs for its own sake, but there seems to be a bigger bug lurking underneath. (creating a new bug report for it)
Comment 8 Victor Grousset/tuxayo 2023-09-05 01:30:53 UTC
Created attachment 155207 [details] [review]
Bug 34656: Unit test

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 9 Victor Grousset/tuxayo 2023-09-05 01:30:56 UTC
Created attachment 155208 [details] [review]
Bug 34656: Do not update real Time Holds Queue when moving from cart to shelf

To test:
1 - Enable RealTimeHoldsQueue system preference
2 - Set UpdateItemLocationOnCheckin to _ALL_: CART
3 - Check in an item
4 - Check the background jobs - the RTHQ is updated
5 - perl -e 'use C4::Items; C4::Items::CartToShelf(##);' -- substitute the itemnumber from above
6 - Check the background jobs - no new update to RTHQ
7 - Confirm item was set back to correct permanent_location

Signed-off-by: Emily Lamancusa <emily.lamancusa@montgomerycountymd.gov>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 10 Victor Grousset/tuxayo 2023-09-05 01:30:59 UTC
Created attachment 155209 [details] [review]
Bug 34656: (QA follow-up) perltidy

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 11 Victor Grousset/tuxayo 2023-09-05 01:54:36 UTC
I don't get enough the functional side to fully validate this, at least I can tell that the test code looks good and add a 2nd signoff so maybe Emily can pass QA.
Comment 12 Emily Lamancusa (emlam) 2023-09-13 20:42:46 UTC
Thanks, Victor! Passing QA
Comment 13 Tomás Cohen Arazi (tcohen) 2023-09-15 14:58:53 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 14 Fridolin Somers 2023-09-18 08:07:01 UTC
Pushed to 23.05.x for 23.05.04
Comment 15 Matt Blenkinsop 2023-10-12 15:32:20 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x