This feature would allow libraries to specify that, when items a returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds.
Created attachment 22328 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t
Created attachment 22329 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t
Applying: Bug 11126 - Make the holds system optionally give precedence to local holds Using index info to reconstruct a base tree... M installer/data/mysql/updatedatabase.pl Falling back to patching base and 3-way merge... Auto-merging installer/data/mysql/updatedatabase.pl CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl Patch failed at 0001 Bug 11126 - Make the holds system optionally give precedence to local holds The copy of the patch that failed is found in: /home/christopher/git/koha/.git/rebase-apply/patch When you have resolved this problem, run "git am --continue". If you prefer to skip this patch, run "git am --skip" instead. To restore the original branch and stop patching, run "git am --abort".
Created attachment 22543 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t
Created attachment 22778 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t
Patch tested with a sandbox, by Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 22784 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Comment on attachment 22784 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds Review of attachment 22784 [details] [review]: ----------------------------------------------------------------- Ok, starting with a code review :) Looks good so far: - system preferences are added correctly - new feature is off by default - unit tests are provided Only one small issue with one of the comments: ::: C4/Reserves.pm @@ +915,5 @@ > return ( "Waiting", $res, \@reserves ); # Found it > } else { > + # Lazy fetch for borrower and item. We only need to know about the patron and item > + # each and every time if we are using LocalHoldsPriority. This is a great place to > + # leverage the inherent lazy fetching of DBIx::Class. I think DBIx::Class is currently not yet used here, so the comment could be fixed in a follow up.
Coninuing testing...
Created attachment 24195 [details] test case home library/pickup library Kyle, I have double checked everything, but the test case described in the attached PDF doesn't seem to give the correct result to me. Could you please take a look? When I turn off LocalHoldsPriority the hold for R is triggered.
(In reply to Katrin Fischer from comment #10) > Created attachment 24195 [details] > test case home library/pickup library > > Kyle, I have double checked everything, but the test case described in the > attached PDF doesn't seem to give the correct result to me. Could you please > take a look? > > When I turn off LocalHoldsPriority the hold for R is triggered. I think I made the assumption that the holds would be iterated over in order of priority. I'll try to reproduce your results and see what I can do!
Created attachment 25589 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Katrin, can you take a look at this again? Thanks.
There are lots of conflicts in Reserves.pm now - with the recent changes to Reserves.pm. Please fix and test.
Created attachment 28145 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 28146 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Created attachment 28181 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 28182 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Created attachment 30215 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 30216 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Planning to work on this in the next few days.
Created attachment 32189 [details] [review] Bug 11126: qa follow-up - use Modern::Perl; - fix a typo - remove an old comment
I am not sure about all the consequences of this enhancement. The C4::CheckReserves subroutine is called at several places. It would have been good to have a better test plan :) Maybe should we highlight the fact that if will be applied for new holds, not existing ones. All looks good to me and I pass QA on it.
Created attachment 32191 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net>
Created attachment 32192 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority
Created attachment 32193 [details] [review] Bug 11126: qa follow-up - use Modern::Perl; - fix a typo - remove an old comment
Created attachment 32195 [details] [review] Bug 11126 - Make the holds system optionally give precedence to local holds This feature will allow libraries to specify that, when an item is returned, a local hold may be given priority for fulfillment even though it is of lower priority in the list of unfilled holds. This feature has three settings: * LocalHoldsPriority, which enables the feature * LocalHoldsPriorityPatronControl, which selects for either tha patron's home library, or the patron's pickup library for the hold * LocalHoldsPriorityItemControl, which selects for either the item's holding library, or home library. So, this feature can "give priority for filling holds to patrons whose (home library|pickup library) matches the item's (home library|holding library)" Test Plan: 1) Apply this patch 2) Run t/db_dependent/Holds/LocalHoldsPriority.t Signed-off-by: Joel Sasse <jsasse@plumcreeklibrary.net> Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 32196 [details] [review] Bug 11126 [QA Followup] - Make reserves returned by _Findgroupreserve sorted by priority Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Created attachment 32197 [details] [review] Bug 11126: qa follow-up - use Modern::Perl; - fix a typo - remove an old comment Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Patches pushed to master. Thanks Kyle!