From b7ef3d4c2f531ee9561b12b310b08587bf15603a Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Mon, 4 Jul 2011 16:44:16 +0100
Subject: [PATCH] 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>
---
 C4/SIP/ILS/Item.pm                | 30 +++++++++++++++++++++---------
 C4/SIP/ILS/Transaction/Checkin.pm | 16 ++++++++++++++--
 C4/SIP/Sip/MsgType.pm             |  9 +--------
 3 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/C4/SIP/ILS/Item.pm b/C4/SIP/ILS/Item.pm
index b05476c684..1e900bdaef 100644
--- a/C4/SIP/ILS/Item.pm
+++ b/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;
diff --git a/C4/SIP/ILS/Transaction/Checkin.pm b/C4/SIP/ILS/Transaction/Checkin.pm
index d3a4700f5e..a705f5715b 100644
--- a/C4/SIP/ILS/Transaction/Checkin.pm
+++ b/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);
diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm
index 6da2d36c3e..c18186d4bb 100644
--- a/C4/SIP/Sip/MsgType.pm
+++ b/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
         }
-- 
2.39.5