http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 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=10628 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- I could not come up with a reliable test plan for this bug. However, inspection of the code should be extremely simple. The patch actually contains two changes, either of which solves the problem, but I see no reason not to include both. The first is is the removal of the use of AutomaticItemReturn to control if we should try filling local holds with local items. The second is the rather strange situation where the least_cost_branch subroutine completely ignores the fact that the pickup branch might be in the list of branches that can be pulled from. I suggest both be implemented, the former for the fact that I cannot find any reason for this behavior, and the latter ( though obviated by the former ) because in the future someone may again modify the holds queue system and not realize that some code in MapItemsToHoldRequests would make this addition to least_cost_branch unnecessary. It seems to me that a unit test for least_cost_branch should fail immediately in it's current state. However, there is, for some reason, no unit test for least_cost_branch itself in HoldsQueue.t -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 kenza <kenza.zaki@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |10630 -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |srdjan@catalyst.net.nz --- Comment #4 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- my ($self) = grep( /$to/, @$from ); looks a bit inefficient. Would you consider replacing with my $self; foreach (@$from) { if ($_ eq $to) { $self = $to; last; } } or maybe even better List::MoreUtils::any() -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 --- Comment #6 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- # Failed test 'take from holdingbranch AutomaticItemReturn on (use cost matrix) holding branch' # at t/db_dependent/HoldsQueue.t line 196. # got: 'CPL' # expected: 'IPT' # Wrong pick-up/hold for first target (pick_branch, hold_branch, reserves, hold_fill_targets, tmp_holdsqueue): $VAR1 = 'CPL'; # $VAR2 = 'IPT'; # $VAR3 = [ # { # 'priority' => '1', # 'reservenotes' => undef, # 'reservedate' => '2013-08-00', # 'found' => undef, # 'expirationdate' => undef, # 'suspend_until' => undef, # 'reminderdate' => undef, # 'biblionumber' => '92704', # 'timestamp' => '2013-08-01 12:27:34', # 'notificationdate' => undef, # 'borrowernumber' => '1000022266', # 'branchcode' => 'CPL', # 'itemnumber' => undef, # 'constrainttype' => '', # 'lowestPriority' => '0', # 'suspend' => '0', # 'waitingdate' => undef, # 'cancellationdate' => undef, # 'reserve_id' => '4361' # } # ]; # $VAR4 = [ # { # 'itemnumber' => '68955', # 'biblionumber' => '92704', # 'source_branchcode' => 'CPL', # 'borrowernumber' => '1000022266', # 'item_level_request' => '0' # } # ]; # $VAR5 = [ # { # 'firstname' => 'my firstname', # 'pickbranch' => 'CPL', # 'itemnumber' => '68955', # 'itemcallnumber' => undef, # 'reservedate' => '2013-08-00', # 'barcode' => '531703236591', # 'phone' => undef, # 'item_level_request' => '0', # 'surname' => 'my surname', # 'biblionumber' => '92704', # 'holdingbranch' => 'CPL', # 'cardnumber' => 'CARDNUMBER42', # 'notes' => undef, # 'title' => 'Test Holds Queue XXX', # 'borrowernumber' => '1000022266' # } # ]; Illegal date specified (year = 2013, month = 8, day = 00) at C4/HoldsQueue.pm line 138 # Looks like you failed 1 test of 19. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #19862|0 |1 is obsolete| | Attachment #19863|0 |1 is obsolete| | Attachment #20010|0 |1 is obsolete| | --- Comment #8 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 20036 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20036&action=edit Bug 10628 - AutomaticItemReturn prevents holds queue from filling local holds with local items For some reason, C4::HoldsQueue::MapItemsToHoldRequests uses the system preference AutomaticItemReturn to decide if an attempt to fill local holds with local items. No explanation of this behavior is provided. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Srdjan Jankovic <srdjan@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20036|0 |1 is obsolete| | --- Comment #9 from Srdjan Jankovic <srdjan@catalyst.net.nz> --- Created attachment 20052 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20052&action=edit [SIGNED-OFF] Bug 10628 - AutomaticItemReturn prevents holds queue from filling local holds with local items For some reason, C4::HoldsQueue::MapItemsToHoldRequests uses the system preference AutomaticItemReturn to decide if an attempt to fill local holds with local items. No explanation of this behavior is provided. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |katrin.fischer@bsz-bw.de --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Hi Kyle, the code does look alright, but I tried testing it and it didn't give me the results I expected. I turned on the cost matrix and entered and made it look something like this: A B A _ 0 B 0 _ Now I am in A, putting a hold on a record with items from A and B, pickup location is B. I would expect the routine to pick up the available item from B, but it doesn't. I have reran build_holds_queue multiple times. Any hints? -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 --- Comment #13 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- I found it... my items were checked out... -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #20052|0 |1 is obsolete| | --- Comment #14 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 20491 --> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=20491&action=edit [PASSED QA] Bug 10628 - AutomaticItemReturn prevents holds queue from filling local holds with local items For some reason, C4::HoldsQueue::MapItemsToHoldRequests uses the system preference AutomaticItemReturn to decide if an attempt to fill local holds with local items. No explanation of this behavior is provided. Signed-off-by: Srdjan <srdjan@catalyst.net.nz> Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de> Passes QA script and adds unit tests. Tested with some examples and those worked correctly. -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com -- You are receiving this mail because: You are watching all bug changes.

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10628 Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bgkriegel@gmail.com --- Comment #17 from Bernardo Gonzalez Kriegel <bgkriegel@gmail.com> --- Pushed to 3.10.x, will be in 3.10.11 -- You are receiving this mail because: You are watching all bug changes.