Attachment 109680 Details for Bug 25758 – [19.11] Bug 25758: Return on_reserve over too_soon when not calling from automatic_renewals cron

[patch] [19.11] Bug 25758: Return on_reserve over too_soon when not calling from automatic_renewals cron

1911-Bug-25758-Return-onreserve-over-toosoon-when-.patch (text/plain), 6.91 KB, created by Martin Renvoize (ashimema) on 2020-09-04 16:31:03 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Martin Renvoize (ashimema)

Created: 2020-09-04 16:31:03 UTC

Size: 6.91 KB

patch

obsolete

>From 31665e7588118de4ee3c31ecceb8f481f6b4b0e8 Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Fri, 4 Sep 2020 12:47:58 +0000
>Subject: [PATCH] [19.11] Bug 25758: Return on_reserve over too_soon when not
> calling from automatic_renewals cron
>
>Bug 19014 altered CanBookBeRenewed to return (auto_)too_soon over on_reserve
>
>For cron purposes this is the correct behaviour.
>
>For display purposes we wish to see on_reserve over too_soon
>
>This patchset adds a switch to 'CanBookBeRenewed' to alter the priority of these statuses
>
>To test:
> 1 - set NoRenewalBeforePrecision to date only
> 2 - set a circ rule to auto-renewal=yes, no renewal before=0, checkout period to 7 days
> 3 - check item out
> 4 - confirm item shows Scheduled For Automatic Renewal in issues table
> 5 - place a hold on the item for another patron
> 6 - reload issues table for patron 1, confirm checkout still shows "scheduled for automatic renewal" rather than "on hold"
> 7 - change No Renewal Before value to 7
> 8 - reload issues table for patron 1, confirm checkout now shows "on hold"
> 9 - Apply patch
>10 - restart_all
>11 - Reload the issues table - confirm 'on_hold' still shows
>12 - Change No Renewal Before to 0
>13 - Refresh issues table, still shows 'On hold'
>14 - perl misc/cronjobs/automatic_renewals.pl -v
>15 - Result shows 'auto_too_soon'
>16 - prove -v t/db_dependent/Circulation.t
>
>Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
>---
> C4/Circulation.pm                   | 26 ++++++++++++++++----------
> misc/cronjobs/automatic_renewals.pl |  2 +-
> t/db_dependent/Circulation.t        | 10 ++++++++--
> 3 files changed, 25 insertions(+), 13 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 0cedf80d9c..97695c385f 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2683,11 +2683,11 @@ already renewed the loan. $error will contain the reason the renewal can not pro
> =cut
> 
> sub CanBookBeRenewed {
>-    my ( $borrowernumber, $itemnumber, $override_limit ) = @_;
>+    my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_;
> 
>     my $dbh    = C4::Context->dbh;
>     my $renews = 1;
>-    my $auto_renew = 0;
>+    my $auto_renew = "no";
> 
>     my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
>     my $issue = $item->checkout or return ( 0, 'no_checkout' );
>@@ -2778,22 +2778,21 @@ sub CanBookBeRenewed {
> 
>             if ( $soonestrenewal > dt_from_string() )
>             {
>-                return ( 0, "auto_too_soon" ) if $issue->auto_renew;
>-                return ( 0, "too_soon" );
>+                $auto_renew = ($issue->auto_renew) ? "auto_too_soon" : "too_soon";
>             }
>             elsif ( $issue->auto_renew ) {
>-                $auto_renew = 1;
>+                $auto_renew = 'ok';
>             }
>         }
> 
>         # Fallback for automatic renewals:
>         # If norenewalbefore is undef, don't renew before due date.
>-        if ( $issue->auto_renew && !$auto_renew ) {
>+        if ( $issue->auto_renew && $auto_renew eq "no" ) {
>             my $now = dt_from_string;
>             if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
>-                $auto_renew = 1;
>+                $auto_renew = "ok";
>             } else {
>-                return ( 0, "auto_too_soon" );
>+                $auto_renew = "auto_too_soon";
>             }
>         }
>     }
>@@ -2863,8 +2862,15 @@ 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
>+    if( $cron ) { #The cron wants to return 'too_soon' over 'on_reserve'
>+        return ( 0, $auto_renew  ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
>+        return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
>+    } else { # For other purposes we want 'on_reserve' before 'too_soon'
>+        return ( 0, "on_reserve" ) if $resfound;    # '' when no hold was found
>+        return ( 0, $auto_renew  ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
>+    }
>+
>+    return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
> 
>     return ( 1, undef );
> }
>diff --git a/misc/cronjobs/automatic_renewals.pl b/misc/cronjobs/automatic_renewals.pl
>index fb1cf03ba9..92ed6f0ad8 100755
>--- a/misc/cronjobs/automatic_renewals.pl
>+++ b/misc/cronjobs/automatic_renewals.pl
>@@ -76,7 +76,7 @@ my %report;
> 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 );
>+    my ( $ok, $error ) = CanBookBeRenewed( $auto_renew->borrowernumber, $auto_renew->itemnumber, undef, 1 );
>     if ( $error eq 'auto_renew' ) {
>         my $date_due = AddRenewal( $auto_renew->borrowernumber, $auto_renew->itemnumber, $auto_renew->branchcode );
>         $auto_renew->auto_renew_error(undef)->store;
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 9b53952774..6d18724b34 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -279,7 +279,7 @@ $dbh->do(
> 
> my ( $reused_itemnumber_1, $reused_itemnumber_2 );
> subtest "CanBookBeRenewed tests" => sub {
>-    plan tests => 77;
>+    plan tests => 81;
> 
>     C4::Context->set_preference('ItemsDeniedRenewal','');
>     # Generate test biblio
>@@ -615,10 +615,16 @@ 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, 'on_reserve', 'returned code is on_reserve, reserve checked when not checking for cron' );
>+    ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 );
>+    is( $renewokay, 0, 'Still should not be able to renew' );
>+    is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked when checking for cron' );
>     ( $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' );
>+    ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1, 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' );
>     $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' );
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

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