Description
Kyle M Hall (khall)
2015-05-29 10:45:24 UTC
Created attachment 39668 [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 Video Demonstration: http://www.screencast.com/users/kylemhall/folders/Default/media/efa52594-e81b-4672-aa57-e59f8952ea6e It appears that my audio narration in the video did not record, but it's pretty self explanatory without it. Created attachment 41110 [details] [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 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> Any chances to get some tests to highlight this change? (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! Created attachment 42026 [details] [review] Bug 14297 - Unit Tests Created attachment 42027 [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> Created attachment 42028 [details] [review] Bug 14297 - Unit Tests Created 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 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> 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? Shouldn't you change the version to master (instead of 3.18)?; Created attachment 42341 [details] [review] Bug 14297 [QA Followup] - Remove unnecessary line Created attachment 42348 [details] [review] Bug 14297 - Unit Tests Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> 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> Created attachment 42350 [details] [review] Bug 14297 [QA Followup] - Remove unnecessary line Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Patches pushed to master. Thanks Kyle! |