Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch
Summary: Holds Queue building ignoring holds where pickup & home branch don't match an...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-29 10:45 UTC by Kyle M Hall
Modified: 2016-06-21 21:39 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch (3.08 KB, patch)
2015-05-29 10:57 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch (3.15 KB, patch)
2015-07-21 14:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 - Unit Tests (8.08 KB, patch)
2015-08-27 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch (3.14 KB, patch)
2015-08-27 13:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 - Unit Tests (3.66 KB, patch)
2015-08-27 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch (3.14 KB, patch)
2015-08-27 13:39 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 [QA Followup] - Remove unnecessary line (830 bytes, patch)
2015-09-03 15:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14297 - Unit Tests (3.73 KB, patch)
2015-09-03 16:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch (3.19 KB, patch)
2015-09-03 16:12 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14297 [QA Followup] - Remove unnecessary line (879 bytes, patch)
2015-09-03 16:12 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 Kyle M Hall 2015-05-29 10:45:24 UTC
If a record has a hold on it where the pickup and home branch do not match, the holds queue builder will only look at items from the least cost branch ( as defined by the transport cost matrix or the sys pref StaticHoldsQueueWeight.

Test Plan:
1) Create a record with two items, one for library A and one for library B
2) Set your circulation rules such that the book from library A is holdable by all and the book from library B is holdable only by library B patrons
3) Create a hold for a Library C patron for pickup at library C
4) Set the syspref StaticHoldsQueueWeight to by Library B, Library A, Library C in that order
5) Rebuild the holds queue
6) Note the hold wasn't picked up even though the item from library A could have filled the hold
Comment 1 Kyle M Hall 2015-05-29 10:57:23 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-05-29 11:12:48 UTC
It appears that my audio narration in the video did not record, but it's pretty self explanatory without it.
Comment 4 Kyle M Hall 2015-07-21 14:26:07 UTC Comment hidden (obsolete)
Comment 5 Jonathan Druart 2015-08-05 09:49:25 UTC
Any chances to get some tests to highlight this change?
Comment 6 Kyle M Hall 2015-08-14 23:09:28 UTC
(In reply to Jonathan Druart from comment #5)
> Any chances to get some tests to highlight this change?

I'll see what I can do!
Comment 7 Kyle M Hall 2015-08-27 13:37:12 UTC Comment hidden (obsolete)
Comment 8 Kyle M Hall 2015-08-27 13:37:18 UTC Comment hidden (obsolete)
Comment 9 Kyle M Hall 2015-08-27 13:39:04 UTC Comment hidden (obsolete)
Comment 10 Kyle M Hall 2015-08-27 13:39:11 UTC Comment hidden (obsolete)
Comment 11 Jonathan Druart 2015-09-01 11:20:07 UTC
Comment on attachment 42029 [details] [review]
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch

Review of attachment 42029 [details] [review]:
-----------------------------------------------------------------

::: C4/HoldsQueue.pm
@@ +492,5 @@
> +                foreach my $branch (@$pull_branches) {
> +                    my $holding_branch_items = $items_by_branch{$branch}
> +                      or next;
> +
> +                    $holdingbranch ||= $branch;

I am not sure to understand this line, in which case it's needed?
Comment 12 Jonathan Druart 2015-09-01 11:20:39 UTC
Shouldn't you change the version to master (instead of 3.18)?;
Comment 13 Kyle M Hall 2015-09-03 15:03:58 UTC Comment hidden (obsolete)
Comment 14 Jonathan Druart 2015-09-03 16:12:12 UTC
Created attachment 42348 [details] [review]
Bug 14297 - Unit Tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 15 Jonathan Druart 2015-09-03 16:12:15 UTC
Created attachment 42349 [details] [review]
Bug 14297 - Holds Queue building ignoring holds where pickup & home branch don't match and item is not from least cost branch

If a record has a hold on it where the pickup and home branch do not
match, the holds queue builder will only look at items from the least
cost branch ( as defined by the transport cost matrix or the sys pref
StaticHoldsQueueWeight.

Test Plan:
1) Create a record with two items, one for library A and one for library B
2) Set your circulation rules such that the book from library A is
   holdable by all and the book from library B is holdable only by library
   B patrons
3) Create a hold for a Library C patron for pickup at library C
4) Set the syspref StaticHoldsQueueWeight to by Library B, Library A,
   Library C in that order
5) Rebuild the holds queue
6) Note the hold wasn't picked up even though the item from library A
   could have filled the hold
7) Apply this patch
8) Rebuild the holds queue
9) View the holds queue again
10) Note the hold now displays

Signed-off-by: Nora Blake <nblake@masslibsystem.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 16 Jonathan Druart 2015-09-03 16:12:19 UTC
Created attachment 42350 [details] [review]
Bug 14297 [QA Followup] - Remove unnecessary line

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 17 Tomás Cohen Arazi 2015-09-25 18:42:42 UTC
Patches pushed to master.

Thanks Kyle!