Bug 17600: Standardize our EXPORT_OK
[srvgit] / t / db_dependent / Circulation.t
index 3506363..8aa5d4c 100755 (executable)
@@ -18,7 +18,7 @@
 use Modern::Perl;
 use utf8;
 
-use Test::More tests => 50;
+use Test::More tests => 55;
 use Test::Exception;
 use Test::MockModule;
 use Test::Deep qw( cmp_deeply );
@@ -32,13 +32,13 @@ use t::lib::Mocks;
 use t::lib::TestBuilder;
 
 use C4::Accounts;
-use C4::Calendar;
-use C4::Circulation;
+use C4::Calendar qw( new insert_single_holiday insert_week_day_holiday delete_holiday );
+use C4::Circulation qw( AddIssue AddReturn CanBookBeRenewed GetIssuingCharges AddRenewal GetSoonestRenewDate GetLatestAutoRenewDate LostItem GetUpcomingDueIssues CanBookBeIssued AddIssuingCharge ProcessOfflinePayment transferbook updateWrongTransfer );
 use C4::Biblio;
-use C4::Items;
+use C4::Items qw( ModItemTransfer );
 use C4::Log;
-use C4::Reserves;
-use C4::Overdues qw(UpdateFine CalcFine);
+use C4::Reserves qw( AddReserve ModReserve ModReserveCancelAll ModReserveAffect CheckReserves GetOtherReserves );
+use C4::Overdues qw( CalcFine UpdateFine get_chargeable_units );
 use Koha::DateUtils;
 use Koha::Database;
 use Koha::Items;
@@ -122,6 +122,11 @@ for my $branch ( $branches->next ) {
 $dbh->do('DELETE FROM issues');
 $dbh->do('DELETE FROM borrowers');
 
+# Disable recording of the staff who checked out an item until we're ready for it
+t::lib::Mocks::mock_preference('RecordStaffUserOnCheckout', 0);
+
+my $module = Test::MockModule->new('C4::Context');
+
 my $library = $builder->build({
     source => 'Branch',
 });
@@ -277,9 +282,140 @@ Koha::CirculationRules->set_rules(
     }
 );
 
+subtest "CanBookBeRenewed AllowRenewalIfOtherItemsAvailable multiple borrowers and items tests" => sub {
+    plan tests => 5;
+
+    #Can only reserve from home branch
+    Koha::CirculationRules->set_rule(
+        {
+            branchcode   => undef,
+            itemtype     => undef,
+            rule_name    => 'holdallowed',
+            rule_value   => 1
+        }
+    );
+    Koha::CirculationRules->set_rule(
+        {
+            branchcode   => undef,
+            categorycode   => undef,
+            itemtype     => undef,
+            rule_name    => 'onshelfholds',
+            rule_value   => 1
+        }
+    );
+
+    # Patrons from three different branches
+    my $patron_borrower = $builder->build_object({ class => 'Koha::Patrons' });
+    my $patron_hold_1   = $builder->build_object({ class => 'Koha::Patrons' });
+    my $patron_hold_2   = $builder->build_object({ class => 'Koha::Patrons' });
+    my $biblio = $builder->build_sample_biblio();
+
+    # Item at each patron branch
+    my $item_1 = $builder->build_sample_item({
+        biblionumber => $biblio->biblionumber,
+        homebranch   => $patron_borrower->branchcode
+    });
+    my $item_2 = $builder->build_sample_item({
+        biblionumber => $biblio->biblionumber,
+        homebranch   => $patron_hold_2->branchcode
+    });
+    my $item_3 = $builder->build_sample_item({
+        biblionumber => $biblio->biblionumber,
+        homebranch   => $patron_hold_1->branchcode
+    });
+
+    my $issue = AddIssue( $patron_borrower->unblessed, $item_1->barcode);
+    my $datedue = dt_from_string( $issue->date_due() );
+    is (defined $issue->date_due(), 1, "Item 1 checked out, due date: " . $issue->date_due() );
+
+    # Biblio-level holds
+    AddReserve(
+        {
+            branchcode       => $patron_hold_1->branchcode,
+            borrowernumber   => $patron_hold_1->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            priority         => 1,
+            reservation_date => dt_from_string(),
+            expiration_date  => undef,
+            itemnumber       => undef,
+            found            => undef,
+        }
+    );
+    AddReserve(
+        {
+            branchcode       => $patron_hold_2->branchcode,
+            borrowernumber   => $patron_hold_2->borrowernumber,
+            biblionumber     => $biblio->biblionumber,
+            priority         => 2,
+            reservation_date => dt_from_string(),
+            expiration_date  => undef,
+            itemnumber       => undef,
+            found            => undef,
+        }
+    );
+    t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 0 );
+
+    my ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber);
+    is( $renewokay, 0, 'Cannot renew, reserved');
+    is( $error, 'on_reserve', 'Cannot renew, reserved (returned error is on_reserve)');
+
+    t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 );
+
+    ( $renewokay, $error ) = CanBookBeRenewed($patron_borrower->borrowernumber, $item_1->itemnumber);
+    is( $renewokay, 1, 'Can renew, two items available for two holds');
+    is( $error, undef, 'Can renew, each reserve has an item');
+
+
+};
+
+subtest "GetIssuingCharges tests" => sub {
+    plan tests => 4;
+    my $branch_discount = $builder->build_object({ class => 'Koha::Libraries' });
+    my $branch_no_discount = $builder->build_object({ class => 'Koha::Libraries' });
+    Koha::CirculationRules->set_rule(
+        {
+            categorycode => undef,
+            branchcode   => $branch_discount->branchcode,
+            itemtype     => undef,
+            rule_name    => 'rentaldiscount',
+            rule_value   => 15
+        }
+    );
+    my $itype_charge = $builder->build_object({
+        class => 'Koha::ItemTypes',
+        value => {
+            rentalcharge => 10
+        }
+    });
+    my $itype_no_charge = $builder->build_object({
+        class => 'Koha::ItemTypes',
+        value => {
+            rentalcharge => 0
+        }
+    });
+    my $patron = $builder->build_object({ class => 'Koha::Patrons' });
+    my $item_1 = $builder->build_sample_item({ itype => $itype_charge->itemtype });
+    my $item_2 = $builder->build_sample_item({ itype => $itype_no_charge->itemtype });
+
+    t::lib::Mocks::mock_userenv({ branchcode => $branch_no_discount->branchcode });
+    # For now the sub always uses the env branch, this should follow CircControl instead
+    my ($charge, $itemtype) = GetIssuingCharges( $item_1->itemnumber, $patron->borrowernumber);
+    is( $charge + 0, 10.00, "Charge fetched correctly when no discount exists");
+    ($charge, $itemtype) = GetIssuingCharges( $item_2->itemnumber, $patron->borrowernumber);
+    is( $charge + 0, 0.00, "Charge fetched correctly when no discount exists and no charge");
+
+    t::lib::Mocks::mock_userenv({ branchcode => $branch_discount->branchcode });
+    # For now the sub always uses the env branch, this should follow CircControl instead
+    ($charge, $itemtype) = GetIssuingCharges( $item_1->itemnumber, $patron->borrowernumber);
+    is( $charge + 0, 8.50, "Charge fetched correctly when discount exists");
+    ($charge, $itemtype) = GetIssuingCharges( $item_2->itemnumber, $patron->borrowernumber);
+    is( $charge + 0, 0.00, "Charge fetched correctly when discount exists and no charge");
+
+};
+
 my ( $reused_itemnumber_1, $reused_itemnumber_2 );
 subtest "CanBookBeRenewed tests" => sub {
-    plan tests => 87;
+    plan tests => 95;
 
     C4::Context->set_preference('ItemsDeniedRenewal','');
     # Generate test biblio
@@ -414,6 +550,15 @@ subtest "CanBookBeRenewed tests" => sub {
             rule_value   => '1',
         }
     );
+    Koha::CirculationRules->set_rule(
+        {
+            categorycode => undef,
+            branchcode   => undef,
+            itemtype     => undef,
+            rule_name    => 'renewalsallowed',
+            rule_value   => '5',
+        }
+    );
     t::lib::Mocks::mock_preference('AllowRenewalIfOtherItemsAvailable', 1 );
     ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_1->itemnumber);
     is( $renewokay, 1, 'Bug 11634 - Allow renewal of item with unfilled holds if other available items can fill those holds');
@@ -517,6 +662,7 @@ subtest "CanBookBeRenewed tests" => sub {
     is( $renewokay, 1, '(Bug 8236), Can renew, user is not restricted');
     ( $renewokay, $error ) = CanBookBeRenewed($restricted_borrowernumber, $item_5->itemnumber);
     is( $renewokay, 0, '(Bug 8236), Cannot renew, user is restricted');
+    is( $error, 'restriction', "Correct error returned");
 
     # Users cannot renew an overdue item
     my $item_6 = $builder->build_sample_item(
@@ -559,6 +705,9 @@ subtest "CanBookBeRenewed tests" => sub {
         }
     );
 
+    # Make sure fine calculation isn't skipped when adding renewal
+    t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1);
+
     t::lib::Mocks::mock_preference('RenewalLog', 0);
     my $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } );
     my %params_renewal = (
@@ -593,6 +742,27 @@ subtest "CanBookBeRenewed tests" => sub {
     isnt( $fines->next->status, 'UNRETURNED', '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 );
@@ -605,13 +775,6 @@ subtest "CanBookBeRenewed tests" => sub {
     $fines = Koha::Account::Lines->search( { borrowernumber => $renewing_borrower->{borrowernumber}, itemnumber => $item_7->itemnumber } );
     $fines->delete();
 
-    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');
-
-
     $hold = Koha::Holds->search({ biblionumber => $biblio->biblionumber, borrowernumber => $reserving_borrowernumber })->next;
     $hold->cancel;
 
@@ -1158,6 +1321,36 @@ subtest "CanBookBeRenewed tests" => sub {
     is( $renewokay, 0, 'Cannot renew, 0 renewals allowed');
     is( $error, 'too_many', 'Cannot renew, 0 renewals allowed (returned code is too_many)');
 
+    # Too many unseen renewals
+    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');
+    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');
     t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','1');
@@ -1236,13 +1429,6 @@ subtest "CanBookBeRenewed tests" => sub {
     my $units = C4::Overdues::get_chargeable_units('days', $future, $now, $library2->{branchcode});
     ok( $units == 0, '_get_chargeable_units returns 0 for items not past due date (Bug 12596)' );
 
-    # Users cannot renew any item if there is an overdue item
-    t::lib::Mocks::mock_preference('OverduesBlockRenewing','block');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_6->itemnumber);
-    is( $renewokay, 0, '(Bug 8236), Cannot renew, one of the items is overdue');
-    ( $renewokay, $error ) = CanBookBeRenewed($renewing_borrowernumber, $item_7->itemnumber);
-    is( $renewokay, 0, '(Bug 8236), Cannot renew, one of the items is overdue');
-
     my $manager = $builder->build_object({ class => "Koha::Patrons" });
     t::lib::Mocks::mock_userenv({ patron => $manager,branchcode => $manager->branchcode });
     t::lib::Mocks::mock_preference('WhenLostChargeReplacementFee','1');
@@ -2383,7 +2569,7 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub {
 
 
 subtest 'AddReturn | is_overdue' => sub {
-    plan tests => 8;
+    plan tests => 9;
 
     t::lib::Mocks::mock_preference('MarkLostItemsAsReturned', 'batchmod|moredetail|cronjob|additem|pendingreserves|onpayment');
     t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1);
@@ -2696,71 +2882,457 @@ subtest 'AddReturn | is_overdue' => sub {
         Koha::Account::Lines->search(
             { borrowernumber => $patron->borrowernumber } )->delete;
     };
-};
-
-subtest '_RestoreOverdueForLostAndFound' => sub {
 
-    plan tests => 7;
-
-    my $manager = $builder->build_object( { class => "Koha::Patrons" } );
-    t::lib::Mocks::mock_userenv(
-        { patron => $manager, branchcode => $manager->branchcode } );
-
-    my $patron = $builder->build_object( { class => "Koha::Patrons" } );
-    my $item = $builder->build_sample_item();
-
-    # No fine found
-    my $result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
-    is($result, 0, "0 returned when no overdue found");
-
-    # Fine not forgiven
-    my $account = $patron->account;
-    my $overdue = $account->add_debit(
-        {
-            amount     => 30.00,
-            user_id    => $manager->borrowernumber,
-            library_id => $library2->{branchcode},
-            interface  => 'test',
-            item_id    => $item->itemnumber,
-            type       => 'OVERDUE',
-        }
-    )->store();
-    $overdue->status('LOST')->store();
+    subtest 'enh 23091 | Lost item return policies' => sub {
+        plan tests => 4;
 
-    $result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
-    is($result, 0, "0 returned when overdue found without forgival");
-    $overdue->discard_changes;
-    is($overdue->status, 'RETURNED', 'Overdue status updated to RETURNED');
+        my $manager = $builder->build_object({ class => "Koha::Patrons" });
 
-    # Reset status
-    $overdue->status('LOST')->store();
+        my $branchcode_false =
+          $builder->build( { source => 'Branch' } )->{branchcode};
+        my $specific_rule_false = $builder->build(
+            {
+                source => 'CirculationRule',
+                value  => {
+                    branchcode   => $branchcode_false,
+                    categorycode => undef,
+                    itemtype     => undef,
+                    rule_name    => 'lostreturn',
+                    rule_value   => 0
+                }
+            }
+        );
+        my $branchcode_refund =
+          $builder->build( { source => 'Branch' } )->{branchcode};
+        my $specific_rule_refund = $builder->build(
+            {
+                source => 'CirculationRule',
+                value  => {
+                    branchcode   => $branchcode_refund,
+                    categorycode => undef,
+                    itemtype     => undef,
+                    rule_name    => 'lostreturn',
+                    rule_value   => 'refund'
+                }
+            }
+        );
+        my $branchcode_restore =
+          $builder->build( { source => 'Branch' } )->{branchcode};
+        my $specific_rule_restore = $builder->build(
+            {
+                source => 'CirculationRule',
+                value  => {
+                    branchcode   => $branchcode_restore,
+                    categorycode => undef,
+                    itemtype     => undef,
+                    rule_name    => 'lostreturn',
+                    rule_value   => 'restore'
+                }
+            }
+        );
+        my $branchcode_charge =
+          $builder->build( { source => 'Branch' } )->{branchcode};
+        my $specific_rule_charge = $builder->build(
+            {
+                source => 'CirculationRule',
+                value  => {
+                    branchcode   => $branchcode_charge,
+                    categorycode => undef,
+                    itemtype     => undef,
+                    rule_name    => 'lostreturn',
+                    rule_value   => 'charge'
+                }
+            }
+        );
 
-    # Fine forgiven
-    my $credit = $account->add_credit(
-        {
-            amount     => 30.00,
-            user_id    => $manager->borrowernumber,
-            library_id => $library2->{branchcode},
-            interface  => 'test',
-            type       => 'FORGIVEN',
-            item_id    => $item->itemnumber
-        }
-    );
-    $credit->apply( { debits => [$overdue], offset_type => 'Forgiven' } );
+        my $replacement_amount = 99.00;
+        t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' );
+        t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 );
+        t::lib::Mocks::mock_preference( 'WhenLostForgiveFine',          0 );
+        t::lib::Mocks::mock_preference( 'BlockReturnOfLostItems',       0 );
+        t::lib::Mocks::mock_preference( 'RefundLostOnReturnControl',
+            'CheckinLibrary' );
+        t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge',
+            undef );
 
-    $result = C4::Circulation::_RestoreOverdueForLostAndFound( $item->itemnumber);
+        subtest 'lostreturn | false' => sub {
+            plan tests => 12;
 
-    is( ref($result), 'Koha::Account::Line', 'Return a Koha::Account::Line object on sucess');
-    $overdue->discard_changes;
-    is($overdue->status, 'RETURNED', 'Overdue status updated to RETURNED');
-    is($overdue->amountoutstanding, $overdue->amount, 'Overdue outstanding restored');
+            t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_false });
 
-    # Missing parameters
-    warning_like {
-        C4::Circulation::_RestoreOverdueForLostAndFound( undef )
-    }
-    qr/_RestoreOverdueForLostAndFound\(\) not supplied valid itemnumber/,
-      "parameter warning received for missing itemnumbernumber";
+            my $item = $builder->build_sample_item(
+                {
+                    replacementprice => $replacement_amount
+                }
+            );
+
+            # Issue the item
+            my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago );
+
+            # Fake fines cronjob on this checkout
+            my ($fine) =
+              CalcFine( $item, $patron->categorycode, $library->{branchcode},
+                $ten_days_ago, $now );
+            UpdateFine(
+                {
+                    issue_id       => $issue->issue_id,
+                    itemnumber     => $item->itemnumber,
+                    borrowernumber => $patron->borrowernumber,
+                    amount         => $fine,
+                    due            => output_pref($ten_days_ago)
+                }
+            );
+            my $overdue_fees = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'OVERDUE'
+                }
+            );
+            is( $overdue_fees->count, 1, 'Overdue item fee produced' );
+            my $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                10, 'The right OVERDUE amount is generated' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The right OVERDUE amountoutstanding is generated' );
+
+            # Simulate item marked as lost
+            $item->itemlost(3)->store;
+            C4::Circulation::LostItem( $item->itemnumber, 1 );
+
+            my $lost_fee_lines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'LOST'
+                }
+            );
+            is( $lost_fee_lines->count, 1, 'Lost item fee produced' );
+            my $lost_fee_line = $lost_fee_lines->next;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The right LOST amount is generated' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                $replacement_amount,
+                'The right LOST amountoutstanding is generated' );
+            is( $lost_fee_line->status, undef, 'The LOST status was not set' );
+
+            # Return lost item
+            my ( $returned, $message ) =
+              AddReturn( $item->barcode, $branchcode_false, undef, $five_days_ago );
+
+            $overdue_fee->discard_changes;
+            is( $overdue_fee->amount + 0,
+                10, 'The OVERDUE amount is left intact' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The OVERDUE amountoutstanding is left intact' );
+
+            $lost_fee_line->discard_changes;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The LOST amount is left intact' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                $replacement_amount,
+                'The LOST amountoutstanding is left intact' );
+            # FIXME: Should we set the LOST fee status to 'FOUND' regardless of whether we're refunding or not?
+            is( $lost_fee_line->status, undef, 'The LOST status was not set' );
+        };
+
+        subtest 'lostreturn | refund' => sub {
+            plan tests => 12;
+
+            t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_refund });
+
+            my $item = $builder->build_sample_item(
+                {
+                    replacementprice => $replacement_amount
+                }
+            );
+
+            # Issue the item
+            my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago );
+
+            # Fake fines cronjob on this checkout
+            my ($fine) =
+              CalcFine( $item, $patron->categorycode, $library->{branchcode},
+                $ten_days_ago, $now );
+            UpdateFine(
+                {
+                    issue_id       => $issue->issue_id,
+                    itemnumber     => $item->itemnumber,
+                    borrowernumber => $patron->borrowernumber,
+                    amount         => $fine,
+                    due            => output_pref($ten_days_ago)
+                }
+            );
+            my $overdue_fees = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'OVERDUE'
+                }
+            );
+            is( $overdue_fees->count, 1, 'Overdue item fee produced' );
+            my $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                10, 'The right OVERDUE amount is generated' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The right OVERDUE amountoutstanding is generated' );
+
+            # Simulate item marked as lost
+            $item->itemlost(3)->store;
+            C4::Circulation::LostItem( $item->itemnumber, 1 );
+
+            my $lost_fee_lines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'LOST'
+                }
+            );
+            is( $lost_fee_lines->count, 1, 'Lost item fee produced' );
+            my $lost_fee_line = $lost_fee_lines->next;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The right LOST amount is generated' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                $replacement_amount,
+                'The right LOST amountoutstanding is generated' );
+            is( $lost_fee_line->status, undef, 'The LOST status was not set' );
+
+            # Return the lost item
+            my ( undef, $message ) =
+              AddReturn( $item->barcode, $branchcode_refund, undef, $five_days_ago );
+
+            $overdue_fee->discard_changes;
+            is( $overdue_fee->amount + 0,
+                10, 'The OVERDUE amount is left intact' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The OVERDUE amountoutstanding is left intact' );
+
+            $lost_fee_line->discard_changes;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The LOST amount is left intact' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                0,
+                'The LOST amountoutstanding is refunded' );
+            is( $lost_fee_line->status, 'FOUND', 'The LOST status was set to FOUND' );
+        };
+
+        subtest 'lostreturn | restore' => sub {
+            plan tests => 13;
+
+            t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_restore });
+
+            my $item = $builder->build_sample_item(
+                {
+                    replacementprice => $replacement_amount
+                }
+            );
+
+            # Issue the item
+            my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode , $ten_days_ago);
+
+            # Fake fines cronjob on this checkout
+            my ($fine) =
+              CalcFine( $item, $patron->categorycode, $library->{branchcode},
+                $ten_days_ago, $now );
+            UpdateFine(
+                {
+                    issue_id       => $issue->issue_id,
+                    itemnumber     => $item->itemnumber,
+                    borrowernumber => $patron->borrowernumber,
+                    amount         => $fine,
+                    due            => output_pref($ten_days_ago)
+                }
+            );
+            my $overdue_fees = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'OVERDUE'
+                }
+            );
+            is( $overdue_fees->count, 1, 'Overdue item fee produced' );
+            my $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                10, 'The right OVERDUE amount is generated' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The right OVERDUE amountoutstanding is generated' );
+
+            # Simulate item marked as lost
+            $item->itemlost(3)->store;
+            C4::Circulation::LostItem( $item->itemnumber, 1 );
+
+            my $lost_fee_lines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'LOST'
+                }
+            );
+            is( $lost_fee_lines->count, 1, 'Lost item fee produced' );
+            my $lost_fee_line = $lost_fee_lines->next;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The right LOST amount is generated' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                $replacement_amount,
+                'The right LOST amountoutstanding is generated' );
+            is( $lost_fee_line->status, undef, 'The LOST status was not set' );
+
+            # Simulate refunding overdue fees upon marking item as lost
+            my $overdue_forgive = $patron->account->add_credit(
+                {
+                    amount     => 10.00,
+                    user_id    => $manager->borrowernumber,
+                    library_id => $branchcode_restore,
+                    interface  => 'test',
+                    type       => 'FORGIVEN',
+                    item_id    => $item->itemnumber
+                }
+            );
+            $overdue_forgive->apply(
+                { debits => [$overdue_fee], offset_type => 'Forgiven' } );
+            $overdue_fee->discard_changes;
+            is($overdue_fee->amountoutstanding + 0, 0, 'Overdue fee forgiven');
+
+            # Do nothing
+            my ( undef, $message ) =
+              AddReturn( $item->barcode, $branchcode_restore, undef, $five_days_ago );
+
+            $overdue_fee->discard_changes;
+            is( $overdue_fee->amount + 0,
+                10, 'The OVERDUE amount is left intact' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The OVERDUE amountoutstanding is restored' );
+
+            $lost_fee_line->discard_changes;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The LOST amount is left intact' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                0,
+                'The LOST amountoutstanding is refunded' );
+            is( $lost_fee_line->status, 'FOUND', 'The LOST status was set to FOUND' );
+        };
+
+        subtest 'lostreturn | charge' => sub {
+            plan tests => 16;
+
+            t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_charge });
+
+            my $item = $builder->build_sample_item(
+                {
+                    replacementprice => $replacement_amount
+                }
+            );
+
+            # Issue the item
+            my $issue = C4::Circulation::AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago );
+
+            # Fake fines cronjob on this checkout
+            my ($fine) =
+              CalcFine( $item, $patron->categorycode, $library->{branchcode},
+                $ten_days_ago, $now );
+            UpdateFine(
+                {
+                    issue_id       => $issue->issue_id,
+                    itemnumber     => $item->itemnumber,
+                    borrowernumber => $patron->borrowernumber,
+                    amount         => $fine,
+                    due            => output_pref($ten_days_ago)
+                }
+            );
+            my $overdue_fees = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'OVERDUE'
+                }
+            );
+            is( $overdue_fees->count, 1, 'Overdue item fee produced' );
+            my $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                10, 'The right OVERDUE amount is generated' );
+            is( $overdue_fee->amountoutstanding + 0,
+                10,
+                'The right OVERDUE amountoutstanding is generated' );
+
+            # Simulate item marked as lost
+            $item->itemlost(3)->store;
+            C4::Circulation::LostItem( $item->itemnumber, 1 );
+
+            my $lost_fee_lines = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'LOST'
+                }
+            );
+            is( $lost_fee_lines->count, 1, 'Lost item fee produced' );
+            my $lost_fee_line = $lost_fee_lines->next;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The right LOST amount is generated' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                $replacement_amount,
+                'The right LOST amountoutstanding is generated' );
+            is( $lost_fee_line->status, undef, 'The LOST status was not set' );
+
+            # Simulate refunding overdue fees upon marking item as lost
+            my $overdue_forgive = $patron->account->add_credit(
+                {
+                    amount     => 10.00,
+                    user_id    => $manager->borrowernumber,
+                    library_id => $branchcode_charge,
+                    interface  => 'test',
+                    type       => 'FORGIVEN',
+                    item_id    => $item->itemnumber
+                }
+            );
+            $overdue_forgive->apply(
+                { debits => [$overdue_fee], offset_type => 'Forgiven' } );
+            $overdue_fee->discard_changes;
+            is($overdue_fee->amountoutstanding + 0, 0, 'Overdue fee forgiven');
+
+            # Do nothing
+            my ( undef, $message ) =
+              AddReturn( $item->barcode, $branchcode_charge, undef, $five_days_ago );
+
+            $lost_fee_line->discard_changes;
+            is( $lost_fee_line->amount + 0,
+                $replacement_amount, 'The LOST amount is left intact' );
+            is( $lost_fee_line->amountoutstanding + 0,
+                0,
+                'The LOST amountoutstanding is refunded' );
+            is( $lost_fee_line->status, 'FOUND', 'The LOST status was set to FOUND' );
+
+            $overdue_fees = Koha::Account::Lines->search(
+                {
+                    borrowernumber  => $patron->id,
+                    itemnumber      => $item->itemnumber,
+                    debit_type_code => 'OVERDUE'
+                },
+                {
+                    order_by => { '-asc' => 'accountlines_id'}
+                }
+            );
+            is( $overdue_fees->count, 2, 'A second OVERDUE fee has been added' );
+            $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                10, 'The original OVERDUE amount is left intact' );
+            is( $overdue_fee->amountoutstanding + 0,
+                0,
+                'The original OVERDUE amountoutstanding is left as forgiven' );
+            $overdue_fee = $overdue_fees->next;
+            is( $overdue_fee->amount + 0,
+                5, 'The new OVERDUE amount is correct for the backdated return' );
+            is( $overdue_fee->amountoutstanding + 0,
+                5,
+                'The new OVERDUE amountoutstanding is correct for the backdated return' );
+        };
+    };
 };
 
 subtest '_FixOverduesOnReturn' => sub {
@@ -2881,7 +3453,7 @@ subtest 'Set waiting flag' => sub {
     is( $hold->found, 'T', 'Hold is in transit' );
 
     my ( $status ) = CheckReserves($item->itemnumber);
-    is( $status, 'Reserved', 'Hold is not waiting yet');
+    is( $status, 'Transferred', 'Hold is not waiting yet');
 
     set_userenv( $library_2 );
     $do_transfer = 0;
@@ -2907,55 +3479,73 @@ subtest 'Set waiting flag' => sub {
 
 subtest 'Cancel transfers on lost items' => sub {
     plan tests => 6;
-    my $library_1 = $builder->build( { source => 'Branch' } );
-    my $patron_1 = $builder->build( { source => 'Borrower', value => { branchcode => $library_1->{branchcode}, categorycode => $patron_category->{categorycode} } } );
-    my $library_2 = $builder->build( { source => 'Branch' } );
-    my $patron_2  = $builder->build( { source => 'Borrower', value => { branchcode => $library_2->{branchcode}, categorycode => $patron_category->{categorycode} } } );
-    my $biblio = $builder->build_sample_biblio({branchcode => $library->{branchcode}});
-    my $item   = $builder->build_sample_item({
-        biblionumber  => $biblio->biblionumber,
-        library    => $library_1->{branchcode},
-    });
 
-    set_userenv( $library_2 );
-    my $reserve_id = AddReserve(
+    my $library_to = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $item   = $builder->build_sample_item();
+    my $holdingbranch = $item->holdingbranch;
+    # Historic transfer (datearrived is defined)
+    my $old_transfer = $builder->build_object(
         {
-            branchcode     => $library_2->{branchcode},
-            borrowernumber => $patron_2->{borrowernumber},
-            biblionumber   => $item->biblionumber,
-            priority       => 1,
-            itemnumber     => $item->itemnumber,
+            class => 'Koha::Item::Transfers',
+            value => {
+                itemnumber    => $item->itemnumber,
+                frombranch    => $holdingbranch,
+                tobranch      => $library_to->branchcode,
+                reason        => 'Manual',
+                datesent      => \'NOW()',
+                datearrived   => \'NOW()',
+                datecancelled => undef,
+                daterequested => \'NOW()'
+            }
+        }
+    );
+    # Queued transfer (datesent is undefined)
+    my $transfer_1 = $builder->build_object(
+        {
+            class => 'Koha::Item::Transfers',
+            value => {
+                itemnumber    => $item->itemnumber,
+                frombranch    => $holdingbranch,
+                tobranch      => $library_to->branchcode,
+                reason        => 'Manual',
+                datesent      => undef,
+                datearrived   => undef,
+                datecancelled => undef,
+                daterequested => \'NOW()'
+            }
+        }
+    );
+    # In transit transfer (datesent is defined, datearrived and datecancelled are both undefined)
+    my $transfer_2 = $builder->build_object(
+        {
+            class => 'Koha::Item::Transfers',
+            value => {
+                itemnumber    => $item->itemnumber,
+                frombranch    => $holdingbranch,
+                tobranch      => $library_to->branchcode,
+                reason        => 'Manual',
+                datesent      => \'NOW()',
+                datearrived   => undef,
+                datecancelled => undef,
+                daterequested => \'NOW()'
+            }
         }
     );
 
-    #Return book and add transfer
-    set_userenv( $library_1 );
-    my $do_transfer = 1;
-    my ( $res, $rr ) = AddReturn( $item->barcode, $library_1->{branchcode} );
-    ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id );
-    C4::Circulation::transferbook({
-        from_branch => $library_1->{branchcode},
-        to_branch => $library_2->{branchcode},
-        barcode   => $item->barcode,
-    });
-    my $hold = Koha::Holds->find( $reserve_id );
-    is( $hold->found, 'T', 'Hold is in transit' );
-
-    #Check transfer exists and the items holding branch is the transfer destination branch before marking it as lost
-    my ($datesent,$frombranch,$tobranch) = GetTransfers($item->itemnumber);
-    is( $frombranch, $library_1->{branchcode}, 'The transfer is generated from the correct library');
-    is( $tobranch, $library_2->{branchcode}, 'The transfer is generated to the correct library');
-    my $itemcheck = Koha::Items->find($item->itemnumber);
-    is( $itemcheck->holdingbranch, $library_1->{branchcode}, 'Items holding branch is the transfers origination branch before it is marked as lost' );
-
-    #Simulate item being marked as lost and confirm the transfer is deleted and the items holding branch is the transfers source branch
+    # Simulate item being marked as lost
     $item->itemlost(1)->store;
     LostItem( $item->itemnumber, 'test', 1 );
-    ($datesent,$frombranch,$tobranch) = GetTransfers($item->itemnumber);
-    is( $tobranch, undef, 'The transfer on the lost item has been deleted as the LostItemCancelOutstandingTransfer is enabled');
-    $itemcheck = Koha::Items->find($item->itemnumber);
-    is( $itemcheck->holdingbranch, $library_1->{branchcode}, 'Lost item with cancelled hold has holding branch equallying the transfers source branch' );
 
+    $transfer_1->discard_changes;
+    isnt($transfer_1->datecancelled, undef, "Queud transfer was cancelled upon item lost");
+    is($transfer_1->cancellation_reason, 'ItemLost', "Cancellation reason was set to 'ItemLost'");
+    $transfer_2->discard_changes;
+    isnt($transfer_2->datecancelled, undef, "Active transfer was cancelled upon item lost");
+    is($transfer_2->cancellation_reason, 'ItemLost', "Cancellation reason was set to 'ItemLost'");
+    $old_transfer->discard_changes;
+    is($old_transfer->datecancelled, undef, "Old transfers are unaffected");
+    $item->discard_changes;
+    is($item->holdingbranch, $holdingbranch, "Items holding branch remains unchanged");
 };
 
 subtest 'CanBookBeIssued | is_overdue' => sub {
@@ -3273,7 +3863,7 @@ subtest 'AddReturn should clear items.onloan for unissued items' => sub {
 
 subtest 'AddRenewal and AddIssuingCharge tests' => sub {
 
-    plan tests => 12;
+    plan tests => 13;
 
 
     t::lib::Mocks::mock_preference('item-level_itypes', 1);
@@ -3316,6 +3906,11 @@ subtest 'AddRenewal and AddIssuingCharge tests' => sub {
 
     # Check the item out
     AddIssue( $patron->unblessed, $item->barcode );
+
+    throws_ok {
+        AddRenewal( $patron->borrowernumber, $item->itemnumber, $library->id, undef, {break=>"the_renewal"} );
+    } 'Koha::Exceptions::Checkout::FailedRenewal', 'Exception is thrown when renewal update to issues fails';
+
     t::lib::Mocks::mock_preference( 'RenewalLog', 0 );
     my $date = output_pref( { dt => dt_from_string(), dateonly => 1, dateformat => 'iso' } );
     my %params_renewal = (
@@ -3411,7 +4006,6 @@ subtest 'Incremented fee tests' => sub {
     my $library =
       $builder->build_object( { class => 'Koha::Libraries' } )->store;
 
-    my $module = Test::MockModule->new('C4::Context');
     $module->mock( 'userenv', sub { { branch => $library->id } } );
 
     my $patron = $builder->build_object(
@@ -4195,7 +4789,7 @@ subtest 'Checkout should correctly terminate a transfer' => sub {
 
     my $do_transfer = 1;
     ModItemTransfer( $item->itemnumber, $library_1->branchcode,
-        $library_2->branchcode );
+        $library_2->branchcode, 'Manual' );
     ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id );
     GetOtherReserves( $item->itemnumber )
       ;    # To put the Reason, it's what does returns.pl...
@@ -4215,6 +4809,116 @@ subtest 'Checkout should correctly terminate a transfer' => sub {
     is( $hold->priority, 1, );
 };
 
+subtest 'AddIssue records staff who checked out item if appropriate' => sub  {
+    plan tests => 2;
+
+    $module->mock( 'userenv', sub { { branch => $library->{id} } } );
+
+    my $library = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $patron = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { categorycode => $patron_category->{categorycode} }
+        }
+    );
+    my $issuer = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { categorycode => $patron_category->{categorycode} }
+        }
+    );
+    my $item = $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 );
+
+    is( $issue->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 );
+
+    is( $issue->issuer, $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 {
+    plan tests => 2;
+
+    my $library_1 = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $patron_1 = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $library_1->branchcode }
+        }
+    );
+    my $patron_2 = $builder->build_object(
+        {
+            class => 'Koha::Patrons',
+            value => { branchcode => $library_1->branchcode }
+        }
+    );
+
+    my $item = $builder->build_sample_item(
+        {
+            library => $library_1->branchcode,
+        }
+    );
+
+    AddIssue( $patron_1->unblessed, $item->barcode );
+    ok( $item->get_from_storage->onloan, "Item's onloan column is set after initial checkout" );
+    AddIssue( $patron_2->unblessed, $item->barcode );
+    ok( $item->get_from_storage->onloan, "Item's onloan column is set after second checkout" );
+};
+
+subtest "updateWrongTransfer tests" => sub {
+    plan tests => 5;
+
+    my $library1 = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $library2 = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $library3 = $builder->build_object( { class => 'Koha::Libraries' } );
+    my $item     = $builder->build_sample_item(
+        {
+            homebranch    => $library1->branchcode,
+            holdingbranch => $library2->branchcode,
+            datelastseen  => undef
+        }
+    );
+
+    my $transfer = $builder->build_object(
+        {
+            class => 'Koha::Item::Transfers',
+            value => {
+                itemnumber    => $item->itemnumber,
+                frombranch    => $library2->branchcode,
+                tobranch      => $library1->branchcode,
+                daterequested => dt_from_string,
+                datesent      => dt_from_string,
+                datecancelled => undef,
+                datearrived   => undef,
+                reason        => 'Manual'
+            }
+        }
+    );
+    is( ref($transfer), 'Koha::Item::Transfer', 'Mock transfer added' );
+
+    my $new_transfer = C4::Circulation::updateWrongTransfer($item->itemnumber, $library1->branchcode);
+    is(ref($new_transfer), 'Koha::Item::Transfer', "updateWrongTransfer returns a 'Koha::Item::Transfer' object");
+    ok( !$new_transfer->in_transit, "New transfer is NOT created as in transit (or cancelled)");
+
+    my $original_transfer = $transfer->get_from_storage;
+    ok( defined($original_transfer->datecancelled), "Original transfer was cancelled");
+    is( $original_transfer->cancellation_reason, 'WrongTransfer', "Original transfer cancellation reason is 'WrongTransfer'");
+};
+
 $schema->storage->txn_rollback;
 C4::Context->clear_syspref_cache();
 $branches = Koha::Libraries->search();