X-Git-Url: http://koha-dev.rot13.org:8081/gitweb/?a=blobdiff_plain;f=t%2Fdb_dependent%2FCirculation.t;h=8aa5d4c970dba5331fd367f28df127429c34388e;hb=9d6d641d1f8b77271800f43bc027b651f9aea52b;hp=104fc8fc8595367a0445536feb5f38e1d51fc124;hpb=2849b188c885bb6752e9e5c7db381e291ed4bb30;p=srvgit diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index 104fc8fc85..8aa5d4c970 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -18,9 +18,11 @@ use Modern::Perl; use utf8; -use Test::More tests => 46; +use Test::More tests => 55; +use Test::Exception; use Test::MockModule; use Test::Deep qw( cmp_deeply ); +use Test::Warn; use Data::Dumper; use DateTime; @@ -30,18 +32,20 @@ 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; +use Koha::Item::Transfers; use Koha::Checkouts; use Koha::Patrons; +use Koha::Holds; use Koha::CirculationRules; use Koha::Subscriptions; use Koha::Account::Lines; @@ -80,9 +84,9 @@ sub test_debarment_on_checkout { ); my @caller = caller; my $line_number = $caller[2]; - AddIssue( $patron, $item->{barcode}, $due_date ); + AddIssue( $patron, $item->barcode, $due_date ); - my ( undef, $message ) = AddReturn( $item->{barcode}, $library->{branchcode}, undef, $return_date ); + my ( undef, $message ) = AddReturn( $item->barcode, $library->{branchcode}, undef, $return_date ); is( $message->{WasReturned} && exists $message->{Debarred}, 1, 'AddReturn must have debarred the patron' ) or diag('AddReturn returned message ' . Dumper $message ); my $debarments = Koha::Patron::Debarments::GetDebarments( @@ -108,12 +112,21 @@ $mocked_datetime->mock( 'now', sub { return $now_value->clone; } ); my $cache = Koha::Caches->get_instance(); $dbh->do(q|DELETE FROM special_holidays|); $dbh->do(q|DELETE FROM repeatable_holidays|); -$cache->clear_from_cache('single_holidays'); +my $branches = Koha::Libraries->search(); +for my $branch ( $branches->next ) { + my $key = $branch->branchcode . "_holidays"; + $cache->clear_from_cache($key); +} # Start with a clean slate $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', }); @@ -269,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 => 83; + plan tests => 95; C4::Context->set_preference('ItemsDeniedRenewal',''); # Generate test biblio @@ -406,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'); @@ -509,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( @@ -551,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 = ( @@ -585,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 ); @@ -597,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; @@ -642,10 +813,16 @@ subtest "CanBookBeRenewed tests" => sub { ); ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber ); is( $renewokay, 0, 'Still should not be able to renew' ); - is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked' ); + is( $error, 'on_reserve', 'returned code is on_reserve, reserve checked when not checking for cron' ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, undef, 1 ); + is( $renewokay, 0, 'Still should not be able to renew' ); + is( $error, 'auto_too_soon', 'returned code is auto_too_soon, reserve not checked when checking for cron' ); ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); + ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1, 1 ); + is( $renewokay, 0, 'Still should not be able to renew' ); + is( $error, 'on_reserve', 'returned code is on_reserve, auto_too_soon limit is overridden' ); $dbh->do('UPDATE circulation_rules SET rule_value = 0 where rule_name = "norenewalbefore"'); ( $renewokay, $error ) = CanBookBeRenewed( $renewing_borrowernumber, $item_4->itemnumber, 1 ); is( $renewokay, 0, 'Still should not be able to renew' ); @@ -742,19 +919,16 @@ subtest "CanBookBeRenewed tests" => sub { subtest "too_late_renewal / no_auto_renewal_after" => sub { plan tests => 14; - my $item_to_auto_renew = $builder->build( - { source => 'Item', - value => { - biblionumber => $biblio->biblionumber, - homebranch => $branch, - holdingbranch => $branch, - } + my $item_to_auto_renew = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $branch, } ); my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -768,7 +942,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -784,7 +958,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot auto renew, too late - no_auto_renewal_after is inclusive(returned code is auto_too_late)' ); @@ -800,7 +974,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_soon', 'Cannot auto renew, too soon - no_auto_renewal_after is defined(returned code is auto_too_soon)' ); @@ -816,7 +990,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); @@ -833,7 +1007,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -850,7 +1024,7 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_late', 'Cannot renew, too late(returned code is auto_too_late)' ); @@ -867,25 +1041,23 @@ subtest "CanBookBeRenewed tests" => sub { } ); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Cannot renew, renew is automatic' ); }; subtest "auto_too_much_oweing | OPACFineNoRenewalsBlockAutoRenew & OPACFineNoRenewalsIncludeCredit" => sub { plan tests => 10; - my $item_to_auto_renew = $builder->build({ - source => 'Item', - value => { + my $item_to_auto_renew = $builder->build_sample_item( + { biblionumber => $biblio->biblionumber, - homebranch => $branch, - holdingbranch => $branch, + library => $branch, } - }); + ); my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { @@ -908,12 +1080,12 @@ subtest "CanBookBeRenewed tests" => sub { amount => $fines_amount, interface => 'test', type => 'OVERDUE', - item_id => $item_to_auto_renew->{itemnumber}, + item_id => $item_to_auto_renew->itemnumber, description => "Some fines" } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 5' ); @@ -922,12 +1094,12 @@ subtest "CanBookBeRenewed tests" => sub { amount => $fines_amount, interface => 'test', type => 'OVERDUE', - item_id => $item_to_auto_renew->{itemnumber}, + item_id => $item_to_auto_renew->itemnumber, description => "Some fines" } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, patron has 10' ); @@ -936,12 +1108,12 @@ subtest "CanBookBeRenewed tests" => sub { amount => $fines_amount, interface => 'test', type => 'OVERDUE', - item_id => $item_to_auto_renew->{itemnumber}, + item_id => $item_to_auto_renew->itemnumber, description => "Some fines" } )->status('RETURNED')->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, patron has 15' ); @@ -954,13 +1126,13 @@ subtest "CanBookBeRenewed tests" => sub { } )->store; ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); C4::Context->set_preference('OPACFineNoRenewalsIncludeCredit','0'); ( $renewokay, $error ) = - CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_too_much_oweing', 'Cannot auto renew, OPACFineNoRenewals=10, OPACFineNoRenewalsIncludeCredit=1, patron has 15 debt, 5 credit' ); @@ -970,14 +1142,12 @@ subtest "CanBookBeRenewed tests" => sub { subtest "auto_account_expired | BlockExpiredPatronOpacActions" => sub { plan tests => 6; - my $item_to_auto_renew = $builder->build({ - source => 'Item', - value => { + my $item_to_auto_renew = $builder->build_sample_item( + { biblionumber => $biblio->biblionumber, - homebranch => $branch, - holdingbranch => $branch, + library => $branch, } - }); + ); Koha::CirculationRules->set_rules( { @@ -998,9 +1168,9 @@ subtest "CanBookBeRenewed tests" => sub { # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 0); my $patron = $expired_borrower; - my $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + my $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, patron is expired but BlockExpiredPatronOpacActions=0' ); Koha::Checkouts->find( $checkout->issue_id )->delete; @@ -1010,9 +1180,9 @@ subtest "CanBookBeRenewed tests" => sub { # => auto renew is not allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); $patron = $expired_borrower; - $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_account_expired', 'Can not auto renew, lockExpiredPatronOpacActions=1 and patron is expired' ); Koha::Checkouts->find( $checkout->issue_id )->delete; @@ -1022,9 +1192,9 @@ subtest "CanBookBeRenewed tests" => sub { # => auto renew is allowed t::lib::Mocks::mock_preference('BlockExpiredPatronOpacActions', 1); $patron = $renewing_borrower; - $checkout = AddIssue( $patron, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + $checkout = AddIssue( $patron, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); ( $renewokay, $error ) = - CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->{itemnumber} ); + CanBookBeRenewed( $patron->{borrowernumber}, $item_to_auto_renew->itemnumber ); is( $renewokay, 0, 'Do not renew, renewal is automatic' ); is( $error, 'auto_renew', 'Can auto renew, BlockExpiredPatronOpacActions=1 but patron is not expired' ); Koha::Checkouts->find( $checkout->issue_id )->delete; @@ -1032,19 +1202,16 @@ subtest "CanBookBeRenewed tests" => sub { subtest "GetLatestAutoRenewDate" => sub { plan tests => 5; - my $item_to_auto_renew = $builder->build( - { source => 'Item', - value => { - biblionumber => $biblio->biblionumber, - homebranch => $branch, - holdingbranch => $branch, - } + my $item_to_auto_renew = $builder->build_sample_item( + { + biblionumber => $biblio->biblionumber, + library => $branch, } ); my $ten_days_before = dt_from_string->add( days => -10 ); my $ten_days_ahead = dt_from_string->add( days => 10 ); - AddIssue( $renewing_borrower, $item_to_auto_renew->{barcode}, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); + AddIssue( $renewing_borrower, $item_to_auto_renew->barcode, $ten_days_ahead, undef, $ten_days_before, undef, { auto_renew => 1 } ); Koha::CirculationRules->set_rules( { categorycode => undef, @@ -1057,7 +1224,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + my $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $latest_auto_renew_date, undef, 'GetLatestAutoRenewDate should return undef if no_auto_renewal_after or no_auto_renewal_after_hard_limit are not defined' ); my $five_days_before = dt_from_string->add( days => -5 ); Koha::CirculationRules->set_rules( @@ -1072,7 +1239,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_before->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return -5 days if no_auto_renewal_after = 5 and date_due is 10 days before' @@ -1093,7 +1260,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $latest_auto_renew_date->truncate( to => 'minute' ), $five_days_ahead->truncate( to => 'minute' ), 'GetLatestAutoRenewDate should return +5 days if no_auto_renewal_after = 15 and date_due is 10 days before' @@ -1111,7 +1278,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is defined and not no_auto_renewal_after' @@ -1128,7 +1295,7 @@ subtest "CanBookBeRenewed tests" => sub { } } ); - $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->{itemnumber} ); + $latest_auto_renew_date = GetLatestAutoRenewDate( $renewing_borrowernumber, $item_to_auto_renew->itemnumber ); is( $latest_auto_renew_date->truncate( to => 'day' ), $two_days_ahead->truncate( to => 'day' ), 'GetLatestAutoRenewDate should return +2 days if no_auto_renewal_after_hard_limit is < no_auto_renewal_after' @@ -1154,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'); @@ -1232,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'); @@ -1595,11 +1785,11 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { homebranch => $homebranch->{branchcode}, holdingbranch => $holdingbranch->{branchcode}, } - )->unblessed; + ); set_userenv($holdingbranch); - my $issue = AddIssue( $patron_1->unblessed, $item->{barcode} ); + my $issue = AddIssue( $patron_1->unblessed, $item->barcode ); is( ref($issue), 'Koha::Checkout', 'AddIssue should return a Koha::Checkout object' ); my ( $error, $question, $alerts ); @@ -1612,16 +1802,16 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { ok( $error->{UNKNOWN_BARCODE}, '"KohaIsAwesome" is not a valid barcode as expected.' ); ## Can be issued from homebranch set_userenv($homebranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER must be set' ); ## Can be issued from holdingbranch set_userenv($holdingbranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER must be set' ); ## Can be issued from another branch - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER must be set' ); @@ -1629,18 +1819,18 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); ## Cannot be issued from homebranch set_userenv($homebranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); is( $error->{branch_to_return}, $holdingbranch->{branchcode}, 'branch_to_return matched holdingbranch' ); ## Can be issued from holdinbranch set_userenv($holdingbranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER must be set' ); ## Cannot be issued from another branch set_userenv($otherbranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); is( $error->{branch_to_return}, $holdingbranch->{branchcode}, 'branch_to_return matches holdingbranch' ); @@ -1649,18 +1839,18 @@ subtest 'CanBookBeIssued & AllowReturnToBranch' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); ## Can be issued from holdinbranch set_userenv($homebranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER must be set' ); ## Cannot be issued from holdinbranch set_userenv($holdingbranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); is( $error->{branch_to_return}, $homebranch->{branchcode}, 'branch_to_return matches homebranch' ); ## Cannot be issued from holdinbranch set_userenv($otherbranch); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron_2, $item->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'There should not be any errors or alerts (impossible)' . str($error, $question, $alerts) ); is( exists $error->{RETURN_IMPOSSIBLE}, 1, 'RETURN_IMPOSSIBLE must be set' ); is( $error->{branch_to_return}, $homebranch->{branchcode}, 'branch_to_return matches homebranch' ); @@ -1682,12 +1872,12 @@ subtest 'AddIssue & AllowReturnToBranch' => sub { homebranch => $homebranch->{branchcode}, holdingbranch => $holdingbranch->{branchcode}, } - )->unblessed; + ); set_userenv($holdingbranch); my $ref_issue = 'Koha::Checkout'; - my $issue = AddIssue( $patron_1, $item->{barcode} ); + my $issue = AddIssue( $patron_1, $item->barcode ); my ( $error, $question, $alerts ); @@ -1695,42 +1885,42 @@ subtest 'AddIssue & AllowReturnToBranch' => sub { t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'anywhere' ); ## Can be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from homebranch'); - set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue + is ( ref( AddIssue( $patron_2, $item->barcode ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from homebranch'); + set_userenv($holdingbranch); AddIssue( $patron_1, $item->barcode ); # Reinsert the original issue ## Can be issued from holdinbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from holdingbranch'); - set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue + is ( ref( AddIssue( $patron_2, $item->barcode ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from holdingbranch'); + set_userenv($holdingbranch); AddIssue( $patron_1, $item->barcode ); # Reinsert the original issue ## Can be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from otherbranch'); - set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue + is ( ref( AddIssue( $patron_2, $item->barcode ) ), $ref_issue, 'AllowReturnToBranch - anywhere | Can be issued from otherbranch'); + set_userenv($holdingbranch); AddIssue( $patron_1, $item->barcode ); # Reinsert the original issue # AllowReturnToBranch == holdinbranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'holdingbranch' ); ## Cannot be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from homebranch'); + is ( ref( AddIssue( $patron_2, $item->barcode ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from homebranch'); ## Can be issued from holdingbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - holdingbranch | Can be issued from holdingbranch'); - set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue + is ( ref( AddIssue( $patron_2, $item->barcode ) ), $ref_issue, 'AllowReturnToBranch - holdingbranch | Can be issued from holdingbranch'); + set_userenv($holdingbranch); AddIssue( $patron_1, $item->barcode ); # Reinsert the original issue ## Cannot be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from otherbranch'); + is ( ref( AddIssue( $patron_2, $item->barcode ) ), '', 'AllowReturnToBranch - holdingbranch | Cannot be issued from otherbranch'); # AllowReturnToBranch == homebranch t::lib::Mocks::mock_preference( 'AllowReturnToBranch', 'homebranch' ); ## Can be issued from homebranch set_userenv($homebranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), $ref_issue, 'AllowReturnToBranch - homebranch | Can be issued from homebranch' ); - set_userenv($holdingbranch); AddIssue( $patron_1, $item->{barcode} ); # Reinsert the original issue + is ( ref( AddIssue( $patron_2, $item->barcode ) ), $ref_issue, 'AllowReturnToBranch - homebranch | Can be issued from homebranch' ); + set_userenv($holdingbranch); AddIssue( $patron_1, $item->barcode ); # Reinsert the original issue ## Cannot be issued from holdinbranch set_userenv($holdingbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from holdingbranch' ); + is ( ref( AddIssue( $patron_2, $item->barcode ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from holdingbranch' ); ## Cannot be issued from another branch set_userenv($otherbranch); - is ( ref( AddIssue( $patron_2, $item->{barcode} ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from otherbranch' ); + is ( ref( AddIssue( $patron_2, $item->barcode ) ), '', 'AllowReturnToBranch - homebranch | Cannot be issued from otherbranch' ); # TODO t::lib::Mocks::mock_preference('AllowReturnToBranch', 'homeorholdingbranch'); }; @@ -1743,38 +1933,38 @@ subtest 'CanBookBeIssued + Koha::Patron->is_debarred|has_overdues' => sub { { library => $library->{branchcode}, } - )->unblessed; + ); my $item_2 = $builder->build_sample_item( { library => $library->{branchcode}, } - )->unblessed; + ); my ( $error, $question, $alerts ); # Patron cannot issue item_1, they have overdues my $yesterday = DateTime->today( time_zone => C4::Context->tz() )->add( days => -1 ); - my $issue = AddIssue( $patron->unblessed, $item_1->{barcode}, $yesterday ); # Add an overdue + my $issue = AddIssue( $patron->unblessed, $item_1->barcode, $yesterday ); # Add an overdue t::lib::Mocks::mock_preference( 'OverduesBlockCirc', 'confirmation' ); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); is( keys(%$error) + keys(%$alerts), 0, 'No key for error and alert' . str($error, $question, $alerts) ); is( $question->{USERBLOCKEDOVERDUE}, 1, 'OverduesBlockCirc=confirmation, USERBLOCKEDOVERDUE should be set for question' ); t::lib::Mocks::mock_preference( 'OverduesBlockCirc', 'block' ); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'No key for question and alert ' . str($error, $question, $alerts) ); is( $error->{USERBLOCKEDOVERDUE}, 1, 'OverduesBlockCirc=block, USERBLOCKEDOVERDUE should be set for error' ); # Patron cannot issue item_1, they are debarred my $tomorrow = DateTime->today( time_zone => C4::Context->tz() )->add( days => 1 ); Koha::Patron::Debarments::AddDebarment( { borrowernumber => $patron->borrowernumber, expiration => $tomorrow } ); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'No key for question and alert ' . str($error, $question, $alerts) ); is( $error->{USERBLOCKEDWITHENDDATE}, output_pref( { dt => $tomorrow, dateformat => 'sql', dateonly => 1 } ), 'USERBLOCKEDWITHENDDATE should be tomorrow' ); Koha::Patron::Debarments::AddDebarment( { borrowernumber => $patron->borrowernumber } ); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); is( keys(%$question) + keys(%$alerts), 0, 'No key for question and alert ' . str($error, $question, $alerts) ); is( $error->{USERBLOCKEDNOENDDATE}, '9999-12-31', 'USERBLOCKEDNOENDDATE should be 9999-12-31 for unlimited debarments' ); }; @@ -1805,9 +1995,9 @@ subtest 'CanBookBeIssued + Statistic patrons "X"' => sub { { library => $library->{branchcode}, } - )->unblessed; + ); - my ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_1->{barcode} ); + my ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_1->barcode ); is( $error->{STATS}, 1, '"Error" flag "STATS" must be set if CanBookBeIssued is called with a statistic patron (category_type=X)' ); # TODO There are other tests to provide here @@ -1937,20 +2127,20 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { biblionumber => $biblionumber, library => $library->{branchcode}, } - )->unblessed; + ); my $item_2 = $builder->build_sample_item( { biblionumber => $biblionumber, library => $library->{branchcode}, } - )->unblessed; + ); my ( $error, $question, $alerts ); - my $issue = AddIssue( $patron->unblessed, $item_1->{barcode}, dt_from_string->add( days => 1 ) ); + my $issue = AddIssue( $patron->unblessed, $item_1->barcode, dt_from_string->add( days => 1 ) ); t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); cmp_deeply( { error => $error, alerts => $alerts }, { error => {}, alerts => {} }, @@ -1959,7 +2149,7 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { is( $question->{BIBLIO_ALREADY_ISSUED}, 1, 'BIBLIO_ALREADY_ISSUED question flag should be set if AllowMultipleIssuesOnABiblio=0 and issue already exists' ); t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); cmp_deeply( { error => $error, question => $question, alerts => $alerts }, { error => {}, question => {}, alerts => {} }, @@ -1970,7 +2160,7 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { Koha::Subscription->new({ biblionumber => $biblionumber })->store; t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 0); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); cmp_deeply( { error => $error, question => $question, alerts => $alerts }, { error => {}, question => {}, alerts => {} }, @@ -1978,7 +2168,7 @@ subtest 'CanBookBeIssued + AllowMultipleIssuesOnABiblio' => sub { ); t::lib::Mocks::mock_preference('AllowMultipleIssuesOnABiblio', 1); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron, $item_2->barcode ); cmp_deeply( { error => $error, question => $question, alerts => $alerts }, { error => {}, question => {}, alerts => {} }, @@ -2003,13 +2193,13 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { biblionumber => $biblionumber, library => $library->{branchcode}, } - )->unblessed; + ); my $item_2 = $builder->build_sample_item( { biblionumber => $biblionumber, library => $library->{branchcode}, } - )->unblessed; + ); # And the circulation rule Koha::CirculationRules->search->delete; @@ -2031,12 +2221,12 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { my $now = dt_from_string; my $five_days_ago = $now->clone->subtract( days => 5 ); my $ten_days_ago = $now->clone->subtract( days => 10 ); - AddIssue( $patron, $item_1->{barcode}, $five_days_ago ); # Add an overdue - AddIssue( $patron, $item_2->{barcode}, $ten_days_ago ) + AddIssue( $patron, $item_1->barcode, $five_days_ago ); # Add an overdue + AddIssue( $patron, $item_2->barcode, $ten_days_ago ) ; # Add another overdue t::lib::Mocks::mock_preference( 'CumulativeRestrictionPeriods', '0' ); - AddReturn( $item_1->{barcode}, $library->{branchcode}, undef, $now ); + AddReturn( $item_1->barcode, $library->{branchcode}, undef, $now ); my $debarments = Koha::Patron::Debarments::GetDebarments( { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); is( scalar(@$debarments), 1 ); @@ -2052,7 +2242,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { ); is( $debarments->[0]->{expiration}, $expected_expiration ); - AddReturn( $item_2->{barcode}, $library->{branchcode}, undef, $now ); + AddReturn( $item_2->barcode, $library->{branchcode}, undef, $now ); $debarments = Koha::Patron::Debarments::GetDebarments( { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); is( scalar(@$debarments), 1 ); @@ -2069,10 +2259,10 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); t::lib::Mocks::mock_preference( 'CumulativeRestrictionPeriods', '1' ); - AddIssue( $patron, $item_1->{barcode}, $five_days_ago ); # Add an overdue - AddIssue( $patron, $item_2->{barcode}, $ten_days_ago ) + AddIssue( $patron, $item_1->barcode, $five_days_ago ); # Add an overdue + AddIssue( $patron, $item_2->barcode, $ten_days_ago ) ; # Add another overdue - AddReturn( $item_1->{barcode}, $library->{branchcode}, undef, $now ); + AddReturn( $item_1->barcode, $library->{branchcode}, undef, $now ); $debarments = Koha::Patron::Debarments::GetDebarments( { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); is( scalar(@$debarments), 1 ); @@ -2085,7 +2275,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { ); is( $debarments->[0]->{expiration}, $expected_expiration ); - AddReturn( $item_2->{barcode}, $library->{branchcode}, undef, $now ); + AddReturn( $item_2->barcode, $library->{branchcode}, undef, $now ); $debarments = Koha::Patron::Debarments::GetDebarments( { borrowernumber => $patron->{borrowernumber}, type => 'SUSPENSION' } ); is( scalar(@$debarments), 1 ); @@ -2100,7 +2290,7 @@ subtest 'AddReturn + CumulativeRestrictionPeriods' => sub { }; subtest 'AddReturn + suspension_chargeperiod' => sub { - plan tests => 21; + plan tests => 27; my $library = $builder->build( { source => 'Branch' } ); my $patron = $builder->build( { source => 'Borrower', value => { categorycode => $patron_category->{categorycode} } } ); @@ -2115,7 +2305,7 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { biblionumber => $biblionumber, library => $library->{branchcode}, } - )->unblessed; + ); # And the issuing rule Koha::CirculationRules->search->delete; @@ -2149,6 +2339,38 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { } ); + # Same with undef firstremind + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => '*', + itemtype => '*', + branchcode => '*', + rules => { + issuelength => 1, + firstremind => undef, # 0 day of grace + finedays => 2, # 2 days of fine per day of overdue + suspension_chargeperiod => 1, + lengthunit => 'days', + } + } + ); + { + my $now = dt_from_string; + my $five_days_ago = $now->clone->subtract( days => 5 ); + # We want to charge 2 days every day, without grace + # With 5 days of overdue: 5 * Z + my $expected_expiration = $now->clone->add( days => ( 5 * 2 ) / 1 ); + test_debarment_on_checkout( + { + item => $item_1, + library => $library, + patron => $patron, + due_date => $five_days_ago, + expiration_date => $expected_expiration, + } + ); + } # We want to charge 2 days every 2 days, without grace # With 5 days of overdue: (5 * 2) / 2 Koha::CirculationRules->set_rule( @@ -2286,6 +2508,18 @@ subtest 'AddReturn + suspension_chargeperiod' => sub { expiration_date => $now->clone->add(days => 5 + (5 * 2 - 1) ), } ); + + test_debarment_on_checkout( + { + item => $item_1, + library => $library, + patron => $patron, + due_date => $now->clone->add(days => 1), + return_date => $now->clone->add(days => 5), + expiration_date => $now->clone->add(days => 5 + (4 * 2 - 1) ), + } + ); + }; subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { @@ -2317,17 +2551,17 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { { library => $library->branchcode, } - )->unblessed; + ); my ( $error, $question, $alerts ); - my $issue = AddIssue( $patron1->unblessed, $item->{barcode} ); + my $issue = AddIssue( $patron1->unblessed, $item->barcode ); t::lib::Mocks::mock_preference('AutoReturnCheckedOutItems', 0); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron2, $item->barcode ); is( $question->{ISSUED_TO_ANOTHER}, 1, 'ISSUED_TO_ANOTHER question flag should be set if AutoReturnCheckedOutItems is disabled and item is checked out to another' ); t::lib::Mocks::mock_preference('AutoReturnCheckedOutItems', 1); - ( $error, $question, $alerts ) = CanBookBeIssued( $patron2, $item->{barcode} ); + ( $error, $question, $alerts ) = CanBookBeIssued( $patron2, $item->barcode ); is( $alerts->{RETURNED_FROM_ANOTHER}->{patron}->borrowernumber, $patron1->borrowernumber, 'RETURNED_FROM_ANOTHER alert flag should be set if AutoReturnCheckedOutItems is enabled and item is checked out to another' ); t::lib::Mocks::mock_preference('AutoReturnCheckedOutItems', 0); @@ -2335,8 +2569,9 @@ subtest 'CanBookBeIssued + AutoReturnCheckedOutItems' => sub { subtest 'AddReturn | is_overdue' => sub { - plan tests => 5; + plan tests => 9; + t::lib::Mocks::mock_preference('MarkLostItemsAsReturned', 'batchmod|moredetail|cronjob|additem|pendingreserves|onpayment'); t::lib::Mocks::mock_preference('CalculateFinesOnReturn', 1); t::lib::Mocks::mock_preference('finesMode', 'production'); t::lib::Mocks::mock_preference('MaxFine', '100'); @@ -2351,7 +2586,7 @@ subtest 'AddReturn | is_overdue' => sub { library => $library->{branchcode}, replacementprice => 7 } - )->unblessed; + ); Koha::CirculationRules->search->delete; Koha::CirculationRules->set_rules( @@ -2370,39 +2605,40 @@ subtest 'AddReturn | is_overdue' => sub { my $now = dt_from_string; my $one_day_ago = $now->clone->subtract( days => 1 ); + my $two_days_ago = $now->clone->subtract( days => 2 ); my $five_days_ago = $now->clone->subtract( days => 5 ); my $ten_days_ago = $now->clone->subtract( days => 10 ); $patron = Koha::Patrons->find( $patron->{borrowernumber} ); # 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} ); + 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 charged - AddIssue( $patron->unblessed, $item->{barcode}, $five_days_ago ); # date due was 5d ago - AddReturn( $item->{barcode}, $library->{branchcode}, undef, $ten_days_ago ); + 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 => 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 ); + 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 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}, 1, $five_days_ago ); + AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago + 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; - subtest 'bug 22877' => sub { + subtest 'bug 22877 | Lost item return' => sub { plan tests => 3; - my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $ten_days_ago ); # date due was 10d ago + my $issue = AddIssue( $patron->unblessed, $item->barcode, $ten_days_ago ); # date due was 10d ago # Fake fines cronjob on this checkout my ($fine) = @@ -2411,7 +2647,7 @@ subtest 'AddReturn | is_overdue' => sub { UpdateFine( { issue_id => $issue->issue_id, - itemnumber => $item->{itemnumber}, + itemnumber => $item->itemnumber, borrowernumber => $patron->borrowernumber, amount => $fine, due => output_pref($ten_days_ago) @@ -2421,410 +2657,686 @@ subtest 'AddReturn | is_overdue' => sub { 10, "Overdue fine of 10 days overdue" ); # Fake longoverdue with charge and not marking returned - LostItem( $item->{itemnumber}, 'cronjob', 0 ); + 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} ); + AddReturn( $item->barcode, $library->{branchcode} ); is( int( $patron->account->balance() ), 17, "Should have a single 10 days overdue fine and lost charge" ); - } -}; -subtest '_FixAccountForLostAndFound' => sub { + # Cleanup + Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; + }; - plan tests => 5; + subtest 'bug 8338 | backdated return resulting in zero amount fine' => sub { - t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); - t::lib::Mocks::mock_preference( 'WhenLostForgiveFine', 0 ); + plan tests => 17; - my $processfee_amount = 20; - my $replacement_amount = 99.00; - my $item_type = $builder->build_object( - { class => 'Koha::ItemTypes', - value => { - notforloan => undef, - rentalcharge => 0, - defaultreplacecost => undef, - processfee => $processfee_amount, - rentalcharge_daily => 0, - } - } - ); - my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); - my $biblio = $builder->build_sample_biblio({ author => 'Hall, Daria' }); + my $issue = AddIssue( $patron->unblessed, $item->barcode, $one_day_ago ); # date due was 1d ago - subtest 'Full write-off tests' => sub { + # Fake fines cronjob on this checkout + my ($fine) = + CalcFine( $item, $patron->categorycode, $library->{branchcode}, + $one_day_ago, $now ); + UpdateFine( + { + issue_id => $issue->issue_id, + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + amount => $fine, + due => output_pref($one_day_ago) + } + ); + is( int( $patron->account->balance() ), + 1, "Overdue fine of 1 day overdue" ); - plan tests => 12; + # Backdated return (dropbox mode example - charge should be removed) + AddReturn( $item->barcode, $library->{branchcode}, 1, $one_day_ago ); + is( int( $patron->account->balance() ), + 0, "Overdue fine should be annulled" ); + my $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); + is( $lines->count, 0, "Overdue fine accountline has been removed"); - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $manager = $builder->build_object({ class => "Koha::Patrons" }); - t::lib::Mocks::mock_userenv({ patron => $manager,branchcode => $manager->branchcode }); + $issue = AddIssue( $patron->unblessed, $item->barcode, $two_days_ago ); # date due was 2d ago - my $item = $builder->build_sample_item( + # Fake fines cronjob on this checkout + ($fine) = + CalcFine( $item, $patron->categorycode, $library->{branchcode}, + $two_days_ago, $now ); + UpdateFine( { - biblionumber => $biblio->biblionumber, - library => $library->branchcode, - replacementprice => $replacement_amount, - itype => $item_type->itemtype, + issue_id => $issue->issue_id, + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + amount => $fine, + due => output_pref($one_day_ago) } ); + is( int( $patron->account->balance() ), + 2, "Overdue fine of 2 days overdue" ); - AddIssue( $patron->unblessed, $item->barcode ); - - # Simulate item marked as lost - $item->itemlost(3)->store; - LostItem( $item->itemnumber, 1 ); - - my $processing_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'PROCESSING' } ); - is( $processing_fee_lines->count, 1, 'Only one processing fee produced' ); - my $processing_fee_line = $processing_fee_lines->next; - is( $processing_fee_line->amount + 0, - $processfee_amount, 'The right PROCESSING amount is generated' ); - is( $processing_fee_line->amountoutstanding + 0, - $processfee_amount, 'The right PROCESSING amountoutstanding is generated' ); - - my $lost_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'LOST' } ); - is( $lost_fee_lines->count, 1, 'Only one 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' ); - - my $account = $patron->account; - my $debts = $account->outstanding_debits; - - # Write off the debt - my $credit = $account->add_credit( - { amount => $account->balance, - type => 'WRITEOFF', + # Payment made against fine + $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); + my $debit = $lines->next; + my $credit = $patron->account->add_credit( + { + amount => 2, + type => 'PAYMENT', interface => 'test', } ); - $credit->apply( { debits => [ $debts->as_list ], offset_type => 'Writeoff' } ); + $credit->apply( + { debits => [ $debit ], offset_type => 'Payment' } ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); - is( $credit_return_id, undef, 'No LOST_FOUND account line added' ); - - $lost_fee_line->discard_changes; # reload from DB - is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); - is( $lost_fee_line->debit_type_code, - 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); - - is( $patron->account->balance, -0, 'The patron balance is 0, everything was written off' ); + is( int( $patron->account->balance() ), + 0, "Overdue fine should be paid off" ); + $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }); + is ( $lines->count, 2, "Overdue (debit) and Payment (credit) present"); + my $line = $lines->next; + is( $line->amount+0, 2, "Overdue fine amount remains as 2 days"); + is( $line->amountoutstanding+0, 0, "Overdue fine amountoutstanding reduced to 0"); + + # Backdated return (dropbox mode example - charge should be removed) + AddReturn( $item->barcode, $library->{branchcode}, undef, $one_day_ago ); + is( int( $patron->account->balance() ), + -1, "Refund credit has been applied" ); + $lines = Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber }, { order_by => { '-asc' => 'accountlines_id' }}); + is( $lines->count, 3, "Overdue (debit), Payment (credit) and Refund (credit) are all present"); + + $line = $lines->next; + is($line->amount+0,1, "Overdue fine amount has been reduced to 1"); + is($line->amountoutstanding+0,0, "Overdue fine amount outstanding remains at 0"); + is($line->status,'RETURNED', "Overdue fine is fixed"); + $line = $lines->next; + is($line->amount+0,-2, "Original payment amount remains as 2"); + is($line->amountoutstanding+0,0, "Original payment remains applied"); + $line = $lines->next; + is($line->amount+0,-1, "Refund amount correctly set to 1"); + is($line->amountoutstanding+0,-1, "Refund amount outstanding unspent"); + + # Cleanup + Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; }; - subtest 'Full payment tests' => sub { + subtest 'bug 25417 | backdated return + exemptfine' => sub { - plan tests => 13; + plan tests => 2; - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + t::lib::Mocks::mock_preference('CalculateFinesOnBackdate', 1); - my $item = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - library => $library->branchcode, - replacementprice => $replacement_amount, - itype => $item_type->itemtype - } - ); + my $issue = AddIssue( $patron->unblessed, $item->barcode, $one_day_ago ); # date due was 1d ago - AddIssue( $patron->unblessed, $item->barcode ); - - # Simulate item marked as lost - $item->itemlost(1)->store; - LostItem( $item->itemnumber, 1 ); - - my $processing_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'PROCESSING' } ); - is( $processing_fee_lines->count, 1, 'Only one processing fee produced' ); - my $processing_fee_line = $processing_fee_lines->next; - is( $processing_fee_line->amount + 0, - $processfee_amount, 'The right PROCESSING amount is generated' ); - is( $processing_fee_line->amountoutstanding + 0, - $processfee_amount, 'The right PROCESSING amountoutstanding is generated' ); - - my $lost_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'LOST' } ); - is( $lost_fee_lines->count, 1, 'Only one 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 amountountstanding is generated' ); - - my $account = $patron->account; - my $debts = $account->outstanding_debits; - - # Write off the debt - my $credit = $account->add_credit( - { amount => $account->balance, - type => 'PAYMENT', - interface => 'test', + # Fake fines cronjob on this checkout + my ($fine) = + CalcFine( $item, $patron->categorycode, $library->{branchcode}, + $one_day_ago, $now ); + UpdateFine( + { + issue_id => $issue->issue_id, + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + amount => $fine, + due => output_pref($one_day_ago) } ); - $credit->apply( { debits => [ $debts->as_list ], offset_type => 'Payment' } ); - - my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); - my $credit_return = Koha::Account::Lines->find($credit_return_id); - - is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); - is( $credit_return->amount + 0, - -99.00, 'The account line of type LOST_FOUND has an amount of -99' ); - is( $credit_return->amountoutstanding + 0, - -99.00, 'The account line of type LOST_FOUND has an amountoutstanding of -99' ); + is( int( $patron->account->balance() ), + 1, "Overdue fine of 1 day overdue" ); - $lost_fee_line->discard_changes; - is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); - is( $lost_fee_line->debit_type_code, - 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); + # Backdated return (dropbox mode example - charge should no longer exist) + AddReturn( $item->barcode, $library->{branchcode}, 1, $one_day_ago ); + is( int( $patron->account->balance() ), + 0, "Overdue fine should be annulled" ); - is( $patron->account->balance, - -99, 'The patron balance is -99, a credit that equals the lost fee payment' ); + # Cleanup + Koha::Account::Lines->search({ borrowernumber => $patron->borrowernumber })->delete; }; - subtest 'Test without payment or write off' => sub { + subtest 'bug 24075 | backdated return with return datetime matching due datetime' => sub { + plan tests => 7; - plan tests => 13; + t::lib::Mocks::mock_preference( 'CalculateFinesOnBackdate', 1 ); - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + my $due_date = dt_from_string; + my $issue = AddIssue( $patron->unblessed, $item->barcode, $due_date ); - my $item = $builder->build_sample_item( + # Add fine + UpdateFine( { - biblionumber => $biblio->biblionumber, - library => $library->branchcode, - replacementprice => 23.00, - replacementprice => $replacement_amount, - itype => $item_type->itemtype + issue_id => $issue->issue_id, + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + amount => 0.25, + due => output_pref($due_date) } ); + is( $patron->account->balance(), + 0.25, 'Overdue fine of $0.25 recorded' ); - AddIssue( $patron->unblessed, $item->barcode ); - - # Simulate item marked as lost - $item->itemlost(3)->store; - LostItem( $item->itemnumber, 1 ); - - my $processing_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'PROCESSING' } ); - is( $processing_fee_lines->count, 1, 'Only one processing fee produced' ); - my $processing_fee_line = $processing_fee_lines->next; - is( $processing_fee_line->amount + 0, - $processfee_amount, 'The right PROCESSING amount is generated' ); - is( $processing_fee_line->amountoutstanding + 0, - $processfee_amount, 'The right PROCESSING amountoutstanding is generated' ); - - my $lost_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'LOST' } ); - is( $lost_fee_lines->count, 1, 'Only one 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 amountountstanding is generated' ); - - my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); - my $credit_return = Koha::Account::Lines->find($credit_return_id); - - is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); - is( $credit_return->amount + 0, -99.00, 'The account line of type LOST_FOUND has an amount of -99' ); - is( $credit_return->amountoutstanding + 0, 0, 'The account line of type LOST_FOUND has an amountoutstanding of 0' ); - - $lost_fee_line->discard_changes; - is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); - is( $lost_fee_line->debit_type_code, - 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); - - is( $patron->account->balance, 20, 'The patron balance is 20, still owes the processing fee' ); - }; + # Backdate return to exact due date and time + my ( undef, $message ) = + AddReturn( $item->barcode, $library->{branchcode}, + undef, $due_date ); - subtest 'Test with partial payement and write off, and remaining debt' => sub { + my $accountline = + Koha::Account::Lines->find( { issue_id => $issue->id } ); + ok( !$accountline, 'accountline removed as expected' ); - plan tests => 16; + # Re-issue + $issue = AddIssue( $patron->unblessed, $item->barcode, $due_date ); - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $item = $builder->build_sample_item( + # Add fine + UpdateFine( { - biblionumber => $biblio->biblionumber, - library => $library->branchcode, - replacementprice => $replacement_amount, - itype => $item_type->itemtype + issue_id => $issue->issue_id, + itemnumber => $item->itemnumber, + borrowernumber => $patron->borrowernumber, + amount => .25, + due => output_pref($due_date) } ); + is( $patron->account->balance(), + 0.25, 'Overdue fine of $0.25 recorded' ); - AddIssue( $patron->unblessed, $item->barcode ); - - # Simulate item marked as lost - $item->itemlost(1)->store; - LostItem( $item->itemnumber, 1 ); - - my $processing_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'PROCESSING' } ); - is( $processing_fee_lines->count, 1, 'Only one processing fee produced' ); - my $processing_fee_line = $processing_fee_lines->next; - is( $processing_fee_line->amount + 0, - $processfee_amount, 'The right PROCESSING amount is generated' ); - is( $processing_fee_line->amountoutstanding + 0, - $processfee_amount, 'The right PROCESSING amountoutstanding is generated' ); - - my $lost_fee_lines = Koha::Account::Lines->search( - { borrowernumber => $patron->id, itemnumber => $item->itemnumber, debit_type_code => 'LOST' } ); - is( $lost_fee_lines->count, 1, 'Only one 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 amountountstanding is generated' ); - - my $account = $patron->account; - is( $account->balance, $processfee_amount + $replacement_amount, 'Balance is PROCESSING + L' ); - - # Partially pay fee - my $payment_amount = 27; - my $payment = $account->add_credit( - { amount => $payment_amount, - type => 'PAYMENT', - interface => 'test', + # Partial pay accruing fine + my $lines = Koha::Account::Lines->search( + { + borrowernumber => $patron->borrowernumber, + issue_id => $issue->id } ); - - $payment->apply( { debits => [ $lost_fee_line ], offset_type => 'Payment' } ); - - # Partially write off fee - my $write_off_amount = 25; - my $write_off = $account->add_credit( - { amount => $write_off_amount, - type => 'WRITEOFF', + my $debit = $lines->next; + my $credit = $patron->account->add_credit( + { + amount => .20, + type => 'PAYMENT', interface => 'test', } ); - $write_off->apply( { debits => [ $lost_fee_line ], offset_type => 'Writeoff' } ); - - is( $account->balance, - $processfee_amount + $replacement_amount - $payment_amount - $write_off_amount, - 'Payment and write off applied' - ); - - # Store the amountoutstanding value - $lost_fee_line->discard_changes; - my $outstanding = $lost_fee_line->amountoutstanding; - - my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); - my $credit_return = Koha::Account::Lines->find($credit_return_id); - - is( $account->balance, $processfee_amount - $payment_amount, 'Balance is PROCESSING - PAYMENT (LOST_FOUND)' ); + $credit->apply( { debits => [$debit], offset_type => 'Payment' } ); - $lost_fee_line->discard_changes; - is( $lost_fee_line->amountoutstanding + 0, 0, 'Lost fee has no outstanding amount' ); - is( $lost_fee_line->debit_type_code, - 'LOST', 'Lost fee now still has account type of LOST' ); - is( $lost_fee_line->status, 'FOUND', "Lost fee now has account status of FOUND"); + is( $patron->account->balance(), .05, 'Overdue fine reduced to $0.05' ); - is( $credit_return->credit_type_code, 'LOST_FOUND', 'An account line of type LOST_FOUND is added' ); - is( $credit_return->amount + 0, - ($payment_amount + $outstanding ) * -1, - 'The account line of type LOST_FOUND has an amount equal to the payment + outstanding' - ); - is( $credit_return->amountoutstanding + 0, - $payment_amount * -1, - 'The account line of type LOST_FOUND has an amountoutstanding equal to the payment' - ); + # Backdate return to exact due date and time + ( undef, $message ) = + AddReturn( $item->barcode, $library->{branchcode}, + undef, $due_date ); - is( $account->balance, - $processfee_amount - $payment_amount, - 'The patron balance is the difference between the PROCESSING and the credit' + $lines = Koha::Account::Lines->search( + { + borrowernumber => $patron->borrowernumber, + issue_id => $issue->id + } ); + $accountline = $lines->next; + is( $accountline->amountoutstanding + 0, + 0, 'Partially paid fee amount outstanding was reduced to 0' ); + is( $accountline->amount + 0, + 0, 'Partially paid fee amount was reduced to 0' ); + is( $patron->account->balance(), -0.20, 'Patron refund recorded' ); + + # Cleanup + Koha::Account::Lines->search( + { borrowernumber => $patron->borrowernumber } )->delete; }; - subtest 'Partial payement, existing debits and AccountAutoReconcile' => sub { - - plan tests => 8; + subtest 'enh 23091 | Lost item return policies' => sub { + plan tests => 4; - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - my $barcode = 'KD123456793'; - my $replacement_amount = 100; - my $processfee_amount = 20; + my $manager = $builder->build_object({ class => "Koha::Patrons" }); - my $item_type = $builder->build_object( - { class => 'Koha::ItemTypes', - value => { - notforloan => undef, - rentalcharge => 0, - defaultreplacecost => undef, - processfee => 0, - rentalcharge_daily => 0, + 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 $item = Koha::Item->new( + my $branchcode_refund = + $builder->build( { source => 'Branch' } )->{branchcode}; + my $specific_rule_refund = $builder->build( { - biblionumber => $biblio->biblionumber, - homebranch => $library->branchcode, - holdingbranch => $library->branchcode, - barcode => $barcode, - replacementprice => $replacement_amount, - itype => $item_type->itemtype - }, - )->store; - - AddIssue( $patron->unblessed, $barcode ); - - # Simulate item marked as lost - $item->itemlost(1)->store; - 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, 'Only one 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 amountountstanding is generated' ); - - my $account = $patron->account; - is( $account->balance, $replacement_amount, 'Balance is L' ); - - # Partially pay fee - my $payment_amount = 27; - my $payment = $account->add_credit( - { amount => $payment_amount, - type => 'PAYMENT', - interface => 'test', + source => 'CirculationRule', + value => { + branchcode => $branchcode_refund, + categorycode => undef, + itemtype => undef, + rule_name => 'lostreturn', + rule_value => 'refund' + } } ); - $payment->apply({ debits => [ $lost_fee_line ], offset_type => 'Payment' }); - - is( $account->balance, - $replacement_amount - $payment_amount, - 'Payment applied' + 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' + } + } ); - my $manual_debit_amount = 80; - $account->add_debit( { amount => $manual_debit_amount, type => 'OVERDUE', interface =>'test' } ); - - is( $account->balance, $manual_debit_amount + $replacement_amount - $payment_amount, 'Manual debit applied' ); - - t::lib::Mocks::mock_preference( 'AccountAutoReconcile', 1 ); + 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 ); - my $credit_return_id = C4::Circulation::_FixAccountForLostAndFound( $item->itemnumber, $patron->id ); - my $credit_return = Koha::Account::Lines->find($credit_return_id); + subtest 'lostreturn | false' => sub { + plan tests => 12; - is( $account->balance, $manual_debit_amount - $payment_amount, 'Balance is PROCESSING - payment (LOST_FOUND)' ); + t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $branchcode_false }); - my $manual_debit = Koha::Account::Lines->search({ borrowernumber => $patron->id, debit_type_code => 'OVERDUE', status => 'UNRETURNED' })->next; - is( $manual_debit->amountoutstanding + 0, $manual_debit_amount - $payment_amount, 'reconcile_balance was called' ); + 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 { - plan tests => 11; + plan tests => 14; my $manager = $builder->build_object({ class => "Koha::Patrons" }); t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $manager->branchcode }); @@ -2881,55 +3393,31 @@ subtest '_FixOverduesOnReturn' => sub { is( $accountline->amountoutstanding + 0, 0, 'Fine amountoutstanding has been reduced to 0' ); isnt( $accountline->status, 'UNRETURNED', 'Open fine ( account type OVERDUE ) has been closed out ( status not UNRETURNED )'); - is( $accountline->status, 'FORGIVEN', 'Open fine ( account type OVERDUE ) has been set to fine forgiven ( status FORGIVEN )'); + is( $accountline->status, 'RETURNED', 'Open fine ( account type OVERDUE ) has been set to returned ( status RETURNED )'); is( ref $offset, "Koha::Account::Offset", "Found matching offset for fine reduction via forgiveness" ); is( $offset->amount + 0, -99, "Amount of offset is correct" ); my $credit = $offset->credit; is( ref $credit, "Koha::Account::Line", "Found matching credit for fine forgiveness" ); is( $credit->amount + 0, -99, "Credit amount is set correctly" ); is( $credit->amountoutstanding + 0, 0, "Credit amountoutstanding is correctly set to 0" ); -}; -subtest '_FixAccountForLostAndFound returns undef if patron is deleted' => sub { - plan tests => 1; - - my $manager = $builder->build_object({ class => "Koha::Patrons" }); - t::lib::Mocks::mock_userenv({ patron => $manager, branchcode => $manager->branchcode }); - - my $biblio = $builder->build_sample_biblio({ author => 'Hall, Kylie' }); - - my $branchcode = $library2->{branchcode}; - - my $item = $builder->build_sample_item( - { - biblionumber => $biblio->biblionumber, - library => $branchcode, - replacementprice => 99.00, - itype => $itemtype, - } - ); - - my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); - - ## Start with basic call, should just close out the open fine - my $accountline = Koha::Account::Line->new( + # Bug 25417 - Only forgive fines where there is an amount outstanding to forgive + $accountline->set( { - borrowernumber => $patron->id, - debit_type_code => 'LOST', - status => undef, - itemnumber => $item->itemnumber, - amount => 99.00, - amountoutstanding => 99.00, - interface => 'test', + debit_type_code => 'OVERDUE', + status => 'UNRETURNED', + amountoutstanding => 0.00, } )->store(); + $offset->delete; - $patron->delete(); - - my $return_value = C4::Circulation::_FixAccountForLostAndFound( $patron->id, $item->itemnumber ); - - is( $return_value, undef, "_FixAccountForLostAndFound returns undef if patron is deleted" ); + C4::Circulation::_FixOverduesOnReturn( $patron->{borrowernumber}, $item->itemnumber, 1, 'RETURNED' ); + $accountline->_result()->discard_changes(); + $offset = Koha::Account::Offsets->search({ debit_id => $accountline->id, type => 'Forgiven' })->next(); + is( $offset, undef, "No offset created when trying to forgive fine with no outstanding balance" ); + isnt( $accountline->status, 'UNRETURNED', 'Open fine ( account type OVERDUE ) has been closed out ( status not UNRETURNED )'); + is( $accountline->status, 'RETURNED', 'Passed status has been used to set as RETURNED )'); }; subtest 'Set waiting flag' => sub { @@ -2944,96 +3432,120 @@ subtest 'Set waiting flag' => sub { { library => $library_1->{branchcode}, } - )->unblessed; + ); set_userenv( $library_2 ); my $reserve_id = AddReserve( { branchcode => $library_2->{branchcode}, borrowernumber => $patron_2->{borrowernumber}, - biblionumber => $item->{biblionumber}, + biblionumber => $item->biblionumber, priority => 1, - itemnumber => $item->{itemnumber}, + itemnumber => $item->itemnumber, } ); 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 ); + my ( $res, $rr ) = AddReturn( $item->barcode, $library_1->{branchcode} ); + ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); my $hold = Koha::Holds->find( $reserve_id ); is( $hold->found, 'T', 'Hold is in transit' ); - my ( $status ) = CheckReserves($item->{itemnumber}); - is( $status, 'Reserved', 'Hold is not waiting yet'); + my ( $status ) = CheckReserves($item->itemnumber); + is( $status, 'Transferred', 'Hold is not waiting yet'); set_userenv( $library_2 ); $do_transfer = 0; - AddReturn( $item->{barcode}, $library_2->{branchcode} ); - ModReserveAffect( $item->{itemnumber}, undef, $do_transfer, $reserve_id ); + AddReturn( $item->barcode, $library_2->{branchcode} ); + ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); $hold = Koha::Holds->find( $reserve_id ); is( $hold->found, 'W', 'Hold is waiting' ); - ( $status ) = CheckReserves($item->{itemnumber}); + ( $status ) = CheckReserves($item->itemnumber); is( $status, 'Waiting', 'Now the hold is waiting'); #Bug 21944 - Waiting transfer checked in at branch other than pickup location set_userenv( $library_1 ); - (undef, my $messages, undef, undef ) = AddReturn ( $item->{barcode}, $library_1->{branchcode} ); + (undef, my $messages, undef, undef ) = AddReturn ( $item->barcode, $library_1->{branchcode} ); $hold = Koha::Holds->find( $reserve_id ); is( $hold->found, undef, 'Hold is no longer marked waiting' ); is( $hold->priority, 1, "Hold is now priority one again"); is( $hold->waitingdate, undef, "Hold no longer has a waiting date"); - is( $hold->itemnumber, $item->{itemnumber}, "Hold has retained its' itemnumber"); + is( $hold->itemnumber, $item->itemnumber, "Hold has retained its' itemnumber"); is( $messages->{ResFound}->{ResFound}, "Reserved", "Hold is still returned"); is( $messages->{ResFound}->{found}, undef, "Hold is no longer marked found in return message"); is( $messages->{ResFound}->{priority}, 1, "Hold is priority 1 in return message"); }; subtest 'Cancel transfers on lost items' => sub { - plan tests => 5; - 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}, - }); + plan tests => 6; - 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( $library_2->{branchcode}, $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( $tobranch, $library_2->{branchcode}, 'The transfer record exists in the branchtransfers table'); - my $itemcheck = Koha::Items->find($item->itemnumber); - is( $itemcheck->holdingbranch, $library_1->{branchcode}, 'Items holding branch is the transfers origin 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 { @@ -3070,12 +3582,12 @@ subtest 'CanBookBeIssued | is_overdue' => sub { { library => $library->{branchcode}, } - )->unblessed; + ); - my $issue = AddIssue( $patron->unblessed, $item->{barcode}, $five_days_go ); # date due was 10d ago - my $actualissue = Koha::Checkouts->find( { itemnumber => $item->{itemnumber} } ); + 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}), $five_days_go, "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"); }; @@ -3351,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); @@ -3394,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 = ( @@ -3489,7 +4006,6 @@ subtest 'Incremented fee tests' => sub { my $library = $builder->build_object( { class => 'Koha::Libraries' } )->store; - my $module = new Test::MockModule('C4::Context'); $module->mock( 'userenv', sub { { branch => $library->id } } ); my $patron = $builder->build_object( @@ -3696,20 +4212,13 @@ subtest 'CanBookBeIssued & RentalFeesCheckoutConfirmation' => sub { } ); - my $biblioitem = $builder->build( { source => 'Biblioitem' } ); - my $item = $builder->build_object( + my $item = $builder->build_sample_item( { - class => 'Koha::Items', - value => { - homebranch => $library->id, - holdingbranch => $library->id, - notforloan => 0, - itemlost => 0, - withdrawn => 0, - itype => $itemtype->id, - biblionumber => $biblioitem->{biblionumber}, - biblioitemnumber => $biblioitem->{biblioitemnumber}, - } + library => $library->id, + notforloan => 0, + itemlost => 0, + withdrawn => 0, + itype => $itemtype->id, } )->store; @@ -3727,49 +4236,693 @@ subtest 'CanBookBeIssued & RentalFeesCheckoutConfirmation' => sub { $itemtype->rentalcharge_daily('0')->store; }; -subtest "Test Backdating of Returns" => sub { - plan tests => 2; +subtest 'CanBookBeIssued & CircConfirmItemParts' => sub { + plan tests => 1; + + t::lib::Mocks::mock_preference('CircConfirmItemParts', 1); + + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + } + )->store; + + my $item = $builder->build_sample_item( + { + materials => 'includes DVD', + } + )->store; + + my $dt_due = dt_from_string->add( days => 3 ); + + my ( $issuingimpossible, $needsconfirmation ) = CanBookBeIssued( $patron, $item->barcode, $dt_due, undef, undef, undef ); + is_deeply( $needsconfirmation, { ADDITIONAL_MATERIALS => 'includes DVD' }, 'Item needs confirmation of additional parts' ); +}; + +subtest 'Do not return on renewal (LOST charge)' => sub { + plan tests => 1; + + t::lib::Mocks::mock_preference('MarkLostItemsAsReturned', 'onpayment'); + my $library = $builder->build_object( { class => "Koha::Libraries" } ); + my $manager = $builder->build_object( { class => "Koha::Patrons" } ); + t::lib::Mocks::mock_userenv({ patron => $manager,branchcode => $manager->branchcode }); + + my $biblio = $builder->build_sample_biblio; - my $branch = $library2->{branchcode}; - my $biblio = $builder->build_sample_biblio(); my $item = $builder->build_sample_item( { biblionumber => $biblio->biblionumber, - library => $branch, + library => $library->branchcode, + replacementprice => 99.00, itype => $itemtype, } ); - my %a_borrower_data = ( - firstname => 'Kyle', - surname => 'Hall', - categorycode => $patron_category->{categorycode}, - branchcode => $branch, - ); - my $borrowernumber = Koha::Patron->new(\%a_borrower_data)->store->borrowernumber; - my $borrower = Koha::Patrons->find( $borrowernumber )->unblessed; + my $patron = $builder->build_object( { class => 'Koha::Patrons' } ); + AddIssue( $patron->unblessed, $item->barcode ); - my $due_date = dt_from_string; - my $issue = AddIssue( $borrower, $item->barcode, $due_date ); - UpdateFine( + my $accountline = Koha::Account::Line->new( { - issue_id => $issue->id(), + borrowernumber => $patron->borrowernumber, + debit_type_code => 'LOST', + status => undef, itemnumber => $item->itemnumber, - borrowernumber => $borrowernumber, - amount => .25, - amountoutstanding => .25, - type => q{} + amount => 12, + amountoutstanding => 12, + interface => 'something', + } + )->store(); + + # AddRenewal doesn't call _FixAccountForLostAndFound + AddIssue( $patron->unblessed, $item->barcode ); + + is( $patron->checkouts->count, 1, + 'Renewal should not return the item even if a LOST payment has been made earlier' + ); +}; + +subtest 'Filling a hold should cancel existing transfer' => sub { + plan tests => 4; + + t::lib::Mocks::mock_preference('AutomaticItemReturn', 1); + + my $libraryA = $builder->build_object( { class => 'Koha::Libraries' } ); + my $libraryB = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { + categorycode => $patron_category->{categorycode}, + branchcode => $libraryA->branchcode, + } + } + )->store; + + my $item = $builder->build_sample_item({ + homebranch => $libraryB->branchcode, + }); + + my ( undef, $message ) = AddReturn( $item->barcode, $libraryA->branchcode, undef, undef ); + is( Koha::Item::Transfers->search({ itemnumber => $item->itemnumber, datearrived => undef })->count, 1, "We generate a transfer on checkin"); + AddReserve({ + branchcode => $libraryA->branchcode, + borrowernumber => $patron->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber + }); + my $reserves = Koha::Holds->search({ itemnumber => $item->itemnumber }); + is( $reserves->count, 1, "Reserve is placed"); + ( undef, $message ) = AddReturn( $item->barcode, $libraryA->branchcode, undef, undef ); + my $reserve = $reserves->next; + ModReserveAffect( $item->itemnumber, $patron->borrowernumber, 0, $reserve->reserve_id ); + $reserve->discard_changes; + ok( $reserve->found eq 'W', "Reserve is marked waiting" ); + is( Koha::Item::Transfers->search({ itemnumber => $item->itemnumber, datearrived => undef })->count, 0, "No outstanding transfers when hold is waiting"); +}; + +subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddReturn' => sub { + + plan tests => 4; + + t::lib::Mocks::mock_preference('BlockReturnOfLostItems', 0); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + } + ); + + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->branchcode, + } + )->biblionumber; + + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'lostreturn', + rule_value => 'refund' + } + } + ); + + subtest 'NoRefundOnLostReturnedItemsAge = undef' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', undef ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->branchcode, undef, dt_from_string ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); + $a->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge > length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 6 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->branchcode, undef, dt_from_string ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); + $a->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge = length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->branchcode, undef, dt_from_string ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); + $a->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge < length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 8 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + ); + $a = $a->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + my ( $doreturn, $messages ) = AddReturn( $item->barcode, $library->branchcode, undef, dt_from_string ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); + $a->delete; + }; +}; + +subtest 'Tests for NoRefundOnLostReturnedItemsAge with AddIssue' => sub { + + plan tests => 4; + + t::lib::Mocks::mock_preference('BlockReturnOfLostItems', 0); + my $library = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } + } + ); + my $patron2 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { categorycode => $patron_category->{categorycode} } } ); + my $biblionumber = $builder->build_sample_biblio( + { + branchcode => $library->branchcode, + } + )->biblionumber; - my ( undef, $message ) = AddReturn( $item->barcode, $branch, undef, $due_date ); + # And the circulation rule + Koha::CirculationRules->search->delete; + Koha::CirculationRules->set_rules( + { + categorycode => undef, + itemtype => undef, + branchcode => undef, + rules => { + issuelength => 14, + lengthunit => 'days', + } + } + ); + $builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'lostreturn', + rule_value => 'refund' + } + } + ); + + subtest 'NoRefundOnLostReturnedItemsAge = undef' => sub { + plan tests => 3; - my $accountline = Koha::Account::Lines->find( { issue_id => $issue->id } ); - is( $accountline->amountoutstanding+0, 0, 'Fee amount outstanding was reduced to 0' ); - is( $accountline->amount+0, 0, 'Fee amount was reduced to 0' ); + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', undef ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); + $a->delete; + $issue->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge > length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 6 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 0, "Lost fee was refunded" ); + $a->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge = length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 7 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + )->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); + $a->delete; + }; + + subtest 'NoRefundOnLostReturnedItemsAge < length of days item has been lost' => sub { + plan tests => 3; + + t::lib::Mocks::mock_preference( 'WhenLostChargeReplacementFee', 1 ); + t::lib::Mocks::mock_preference( 'NoRefundOnLostReturnedItemsAge', 7 ); + + my $lost_on = dt_from_string->subtract( days => 8 )->date; + + my $item = $builder->build_sample_item( + { + biblionumber => $biblionumber, + library => $library->branchcode, + replacementprice => '42', + } + ); + my $issue = AddIssue( $patron->unblessed, $item->barcode ); + LostItem( $item->itemnumber, 'cli', 0 ); + $item->_result->itemlost(1); + $item->_result->itemlost_on( $lost_on ); + $item->_result->update(); + + my $a = Koha::Account::Lines->search( + { + itemnumber => $item->id, + borrowernumber => $patron->borrowernumber + } + ); + $a = $a->next; + ok( $a, "Found accountline for lost fee" ); + is( $a->amountoutstanding + 0, 42, "Lost fee charged correctly" ); + $issue = AddIssue( $patron2->unblessed, $item->barcode ); + $a = $a->get_from_storage; + is( $a->amountoutstanding + 0, 42, "Lost fee was not refunded" ); + $a->delete; + }; +}; + +subtest 'transferbook tests' => sub { + plan tests => 9; + + throws_ok + { C4::Circulation::transferbook({}); } + 'Koha::Exceptions::MissingParameter', + 'Koha::Patron->store raises an exception on missing params'; + + throws_ok + { C4::Circulation::transferbook({to_branch=>'anything'}); } + 'Koha::Exceptions::MissingParameter', + 'Koha::Patron->store raises an exception on missing params'; + + throws_ok + { C4::Circulation::transferbook({from_branch=>'anything'}); } + 'Koha::Exceptions::MissingParameter', + 'Koha::Patron->store raises an exception on missing params'; + + my ($doreturn,$messages) = C4::Circulation::transferbook({to_branch=>'there',from_branch=>'here'}); + is( $doreturn, 0, "No return without barcode"); + ok( exists $messages->{BadBarcode}, "We get a BadBarcode message if no barcode passed"); + is( $messages->{BadBarcode}, undef, "No barcode passed means undef BadBarcode" ); + + ($doreturn,$messages) = C4::Circulation::transferbook({to_branch=>'there',from_branch=>'here',barcode=>'BadBarcode'}); + is( $doreturn, 0, "No return without barcode"); + ok( exists $messages->{BadBarcode}, "We get a BadBarcode message if no barcode passed"); + is( $messages->{BadBarcode}, 'BadBarcode', "No barcode passed means undef BadBarcode" ); + +}; + +subtest 'Checkout should correctly terminate a transfer' => sub { + plan tests => 7; + + my $library_1 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron_1 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { branchcode => $library_1->branchcode } + } + ); + my $library_2 = $builder->build_object( { class => 'Koha::Libraries' } ); + my $patron_2 = $builder->build_object( + { + class => 'Koha::Patrons', + value => { branchcode => $library_2->branchcode } + } + ); + + my $item = $builder->build_sample_item( + { + library => $library_1->branchcode, + } + ); + + t::lib::Mocks::mock_userenv( { branchcode => $library_1->branchcode } ); + my $reserve_id = AddReserve( + { + branchcode => $library_2->branchcode, + borrowernumber => $patron_2->borrowernumber, + biblionumber => $item->biblionumber, + itemnumber => $item->itemnumber, + priority => 1, + } + ); + + my $do_transfer = 1; + ModItemTransfer( $item->itemnumber, $library_1->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... + my $hold = Koha::Holds->find($reserve_id); + is( $hold->found, 'T', 'Hold is in transit' ); + my $transfer = $item->get_transfer; + is( $transfer->frombranch, $library_1->branchcode ); + is( $transfer->tobranch, $library_2->branchcode ); + is( $transfer->reason, 'Reserve' ); + + t::lib::Mocks::mock_userenv( { branchcode => $library_2->branchcode } ); + AddIssue( $patron_1->unblessed, $item->barcode ); + $transfer = $transfer->get_from_storage; + isnt( $transfer->datearrived, undef ); + $hold = $hold->get_from_storage; + is( $hold->found, undef, 'Hold is waiting' ); + 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(); -$cache->clear_from_cache('single_holidays'); +$branches = Koha::Libraries->search(); +for my $branch ( $branches->next ) { + my $key = $branch->branchcode . "_holidays"; + $cache->clear_from_cache($key); +}