From 9cc6b926ecd8f8699754d13bfd5f4d2716f28322 Mon Sep 17 00:00:00 2001
From: David Gustafsson <david.gustafsson@ub.gu.se>
Date: Tue, 4 Oct 2022 19:23:28 +0200
Subject: [PATCH] Bug 31676: Fix db_dependent/Circulation.t tests

To test:
1) Set "RenewalPeriodBase" syspref to "the current date",
   "CircControl" to "the library the item is from." and
   "HomeOrHoldingBranch" to "the item's home library (homebranch)."
2) Run tests in t/db_dependent/Circulation.t and verify
   that fails (the script should actually crash before all
   can be run).
3) Apply the patch
4) Verifiy that all tests now pass

Sponsored-by: Gothenburg University Library

Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
---
 t/db_dependent/Circulation.t | 183 ++++++++++++++++++++---------------
 1 file changed, 105 insertions(+), 78 deletions(-)

diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t
index b12e1b27f0..b783a8268b 100755
--- a/t/db_dependent/Circulation.t
+++ b/t/db_dependent/Circulation.t
@@ -420,7 +420,7 @@ subtest "GetIssuingCharges tests" => sub {
 
 my ( $reused_itemnumber_1, $reused_itemnumber_2 );
 subtest "CanBookBeRenewed tests" => sub {
-    plan tests => 105;
+    plan tests => 104;
 
     C4::Context->set_preference('ItemsDeniedRenewal','');
     # Generate test biblio
@@ -719,6 +719,27 @@ subtest "CanBookBeRenewed tests" => sub {
     my $passeddatedue1 = AddIssue($renewing_borrower, $item_7->barcode, $five_weeks_ago);
     is (defined $passeddatedue1, 1, "Item with passed date due checked out, due date: " . $passeddatedue1->date_due);
 
+    t::lib::Mocks::mock_preference('OverduesBlockRenewing','allow');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
+    is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
+    is( $renewokay, 1, '(Bug 8236), Can renew, this item is overdue but not pref does not block');
+
+    t::lib::Mocks::mock_preference('OverduesBlockRenewing','block');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
+    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is not overdue but patron has overdues');
+    is( $error, 'overdue', "Correct error returned");
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
+    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue so patron has overdues');
+    is( $error, 'overdue', "Correct error returned");
+
+    t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
+    is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue');
+    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
+    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue');
+    is( $error, 'overdue', "Correct error returned");
+
     my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower->{categorycode}, $branch, $five_weeks_ago, $now );
     C4::Overdues::UpdateFine(
         {
@@ -733,6 +754,10 @@ subtest "CanBookBeRenewed tests" => sub {
     # Make sure fine calculation isn't skipped when adding renewal
     t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1);
 
+    # Calculate new due-date based on the present date not to incur
+    # multiple fees
+    t::lib::Mocks::mock_preference('RenewalPeriodBase', 'now');
+
     my $staff = $builder->build_object({ class => "Koha::Patrons" });
     t::lib::Mocks::mock_userenv({ patron => $staff });
 
@@ -765,33 +790,10 @@ subtest "CanBookBeRenewed tests" => sub {
     is ($new_log_size, $old_log_size + 1, 'renew log successfully added');
 
     my $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } );
-    is( $fines->count, 2, 'AddRenewal left both fines' );
-    isnt( $fines->next->status, 'UNRETURNED', 'Fine on renewed item is closed out properly' );
-    isnt( $fines->next->status, 'UNRETURNED', 'Fine on renewed item is closed out properly' );
+    is( $fines->count, 1, 'AddRenewal left fine' );
+    is( $fines->next->status, 'RENEWED', 'Fine on renewed item is closed out properly' );
     $fines->delete();
 
-    t::lib::Mocks::mock_preference('OverduesBlockRenewing','allow');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
-    is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
-    is( $renewokay, 1, '(Bug 8236), Can renew, this item is overdue but not pref does not block');
-
-    t::lib::Mocks::mock_preference('OverduesBlockRenewing','block');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
-    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is not overdue but patron has overdues');
-    is( $error, 'overdue', "Correct error returned");
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
-    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue so patron has overdues');
-    is( $error, 'overdue', "Correct error returned");
-
-    t::lib::Mocks::mock_preference('OverduesBlockRenewing','blockitem');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
-    is( $renewokay, 1, '(Bug 8236), Can renew, this item is not overdue');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
-    is( $renewokay, 0, '(Bug 8236), Cannot renew, this item is overdue');
-    is( $error, 'overdue', "Correct error returned");
-
-
     my $old_issue_log_size = Koha::ActionLogs->count( \%params_issue );
     my $old_renew_log_size = Koha::ActionLogs->count( \%params_renewal );
     AddIssue( $renewing_borrower,$item_7->barcode,Koha::DateUtils::output_pref({str=>$datedue6->date_due, dateformat =>'iso'}),0,$date, 0, undef );
@@ -800,9 +802,6 @@ subtest "CanBookBeRenewed tests" => sub {
     $new_log_size = Koha::ActionLogs->count( \%params_issue );
     is ($new_log_size, $old_issue_log_size, 'renew not logged as issue when renewed via issuing');
 
-    $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } );
-    $fines->delete();
-
     $hold = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next;
     $hold->cancel;
 
@@ -4102,7 +4101,7 @@ subtest 'CanBookBeIssued | is_overdue' => sub {
     my $issue = AddIssue( $patron->unblessed, $item->barcode, $five_days_go ); # date due was 10d ago
     my $actualissue = Koha::Checkouts->find( { itemnumber => $item->itemnumber } );
     is( output_pref({ str => $actualissue->date_due, dateonly => 1}), output_pref({ str => $five_days_go, dateonly => 1}), "First issue works");
-    my ($issuingimpossible, $needsconfirmation) = CanBookBeIssued($patron,$item->barcode,$ten_days_go, undef, undef, undef);
+    my ($issuingimpossible, $needsconfirmation) = CanBookBeIssued($patron, $item->barcode, $ten_days_go, undef, undef, undef);
     is( $needsconfirmation->{RENEW_ISSUE}, 1, "This is a renewal");
     is( $needsconfirmation->{TOO_MANY}, undef, "Not too many, is a renewal");
 };
@@ -4623,8 +4622,7 @@ subtest 'Incremented fee tests' => sub {
 
     t::lib::Mocks::mock_preference( 'item-level_itypes', 1 );
 
-    my $library =
-      $builder->build_object( { class => 'Koha::Libraries' } )->store;
+    my $library = $builder->build_object( { class => 'Koha::Libraries' } )->store;
 
     $module->mock( 'userenv', sub { { branch => $library->id } } );
 
@@ -4649,15 +4647,13 @@ subtest 'Incremented fee tests' => sub {
 
     my $item = $builder->build_sample_item(
         {
-            library  => $library->{branchcode},
+            library  => $library->id,
             itype    => $itemtype->id,
         }
     );
 
-    is( $itemtype->rentalcharge_daily+0,
-        1, 'Daily rental charge stored and retreived correctly' );
-    is( $item->effective_itemtype, $itemtype->id,
-        "Itemtype set correctly for item" );
+    is( $itemtype->rentalcharge_daily + 0,1, 'Daily rental charge stored and retreived correctly' );
+    is( $item->effective_itemtype, $itemtype->id, "Itemtype set correctly for item" );
 
     my $now         = dt_from_string;
     my $dt_from     = $now->clone;
@@ -4668,14 +4664,18 @@ subtest 'Incremented fee tests' => sub {
     my $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     my $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 7,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 0"
+    is(
+        $accountline->amount + 0,
+        7,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 0"
     );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 6,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 0, for renewal"
+    is(
+        $accountline->amount + 0,
+        6,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 0, for renewal"
     );
     $accountline->delete();
     $issue->delete();
@@ -4685,14 +4685,18 @@ subtest 'Incremented fee tests' => sub {
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 7,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1"
+    is(
+        $accountline->amount + 0,
+        7,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1"
     );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 6,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1, for renewal"
+    is(
+        $accountline->amount + 0,
+        6,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1, for renewal"
     );
     $accountline->delete();
     $issue->delete();
@@ -4712,37 +4716,37 @@ subtest 'Incremented fee tests' => sub {
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 6,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1 and closed $closed_day_name"
+    is(
+        $accountline->amount + 0,
+        6,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1 and closed $closed_day_name"
     );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount+0, 5,
-"Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1 and closed $closed_day_name, for renewal"
+    is(
+        $accountline->amount + 0,
+        5,
+        "Daily rental charge calculated correctly with rentalcharge_daily_calendar = 1 and closed $closed_day_name, for renewal"
     );
     $accountline->delete();
     $issue->delete();
 
     $itemtype->rentalcharge(2)->store;
-    is( $itemtype->rentalcharge+0, 2,
-        'Rental charge updated and retreived correctly' );
+    is( $itemtype->rentalcharge + 0, 2, 'Rental charge updated and retreived correctly' );
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     my $accountlines =
       Koha::Account::Lines->search( { itemnumber => $item->id } );
-    is( $accountlines->count, '2',
-        "Fixed charge and accrued charge recorded distinctly" );
+    is( $accountlines->count, '2', "Fixed charge and accrued charge recorded distinctly" );
     $accountlines->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountlines = Koha::Account::Lines->search( { itemnumber => $item->id } );
-    is( $accountlines->count, '2',
-        "Fixed charge and accrued charge recorded distinctly, for renewal" );
+    is( $accountlines->count, '2', "Fixed charge and accrued charge recorded distinctly, for renewal" );
     $accountlines->delete();
     $issue->delete();
     $itemtype->rentalcharge(0)->store;
-    is( $itemtype->rentalcharge+0, 0,
-        'Rental charge reset and retreived correctly' );
+    is( $itemtype->rentalcharge + 0, 0, 'Rental charge reset and retreived correctly' );
 
     # Hourly
     Koha::CirculationRules->set_rule(
@@ -4756,8 +4760,7 @@ subtest 'Incremented fee tests' => sub {
     );
 
     $itemtype->rentalcharge_hourly('0.25')->store();
-    is( $itemtype->rentalcharge_hourly,
-        '0.25', 'Hourly rental charge stored and retreived correctly' );
+    is( $itemtype->rentalcharge_hourly, '0.25', 'Hourly rental charge stored and retreived correctly' );
 
     $dt_to       = $now->clone->add( hours => 168 );
     $dt_to_renew = $now->clone->add( hours => 312 );
@@ -4766,13 +4769,19 @@ subtest 'Incremented fee tests' => sub {
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 42,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 0 (168h * 0.25u)" );
+    is(
+        $accountline->amount + 0,
+        42,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 0 (168h * 0.25u)"
+    );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 36,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 0, for renewal (312h - 168h * 0.25u)" );
+    is(
+        $accountline->amount + 0,
+        36,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 0, for renewal (312h - 168h * 0.25u)"
+    );
     $accountline->delete();
     $issue->delete();
 
@@ -4780,13 +4789,19 @@ subtest 'Incremented fee tests' => sub {
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 36,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 and closed $closed_day_name (168h - 24h * 0.25u)" );
+    is(
+        $accountline->amount + 0,
+        36,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 and closed $closed_day_name (168h - 24h * 0.25u)"
+    );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 30,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 and closed $closed_day_name, for renewal (312h - 168h - 24h * 0.25u" );
+    is(
+        $accountline->amount + 0,
+        30,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 and closed $closed_day_name, for renewal (312h - 168h - 24h * 0.25u"
+    );
     $accountline->delete();
     $issue->delete();
 
@@ -4794,13 +4809,19 @@ subtest 'Incremented fee tests' => sub {
     $issue =
       AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 42,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 (168h - 0h * 0.25u" );
+    is(
+        $accountline->amount + 0,
+        42,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1 (168h - 0h * 0.25u"
+    );
     $accountline->delete();
     AddRenewal( $patron->id, $item->id, $library->id, $dt_to_renew, $dt_to );
     $accountline = Koha::Account::Lines->find( { itemnumber => $item->id } );
-    is( $accountline->amount + 0, 36,
-        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1, for renewal (312h - 168h - 0h * 0.25u)" );
+    is(
+        $accountline->amount + 0,
+        36,
+        "Hourly rental charge calculated correctly with rentalcharge_hourly_calendar = 1, for renewal (312h - 168h - 0h * 0.25u)"
+    );
     $accountline->delete();
     $issue->delete();
     Time::Fake->reset;
@@ -5447,27 +5468,33 @@ subtest 'AddIssue records staff who checked out item if appropriate' => sub  {
             value => { categorycode => $patron_category->{categorycode} }
         }
     );
-    my $item = $builder->build_sample_item(
+    my $item_1 = $builder->build_sample_item(
         {
             library  => $library->{branchcode}
         }
     );
 
-    $module->mock( 'userenv', sub { { branch => $library->id, number => $issuer->{borrowernumber} } } );
+    my $item_2 = $builder->build_sample_item(
+        {
+            library  => $library->branchcode
+        }
+    );
+
+    $module->mock( 'userenv', sub { { branch => $library->id, number => $issuer->borrowernumber } } );
 
     my $dt_from = dt_from_string();
     my $dt_to   = dt_from_string()->add( days => 7 );
 
-    my $issue = AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
+    my $issue_1 = AddIssue( $patron->unblessed, $item_1->barcode, $dt_to, undef, $dt_from );
 
-    is( $issue->issuer, undef, "Staff who checked out the item not recorded when RecordStaffUserOnCheckout turned off" );
+    is( $issue_1->issuer, undef, "Staff who checked out the item not recorded when RecordStaffUserOnCheckout turned off" );
 
     t::lib::Mocks::mock_preference('RecordStaffUserOnCheckout', 1);
 
-    my $issue2 =
-      AddIssue( $patron->unblessed, $item->barcode, $dt_to, undef, $dt_from );
+    my $issue_2 =
+      AddIssue( $patron->unblessed, $item_2->barcode, $dt_to, undef, $dt_from );
 
-    is( $issue->issuer, $issuer->{borrowernumber}, "Staff who checked out the item recorded when RecordStaffUserOnCheckout turned on" );
+    is( $issue_2->issuer->borrowernumber, $issuer->borrowernumber, "Staff who checked out the item recorded when RecordStaffUserOnCheckout turned on" );
 };
 
 subtest "Item's onloan value should be set if checked out item is checked out to a different patron" => sub {
-- 
2.30.2