Attachment 107879 Details for Bug 25758 – Bug 25758: Display the different causes of non renewability

[patch] Bug 25758: Display the different causes of non renewability

Bug-25758-Display-the-different-causes-of-non-rene.patch (text/plain), 42.44 KB, created by Owen Leonard on 2020-08-06 12:24:17 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2020-08-06 12:24:17 UTC

Size: 42.44 KB

patch

obsolete

>From 776e2d580f5143cecc34a304027a3d83aa5022f1 Mon Sep 17 00:00:00 2001
>From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
>Date: Wed, 24 Jun 2020 11:08:49 +0200
>Subject: [PATCH] Bug 25758: Display the different causes of non renewability
>
>Bug 25758: Fix handle of checkouts without holds
>
>Signed-off-by: Owen Leonard <oleonard@myacpl.org>
>---
> C4/Circulation.pm | 36 +++---
> C4/SIP/ILS/Transaction/Renew.pm | 1 +
> Koha/Account/Line.pm | 1 +
> Koha/REST/V1/Checkouts.pm | 4 +-
> circ/renew.pl | 1 +
> koha-tmpl/intranet-tmpl/prog/js/checkouts.js | 185 ++++++++++++++-------------
> misc/cronjobs/automatic_renewals.pl | 1 +
> opac/opac-renew.pl | 1 +
> opac/opac-user.pl | 3 +-
> opac/sco/sco-main.pl | 1 +
> svc/checkouts | 21 +--
> t/db_dependent/Circulation.t | 66 +++++-----
> 12 files changed, 173 insertions(+), 148 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index a9cf02ca3d..6d65d4b7e9 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -857,12 +857,13 @@ sub CanBookBeIssued {
> and C4::Context->preference('SwitchOnSiteCheckouts') ) {
> $messages{ONSITE_CHECKOUT_WILL_BE_SWITCHED} = 1;
> } else {
>- my ($CanBookBeRenewed,$renewerror) = CanBookBeRenewed(
>+ my ($CanBookBeRenewed, $renewerror) = CanBookBeRenewed(
> $patron->borrowernumber,
> $item_object->itemnumber,
> );
>+
> if ( $CanBookBeRenewed == 0 ) { # no more renewals allowed
>- if ( $renewerror eq 'onsite_checkout' ) {
>+ if ( $renewerror->[0] eq 'onsite_checkout' ) {
> $issuingimpossible{NO_RENEWAL_FOR_ONSITE_CHECKOUTS} = 1;
> }
> else {
>@@ -2780,8 +2781,9 @@ sub CanBookBeRenewed {
> 
> my $item = Koha::Items->find($itemnumber) or return ( 0, 'no_item' );
> my $issue = $item->checkout or return ( 0, 'no_checkout' );
>- return ( 0, 'onsite_checkout' ) if $issue->onsite_checkout;
>- return ( 0, 'item_denied_renewal') if _item_denied_renewal({ item => $item });
>+ my @errors;
>+ push @errors, 'onsite_checkout' if $issue->onsite_checkout;
>+ push @errors, 'item_denied_renewal' if _item_denied_renewal({ item => $item });
> 
> my $patron = $issue->patron or return;
> 
>@@ -2803,7 +2805,7 @@ sub CanBookBeRenewed {
> }
> );
> 
>- return ( 0, "too_many" )
>+ push @errors, 'too_many'
> if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
> 
> my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
>@@ -2813,15 +2815,15 @@ sub CanBookBeRenewed {
> my $hasoverdues = $patron->has_overdues;
> 
> if ( $restricted and $restrictionblockrenewing ) {
>- return ( 0, 'restriction');
>+ push @errors, 'restriction';
> } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) {
>- return ( 0, 'overdue');
>+ push @errors, 'overdue';
> }
> 
> if ( $issue->auto_renew && $patron->autorenew_checkouts ) {
> 
> if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
>- return ( 0, 'auto_account_expired' );
>+ push @errors, 'auto_account_expired';
> }
> 
> if ( defined $issuing_rule->{no_auto_renewal_after}
>@@ -2834,14 +2836,14 @@ sub CanBookBeRenewed {
> );
> my $now = dt_from_string;
> if ( $now >= $maximum_renewal_date ) {
>- return ( 0, "auto_too_late" );
>+ push @errors, 'auto_too_late';
> }
> }
> if ( defined $issuing_rule->{no_auto_renewal_after_hard_limit}
> and $issuing_rule->{no_auto_renewal_after_hard_limit} ne "" ) {
> # If no_auto_renewal_after_hard_limit is >= today, it's also too late for renewal
> if ( dt_from_string >= dt_from_string( $issuing_rule->{no_auto_renewal_after_hard_limit} ) ) {
>- return ( 0, "auto_too_late" );
>+ push @errors, 'auto_too_late';
> }
> }
> 
>@@ -2852,7 +2854,7 @@ sub CanBookBeRenewed {
> ? $patron->account->balance
> : $patron->account->outstanding_debits->total_outstanding;
> if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
>- return ( 0, "auto_too_much_oweing" );
>+ push @errors, 'auto_too_much_oweing';
> }
> }
> }
>@@ -2874,8 +2876,8 @@ sub CanBookBeRenewed {
> 
> if ( $soonestrenewal > dt_from_string() )
> {
>- return ( 0, "auto_too_soon" ) if $issue->auto_renew && $patron->autorenew_checkouts;
>- return ( 0, "too_soon" );
>+ push @errors, 'auto_too_soon' if $issue->auto_renew && $patron->autorenew_checkouts;
>+ push @errors, 'too_soon';
> }
> elsif ( $issue->auto_renew && $patron->autorenew_checkouts ) {
> $auto_renew = 1;
>@@ -2889,7 +2891,7 @@ sub CanBookBeRenewed {
> if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
> $auto_renew = 1;
> } else {
>- return ( 0, "auto_too_soon" );
>+ push @errors, 'auto_too_soon';
> }
> }
> }
>@@ -2959,8 +2961,10 @@ sub CanBookBeRenewed {
> }
> }
> }
>- return ( 0, "on_reserve" ) if $resfound; # '' when no hold was found
>- return ( 0, "auto_renew" ) if $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed
>+ push @errors, 'on_reserve' if $resfound; # '' when no hold was found
>+ push @errors, 'auto_renew' if $auto_renew && !$override_limit; # 0 if auto-renewal should not succeed
>+
>+ return ( 0, \@errors ) if @errors;
> 
> return ( 1, undef );
> }
>diff --git a/C4/SIP/ILS/Transaction/Renew.pm b/C4/SIP/ILS/Transaction/Renew.pm
>index 7a21c89144..ac5dfca6a7 100644
>--- a/C4/SIP/ILS/Transaction/Renew.pm
>+++ b/C4/SIP/ILS/Transaction/Renew.pm
>@@ -49,6 +49,7 @@ sub do_renew_for {
> $self->{due} = $self->duedatefromissue($issue, $self->{item}->{itemnumber});
> $self->renewal_ok(1);
> } else {
>+ $renewerror = $renewerror->[0];
> $renewerror=~s/on_reserve/Item unavailable due to outstanding holds/;
> $renewerror=~s/too_many/Item has reached maximum renewals/;
> $renewerror=~s/item_denied_renewal/Item renewal is not allowed/;
>diff --git a/Koha/Account/Line.pm b/Koha/Account/Line.pm
>index 7ed59a9ed0..1d0e0ad7f4 100644
>--- a/Koha/Account/Line.pm
>+++ b/Koha/Account/Line.pm
>@@ -852,6 +852,7 @@ sub renew_item {
> $borrowernumber,
> $itemnumber
> );
>+ $error = $error->[0] if @$error;
> if ( $can_renew ) {
> my $due_date = C4::Circulation::AddRenewal(
> $borrowernumber,
>diff --git a/Koha/REST/V1/Checkouts.pm b/Koha/REST/V1/Checkouts.pm
>index 9d5e5b42a1..412a74561c 100644
>--- a/Koha/REST/V1/Checkouts.pm
>+++ b/Koha/REST/V1/Checkouts.pm
>@@ -165,7 +165,7 @@ sub renew {
> if (!$can_renew) {
> return $c->render(
> status => 403,
>- openapi => { error => "Renewal not authorized ($error)" }
>+ openapi => { error => sprintf("Renewal not authorized (%s)", $error->[0]) }
> );
> }
> 
>@@ -206,6 +206,8 @@ sub allows_renewal {
> my ($can_renew, $error) = C4::Circulation::CanBookBeRenewed(
> $checkout->borrowernumber, $checkout->itemnumber);
> 
>+ $error = $error->[0] if @$error;
>+
> my $renewable = Mojo::JSON->false;
> $renewable = Mojo::JSON->true if $can_renew;
> 
>diff --git a/circ/renew.pl b/circ/renew.pl
>index c3f1d2a4b3..8f0013c0ac 100755
>--- a/circ/renew.pl
>+++ b/circ/renew.pl
>@@ -71,6 +71,7 @@ if ($barcode) {
> CanBookBeRenewed( $borrower->borrowernumber(),
> $item->itemnumber(), $override_limit );
> 
>+ $error = $error->[0] if @$error;
> if ( $error && ($error eq 'on_reserve') ) {
> if ($override_holds) {
> $can_renew = 1;
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>index 41c3562a67..531696881a 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>@@ -419,98 +419,107 @@ $(document).ready(function() {
> 
> if ( oObj.can_renew ) {
> // Do nothing
>- } else if ( oObj.can_renew_error == "on_reserve" ) {
>- msg += ""
>- + "<a href='/cgi-bin/koha/reserve/request.pl?biblionumber=" + oObj.biblionumber + "'>" + ON_HOLD + "</a>"
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed-on_reserve";
>- } else if ( oObj.can_renew_error == "too_many" ) {
>- msg += ""
>- + NOT_RENEWABLE
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "restriction" ) {
>- msg += ""
>- + NOT_RENEWABLE_RESTRICTION
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "overdue" ) {
>- msg += ""
>- + NOT_RENEWABLE_OVERDUE
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "too_soon" ) {
>- msg += ""
>- + NOT_RENEWABLE_TOO_SOON.format( oObj.can_renew_date )
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "auto_too_soon" ) {
>- msg += ""
>- + NOT_RENEWABLE_AUTO_TOO_SOON
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "auto_too_late" ) {
>- msg += ""
>- + NOT_RENEWABLE_AUTO_TOO_LATE
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "auto_too_much_oweing" ) {
>- msg += ""
>- + NOT_RENEWABLE_AUTO_TOO_MUCH_OWEING
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "auto_account_expired" ) {
>- msg += ""
>- + NOT_RENEWABLE_AUTO_ACCOUNT_EXPIRED
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "auto_renew" ) {
>- msg += ""
>- + NOT_RENEWABLE_AUTO_RENEW
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>- } else if ( oObj.can_renew_error == "onsite_checkout" ) {
>- // Don't display something if it's an onsite checkout
>- } else if ( oObj.can_renew_error == "item_denied_renewal" ) {
>- content += ""
>- + NOT_RENEWABLE_DENIED
>- + "";
>-
>- span_style = "display: none";
>- span_class = "renewals-allowed";
> } else {
>- msg += ""
>- + oObj.can_renew_error
>- + "";
>+ $(oObj.can_renew_error).each(function(i, error){
>+ if ( error == "on_reserve" ) {
>+ msg += ""
>+ + "<a href='/cgi-bin/koha/reserve/request.pl?biblionumber=" + oObj.biblionumber + "'>" + ON_HOLD + "</a>"
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed-on_reserve";
>+ }
>+ if ( error == "too_many" ) {
>+ msg += ""
>+ + NOT_RENEWABLE
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "restriction" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_RESTRICTION
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "overdue" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_OVERDUE
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "too_soon" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_TOO_SOON.format( oObj.can_renew_date )
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "auto_too_soon" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_AUTO_TOO_SOON
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "auto_too_late" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_AUTO_TOO_LATE
>+ + "";
> 
>- span_style = "display: none";
>- span_class = "renewals-allowed";
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "auto_too_much_oweing" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_AUTO_TOO_MUCH_OWEING
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "auto_account_expired" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_AUTO_ACCOUNT_EXPIRED
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "auto_renew" ) {
>+ msg += ""
>+ + NOT_RENEWABLE_AUTO_RENEW
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ if ( error == "onsite_checkout" ) {
>+ // Don't display something if it's an onsite checkout
>+ }
>+ if ( error == "item_denied_renewal" ) {
>+ content += ""
>+ + NOT_RENEWABLE_DENIED
>+ + "";
>+
>+ span_style = "display: none";
>+ span_class = "renewals-allowed";
>+ }
>+ msg += " ";
>+ });
> }
> 
> var can_force_renew = ( oObj.onsite_checkout == 0 ) &&
>- ( oObj.can_renew_error != "on_reserve" || (oObj.can_renew_error == "on_reserve" && AllowRenewalOnHoldOverride))
>+ ( !oObj.can_renew_error || !oObj.can_renew_error.includes("on_reserve") || (oObj.can_renew_error.includes("on_reserve") && AllowRenewalOnHoldOverride))
> ? true : false;
>- var can_renew = ( oObj.renewals_remaining > 0 && !oObj.can_renew_error );
>+ var can_renew = ( oObj.renewals_remaining > 0 && ( !oObj.can_renew_error || !oObj.can_renew_error.length ) );
> content += "";
> if ( can_renew || can_force_renew ) {
> content += "" + oObj.renewals_count + "";
>@@ -519,7 +528,7 @@ $(document).ready(function() {
> if ( oObj.date_due_overdue && can_renew ) {
> content += "checked='checked' ";
> }
>- if (oObj.can_renew_error == "on_reserve") {
>+ if (oObj.can_renew_error && oObj.can_renew_error.includes("on_reserve")) {
> content += "data-on-reserve ";
> }
> content += "class='renew' id='renew_" + oObj.itemnumber + "' name='renew' value='" + oObj.itemnumber +"'/>"
>@@ -541,7 +550,7 @@ $(document).ready(function() {
> "bSortable": false,
> "bVisible": AllowCirculate ? true : false,
> "mDataProp": function ( oObj ) {
>- if ( oObj.can_renew_error == "on_reserve" ) {
>+ if ( oObj.can_renew_error && oObj.can_renew_error.includes("on_reserve") ) {
> return "<a href='/cgi-bin/koha/reserve/request.pl?biblionumber=" + oObj.biblionumber + "'>" + ON_HOLD + "</a>";
> } else {
> return "<input type='checkbox' class='checkin' id='checkin_" + oObj.itemnumber + "' name='checkin' value='" + oObj.itemnumber +"'></input>";
>diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl
>index a76f7dfb35..ea7526f040 100755
>--- a/misc/cronjobs/automatic_renewals.pl
>+++ b/misc/cronjobs/automatic_renewals.pl
>@@ -89,6 +89,7 @@ while ( my $auto_renew = $auto_renews->next ) {
> 
> # CanBookBeRenewed returns 'auto_renew' when the renewal should be done by this script
> my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber );
>+ $error = $error->[0] if @$error;
> if ( $error eq 'auto_renew' ) {
> if ($verbose) {
> say sprintf "Issue id: %s for borrower: %s and item: %s ". ( $confirm ? 'will' : 'would') . " be renewed.",
>diff --git a/opac/opac-renew.pl b/opac/opac-renew.pl
>index cb9043d9ad..2246725dff 100755
>--- a/opac/opac-renew.pl
>+++ b/opac/opac-renew.pl
>@@ -62,6 +62,7 @@ else {
> for my $itemnumber (@items) {
> my ( $status, $error ) =
> CanBookBeRenewed( $borrowernumber, $itemnumber );
>+ $error = $error->[0] if @$error;
> if ( $status == 1 && $opacrenew == 1 ) {
> AddRenewal( $borrowernumber, $itemnumber, undef, undef, undef );
> push( @renewed, $itemnumber );
>diff --git a/opac/opac-user.pl b/opac/opac-user.pl
>index 6d967c8803..d931ee9142 100755
>--- a/opac/opac-user.pl
>+++ b/opac/opac-user.pl
>@@ -232,7 +232,8 @@ if ( $pending_checkouts->count ) { # Useless test
> 
> $issue->{'renewed'} = $renewed{ $issue->{'itemnumber'} };
> 
>- if ($renewerror) {
>+ if (@$renewerror) {
>+ $renewerror = $renewerror->[0];
> $issue->{'too_many'} = 1 if $renewerror eq 'too_many';
> $issue->{'on_reserve'} = 1 if $renewerror eq 'on_reserve';
> $issue->{'norenew_overdue'} = 1 if $renewerror eq 'overdue';
>diff --git a/opac/sco/sco-main.pl b/opac/sco/sco-main.pl
>index 83a37aca46..0dc452fe51 100755
>--- a/opac/sco/sco-main.pl
>+++ b/opac/sco/sco-main.pl
>@@ -273,6 +273,7 @@ if ($borrower) {
> $borrower->{borrowernumber},
> $checkout->{itemnumber},
> );
>+ $renew_error = $renew_error->[0] if @$renew_error;
> $checkout->{can_be_renewed} = $can_be_renewed; # In the future this will be $checkout->can_be_renewed
> $checkout->{renew_error} = $renew_error;
> $checkout->{overdue} = $c->is_overdue;
>diff --git a/svc/checkouts b/svc/checkouts
>index fa470b3a2a..081d99fb05 100755
>--- a/svc/checkouts
>+++ b/svc/checkouts
>@@ -21,6 +21,7 @@ use Modern::Perl;
> 
> use CGI;
> use JSON qw(to_json);
>+use List::MoreUtils qw( uniq );
> 
> use C4::Auth qw(check_cookie_auth haspermission get_session);
> use C4::Circulation qw(GetIssuingCharges CanBookBeRenewed GetRenewCount GetSoonestRenewDate);
>@@ -148,15 +149,17 @@ while ( my $c = $sth->fetchrow_hashref() ) {
> 
> my ( $can_renew, $can_renew_error ) =
> CanBookBeRenewed( $c->{borrowernumber}, $c->{itemnumber} );
>- my $can_renew_date =
>- $can_renew_error && $can_renew_error eq 'too_soon'
>- ? output_pref(
>- {
>- dt => GetSoonestRenewDate( $c->{borrowernumber}, $c->{itemnumber} ),
>- as_due_date => 1
>- }
>- )
>- : undef;
>+
>+ my $can_renew_date;
>+ $can_renew_error &&= [uniq @$can_renew_error];
>+ if ( $can_renew_error && grep { $_ eq 'too_soon' } @$can_renew_error ) {
>+ $can_renew_date = output_pref(
>+ {
>+ dt => GetSoonestRenewDate( $c->{borrowernumber}, $c->{itemnumber} ),
>+ as_due_date => 1
>+ }
>+ );
>+ }
> 
> my ( $renewals_count, $renewals_allowed, $renewals_remaining ) =
> GetRenewCount( $c->{borrowernumber}, $c->{itemnumber} );
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index bb2ff5f978..179ba0ab99 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -452,11 +452,11 @@ subtest "CanBookBeRenewed tests" => sub {
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
> is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved');
>- is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)');
>+ is( $error->[0], 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)');
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber);
> is( $renewokay, 0, '(Bug 10663) Cannot renew, reserved');
>- is( $error, 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)');
>+ is( $error->[0], 'on_reserve', '(Bug 10663) Cannot renew, reserved (returned error is on_reserve)');
> 
> my $reserveid = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next->reserve_id;
> my $reserving_borrower = Koha::Patrons->find( $reserving_borrowernumber )->unblessed;
>@@ -485,7 +485,7 @@ subtest "CanBookBeRenewed tests" => sub {
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber, 1);
> is( $renewokay, 0, '(Bug 10663) Cannot renew, item reserved');
>- is( $error, 'on_reserve', '(Bug 10663) Cannot renew, item reserved (returned error is on_reserve)');
>+ is( $error->[0], 'on_reserve', '(Bug 10663) Cannot renew, item reserved (returned error is on_reserve)');
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_2->itemnumber, 1);
> is( $renewokay, 1, 'Can renew item 2, item-level hold is on item 1');
>@@ -629,7 +629,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>- is( $error, 'auto_too_soon',
>+ is( $error->[0], 'auto_too_soon',
> 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' );
> AddReserve(
> {
>@@ -648,14 +648,14 @@ subtest "CanBookBeRenewed tests" => sub {
> );
> ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'Still should not be able to renew' );
>- is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked' );
>+ is( $error->[0], 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked' );
> ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 );
> is( $renewokay, 0, 'Still should not be able to renew' );
>- is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' );
>+ is( $error->[0], 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' );
> $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"');
> ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 );
> is( $renewokay, 0, 'Still should not be able to renew' );
>- is( $error, 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' );
>+ is( $error->[0], 'on_reserve', 'returned code is on_reserve, auto_renew only happens if not on reserve' );
> ModReserveCancelAll($item_4->itemnumber, $reserving_borrowernumber);
> 
> 
>@@ -680,7 +680,7 @@ subtest "CanBookBeRenewed tests" => sub {
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
> is( $renewokay, 0, 'Bug 7413: Cannot renew, renewal is premature');
>- is( $error, 'too_soon', 'Bug 7413: Cannot renew, renewal is premature (returned code is too_soon)');
>+ is( $error->[0], 'too_soon', 'Bug 7413: Cannot renew, renewal is premature (returned code is too_soon)');
> 
> # Bug 14395
> # Test 'exact time' setting for syspref NoRenewalBeforePrecision
>@@ -705,14 +705,14 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>- is( $error, 'auto_too_soon',
>+ is( $error->[0], 'auto_too_soon',
> 'Bug 14101: Cannot renew, renewal is automatic and premature (returned code is auto_too_soon)'
> );
> 
> $renewing_borrower_obj->autorenew_checkouts(0)->store;
> ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'No renewal before is 7, patron opted out of auto_renewal still cannot renew early' );
>- is( $error, 'too_soon', 'Error is too_soon, no auto' );
>+ is( $error->[0], 'too_soon', 'Error is too_soon, no auto' );
> $renewing_borrower_obj->autorenew_checkouts(1)->store;
> 
> # Change policy so that loans can only be renewed exactly on due date (0 days prior to due date)
>@@ -721,14 +721,14 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>- is( $error, 'auto_too_soon',
>+ is( $error->[0], 'auto_too_soon',
> 'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = 0 (returned code is auto_too_soon)'
> );
> 
> $renewing_borrower_obj->autorenew_checkouts(0)->store;
> ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'No renewal before is 0, patron opted out of auto_renewal still cannot renew early' );
>- is( $error, 'too_soon', 'Error is too_soon, no auto' );
>+ is( $error->[0], 'too_soon', 'Error is too_soon, no auto' );
> $renewing_borrower_obj->autorenew_checkouts(1)->store;
> 
> # Change policy so that loans can be renewed 99 days prior to the due date
>@@ -737,7 +737,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
> is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic' );
>- is( $error, 'auto_renew',
>+ is( $error->[0], 'auto_renew',
> 'Bug 14101: Cannot renew, renewal is automatic (returned code is auto_renew)'
> );
> 
>@@ -776,7 +776,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
>+ is( $error->[0], 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -792,7 +792,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_late', 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)' );
>+ is( $error->[0], 'auto_too_late', 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -808,7 +808,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_soon', 'Cannot auto renew, too soon - no_auto_renewal_after is defined(returned code is auto_too_soon)' );
>+ is( $error->[0], 'auto_too_soon', 'Cannot auto renew, too soon - no_auto_renewal_after is defined(returned code is auto_too_soon)' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -824,7 +824,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Cannot renew, renew is automatic' );
>+ is( $error->[0], 'auto_renew', 'Cannot renew, renew is automatic' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -841,7 +841,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
>+ is( $error->[0], 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -858,7 +858,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
>+ is( $error->[0], 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' );
> 
> Koha::CirculationRules->set_rules(
> {
>@@ -875,7 +875,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Cannot renew, renew is automatic' );
>+ is( $error->[0], 'auto_renew', 'Cannot renew, renew is automatic' );
> };
> 
> subtest "auto_too_much_oweing | OPACFineNoRenewalsBlockAutoRenew & OPACFineNoRenewalsIncludeCredit" => sub {
>@@ -921,7 +921,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 5' );
>+ is( $error->[0], 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 5' );
> 
> $account->add_debit(
> {
>@@ -935,7 +935,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 10' );
>+ is( $error->[0], 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 10' );
> 
> $account->add_debit(
> {
>@@ -949,7 +949,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, patron has 15' );
>+ is( $error->[0], 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, patron has 15' );
> 
> $account->add_credit(
> {
>@@ -962,13 +962,13 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' );
>+ is( $error->[0], 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' );
> 
> C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','0');
> ( $renewokay, $error ) =
> CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' );
>+ is( $error->[0], 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' );
> 
> $dbh->do('DELETE FROM accountlines WHERE borrowernumber=?', undef, $renewing_borrowernumber);
> C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','1');
>@@ -1008,7 +1008,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' );
>+ is( $error->[0], 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' );
> Koha::Checkouts->find( $checkout->issue_id )->delete;
> 
> 
>@@ -1020,7 +1020,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' );
>+ is( $error->[0], 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' );
> Koha::Checkouts->find( $checkout->issue_id )->delete;
> 
> 
>@@ -1032,7 +1032,7 @@ subtest "CanBookBeRenewed tests" => sub {
> ( $renewokay, $error ) =
> CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} );
> is( $renewokay, 0, 'Do not renew, renewal is automatic' );
>- is( $error, 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' );
>+ is( $error->[0], 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' );
> Koha::Checkouts->find( $checkout->issue_id )->delete;
> };
> 
>@@ -1158,7 +1158,7 @@ subtest "CanBookBeRenewed tests" => sub {
> 
> ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
> is( $renewokay, 0, 'Cannot renew, 0 renewals allowed');
>- is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)');
>+ is( $error->[0], 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)');
> 
> # Test WhenLostForgiveFine and WhenLostChargeReplacementFee
> t::lib::Mocks::mock_preference('WhenLostForgiveFine','1');
>@@ -1547,7 +1547,7 @@ subtest "AllowRenewalIfOtherItemsAvailable tests" => sub {
> my $issue = AddIssue( $borrower, $item->barcode, undef, undef, undef, undef, { onsite_checkout => 1 } );
> my ( $renewed, $error ) = CanBookBeRenewed( $borrowernumber, $item->itemnumber );
> is( $renewed, 0, 'CanBookBeRenewed should not allow to renew on-site checkout' );
>- is( $error, 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' );
>+ is( $error->[0], 'onsite_checkout', 'A correct error code should be returned by CanBookBeRenewed for on-site checkout' );
> }
> 
> {
>@@ -3415,7 +3415,7 @@ subtest 'ItemsDeniedRenewal preference' => sub {
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber );
> is( $idr_mayrenew, 0, 'Renewal blocked when 1 rules (withdrawn)' );
>- is( $idr_error, 'item_denied_renewal', 'Renewal blocked when 1 rule (withdrawn)' );
>+ is( $idr_error->[0], 'item_denied_renewal', 'Renewal blocked when 1 rule (withdrawn)' );
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $allow_issue->itemnumber );
> is( $idr_mayrenew, 1, 'Renewal allowed when 1 rules not matched (withdrawn)' );
>@@ -3427,7 +3427,7 @@ subtest 'ItemsDeniedRenewal preference' => sub {
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber );
> is( $idr_mayrenew, 0, 'Renewal blocked when 2 rules matched (withdrawn, itype)' );
>- is( $idr_error, 'item_denied_renewal', 'Renewal blocked when 2 rules matched (withdrawn,itype)' );
>+ is( $idr_error->[0], 'item_denied_renewal', 'Renewal blocked when 2 rules matched (withdrawn,itype)' );
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $allow_issue->itemnumber );
> is( $idr_mayrenew, 1, 'Renewal allowed when 2 rules not matched (withdrawn, itype)' );
>@@ -3439,7 +3439,7 @@ subtest 'ItemsDeniedRenewal preference' => sub {
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $deny_issue->itemnumber );
> is( $idr_mayrenew, 0, 'Renewal blocked when 3 rules matched (withdrawn, itype, location)' );
>- is( $idr_error, 'item_denied_renewal', 'Renewal blocked when 3 rules matched (withdrawn,itype, location)' );
>+ is( $idr_error->[0], 'item_denied_renewal', 'Renewal blocked when 3 rules matched (withdrawn,itype, location)' );
> ( $idr_mayrenew, $idr_error ) =
> CanBookBeRenewed( $idr_borrower->borrowernumber, $allow_issue->itemnumber );
> is( $idr_mayrenew, 1, 'Renewal allowed when 3 rules not matched (withdrawn, itype, location)' );
>-- 
>2.11.0

Actions: View | Diff | Splinter Review

Attachments on bug 25758: 106229 | 107409 | 107495 | 107496 | 107504 | 107879 | 107894 | 109139 | 109658 | 109668 | 109672 | 109680 | 110008 | 110009 | 110010