Bug 25786 - Holds Queue building may target the wrong item for item level requests that match holds queue priority
Summary: Holds Queue building may target the wrong item for item level requests that m...
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low critical (vote)
Assignee: Kyle M Hall
QA Contact: Tomás Cohen Arazi
URL:
Keywords:
Depends on: 25783 23934
Blocks:
  Show dependency treegraph
 
Reported: 2020-06-17 16:31 UTC by Kyle M Hall
Modified: 2020-09-20 15:39 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.01, 19.11.10


Attachments
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority (4.31 KB, patch)
2020-06-17 16:56 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority (4.35 KB, patch)
2020-06-18 01:43 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 25786: AddReserve now takes a hashref (1.17 KB, patch)
2020-06-18 13:13 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 25786: Holds Queue building may target the wrong item for item level requests that match holds queue priority (4.40 KB, patch)
2020-06-18 15:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 25786: AddReserve now takes a hashref (1.22 KB, patch)
2020-06-18 15:28 UTC, Tomás Cohen Arazi
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 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.