Attachment 22971 Details for Bug 11258 – Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy

[patch] Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy

Bug-11258---Holds-queue-is-still-making-transfer-r.patch (text/plain), 4.17 KB, created by Kyle M Hall (khall) on 2013-11-15 17:50:00 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Kyle M Hall (khall)

Created: 2013-11-15 17:50:00 UTC

Size: 4.17 KB

patch

obsolete

>From 4a8079b548f2b883a89a140a8fde5f148413ed6d Mon Sep 17 00:00:00 2001
>From: ByWater Migration Support <migrations@bywatersolutions.com>
>Date: Fri, 15 Nov 2013 08:29:23 -0800
>Subject: [PATCH] Bug 11258 - Holds queue is still making transfer requests that contradict the branch holds policy
>
>The holds queue generator is still making requests that are not allowed.
>Previously it was making transfer requests that were not allowed.
>Currently, it is still making requests where the pickup branch is the
>same as the item's branch but not the patron's branch,
>even if there is a "Default holds policy by item type" rule that states
>this item can only fill holds for patron's of the same branch as the item.
>
>Test Plan:
>1) Create a test record with 2 items with different itemtypes
>2) Set the Default holds policy by item type for the first
>   item to "From any library"
>3) Set the Default holds policy by item type for the second
>   item to "From home library"
>4) Place a record level hold for a patron from another library,
>   but for pickup at the same library as the item is from
>5) Rebuild the holds queue
>6) View the holds queue, note the item is listed, though this
>   patron cannot place a hold on this item
>7) Apply this patch
>8) Repeat step 5, note the hold is no longer in the queue
>---
> C4/HoldsQueue.pm            |    4 ++--
> t/db_dependent/HoldsQueue.t |    7 ++++---
> 2 files changed, 6 insertions(+), 5 deletions(-)
>
>diff --git a/C4/HoldsQueue.pm b/C4/HoldsQueue.pm
>index 7062cbe..503ed90 100755
>--- a/C4/HoldsQueue.pm
>+++ b/C4/HoldsQueue.pm
>@@ -434,7 +434,6 @@ sub MapItemsToHoldRequests {
>                 }
>             }
>             $holdingbranch = $pickup_branch;
>-            $itemnumber ||= $holding_branch_items->[0]->{itemnumber};
>         }
>         elsif ($transport_cost_matrix) {
>             $pull_branches = [keys %items_by_branch];
>@@ -469,6 +468,7 @@ sub MapItemsToHoldRequests {
>                 $holdingbranch ||= $branch;
>                 foreach my $item (@$holding_branch_items) {
>                     next if $pickup_branch ne $item->{homebranch};
>+                    next if ( $item->{holdallowed} == 1 && $item->{homebranch} ne $request->{borrowerbranch} );
> 
>                     $itemnumber = $item->{itemnumber};
>                     $holdingbranch = $branch;
>@@ -478,7 +478,7 @@ sub MapItemsToHoldRequests {
> 
>             unless ( $itemnumber ) {
>                 foreach my $current_item ( @{ $items_by_branch{$holdingbranch} } ) {
>-                    if ( $holdingbranch && ( $current_item->{holdallowed} == 2 || $pickup_branch eq $current_item->{homebranch} ) ) {
>+                    if ( $holdingbranch && ( $current_item->{holdallowed} == 2 || $request->{borrowerbranch} eq $current_item->{homebranch} ) ) {
>                         $itemnumber = $current_item->{itemnumber};
>                         last; # quit this loop as soon as we have a suitable item
>                     }
>diff --git a/t/db_dependent/HoldsQueue.t b/t/db_dependent/HoldsQueue.t
>index 92bd843..0a96143 100755
>--- a/t/db_dependent/HoldsQueue.t
>+++ b/t/db_dependent/HoldsQueue.t
>@@ -12,7 +12,7 @@ use C4::Context;
> 
> use Data::Dumper;
> 
>-use Test::More tests => 20;
>+use Test::More tests => 21;
> 
> 
> use C4::Branch;
>@@ -287,8 +287,9 @@ $dbh->do("DELETE FROM default_circ_rules");
> $dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 1 )");
> C4::HoldsQueue::CreateQueue();
> $holds_queue = $dbh->selectall_arrayref("SELECT * FROM tmp_holdsqueue", { Slice => {} });
>-ok( @$holds_queue == 1, "Holds queue filling correct number for default holds policy 'from home library'" );
>-ok( $holds_queue->[0]->{cardnumber} eq 'CARDNUMBER1', "Holds queue filling correct hold for default holds policy 'from home library'");
>+ok( @$holds_queue == 2, "Holds queue filling correct number for default holds policy 'from home library'" );
>+ok( $holds_queue->[0]->{cardnumber} eq 'CARDNUMBER1', "Holds queue filling 1st correct hold for default holds policy 'from home library'");
>+ok( $holds_queue->[1]->{cardnumber} eq 'CARDNUMBER2', "Holds queue filling 2nd correct hold for default holds policy 'from home library'");
> 
> $dbh->do("DELETE FROM default_circ_rules");
> $dbh->do("INSERT INTO default_circ_rules ( holdallowed ) VALUES ( 2 )");
>-- 
>1.7.2.5

Actions: View | Diff | Splinter Review

Attachments on bug 11258: 22965 | 22970 | 22971 | 23011 | 25224 | 27267