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

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
This commit is contained in:
Colin Campbell 2012-06-29 18:43:28 +01:00 committed by Chris Cormack
parent 0bf583cc42
commit 74eaf9bfdb
4 changed files with 72 additions and 63 deletions

View File

@ -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;

View File

@ -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;

View File

@ -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;

View File

@ -1344,7 +1344,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')) {
@ -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);