Bug 29786 - Holds to pull report shows incorrect item for item level holds
Summary: Holds to pull report shows incorrect item for item level holds
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
: 29161 (view as bug list)
Depends on: 29148
Blocks:
  Show dependency treegraph
 
Reported: 2022-01-04 13:35 UTC by Nick Clemens
Modified: 2022-12-12 21:23 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This patch corrects an issue with the Holds to Pull report in which an incorrect barcode number could be shown for an item-level hold. The correct barcode will now be shown.
Version(s) released in:
22.05.00,21.11.03,21.05.11


Attachments
Bug 29786: Unit tests (2.49 KB, patch)
2022-01-04 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29786: Select only specific items for item level holds (4.36 KB, patch)
2022-01-04 13:42 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 29786: Unit tests (2.56 KB, patch)
2022-01-04 14:06 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29786: Select only specific items for item level holds (4.42 KB, patch)
2022-01-04 14:06 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 29786: Unit tests (2.63 KB, patch)
2022-01-07 11:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 29786: Select only specific items for item level holds (4.50 KB, patch)
2022-01-07 11:27 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2022-01-04 13:35:55 UTC
When running holds to pull it can show the wrong item for an item level hold. This is because we simply get the first barcode from the list of items that can fill the hold.

The issue seems to come from 'get_items_that_can_fill' in Koha::Holds

This does not take into account that an item level hold can only be filled by one item

To recreate:
1 - Find a record with many items available
2 - Place an item level hold for an item on the record, not the one with lowest itemnumber
3 - Run 'Hold to pull' report
4 - Note the barcode does not match
Comment 1 Nick Clemens 2022-01-04 13:42:39 UTC
Created attachment 128987 [details] [review]
Bug 29786: Unit tests

I add a new test for the case of an item level hold, and additionally
adjust later tests to use a title level hold.

I also fix a typo where the comment says item 1 when it means item 2
Comment 2 Nick Clemens 2022-01-04 13:42:43 UTC
Created attachment 128988 [details] [review]
Bug 29786: Select only specific items for item level holds

This patch adjusts get_items_that_can_fill to make two requests:
first the list of items for item-level holds
second the list of biblionumbers for title-level holds

This stops the report from pulling more items for item-level hold

This patch also removes the aliases used in the code - while readability
is a bit harder, it allows for using 'me' in get_items_that_can_fill
Otherwise, this routine would need a parameter to know what we called the table.

To test:
1 - Find a record with many items available
2 - Place an item level hold for an item on the record, not the one with lowest itemnumber
3 - Run 'Hold to pull' report
4 - Note the barcode does not match
5 - Apply patch
6 - Reload report
7 - It matches!
Comment 3 Andrew Fuerste-Henry 2022-01-04 14:06:21 UTC
Created attachment 128989 [details] [review]
Bug 29786: Unit tests

I add a new test for the case of an item level hold, and additionally
adjust later tests to use a title level hold.

I also fix a typo where the comment says item 1 when it means item 2

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 4 Andrew Fuerste-Henry 2022-01-04 14:06:24 UTC
Created attachment 128990 [details] [review]
Bug 29786: Select only specific items for item level holds

This patch adjusts get_items_that_can_fill to make two requests:
first the list of items for item-level holds
second the list of biblionumbers for title-level holds

This stops the report from pulling more items for item-level hold

This patch also removes the aliases used in the code - while readability
is a bit harder, it allows for using 'me' in get_items_that_can_fill
Otherwise, this routine would need a parameter to know what we called the table.

To test:
1 - Find a record with many items available
2 - Place an item level hold for an item on the record, not the one with lowest itemnumber
3 - Run 'Hold to pull' report
4 - Note the barcode does not match
5 - Apply patch
6 - Reload report
7 - It matches!

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Comment 5 Andrew Fuerste-Henry 2022-01-06 14:53:36 UTC
*** Bug 29161 has been marked as a duplicate of this bug. ***
Comment 6 Jonathan Druart 2022-01-07 11:27:46 UTC
Created attachment 129139 [details] [review]
Bug 29786: Unit tests

I add a new test for the case of an item level hold, and additionally
adjust later tests to use a title level hold.

I also fix a typo where the comment says item 1 when it means item 2

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2022-01-07 11:27:50 UTC
Created attachment 129140 [details] [review]
Bug 29786: Select only specific items for item level holds

This patch adjusts get_items_that_can_fill to make two requests:
first the list of items for item-level holds
second the list of biblionumbers for title-level holds

This stops the report from pulling more items for item-level hold

This patch also removes the aliases used in the code - while readability
is a bit harder, it allows for using 'me' in get_items_that_can_fill
Otherwise, this routine would need a parameter to know what we called the table.

To test:
1 - Find a record with many items available
2 - Place an item level hold for an item on the record, not the one with lowest itemnumber
3 - Run 'Hold to pull' report
4 - Note the barcode does not match
5 - Apply patch
6 - Reload report
7 - It matches!

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Fridolin Somers 2022-01-18 00:50:37 UTC
What a big dirty bug ^^

A short release note would be great
Comment 9 Fridolin Somers 2022-01-18 00:56:34 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 10 Kyle M Hall 2022-02-04 18:58:45 UTC
Pushed to 21.11.x for 21.11.03
Comment 11 Andrew Fuerste-Henry 2022-02-10 20:47:18 UTC
Pushed to 21.05.x for 21.05.11
I also backported 3142, which is seemed like this might require.
Comment 12 Victor Grousset/tuxayo 2022-02-18 23:17:38 UTC
Missing dependencies for 20.11.x, it shouldn't be affected, no backport.