[Bug 8562] New: RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Priority: P5 - low Change sponsored?: --- Bug ID: 8562 CC: gmcharlt@gmail.com Assignee: koha-bugs@lists.koha-community.org Summary: RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. Severity: minor Classification: Unclassified OS: All Reporter: kyle@bywatersolutions.com Hardware: All Status: NEW Version: master Component: Hold requests Product: Koha If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |kyle@bywatersolutions.com |ity.org | -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #1 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 11302 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11302&action=edit Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- One thought, I wonder if we should just always randomize them in the case that StaticHoldsQueueWeight is empty. I cannot imagine an instance where a library system would want holds to be filled in alphabetical order. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #3 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 11303 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=11303&action=edit Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. Force randomization instead. If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. This patch forces randomization in the event that StaticHoldsQueueWeight is empty, under the assumption that no library would wish to have libraries fulfill hold requests in alphabetical order. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #4 from Kyle M Hall <kyle@bywatersolutions.com> --- This second variation forces the shuffling in the case that StaticHoldsQueueWeight is empty. The larger benefit is the reduction of calls to C4::Context->preference from @bibs_with_pending_requests + 1 back down to 1. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #5 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Do these patches work together, or are they two options? If the latter, what is the difference? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #6 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to comment #5)
Do these patches work together, or are they two options? If the latter, what is the difference?
The are mutually exclusive. Either one will fix the bug. The first patch shuffles the branches to pull if RandomizeHoldsQueueWeight is enabled, even when StaticHoldsQueueWeight is empty. The second patch forces randomization in the event that StaticHoldsQueueWeight is empty, under the assumption that no library would wish to have libraries fulfill hold requests in alphabetical order. It also is slightly more efficient in that it does not make a call to C4::Context->preference(). -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #7 from Owen Leonard <oleonard@myacpl.org> --- These patches have been in the queue for a long time. Perhaps good test plans for each would help? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11302|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #11303|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 16323 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16323&action=edit Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. Test Plan: 1) Apply patch 2) Set the system preference StaticHoldsQueueWeight to empty 3) Create a hold on an record with items at multiple libraries 4) Run build_holds_queue.pl 5) Look at the holds queue report from the circulation home page 6) Run build_holds_queue.pl again 7) Look at the holds queue report again, note the library chosen for fulfillment of the hold has likely changed. 8) Repeat steps 6-7 until you are satisfied the library to fulfill the hold is chosen at random. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16323|0 |1 is obsolete| | --- Comment #9 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 16324 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16324&action=edit Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. Test Plan: 1) Apply patch 2) Set the system preference StaticHoldsQueueWeight to empty 3) Create a hold on an record with items at multiple libraries 4) Run build_holds_queue.pl 5) Look at the holds queue report from the circulation home page 6) Run build_holds_queue.pl again 7) Look at the holds queue report again, note the library chosen for fulfillment of the hold has likely changed. 8) Repeat steps 6-7 until you are satisfied the library to fulfill the hold is chosen at random. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Koha Team AMU <koha.aixmarseille@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off CC| |koha.aixmarseille@gmail.com --- Comment #10 from Koha Team AMU <koha.aixmarseille@gmail.com> --- Tested on a sandbox. The hold is chosen at random. Test plan needed to be specified. An hold (3 items, one in each library, and the pickup library is a 4th one because the pickup library for this test must be a library without the item). Don't forget the syspref AllowOnShelfHolds. Look at the holds queue report from the circulation home page. If you choose all libraries : no barcode specified for the hold. If you specify one library, the first library you choose is the one that must send the book. Run build_holds_queue.pl Change the library of the holds queue report. So : ok. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |paul.poulain@biblibre.com --- Comment #11 from Paul Poulain <paul.poulain@biblibre.com> --- Patch tested with a sandbox, by Leila and Frido <koha.aixmarseille@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Paul Poulain <paul.poulain@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #16324|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #12 from Paul Poulain <paul.poulain@biblibre.com> --- Created attachment 16794 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16794&action=edit Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. If StaticHoldsQueueWeight is empty, but RandomizeHoldsQueueWeight is enabled, the script build_holds_queue.pl should select branches for fulfillment requests at random. Instead, it will select them in alphabetical order. Test Plan: 1) Apply patch 2) Set the system preference StaticHoldsQueueWeight to empty 3) Create a hold on an record with items at multiple libraries 4) Run build_holds_queue.pl 5) Look at the holds queue report from the circulation home page 6) Run build_holds_queue.pl again 7) Look at the holds queue report again, note the library chosen for fulfillment of the hold has likely changed. 8) Repeat steps 6-7 until you are satisfied the library to fulfill the hold is chosen at random. Signed-off-by: Leila and Frido <koha.aixmarseille@gmail.com> -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 --- Comment #13 from Jared Camins-Esakov <jcamins@cpbibliography.com> --- Comment on attachment 16794 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16794 Bug 8562 - RandomizeHoldsQueueWeight ignored if StaticHoldsQueueWeight is empty. Review of attachment 16794: --> (http://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=8562&attachment=16794) ----------------------------------------------------------------- ::: C4/HoldsQueue.pm @@ +575,5 @@
my @branches_to_use = map _trim($_), split /,/, $static_branch_list;
@branches_to_use = shuffle(@branches_to_use) if C4::Context->preference("RandomizeHoldsQueueWeight");
+ @branches_to_use = shuffle keys %{GetBranches()} unless ( @branches_to_use );
I do not think that this is correct. The branches are shuffled regardless of the value of RandomizeHoldsQueueWeight. Perhaps better would be: @branches_to_use ||= keys %{GetBranches()}; # (or the correct version of that statement, since I didn't test my suggestion) right after the definition of @branches_to_use? Also, I think there is a problem here with the default value of StaticHoldsQueueWeight, which is '0'(?). This could be addressed on a different bug. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Jared Camins-Esakov <jcamins@cpbibliography.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Kyle M Hall <kyle.m.hall@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff CC| |kyle.m.hall@gmail.com --- Comment #14 from Kyle M Hall <kyle.m.hall@gmail.com> ---
I do not think that this is correct. The branches are shuffled regardless of the value of RandomizeHoldsQueueWeight. Perhaps better would be: @branches_to_use ||= keys %{GetBranches()}; # (or the correct version of that statement, since I didn't test my suggestion) right after the definition of @branches_to_use?
The justification for this is that it is extremely unlikely that a library would want to fill holds from libraries in alphabetical order in the case that the library has not populated StaticHoldsQueueWeight. Do you believe this is an unreasonable assumption? -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA CC| |m.de.rooy@rijksmuseum.nl --- Comment #15 from M. de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Kyle M Hall from comment #14)
I do not think that this is correct. The branches are shuffled regardless of the value of RandomizeHoldsQueueWeight. Perhaps better would be: @branches_to_use ||= keys %{GetBranches()}; # (or the correct version of that statement, since I didn't test my suggestion) right after the definition of @branches_to_use?
The justification for this is that it is extremely unlikely that a library would want to fill holds from libraries in alphabetical order in the case that the library has not populated StaticHoldsQueueWeight. Do you believe this is an unreasonable assumption?
I tend to agree with Jared here. If the Randomize pref is not set, you should not shuffle. Otherwise this pref is really useless. You have a point however. I think that what you say, more pertains to the default values set for these prefs in Koha. Randomize should be 1 by default. The description on the pref form could be improved/extended. I also think you should take into account the '0' default value. Just interpret it as all branches here (empty string as advertised). If we definitely want to get rid of this zero, it could be removed completely on a new report. -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 M. de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |m.de.rooy@rijksmuseum.nl -- You are receiving this mail because: You are watching all bug changes.
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8562 Marc Véron <veron@veron.ch> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |veron@veron.ch --- Comment #16 from Marc Véron <veron@veron.ch> --- Still valid? -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org