Attachment 5423 Details for Bug 3638 – Proposed patch

[patch] Proposed patch

0001-Bug-3638-Self-Check-Should-Capture-Hold-Items.patch (text/plain), 5.08 KB, created by Colin Campbell on 2011-09-14 12:10:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Colin Campbell

Created: 2011-09-14 12:10:06 UTC

Size: 5.08 KB

patch

obsolete

>From 23a3a78170cd420ac2b0eda19bf7bd8ada5d7869 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 1/2] Bug 3638 Self Check Should Capture Hold Items
>Content-Type: text/plain; charset="utf-8"
>
>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
>---
> 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 b05476c..1e900bd 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 d3a4700..a705f57 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 6a11d0e..3736bb7 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
>         }
>-- 
>1.7.6
>

Actions: View | Diff | Splinter Review

Attachments on bug 3638: 1471 | 4571 | 5019 | 5423 | 5424 | 9901 | 9902