From 6f296277d9b230c21121a4fc95fdbf18b8d858f8 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Thu, 9 May 2019 16:34:09 +0100 Subject: [PATCH] Bug 22877: (QA follow-up) Clarify intent of tests and remove one. This patch embelishes the existing comments slightly and removes a test which had a FIXME, but was in fact just a duplicate of the test directly above it. I beleive it became a duplicate after the subroutine signature changed at some point recently. I also move the bug 22877 specific test into a subtest. Signed-off-by: Martin Renvoize --- t/db_dependent/Circulation.t | 60 +++++++++++++++++++++--------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index b84b2d20f9..4234bb506a 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -1980,7 +1980,7 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { subtest 'AddReturn | is_overdue' => sub { - plan tests => 8; + plan tests => 7; t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1); t::lib::Mocks::mock_preference('finesMode', 'production'); @@ -2039,49 +2039,59 @@ subtest 'AddReturn | is_overdue' => sub { my $ten_days_ago = dt_from_string->subtract( days => 10 ); $patron = Koha::Patrons->find( $patron->{borrowernumber} ); - # No date specify, today will be used + # No return date specified, today will be used => 10 days overdue charged AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago AddReturn( $item->{barcode}, $library->{branchcode} ); is( int($patron->account->balance()), 10, 'Patron should have a charge of 10 (10 days x 1)' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; - # specify return date 5 days before => no overdue + # specify return date 5 days before => no overdue charged AddIssue( $patron->unblessed, $item->{barcode}, $five_days_ago ); # date due was 5d ago AddReturn( $item->{barcode}, $library->{branchcode}, undef, $ten_days_ago ); is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; - # specify return date 5 days later => overdue + # specify return date 5 days later => 5 days overdue charged AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago AddReturn( $item->{barcode}, $library->{branchcode}, undef, $five_days_ago ); is( int($patron->account->balance()), 5, 'AddReturn: pass return_date => overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; - # specify dropbox date 5 days before => no overdue - AddIssue( $patron->unblessed, $item->{barcode}, $five_days_ago ); # date due was 5d ago - AddReturn( $item->{barcode}, $library->{branchcode}, $ten_days_ago ); - is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' ); - Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; - - # specify dropbox date 5 days later => overdue, or... not + # specify return date 5 days later, specify exemptfine => no overdue charge AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago - AddReturn( $item->{barcode}, $library->{branchcode}, $five_days_ago ); - is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue in dropbox mode' ); # FIXME? This is weird, the OVERDUE fine is created ( _CalculateAndUpdateFine > C4::Overdues::UpdateFine ) then remove later (in _FixOverduesOnReturn). Looks like it is a feature + AddReturn( $item->{barcode}, $library->{branchcode}, 1, $five_days_ago ); + is( int($patron->account->balance()), 0, 'AddReturn: pass return_date => no overdue' ); Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; - # Checkout an item 10 days ago - my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d 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) }); - is( int($patron->account->balance()),10, "Overdue fine of 10 days overdue"); - # Fake longoverdue with charge and not marking returned - LostItem( $item->{itemnumber}, 'cronjob',0 ); - is( int($patron->account->balance()),17, "Lost fine of 7 plus 10 days overdue"); - # Now we return it today - AddReturn( $item->{barcode}, $library->{branchcode} ); - is( int($patron->account->balance()),17, "Should have a single 10 days overdue fine and lost charge"); + subtest 'bug 22877' => sub { + my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d 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) + } + ); + is( int( $patron->account->balance() ), + 10, "Overdue fine of 10 days overdue" ); + + # Fake longoverdue with charge and not marking returned + LostItem( $item->{itemnumber}, 'cronjob', 0 ); + is( int( $patron->account->balance() ), + 17, "Lost fine of 7 plus 10 days overdue" ); + + # Now we return it today + AddReturn( $item->{barcode}, $library->{branchcode} ); + is( int( $patron->account->balance() ), + 17, "Should have a single 10 days overdue fine and lost charge" ); + } }; subtest '_FixAccountForLostAndReturned' => sub { -- 2.20.1