Attachment 113080 Details for Bug 24083 – Bug 24083: (follow-up) Squashed follow ups

[patch] Bug 24083: (follow-up) Squashed follow ups

Bug-24083-follow-up-Squashed-follow-ups.patch (text/plain), 25.76 KB, created by Andrew Isherwood on 2020-11-05 10:50:26 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andrew Isherwood

Created: 2020-11-05 10:50:26 UTC

Size: 25.76 KB

patch

obsolete

>From b1db15aed2ba0e912e76a886a5a3d77e50f3b5d5 Mon Sep 17 00:00:00 2001
>From: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
>Date: Mon, 5 Oct 2020 12:39:48 +0100
>Subject: [PATCH] Bug 24083: (follow-up) Squashed follow ups
>
>This squashed commit fixes:
>-  a small error in the checkouts related JS
>
>- GetRenewCount now returns 6 values when a call to it succeeds, a
>failed call should also return the same number of values. This commit
>adds these additional values.
>
>- Some changes in issue.t had broken the tests for unseen renewals (the
>unseen tests were using variables that had been moved out of the tests'
>scope).
>
>- Also now using Koha::CirculationRules::set_rules to set circ rules
>rather than using SQL queries.
>
>- Fixed expected number of return values from GetRenewCount
>
>- Moved unseen tests in issue.t to the bottom of the file to remove the
>risk of interference with other test circ rules.
>
>- There was a real mess in C4/Circulation.pm due to a bad rebase back in
>February. Frankly it's a wonder anything worked at all. This commit
>fixes that problem and reinstates the correct patch for
>C4/Circulation.pm
>
>- Somehow I'd never noticed this before but the columns in smart-rules.tt
>were misaligned when UnseenRenewals was turned off. This was due to the
>display of a <td> not being conditional when it should have been. This
>is now fixed.
>
>- This commit also fixes items 1 & 2 descibed by Katrin in comment #74 ->
>comment #76.
>
>- Fixed missing check for too_unseen in opac-user.tt, this test did used
>to exist but got lost during sizeable rebase a few weeks ago :-(
>- Added test for too_unseen to all AUTO_RENEWAL notice templates apart
>from de-DE (as previously requested by Katrin)
>
>Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
>---
> C4/Circulation.pm                             | 119 +-----------------
> .../mysql/en/mandatory/sample_notices.yml     |   2 +
> .../fr-CA/obligatoire/sample_notices.sql      |   2 +
> .../fr-FR/1-Obligatoire/sample_notices.sql    |   2 +
> .../data/mysql/it-IT/necessari/notices.sql    |   2 +
> .../nb-NO/1-Obligatorisk/sample_notices.sql   |   2 +
> .../mysql/pl-PL/mandatory/sample_notices.sql  |   2 +
> .../mysql/ru-RU/mandatory/sample_notices.sql  |   2 +
> .../mysql/uk-UA/mandatory/sample_notices.sql  |   2 +
> .../prog/en/modules/admin/smart-rules.tt      |   4 +-
> koha-tmpl/intranet-tmpl/prog/js/checkouts.js  |   6 +-
> .../bootstrap/en/modules/opac-user.tt         |   2 +
> t/db_dependent/Circulation.t                  |  28 ++++-
> t/db_dependent/Circulation/issue.t            |  64 ++++++----
> 14 files changed, 95 insertions(+), 144 deletions(-)
>
>diff --git a/C4/Circulation.pm b/C4/Circulation.pm
>index b2173fb68d..10f8306b7b 100644
>--- a/C4/Circulation.pm
>+++ b/C4/Circulation.pm
>@@ -2705,6 +2705,11 @@ sub CanBookBeRenewed {
>         return ( 0, "too_many" )
>           if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
> 
>+        return ( 0, "too_unseen" )
>+          if C4::Context->preference('UnseenRenewals') &&
>+            $issuing_rule->{unseen_renewals_allowed} &&
>+            $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
>+
>         my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
>         my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
>         $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
>@@ -2876,118 +2881,6 @@ sub CanBookBeRenewed {
> 
>     return ( 0, "auto_renew" ) if $auto_renew eq "ok" && !$override_limit; # 0 if auto-renewal should not succeed
> 
>-    return ( 1, undef ) if $override_limit;
>-
>-    my $branchcode = _GetCircControlBranch( $item->unblessed, $patron->unblessed );
>-    my $issuing_rule = Koha::CirculationRules->get_effective_rules(
>-        {
>-            categorycode => $patron->categorycode,
>-            itemtype     => $item->effective_itemtype,
>-            branchcode   => $branchcode,
>-            rules => [
>-                'renewalsallowed',
>-                'no_auto_renewal_after',
>-                'no_auto_renewal_after_hard_limit',
>-                'lengthunit',
>-                'norenewalbefore',
>-                'unseen_renewals_allowed'
>-            ]
>-        }
>-    );
>-
>-    return ( 0, "too_many" )
>-      if not $issuing_rule->{renewalsallowed} or $issuing_rule->{renewalsallowed} <= $issue->renewals;
>-
>-    return ( 0, "too_unseen" )
>-      if C4::Context->preference('UnseenRenewals') &&
>-        $issuing_rule->{unseen_renewals_allowed} &&
>-        $issuing_rule->{unseen_renewals_allowed} <= $issue->unseen_renewals;
>-
>-    my $overduesblockrenewing = C4::Context->preference('OverduesBlockRenewing');
>-    my $restrictionblockrenewing = C4::Context->preference('RestrictionBlockRenewing');
>-    $patron         = Koha::Patrons->find($borrowernumber); # FIXME Is this really useful?
>-    my $restricted  = $patron->is_debarred;
>-    my $hasoverdues = $patron->has_overdues;
>-
>-    if ( $restricted and $restrictionblockrenewing ) {
>-        return ( 0, 'restriction');
>-    } elsif ( ($hasoverdues and $overduesblockrenewing eq 'block') || ($issue->is_overdue and $overduesblockrenewing eq 'blockitem') ) {
>-        return ( 0, 'overdue');
>-    }
>-
>-    if ( $issue->auto_renew ) {
>-
>-        if ( $patron->category->effective_BlockExpiredPatronOpacActions and $patron->is_expired ) {
>-            return ( 0, 'auto_account_expired' );
>-        }
>-
>-        if ( defined $issuing_rule->{no_auto_renewal_after}
>-                and $issuing_rule->{no_auto_renewal_after} ne "" ) {
>-            # Get issue_date and add no_auto_renewal_after
>-            # If this is greater than today, it's too late for renewal.
>-            my $maximum_renewal_date = dt_from_string($issue->issuedate, 'sql');
>-            $maximum_renewal_date->add(
>-                $issuing_rule->{lengthunit} => $issuing_rule->{no_auto_renewal_after}
>-            );
>-            my $now = dt_from_string;
>-            if ( $now >= $maximum_renewal_date ) {
>-                return ( 0, "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" );
>-            }
>-        }
>-
>-        if ( C4::Context->preference('OPACFineNoRenewalsBlockAutoRenew') ) {
>-            my $fine_no_renewals = C4::Context->preference("OPACFineNoRenewals");
>-            my $amountoutstanding =
>-              C4::Context->preference("OPACFineNoRenewalsIncludeCredit")
>-              ? $patron->account->balance
>-              : $patron->account->outstanding_debits->total_outstanding;
>-            if ( $amountoutstanding and $amountoutstanding > $fine_no_renewals ) {
>-                return ( 0, "auto_too_much_oweing" );
>-            }
>-        }
>-    }
>-
>-    if ( defined $issuing_rule->{norenewalbefore}
>-        and $issuing_rule->{norenewalbefore} ne "" )
>-    {
>-
>-        # Calculate soonest renewal by subtracting 'No renewal before' from due date
>-        my $soonestrenewal = dt_from_string( $issue->date_due, 'sql' )->subtract(
>-            $issuing_rule->{lengthunit} => $issuing_rule->{norenewalbefore} );
>-
>-        # Depending on syspref reset the exact time, only check the date
>-        if ( C4::Context->preference('NoRenewalBeforePrecision') eq 'date'
>-            and $issuing_rule->{lengthunit} eq 'days' )
>-        {
>-            $soonestrenewal->truncate( to => 'day' );
>-        }
>-
>-        if ( $soonestrenewal > dt_from_string() )
>-        {
>-            return ( 0, "auto_too_soon" ) if $issue->auto_renew;
>-            return ( 0, "too_soon" );
>-        }
>-        elsif ( $issue->auto_renew ) {
>-            return ( 0, "auto_renew" );
>-        }
>-    }
>-
>-    # Fallback for automatic renewals:
>-    # If norenewalbefore is undef, don't renew before due date.
>-    if ( $issue->auto_renew ) {
>-        my $now = dt_from_string;
>-        return ( 0, "auto_renew" )
>-          if $now >= dt_from_string( $issue->date_due, 'sql' );
>-        return ( 0, "auto_too_soon" );
>-    }
>-
>     return ( 1, undef );
> }
> 
>@@ -3211,7 +3104,7 @@ sub GetRenewCount {
>     my $patron = Koha::Patrons->find( $bornum );
>     my $item   = Koha::Items->find($itemno);
> 
>-    return (0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
>+    return (0, 0, 0, 0, 0, 0) unless $patron or $item; # Wrong call, no renewal allowed
> 
>     # Look in the issues table for this item, lent to this borrower,
>     # and not yet returned.
>diff --git a/installer/data/mysql/en/mandatory/sample_notices.yml b/installer/data/mysql/en/mandatory/sample_notices.yml
>index 50bb421386..547f7cba09 100644
>--- a/installer/data/mysql/en/mandatory/sample_notices.yml
>+++ b/installer/data/mysql/en/mandatory/sample_notices.yml
>@@ -1204,6 +1204,8 @@ tables:
>             - "It's too late to renew this item."
>             - "[% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]"
>             - "Your total unpaid fines are too high."
>+            - "[% ELSIF checkout.auto_renew_error == 'too_unseen' %]"
>+            - "This item must be renewed at the library."
>             - "[% END %]"
>             - "[% ELSE %]"
>             - "The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]"
>diff --git a/installer/data/mysql/fr-CA/obligatoire/sample_notices.sql b/installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
>index 1a231dd12e..45930e1d82 100644
>--- a/installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
>+++ b/installer/data/mysql/fr-CA/obligatoire/sample_notices.sql
>@@ -285,6 +285,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql b/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>index f844c90b2a..e03e90bbb6 100644
>--- a/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>+++ b/installer/data/mysql/fr-FR/1-Obligatoire/sample_notices.sql
>@@ -396,6 +396,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/it-IT/necessari/notices.sql b/installer/data/mysql/it-IT/necessari/notices.sql
>index 9be907b0b5..f840c8bd91 100644
>--- a/installer/data/mysql/it-IT/necessari/notices.sql
>+++ b/installer/data/mysql/it-IT/necessari/notices.sql
>@@ -399,6 +399,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql b/installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
>index e3564ffb1d..a24c56b225 100644
>--- a/installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
>+++ b/installer/data/mysql/nb-NO/1-Obligatorisk/sample_notices.sql
>@@ -414,6 +414,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/pl-PL/mandatory/sample_notices.sql b/installer/data/mysql/pl-PL/mandatory/sample_notices.sql
>index f228b5c68d..22d00e5a1c 100644
>--- a/installer/data/mysql/pl-PL/mandatory/sample_notices.sql
>+++ b/installer/data/mysql/pl-PL/mandatory/sample_notices.sql
>@@ -393,6 +393,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/ru-RU/mandatory/sample_notices.sql b/installer/data/mysql/ru-RU/mandatory/sample_notices.sql
>index 27f002b195..1058414e2c 100644
>--- a/installer/data/mysql/ru-RU/mandatory/sample_notices.sql
>+++ b/installer/data/mysql/ru-RU/mandatory/sample_notices.sql
>@@ -395,6 +395,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/installer/data/mysql/uk-UA/mandatory/sample_notices.sql b/installer/data/mysql/uk-UA/mandatory/sample_notices.sql
>index 17bd9f7606..3c275d3ffb 100644
>--- a/installer/data/mysql/uk-UA/mandatory/sample_notices.sql
>+++ b/installer/data/mysql/uk-UA/mandatory/sample_notices.sql
>@@ -486,6 +486,8 @@ You have overdue items.
> It\'s too late to renew this item.
> [% ELSIF checkout.auto_renew_error == 'auto_too_much_oweing' %]
> Your total unpaid fines are too high.
>+[% ELSIF checkout.auto_renew_error == 'too_unseen' %]
>+This item must be renewed at the library.
> [% END %]
> [% ELSE %]
> The following item, [% biblio.title %], has correctly been renewed and is now due on [% checkout.date_due | $KohaDates as_due_date => 1 %]
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>index 8753055a84..6c59352c4c 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
>@@ -413,7 +413,9 @@
>                     <td><input type="text" name="maxsuspensiondays" id="maxsuspensiondays" size="3" /> </td>
>                     <td><input type="text" name="suspension_chargeperiod" id="suspension_chargeperiod" size="3" /> </td>
>                     <td><input type="text" name="renewalsallowed" id="renewalsallowed" size="2" /></td>
>-                    <td><input type="text" name="unseen_renewals_allowed" id="unseen_renewals_allowed" size="2" /></td>
>+                    [% IF Koha.Preference('UnseenRenewals') %]
>+                        <td><input type="text" name="unseen_renewals_allowed" id="unseen_renewals_allowed" size="2" /></td>
>+                    [% END %]
>                     <td><input type="text" name="renewalperiod" id="renewalperiod" size="3" /></td>
>                     <td><input type="text" name="norenewalbefore" id="norenewalbefore" size="3" /></td>
>                     <td>
>diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>index 52daaacb87..3afff2336d 100644
>--- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>+++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js
>@@ -559,13 +559,11 @@ $(document).ready(function() {
>                             content += "<span class='renewals'>(";
>                             content + __("%s of %s renewals remaining").format(oObj.renewals_remaining, oObj.renewals_allowed);
>                             if (UnseenRenewals && oObj.unseen_allowed) {
>-                                content += __(" / %s of %s unseen renewals remaining").format(oObj.unseen_remaining, oObj.unseen_allowed);
>+                                content += __("%s of %s unseen renewals remaining").format(oObj.unseen_remaining, oObj.unseen_allowed);
>                             }
>-                                    + ")</span>";
>+                            content += ")</span>";
>                         }
> 
>-                        content += "</span>";
>-
>                         return content;
>                     }
>                 },
>diff --git a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
>index 705a7f44ec..c25db59397 100644
>--- a/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
>+++ b/koha-tmpl/opac-tmpl/bootstrap/en/modules/opac-user.tt
>@@ -378,6 +378,8 @@
>                                                             Not renewable <span class="renewals">(on hold)</span>
>                                                         [% ELSIF ( ISSUE.too_many ) %]
>                                                             Not renewable
>+                                                        [% ELSIF ( ISSUE.too_unseen ) %]
>+                                                            Item must be renewed at the library. [% ISSUE.renewsleft | html %] renewals remaining
>                                                         [% ELSIF ( ISSUE.norenew_overdue ) %]
>                                                             Not allowed <span class="renewals">(overdue)</span>
>                                                         [% ELSIF ( ISSUE.auto_too_late ) %]
>diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
>index d5d4e4a6f3..9bfd52bc3f 100755
>--- a/t/db_dependent/Circulation.t
>+++ b/t/db_dependent/Circulation.t
>@@ -278,7 +278,7 @@ Koha::CirculationRules->set_rules(
> 
> my ( $reused_itemnumber_1, $reused_itemnumber_2 );
> subtest "CanBookBeRenewed tests" => sub {
>-    plan tests => 87;
>+    plan tests => 89;
> 
>     C4::Context->set_preference('ItemsDeniedRenewal','');
>     # Generate test biblio
>@@ -1158,12 +1158,34 @@ subtest "CanBookBeRenewed tests" => sub {
>     is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)');
> 
>     # Too many unseen renewals
>-    $dbh->do('UPDATE issuingrules SET unseen_renewals_allowed = 2, renewalsallowed = 10');
>+    Koha::CirculationRules->set_rules(
>+        {
>+            categorycode => undef,
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rules        => {
>+                unseen_renewals_allowed => 2,
>+                renewalsallowed => 10,
>+            }
>+        }
>+    );
>+    t::lib::Mocks::mock_preference('UnseenRenewals', 1);
>     $dbh->do('UPDATE issues SET unseen_renewals = 2 where borrowernumber = ? AND itemnumber = ?', undef, ($renewing_borrowernumber, $item_1->itemnumber));
>     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
>     is( $renewokay, 0, 'Cannot renew, 0 unseen renewals allowed');
>     is( $error, 'too_unseen', 'Cannot renew, returned code is too_unseen');
>-    $dbh->do('UPDATE issuingrules SET norenewalbefore = NULL, renewalsallowed = 0');
>+    Koha::CirculationRules->set_rules(
>+        {
>+            categorycode => undef,
>+            branchcode   => undef,
>+            itemtype     => undef,
>+            rules        => {
>+                norenewalbefore => undef,
>+                renewalsallowed => 0,
>+            }
>+        }
>+    );
>+    t::lib::Mocks::mock_preference('UnseenRenewals', 0);
> 
>     # Test WhenLostForgiveFine and WhenLostChargeReplacementFee
>     t::lib::Mocks::mock_preference('WhenLostForgiveFine','1');
>diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t
>index 6c8c9ba30b..3fbf46c590 100755
>--- a/t/db_dependent/Circulation/issue.t
>+++ b/t/db_dependent/Circulation/issue.t
>@@ -17,7 +17,7 @@
> 
> use Modern::Perl;
> 
>-use Test::More tests => 46;
>+use Test::More tests => 48;
> use DateTime::Duration;
> 
> use t::lib::Mocks;
>@@ -297,20 +297,6 @@ subtest 'Show that AddRenewal respects OpacRenewalBranch and interface' => sub {
>     }
> };
> 
>-# Unseen rewnewals
>-t::lib::Mocks::mock_preference('UnseenRenewals', 1);
>-
>-# Does an unseen renewal increment the issue's count
>-my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-AddRenewal( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber}, $branchcode_1, undef, undef, 0 );
>-my ( $unseen_after ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-is( $unseen_after, $unseen_before + 1, 'unseen_renewals increments' );
>-
>-# Does a seen renewal reset the unseen count
>-AddRenewal( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber}, $branchcode_1, undef, undef, 1 );
>-my ( $unseen_reset ) = ( C4::Circulation::GetRenewCount( $opac_renew_issue->{borrowernumber}, $opac_renew_issue->{itemnumber} ) )[3];
>-is( $unseen_reset, 0, 'seen renewal resets the unseen count' );
>-
> #Test GetBiblioIssues
> is( GetBiblioIssues(), undef, "GetBiblio Issues without parameters" );
> 
>@@ -327,19 +313,19 @@ my $issue3 = C4::Circulation::AddIssue( $borrower_1, $barcode_1 );
> @renewcount = C4::Circulation::GetRenewCount();
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ], # FIXME Need to be fixed, see FIXME in GetRenewCount
>+    [ 0, 0, 0, 0, 0, 0 ], # FIXME Need to be fixed, see FIXME in GetRenewCount
>     "Without issuing rules and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = undef, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount(-1);
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ], # FIXME Need to be fixed
>+    [ 0, 0, 0, 0, 0, 0 ], # FIXME Need to be fixed
>     "Without issuing rules and without wrong parameter, GetRenewCount returns renewcount = 0, renewsallowed = undef, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 0, 0 ],
>+    [ 2, 0, 0, 0, 0, 0 ],
>     "Without issuing rules and with a valid parameter, renewcount = 2, renewsallowed = undef, renewsleft = 0"
> );
> 
>@@ -347,19 +333,19 @@ is_deeply(
> @renewcount = C4::Circulation::GetRenewCount();
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ],
>+    [ 0, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and without parameter, GetRenewCount returns renewcount = 0, renewsallowed = 0, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount(-1);
> is_deeply(
>     \@renewcount,
>-    [ 0, 0, 0 ],
>+    [ 0, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and without wrong parameter, GetRenewCount returns renewcount = 0, renewsallowed = 0, renewsleft = 0"
> );
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 0, 0 ],
>+    [ 2, 0, 0, 0, 0, 0 ],
>     "With issuing rules (renewal disallowed) and with a valid parameter, Getrenewcount returns renewcount = 2, renewsallowed = 0, renewsleft = 0"
> );
> 
>@@ -377,7 +363,7 @@ Koha::CirculationRules->set_rules(
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 2, 3, 1 ],
>+    [ 2, 3, 1, 0, 0, 0 ],
>     "With issuing rules (renewal allowed) and with a valid parameter, Getrenewcount of item1 returns 3 renews left"
> );
> 
>@@ -386,7 +372,7 @@ AddRenewal( $borrower_id1, $item_id1, $branchcode_1,
> @renewcount = C4::Circulation::GetRenewCount($borrower_id1, $item_id1);
> is_deeply(
>     \@renewcount,
>-    [ 3, 3, 0 ],
>+    [ 3, 3, 0, 0, 0, 0 ],
>     "With issuing rules (renewal allowed, 1 remaining) and with a valid parameter, Getrenewcount of item1 returns 0 renews left"
> );
> 
>@@ -504,5 +490,37 @@ AddIssue( $borrower_2, $barcode_1, dt_from_string, 'cancel' );
> my $hold = Koha::Holds->find( $reserve_id );
> is( $hold, undef, 'The reserve should have been correctly cancelled' );
> 
>+# Unseen rewnewals
>+t::lib::Mocks::mock_preference('UnseenRenewals', 1);
>+# Add a default circ rule: 3 unseen renewals allowed
>+Koha::CirculationRules->set_rules(
>+    {
>+        categorycode => undef,
>+        itemtype     => undef,
>+        branchcode   => undef,
>+        rules        => {
>+            renewalsallowed => 10,
>+            unseen_renewals_allowed => 3
>+        }
>+    }
>+);
>+
>+my $unseen_library = $builder->build_object( { class => 'Koha::Libraries' } );
>+my $unseen_patron  = $builder->build_object( { class => 'Koha::Patrons' } );
>+my $unseen_item = $builder->build_sample_item(
>+    { library => $unseen_library->branchcode, itype => $itemtype } );
>+my $unseen_issue = C4::Circulation::AddIssue( $unseen_patron->unblessed, $unseen_item->barcode );
>+
>+# Does an unseen renewal increment the issue's count
>+my ( $unseen_before ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 0 );
>+my ( $unseen_after ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+is( $unseen_after, $unseen_before + 1, 'unseen_renewals increments' );
>+
>+# Does a seen renewal reset the unseen count
>+AddRenewal( $unseen_patron->borrowernumber, $unseen_item->itemnumber, $branchcode_1, undef, undef, undef, 1 );
>+my ( $unseen_reset ) = ( C4::Circulation::GetRenewCount( $unseen_patron->borrowernumber, $unseen_item->itemnumber ) )[3];
>+is( $unseen_reset, 0, 'seen renewal resets the unseen count' );
>+
> #End transaction
> $schema->storage->txn_rollback;
>-- 
>2.20.1

Actions: View | Diff | Splinter Review

Attachments on bug 24083: 95861 | 95862 | 95863 | 95864 | 95865 | 95866 | 95867 | 95868 | 98749 | 98750 | 98751 | 98752 | 98765 | 98766 | 98767 | 98768 | 100244 | 100245 | 100246 | 100247 | 100380 | 100381 | 100382 | 100383 | 100392 | 110607 | 110608 | 110609 | 110610 | 110611 | 111147 | 111148 | 111149 | 111150 | 111151 | 111152 | 111283 | 111284 | 111285 | 111286 | 111287 | 111288 | 111289 | 111290 | 111291 | 111292 | 111293 | 111849 | 111850 | 111851 | 111852 | 111853 | 111854 | 111855 | 111856 | 111857 | 111858 | 111859 | 111860 | 112230 | 112231 | 112232 | 112233 | 112234 | 112235 | 112236 | 112237 | 112238 | 112239 | 112240 | 112241 | 112242 | 112243 | 112410 | 112411 | 112412 | 112413 | 112414 | 112415 | 112416 | 112417 | 112418 | 112419 | 112420 | 112421 | 112422 | 112423 | 113073 | 113074 | 113075 | 113076 | 113077 | 113078 | 113079 | 113080 | 113314 | 113315 | 113484 | 113485 | 113486 | 113487 | 113488 | 113489 | 113490 | 113491 | 113492 | 113493 | 113494 | 113546 | 113556 | 113557 | 113814