[Koha-patches] [PATCH] Bug 8336 Support Sip Renewal Transaction

[Colin Campbell]

Renewals were being rejected for incorrect reasons
Checking was being done against the wrong object
Add more informative messages on failure
Correctly set due_date for renewal response
---
 C4/SIP/ILS.pm                   | 15 +++++----------
 C4/SIP/ILS/Transaction/Renew.pm | 23 +++++++++++++++--------
 C4/SIP/Sip/MsgType.pm           |  6 +++++-
 3 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/C4/SIP/ILS.pm b/C4/SIP/ILS.pm
index f046568..5d59850 100644
--- a/C4/SIP/ILS.pm
+++ b/C4/SIP/ILS.pm
@@ -428,17 +428,12 @@ sub renew {
     if (!defined($item)) {
 		$trans->screen_msg("Item not checked out to " . $patron->name);     # not checked out to $patron_id
         $trans->ok(0);
-    } elsif (!$item->available($patron_id)) {
-		$trans->screen_msg("Item unavailable due to outstanding holds");
-        $trans->ok(0);
     } else {
-		$trans->renewal_ok(1);
-		$trans->desensitize(0);	# It's already checked out
-		$trans->do_renew();
-		syslog("LOG_DEBUG", "done renew (ok:%s): %s renews %s", $trans->renewal_ok, $patron_id, $item_id);
-
-#		$item->{due_date} = $nb_due_date if $no_block eq 'Y';
-#		$item->{sip_item_properties} = $item_props if $item_props;
+        $trans->do_renew();
+        if ($trans->renewal_ok()) {
+            $item->{due_date} = $trans->{due};
+            $trans->desensitize(0);
+        }
     }
 
     return $trans;
diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm
index 57db003..2344c77 100644
--- a/C4/SIP/ILS/Transaction/Renew.pm
+++ b/C4/SIP/ILS/Transaction/Renew.pm
@@ -36,21 +36,28 @@ sub do_renew_for  {
 	my $borrower = shift;
 	my ($renewokay,$renewerror) = CanBookBeRenewed($borrower->{borrowernumber},$self->{item}->{itemnumber});
 	if ($renewokay){
-		$self->{due} = AddIssue( $borrower, $self->{item}->id, undef, 0 );
-		$self->renewal_ok(1);
+        $self->{due} = undef;
+        my $due_date = AddIssue( $borrower, $self->{item}->id, undef, 0 );
+        if ($due_date) {
+            $self->{due} = $due_date;
+        }
+        $self->renewal_ok(1);
 	} else {
-		$self->screen_msg(($self->screen_msg || '') . " " . $renewerror);
+        $renewerror=~s/on_reserve/Item unavailable due to outstanding holds/;
+        $renewerror=~s/too_many/Item has reached maximum renewals/;
+        #$self->screen_msg(($self->screen_msg || '') . " " . $renewerror);
+		$self->screen_msg($renewerror);
 		$self->renewal_ok(0);
 	}
-    $! and warn "do_renew_for error: $!";
-	$self->ok(1) unless $!;
-	return $self;
+	$self->ok(1);
+	return;
 }
 
 sub do_renew {
 	my $self = shift;
 	my $borrower = GetMember( 'cardnumber'=>$self->{patron}->id);
-	return $self->do_renew_for($borrower);
-}	
+    $self->do_renew_for($borrower);
+    return $self;
+}
 
 1;
diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm
index f37b342..c986fe2 100644
--- a/C4/SIP/Sip/MsgType.pm
+++ b/C4/SIP/Sip/MsgType.pm
@@ -1357,7 +1357,11 @@ sub handle_renew {
 	$resp .= add_field(FID_PATRON_ID, $patron->id);
 	$resp .= add_field(FID_ITEM_ID,  $item->id);
 	$resp .= add_field(FID_TITLE_ID, $item->title_id);
-	$resp .= add_field(FID_DUE_DATE, Sip::timestamp($item->due_date));
+    if ($item->due_date) {
+        $resp .= add_field(FID_DUE_DATE, Sip::timestamp($item->due_date));
+    } else {
+        $resp .= add_field(FID_DUE_DATE, q{});
+    }
 	if ($ils->supports('security inhibit')) {
 	    $resp .= add_field(FID_SECURITY_INHIBIT,
 			       $status->security_inhibit);
-- 
1.7.11.1.104.ge7b44f1