Attachment 131046 Details for Bug 30167 – Bug 30167: Return soonest renewal date when renewal is to soon

[patch] Bug 30167: Return soonest renewal date when renewal is to soon

Bug-30167-Return-soonest-renewal-date-when-renewal.patch (text/plain), 9.57 KB, created by Nick Clemens (kidclamp) on 2022-02-23 16:15:36 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Nick Clemens (kidclamp)

Created: 2022-02-23 16:15:36 UTC

Size: 9.57 KB

patch

obsolete

>From a4ed8703c57d4e43185cd0825180c729fccff4aa Mon Sep 17 00:00:00 2001
>From: Nick Clemens <nick@bywatersolutions.com>
>Date: Wed, 23 Feb 2022 16:01:22 +0000
>Subject: [PATCH] Bug 30167: Return soonest renewal date when renewal is to
> soon
>
>This patch adds an 'info' return param to CanBookBeRenewed and passes
>the soonest renewal date when returning too_soon errors
>
>To test:
>1 - prove -v t/db_dependent/Circulation.t
>
>Fix whitespace
>---
> C4/Circulation.pm            | 25 +++++++++++++++----------
> t/db_dependent/Circulation.t | 24 ++++++++++++++++--------
> 2 files changed, 31 insertions(+), 18 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index 17cab0c74b..9c5e60dc72 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2723,7 +2723,7 @@ sub GetUpcomingDueIssues {
> 
> =head2 CanBookBeRenewed
> 
>-  ($ok,$error) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]);
>+  ($ok,$error,$info) = &CanBookBeRenewed($borrowernumber, $itemnumber[, $override_limit]);
> 
> Find out whether a borrowed item may be renewed.
> 
>@@ -2741,7 +2741,9 @@ that are automatically renewed.
> C<$CanBookBeRenewed> returns a true value if the item may be renewed. The
> item must currently be on loan to the specified borrower; renewals
> must be allowed for the item's type; and the borrower must not have
>-already renewed the loan. $error will contain the reason the renewal can not proceed
>+already renewed the loan.
>+    $error will contain the reason the renewal can not proceed
>+    $info will contain the soonest renewal date if the error is 'too soon'
> 
> =cut
> 
>@@ -2749,6 +2751,7 @@ sub CanBookBeRenewed {
>     my ( $borrowernumber, $itemnumber, $override_limit, $cron ) = @_;
> 
>     my $auto_renew = "no";
>+    my $soonest;
> 
>     my $item      = Koha::Items->find($itemnumber)      or return ( 0, 'no_item' );
>     my $issue = $item->checkout or return ( 0, 'no_checkout' );
>@@ -2793,13 +2796,13 @@ sub CanBookBeRenewed {
>             return ( 0, 'overdue');
>         }
> 
>-        $auto_renew = _CanBookBeAutoRenewed({
>+        ( $auto_renew, $soonest ) = _CanBookBeAutoRenewed({
>             patron     => $patron,
>             item       => $item,
>             branchcode => $branchcode,
>             issue      => $issue
>         });
>-        return ( 0, $auto_renew  ) if $auto_renew =~ 'auto_too_soon' && $cron;
>+        return ( 0, $auto_renew, $soonest ) if $auto_renew =~ 'auto_too_soon' && $cron;
>         # cron wants 'too_soon' over 'on_reserve' for performance and to avoid
>         # extra notices being sent. Cron also implies no override
>         return ( 0, $auto_renew  ) if $auto_renew =~ 'auto_account_expired';
>@@ -2867,9 +2870,10 @@ sub CanBookBeRenewed {
>     }
> 
>     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";
>-    if ( GetSoonestRenewDate($borrowernumber, $itemnumber) > dt_from_string() ){
>-        return (0, "too_soon") unless $override_limit;
>+    return ( 0, $auto_renew, $soonest ) if $auto_renew =~ 'too_soon';#$auto_renew ne "no" && $auto_renew ne "ok";
>+    $soonest = GetSoonestRenewDate($borrowernumber, $itemnumber);
>+    if ( $soonest > dt_from_string() ){
>+        return (0, "too_soon", $soonest ) unless $override_limit;
>     }
> 
>     return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
>@@ -4329,8 +4333,9 @@ sub _CanBookBeAutoRenewed {
> 
>     if ( defined $issuing_rule->{norenewalbefore}
>         and $issuing_rule->{norenewalbefore} ne "" ) {
>-        if ( GetSoonestRenewDate($patron->id, $item->id) > dt_from_string()) {
>-            return "auto_too_soon";
>+        my $soonest = GetSoonestRenewDate($patron->id, $item->id);
>+        if ( $soonest > dt_from_string()) {
>+            return ( "auto_too_soon", $soonest );
>         } else {
>             return "ok";
>         }
>@@ -4342,7 +4347,7 @@ sub _CanBookBeAutoRenewed {
>     if ( $now >= dt_from_string( $issue->date_due, 'sql' ) ){
>         return "ok";
>     } else {
>-        return "auto_too_soon";
>+        return ( "auto_too_soon", $issue->date_due );
>     }
> }
> 
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index 3b0b09b83f..eed9b42dea 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -418,7 +418,7 @@ subtest "GetIssuingCharges tests" => sub {
> 
> my ( $reused_itemnumber_1, $reused_itemnumber_2 );
> subtest "CanBookBeRenewed tests" => sub {
>-    plan tests => 93;
>+    plan tests => 100;
> 
>     C4::Context->set_preference('ItemsDeniedRenewal','');
>     # Generate test biblio
>@@ -794,11 +794,13 @@ subtest "CanBookBeRenewed tests" => sub {
>     );
> 
>     $issue = AddIssue( $renewing_borrower, $item_4->barcode, undef, undef, undef, undef, { auto_renew => 1 } );
>-    ( $renewokay, $error ) =
>+    my $info;
>+    ( $renewokay, $error, $info ) =
>       CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>     is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>     is( $error, 'auto_too_soon',
>         'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = undef (returned code is auto_too_soon)' );
>+    is( $info, $issue->date_due, "Due date is returned as earliest renewal date when error is 'auto_too_soon'" );
>     AddReserve(
>         {
>             branchcode       => $branch,
>@@ -817,9 +819,10 @@ subtest "CanBookBeRenewed tests" => sub {
>     ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>     is( $renewokay, 0, 'Still should not be able to renew' );
>     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 );
>+    ( $renewokay, $error, $info ) = 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' );
>+    is( $info, $issue->date_due, "Due date is returned as earliest renewal date when error is 'auto_too_soon'" );
>     ( $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' );
>@@ -852,39 +855,44 @@ subtest "CanBookBeRenewed tests" => sub {
>         }
>     );
> 
>-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
>+    ( $renewokay, $error, $info ) = 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( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'");
> 
>     # Bug 14101
>     # Test premature automatic renewal
>-    ( $renewokay, $error ) =
>+    ( $renewokay, $error, $info ) =
>       CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>     is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>     is( $error, 'auto_too_soon',
>         'Bug 14101: Cannot renew, renewal is automatic and premature (returned code is auto_too_soon)'
>     );
>+    is( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'auto_too_soon'");
> 
>     $renewing_borrower_obj->autorenew_checkouts(0)->store;
>-    ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>+    ( $renewokay, $error, $info ) = 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( $info, dt_from_string($issue->date_due)->subtract( days => 7 ), "Soonest renew date returned when error is 'too_soon'");
>     $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)
>     # and test automatic renewal again
>     $dbh->do(q{UPDATE circulation_rules SET rule_value = '0' WHERE rule_name = 'norenewalbefore'});
>-    ( $renewokay, $error ) =
>+    ( $renewokay, $error, $info ) =
>       CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>     is( $renewokay, 0, 'Bug 14101: Cannot renew, renewal is automatic and premature' );
>     is( $error, 'auto_too_soon',
>         'Bug 14101: Cannot renew, renewal is automatic and premature, "No renewal before" = 0 (returned code is auto_too_soon)'
>     );
>+    is( $info, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'");
> 
>     $renewing_borrower_obj->autorenew_checkouts(0)->store;
>-    ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber );
>+    ( $renewokay, $error, $info ) = 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( $info, dt_from_string($issue->date_due), "Soonest renew date returned when error is 'auto_too_soon'");
>     $renewing_borrower_obj->autorenew_checkouts(1)->store;
> 
>     # Change policy so that loans can be renewed 99 days prior to the due date
>-- 
>2.30.2

Actions: View | Diff | Splinter Review

Attachments on bug 30167: 131046 | 131047 | 134082 | 134083 | 134197 | 134198 | 134380 | 134454 | 134455 | 134456 | 134576 | 134577 | 134578 | 134579 | 135016