Bug 33184 - Warnings when running holds queue builder
Summary: Warnings when running holds queue builder
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-03-09 17:09 UTC by Kyle M Hall
Modified: 2024-04-22 01:50 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 33184: Warnings when running holds queue builder (1.52 KB, patch)
2023-03-09 17:14 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 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 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.