Bug 25786

Summary: Holds Queue building may target the wrong item for item level requests that match holds queue priority
Product: Koha Reporter: Kyle M Hall <kyle>
Component: Hold requestsAssignee: Kyle M Hall <kyle>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: critical    
Priority: P5 - low CC: aleisha, andrewfh, gmcharlt, jonathan.druart, nick, sbcornell, tomascohen, victor
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25783
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.01, 19.11.10
Bug Depends on: 23934, 25783    
Bug Blocks:    
Attachments: Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority
Bug 25786: AddReserve now takes a hashref
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority
Bug 25786: AddReserve now takes a hashref

Description Kyle M Hall 2020-06-17 16:31:18 UTC
Bug 23934 removed the limitation that prevented item level holds from getting local holds priority. The problem is the code has never checked if the item level hold matches the given item! This means the wrong item may be requested to fill an item level hold.
Comment 1 Kyle M Hall 2020-06-17 16:56:14 UTC
Created attachment 105981 [details] [review]
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority

Bug 23934 removed the limitation that prevented item level holds from
getting local holds priority. The problem is the code has never checked
if the item level hold matches the given item! This means the wrong item
may be requested to fill an item level hold.

Test Plan:
1) Create 3 items on a record
2) Place a hold for the 2nd item you created
4) Ensure that hold would be picked up by local holds priority
5) Build the holds queue
6) Note the holds queue is asking for the wrong item!
7) Apply this patch
8) Rebuild the holds queue
9) Holds queue should now be asking for the correct item!
Comment 2 Andrew Fuerste-Henry 2020-06-18 01:43:44 UTC
Created attachment 105994 [details] [review]
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority

Bug 23934 removed the limitation that prevented item level holds from
getting local holds priority. The problem is the code has never checked
if the item level hold matches the given item! This means the wrong item
may be requested to fill an item level hold.

Test Plan:
1) Create 3 items on a record
2) Place a hold for the 2nd item you created
4) Ensure that hold would be picked up by local holds priority
5) Build the holds queue
6) Note the holds queue is asking for the wrong item!
7) Apply this patch
8) Rebuild the holds queue
9) Holds queue should now be asking for the correct item!

Signed-off-by: Kim Peine <kim@williston.lib.vt.us>
Comment 3 Jonathan Druart 2020-06-18 13:11:51 UTC
The tests are failing for me

#   Failed test 'No tests run for subtest "Test Local Holds Priority - Get correct item for item level hold"'
#   at t/db_dependent/HoldsQueue.t line 1163.
Can't use string ("KpHbPJqfr8") as a HASH ref while "strict refs" in use at /kohadevbox/koha/C4/Reserves.pm line 176.
# Looks like your test exited with 255 just after 50.
Comment 4 Jonathan Druart 2020-06-18 13:13:41 UTC
Created attachment 106010 [details] [review]
Bug 25786: AddReserve now takes a hashref
Comment 5 Tomás Cohen Arazi 2020-06-18 15:28:24 UTC
Created attachment 106023 [details] [review]
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority

Bug 23934 removed the limitation that prevented item level holds from
getting local holds priority. The problem is the code has never checked
if the item level hold matches the given item! This means the wrong item
may be requested to fill an item level hold.

Test Plan:
1) Create 3 items on a record
2) Place a hold for the 2nd item you created
4) Ensure that hold would be picked up by local holds priority
5) Build the holds queue
6) Note the holds queue is asking for the wrong item!
7) Apply this patch
8) Rebuild the holds queue
9) Holds queue should now be asking for the correct item!

Signed-off-by: Kim Peine <kim@williston.lib.vt.us>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2020-06-18 15:28:31 UTC
Created attachment 106024 [details] [review]
Bug 25786: AddReserve now takes a hashref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2020-06-18 15:30:01 UTC
Aleisha: The second patch should not be applied to 19.11.
Comment 8 Jonathan Druart 2020-06-18 16:52:40 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 9 Jonathan Druart 2020-06-18 16:56:37 UTC
backported to 20.05.x for 20.05.01
Comment 10 Aleisha Amohia 2020-06-22 04:52:37 UTC
can backport once the tests are fixed on Bug 25783
Comment 11 Nick Clemens 2020-09-03 10:56:01 UTC
(In reply to Aleisha Amohia from comment #10)
> can backport once the tests are fixed on Bug 25783

Hi Alicia, Kyle fixed the tests on 25783 - can you test?
Comment 12 Aleisha Amohia 2020-09-07 03:17:48 UTC
backported to 19.11.x for 19.11.10 (first patch only)

- Aleisha
Comment 13 Victor Grousset/tuxayo 2020-09-20 15:39:25 UTC
Missing dependencies for 19.05.x, can't backport.