From b7ef3d4c2f531ee9561b12b310b08587bf15603a Mon Sep 17 00:00:00 2001 From: Colin Campbell 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 Signed-off-by: Chris Cormack --- 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