Bug 33184

Summary: Warnings when running holds queue builder
Product: Koha Reporter: Kyle M Hall (khall) <kyle>
Component: Hold requestsAssignee: Kyle M Hall (khall) <kyle>
Status: Failed QA --- QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: david, dcook, gmcharlt, magnus, pedro.amorim
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=28966
GIT URL: Change sponsored?: ---
Patch complexity: --- Documentation contact:
Documentation submission: Text to go in the release notes:
Version(s) released in:
Circulation function:
Attachments: Bug 33184: Warnings when running holds queue builder

Description Kyle M Hall (khall) 2023-03-09 17:09:20 UTC
If a hold was fillable when the holds queue builder starts, but is no longer fillable when the builder gets to that hold's record ( for example, an item was trapped to fill that hold so the hold is now "found" ), it will generate warnings like:

Use of uninitialized value in addition (+) at /usr/share/koha/lib/C4/HoldsQueue.pm line 219.
Use of uninitialized value in addition (+) at /usr/share/koha/lib/C4/HoldsQueue.pm line 220.
Comment 1 Kyle M Hall (khall) 2023-03-09 17:14:03 UTC
Created attachment 148019 [details] [review]
Bug 33184: Warnings when running holds queue builder

If a hold was fillable when the holds queue builder starts, but is no longer fillable when the builder gets to that hold's record ( for example, an item was trapped to fill that hold so the hold is now "found" ), it will generate warnings like:

Use of uninitialized value in addition (+) at /usr/share/koha/lib/C4/HoldsQueue.pm line 219.
Use of uninitialized value in addition (+) at /usr/share/koha/lib/C4/HoldsQueue.pm line 220.

Test Plan:
1) Run holds queue builder from the command line
2) While it is running fill a hold
3) Note the warning generated by build_holds_queue.pl
4) Apply this patch
5) Repeat steps 1-2
6) No warnings!
Comment 2 David Nind 2023-09-18 20:58:28 UTC
Is there a script or some way to automatically add a large number of holds, without having to manually add them?

The build_holds_queue.pl runs too quickly with only a small number of holds manually entered.
Comment 3 Magnus Enger 2023-10-06 07:03:12 UTC
(In reply to David Nind from comment #2)
> Is there a script or some way to automatically add a large number of holds,
> without having to manually add them?
> 
> The build_holds_queue.pl runs too quickly with only a small number of holds
> manually entered.

Maybe you could add "sleep 5;" (5 is just an example, could be higher to slow things down even more) somewhere? build_holds_queue.pl just calls CreateQueue(), but I'm guessing there is a loop over the holds in there somwhere, where a little bit of sleep could slow things down.
Comment 4 Pedro Amorim 2023-12-22 11:01:15 UTC
I created a hold manually and ran the following query a couple times to duplicate holds entries in the database:
INSERT INTO reserves (`borrowernumber`,`biblionumber`, `branchcode`) SELECT `borrowernumber`, `biblionumber`, `branchcode` FROM reserves;

This just adds "dummy" data into the reserves table, it appears it's not quite there yet to be able to reproduce what Kyle describes.

(In reply to Kyle M Hall from comment #0)
> If a hold was fillable when the holds queue builder starts, but is no longer
> fillable when the builder gets to that hold's record

I ran perl misc/cronjobs/holds/build_holds_queue.pl but no warnings are showing for me, as per my testing above.

@Kyle are you able to provide test data or a test patch to force this scenario to help reproduce the issue?
Comment 5 David Cook 2024-04-22 01:50:48 UTC
I'm seeing a lot of these warnings in 23.11.x in my email inbox, which do get annoying...

At a glance, this looks like a good change. 

If we could get a unit test for this, I think we could just pass it.