Bugzilla – Attachment 12035 Details for
Bug 8336
SIP Server does not process renewal transaction correctly
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Revised Patch
0001-Bug-8336-Support-Sip-Renewal-Transaction.patch (text/plain), 7.35 KB, created by
Colin Campbell
on 2012-09-07 11:45:36 UTC
(
hide
)
Description:
Revised Patch
Filename:
MIME Type:
Creator:
Colin Campbell
Created:
2012-09-07 11:45:36 UTC
Size:
7.35 KB
patch
obsolete
>From effd6d5247ab3f88cfe2be04bc1a407f50e00af5 Mon Sep 17 00:00:00 2001 >From: Colin Campbell <colin.campbell@ptfs-europe.com> >Date: Fri, 29 Jun 2012 18:43:28 +0100 >Subject: [PATCH] Bug 8336 Support Sip Renewal Transaction > >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 >Avoid crashing the SIPServer because it handles RenewAll >incorrectly >--- > C4/SIP/ILS.pm | 15 +++---- > C4/SIP/ILS/Transaction/Renew.pm | 28 +++++++------ > C4/SIP/ILS/Transaction/RenewAll.pm | 84 ++++++++++++++++++++------------------ > C4/SIP/Sip/MsgType.pm | 8 +++- > 4 files changed, 72 insertions(+), 63 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..73b811f 100644 >--- a/C4/SIP/ILS/Transaction/Renew.pm >+++ b/C4/SIP/ILS/Transaction/Renew.pm >@@ -8,12 +8,11 @@ use warnings; > use strict; > > use ILS; >-use ILS::Transaction; > > use C4::Circulation; > use C4::Members; > >-our @ISA = qw(ILS::Transaction); >+use base qw(ILS::Transaction); > > my %fields = ( > renewal_ok => 0, >@@ -36,21 +35,26 @@ 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($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); >-} >+ my $self = shift; >+ my $borrower = GetMember( cardnumber => $self->{patron}->id ); >+ return $self->do_renew_for($borrower); >+} > > 1; >diff --git a/C4/SIP/ILS/Transaction/RenewAll.pm b/C4/SIP/ILS/Transaction/RenewAll.pm >index adc467a..c7be96b 100644 >--- a/C4/SIP/ILS/Transaction/RenewAll.pm >+++ b/C4/SIP/ILS/Transaction/RenewAll.pm >@@ -1,4 +1,4 @@ >-# >+# > # RenewAll: class to manage status of "Renew All" transaction > > package ILS::Transaction::RenewAll; >@@ -9,57 +9,63 @@ use warnings; > use Sys::Syslog qw(syslog); > > use ILS::Item; >-use ILS::Transaction::Renew; > >-use C4::Members; # GetMember >+use C4::Members qw( GetMember ); > >-our @ISA = qw(ILS::Transaction::Renew); >+use base qw(ILS::Transaction::Renew); > > my %fields = ( >- renewed => [], >- unrenewed => [], >+ renewed => [], >+ unrenewed => [], > ); > > sub new { >- my $class = shift; >- my $self = $class->SUPER::new(); >+ my $class = shift; >+ my $self = $class->SUPER::new(); > >- foreach my $element (keys %fields) { >- $self->{_permitted}->{$element} = $fields{$element}; >- } >+ foreach my $element ( keys %fields ) { >+ $self->{_permitted}->{$element} = $fields{$element}; >+ } > >- @{$self}{keys %fields} = values %fields; >- return bless $self, $class; >+ @{$self}{ keys %fields } = values %fields; >+ return bless $self, $class; > } > > sub do_renew_all { >- my $self = shift; >- my $patron = $self->{patron}; # SIP's patron >- my $borrower = GetMember('cardnumber'=>$patron->id); # Koha's patron >- my $all_ok = 1; >- $self->{renewed} = []; >+ my $self = shift; >+ my $patron = $self->{patron}; # SIP's patron >+ my $borrower = GetMember( cardnumber => $patron->id ); # Koha's patron >+ my $all_ok = 1; >+ $self->{renewed} = []; > $self->{unrenewed} = []; >- foreach my $itemx (@{$patron->{items}}) { >- my $item_id = $itemx->{barcode}; >- my $item = new ILS::Item $item_id; >- if (!defined($item)) { >- syslog("LOG_WARNING", >- "renew_all: Invalid item id '%s' associated with patron '%s'", >- $item_id, $patron->id); >- $all_ok = 0; >- next; >- } >- $self->{item} = $item; >- $self->do_renew_for($borrower); >- if ($self->ok) { >- $item->{due_date} = $self->{due}->clone(); >- push @{$self->renewed }, $item_id; >- } else { >- push @{$self->{unrenewed}}, $item_id; >- } >- } >- $self->ok($all_ok); >- return $self; >+ foreach my $itemx ( @{ $patron->{items} } ) { >+ my $item_id = $itemx->{barcode}; >+ my $item = ILS::Item->new($item_id); >+ if ( !defined($item) ) { >+ syslog( >+ 'LOG_WARNING', >+ q|renew_all: Invalid item id '%s' associated with patron '%s'|, >+ $item_id, >+ $patron->id >+ ); >+ >+ # $all_ok = 0; Do net set as still ok >+ push @{ $self->unrenewed }, $item_id; >+ next; >+ } >+ $self->{item} = $item; >+ $self->do_renew_for($borrower); >+ if ( $self->renewal_ok ) { >+ $item->{due_date} = $self->{due}; >+ push @{ $self->{renewed} }, $item_id; >+ } >+ else { >+ push @{ $self->{unrenewed} }, $item_id; >+ } >+ $self->screen_msg(q{}); # clear indiv message >+ } >+ $self->ok($all_ok); >+ return $self; > } > > 1; >diff --git a/C4/SIP/Sip/MsgType.pm b/C4/SIP/Sip/MsgType.pm >index c8b39a3..43f48dc 100644 >--- a/C4/SIP/Sip/MsgType.pm >+++ b/C4/SIP/Sip/MsgType.pm >@@ -1346,7 +1346,7 @@ sub handle_renew { > $patron = $status->patron; > $item = $status->item; > >- if ($status->ok) { >+ if ($status->renewal_ok) { > $resp .= '1'; > $resp .= $status->renewal_ok ? 'Y' : 'N'; > if ($ils->supports('magnetic media')) { >@@ -1359,7 +1359,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.12.146.g16d26b1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 8336
:
10567
|
12035
|
12260