Bug 3638 Self Check Should Capture Hold Items

Shelf Check was receiving messages saying item was wanted
for a hold but the item was discharged to the shelf
not associated with the hold or transited to
the pickup location. The message was also being sent
on discharge of items when a suitable item had already
been captured.
Checkin now associates the item with the hold and sets
the appropriate data for a correct checkin response

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>

C4/SIP/ILS/Item.pm

@@ -153,15 +153,22 @@ sub next_hold {
 }
 
 # hold_patron_id is NOT the barcode.  It's the borrowernumber.
-# That's because the reserving patron may not have a barcode, or may be from an different system entirely (ILL)!
+# If a return triggers capture for a hold the borrowernumber is passed
+# and saved so that other hold info can be retrieved
 sub hold_patron_id {
-    my $self = shift or return;
-    my $hold = $self->next_hold() or return;
-    return $hold->{borrowernumber};
+    my $self = shift;
+    my $id   = shift;
+    if ($id) {
+        $self->{hold}->{borrowernumber} = $id;
+    }
+    if ($self->{hold} ) {
+        return $self->{hold}->{borrowernumber};
+    }
+    return;
+
 }
 sub hold_patron_name {
     my $self = shift or return;
-    # return $self->{hold_patron_name} if $self->{hold_patron_name};    TODO: consider caching
     my $borrowernumber = (@_ ? shift: $self->hold_patron_id()) or return;
     my $holder = GetMember(borrowernumber=>$borrowernumber);
     unless ($holder) {
@@ -175,7 +182,6 @@ sub hold_patron_name {
                 "" ;                                         # neither populated, empty string
     my $name = $holder->{firstname} ? $holder->{firstname} . ' ' : '';
     $name .= $holder->{surname} . $extra;
-    # $self->{hold_patron_name} = $name;      # TODO: consider caching
     return $name;
 }
 
@@ -192,9 +198,15 @@ sub hold_patron_bcode {
 }
 
 sub destination_loc {
-    my $self = shift or return;
-    my $hold = $self->next_hold();
-    return ($hold ? $hold->{branchcode} : '');
+    my $self = shift;
+    my $set_loc = shift;
+    if ($set_loc) {
+        $self->{dest_loc} = $set_loc;
+    }
+    if ($self->{dest_loc} ) {
+        return $self->{dest_loc};
+    }
+    return q{};
 }
 
 our $AUTOLOAD;

C4/SIP/ILS/Transaction/Checkin.pm

@@ -13,6 +13,7 @@ use ILS;
 use ILS::Transaction;
 
 use C4::Circulation;
+use C4::Reserves qw( ModReserveAffect );
 use C4::Debug;
 
 our @ISA = qw(ILS::Transaction);
@@ -75,8 +76,19 @@ sub do_checkin {
     }
     if ($messages->{ResFound}) {
         $self->hold($messages->{ResFound});
-        $debug and warn "Item returned at $branch reserved at $messages->{ResFound}->{branchcode}";
-        $self->alert_type(($branch eq $messages->{ResFound}->{branchcode}) ? '01' : '02');
+        if ($branch eq $messages->{ResFound}->{branchcode}) {
+            $self->alert_type('01');
+            ModReserveAffect( $messages->{ResFound}->{itemnumber},
+                $messages->{ResFound}->{borrowernumber}, 0);
+
+        } else {
+            $self->alert_type('02');
+            ModReserveAffect( $messages->{ResFound}->{itemnumber},
+                $messages->{ResFound}->{borrowernumber}, 1);
+
+        }
+        $self->{item}->hold_patron_id( $messages->{ResFound}->{borrowernumber} );
+        $self->{item}->destination_loc( $messages->{ResFound}->{branchcode} );
     }
     $self->alert(1) if defined $self->alert_type;  # alert_type could be "00", hypothetically
     $self->ok($return);

C4/SIP/Sip/MsgType.pm

@@ -647,14 +647,7 @@ sub handle_checkin {
     # apparently we can't trust the returns from Checkin yet (because C4::Circulation::AddReturn is faulty)
     # So we reproduce the alert logic here.
     if (not $status->alert) {
-        if ($item->hold_patron_id) {
-            $status->alert(1);
-            if ($item->destination_loc and $item->destination_loc ne $current_loc) {
-                $status->alert_type('02');  # hold at other library
-            } else {
-                $status->alert_type('01');  # hold at this library
-            }
-        } elsif ($item->destination_loc and $item->destination_loc ne $current_loc) {
+        if ($item->destination_loc and $item->destination_loc ne $current_loc) {
             $status->alert(1);
             $status->alert_type('04');  # no hold, just send it
         }