From 0e0a41cebdfa78714c0b89e3e734cf50b7d8e4b8 Mon Sep 17 00:00:00 2001 From: Lari Taskula Date: Wed, 15 Apr 2020 16:08:17 +0000 Subject: [PATCH] Bug 26814: Add onsite_checkout to get_effective_rules To test: 1. Find all occurrences of get_effective_rules that are missing onsite_checkout where required grep --exclude-dir '.git' --exclude-dir 'misc/translator' \ --exclude-dir 'koha-tmpl' \ -Przo '(?s)(::|->)get_effective_rules.*?\)' | \ grep -avz 'onsite_checkout' | less 1.1. Verify command output. Only reserve related calls to get_effective_rules() should be returned. 2. Find all subroutines using get_effective_rules() git grep --no-index -n -p -P 'get_effective_rules\s*\(' \ | grep -v 'sub {' | grep -P 'sub .*' This list should be returned: C4/Circulation.pm=1663=sub GetLoanLength { C4/Circulation.pm=1702=sub GetHardDueDate { C4/Circulation.pm=2386=sub _calculate_new_debar_dt { C4/Circulation.pm=2736=sub CanBookBeRenewed { C4/Circulation.pm=3160=sub GetRenewCount { C4/Circulation.pm=3239=sub GetSoonestRenewDate { C4/Circulation.pm=3303=sub GetLatestAutoRenewDate { C4/Overdues.pm=224=sub CalcFine { C4/Reserves.pm=2263=sub GetHoldRule { Check these subroutines and make sure they are using get_effective_rules() with the new scope, unless not needed. When fetching a hold-related rules, checkout_type is not needed. 3. Check modification made to C4::Circulation::CalcDateDue() 4. Check modification made to C4::Circulation::CalcFine() 5. Check modification made to C4::Circulation::GetLoanLength() 6. Check modification made to C4::Circulation::GetHardDueDate() Make sure all matches (where rule scope allows onsite_checkout) are updated. 7. Run these unit tests: grep -Prl --color=never 'get_effective_rule\(|set_rule\(|set_rules\(' \ t/db_dependent/ | grep '^.*\.t$' | xargs prove 8. Test onboarding tool 8.1. Drop&create your database (take backups first if needed) 8.2. Navigate to web installer 8.3. Continue until you reach onboarding tool step where circulation rules are added. 8.4. Observe new drop down selection "Checkout type". 8.5. Select any checkout type and continue the onboarding process. 8.6. Investigate your database table "circulation_rules". You should now have rules with onsite_checkout of your choice. Sponsored-by: The National Library of Finland --- C4/Circulation.pm | 32 ++++++++++------- C4/Overdues.pm | 7 ++-- ...fix_unclosed_nonaccruing_fines_bug17135.pl | 3 +- misc/cronjobs/fines.pl | 2 +- misc/cronjobs/staticfines.pl | 1 + t/db_dependent/Circulation.t | 18 +++++----- t/db_dependent/Circulation/CalcDateDue.t | 36 +++++++++---------- t/db_dependent/Circulation/CalcFine.t | 10 +++--- t/db_dependent/Circulation/GetHardDueDate.t | 11 ++++-- t/db_dependent/Fines.t | 8 ++--- 10 files changed, 72 insertions(+), 56 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 9788401bfe..7b3c3c69a2 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -754,7 +754,7 @@ sub CanBookBeIssued { unless ( $duedate ) { my $issuedate = $now->clone(); - $duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed ); + $duedate = CalcDateDue( $issuedate, $effective_itemtype, $circ_library->branchcode, $patron_unblessed, $onsite_checkout ); # Offline circ calls AddIssue directly, doesn't run through here # So issuingimpossible should be ok. @@ -1317,7 +1317,7 @@ sub checkHighHolds { ); my $calendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => $daysmode ); - my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); + my $orig_due = C4::Circulation::CalcDateDue( $issuedate, $itype, $branchcode, $borrower, undef, ); my $rule = Koha::CirculationRules->get_effective_rule( { @@ -1440,7 +1440,7 @@ sub AddIssue { else { unless ($datedue) { my $itype = $item_object->effective_itemtype; - $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower ); + $datedue = CalcDateDue( $issuedate, $itype, $branchcode, $borrower, $onsite_checkout ); } $datedue->truncate( to => 'minute' ); @@ -1654,14 +1654,14 @@ sub AddIssue { =head2 GetLoanLength - my $loanlength = &GetLoanLength($borrowertype,$itemtype,branchcode) + my $loanlength = &GetLoanLength($borrowertype,$itemtype,branchcode,$onsite_checkout) Get loan length for an itemtype, a borrower type and a branch =cut sub GetLoanLength { - my ( $categorycode, $itemtype, $branchcode ) = @_; + my ( $categorycode, $itemtype, $branchcode, $onsite_checkout ) = @_; # Initialize default values my $rules = { @@ -1674,6 +1674,7 @@ sub GetLoanLength { branchcode => $branchcode, categorycode => $categorycode, itemtype => $itemtype, + onsite_checkout => $onsite_checkout, rules => [ 'issuelength', 'renewalperiod', @@ -1692,20 +1693,21 @@ sub GetLoanLength { =head2 GetHardDueDate - my ($hardduedate,$hardduedatecompare) = &GetHardDueDate($borrowertype,$itemtype,branchcode) + my ($hardduedate,$hardduedatecompare) = &GetHardDueDate($borrowertype,$itemtype,branchcode,$onsite_checkout) Get the Hard Due Date and it's comparison for an itemtype, a borrower type and a branch =cut sub GetHardDueDate { - my ( $borrowertype, $itemtype, $branchcode ) = @_; + my ( $borrowertype, $itemtype, $branchcode, $onsite_checkout ) = @_; my $rules = Koha::CirculationRules->get_effective_rules( { categorycode => $borrowertype, itemtype => $itemtype, branchcode => $branchcode, + onsite_checkout => $onsite_checkout, rules => [ 'hardduedate', 'hardduedatecompare' ], } ); @@ -2753,6 +2755,7 @@ sub CanBookBeRenewed { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, branchcode => $branchcode, + onsite_checkout => $issue->onsite_checkout, rules => [ 'renewalsallowed', 'no_auto_renewal_after', @@ -3029,7 +3032,7 @@ sub AddRenewal { $datedue = (C4::Context->preference('RenewalPeriodBase') eq 'date_due') ? dt_from_string( $issue->date_due, 'sql' ) : dt_from_string(); - $datedue = CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, 'is a renewal'); + $datedue = CalcDateDue($datedue, $itemtype, $circ_library->branchcode, $patron_unblessed, $issue->onsite_checkout, 'is a renewal'); } my $fees = Koha::Charges::Fees->new( @@ -3250,6 +3253,7 @@ sub GetSoonestRenewDate { { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, branchcode => $branchcode, + onsite_checkout => $itemissue->onsite_checkout, rules => [ 'norenewalbefore', 'lengthunit', @@ -3314,6 +3318,7 @@ sub GetLatestAutoRenewDate { categorycode => $patron->categorycode, itemtype => $item->effective_itemtype, branchcode => $branchcode, + onsite_checkout => $itemissue->onsite_checkout, rules => [ 'no_auto_renewal_after', 'no_auto_renewal_after_hard_limit', @@ -3664,7 +3669,7 @@ sub updateWrongTransfer { =head2 CalcDateDue -$newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower); +$newdatedue = CalcDateDue($startdate,$itemtype,$branchcode,$borrower,$onsite_checkout); this function calculates the due date given the start date and configured circulation rules, checking against the holidays calendar as per the daysmode circulation rule. @@ -3677,13 +3682,13 @@ C<$isrenewal> = Boolean: is true if we want to calculate the date due for a rene =cut sub CalcDateDue { - my ( $startdate, $itemtype, $branch, $borrower, $isrenewal ) = @_; + my ( $startdate, $itemtype, $branch, $borrower, $onsite_checkout, $isrenewal ) = @_; $isrenewal ||= 0; # loanlength now a href my $loanlength = - GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch ); + GetLoanLength( $borrower->{'categorycode'}, $itemtype, $branch, $onsite_checkout ); my $length_key = ( $isrenewal and defined $loanlength->{renewalperiod} ) ? qq{renewalperiod} @@ -3706,6 +3711,7 @@ sub CalcDateDue { categorycode => $borrower->{categorycode}, itemtype => $itemtype, branchcode => $branch, + onsite_checkout => $onsite_checkout, } ); @@ -3737,7 +3743,7 @@ sub CalcDateDue { # if Hard Due Dates are used, retrieve them and apply as necessary my ( $hardduedate, $hardduedatecompare ) = - GetHardDueDate( $borrower->{'categorycode'}, $itemtype, $branch ); + GetHardDueDate( $borrower->{'categorycode'}, $itemtype, $branch, $onsite_checkout ); if ($hardduedate) { # hardduedates are currently dates $hardduedate->truncate( to => 'minute' ); $hardduedate->set_hour(23); @@ -4339,7 +4345,7 @@ sub _CalculateAndUpdateFine { my $date_returned = $return_date ? $return_date : dt_from_string(); my ( $amount, $unitcounttotal, $unitcount ) = - C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $datedue, $date_returned ); + C4::Overdues::CalcFine( $item, $borrower->{categorycode}, $control_branchcode, $issue->onsite_checkout, $datedue, $date_returned ); if ( C4::Context->preference('finesMode') eq 'production' ) { if ( $amount > 0 ) { diff --git a/C4/Overdues.pm b/C4/Overdues.pm index d484c67b0f..5c3c20d279 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -183,7 +183,7 @@ sub checkoverdues { =head2 CalcFine ($amount, $units_minus_grace, $chargeable_units) = &CalcFine($item, - $categorycode, $branch, + $categorycode, $branch, $onsite_checkout, $start_dt, $end_dt ); Calculates the fine for a book. @@ -202,6 +202,8 @@ the book. C<$branchcode> is the library (string) whose issuingrules govern this transaction. +C<$onsite_checkout> boolean value on checkout being onsite checkout. + C<$start_date> & C<$end_date> are DateTime objects defining the date range over which to determine the fine. @@ -222,7 +224,7 @@ or "Final Notice". But CalcFine never defined any value. =cut sub CalcFine { - my ( $item, $bortype, $branchcode, $due_dt, $end_date ) = @_; + my ( $item, $bortype, $branchcode, $onsite_checkout, $due_dt, $end_date ) = @_; # Skip calculations if item is not overdue return ( 0, 0, 0 ) unless (DateTime->compare( $due_dt, $end_date ) == -1); @@ -235,6 +237,7 @@ sub CalcFine { categorycode => $bortype, itemtype => $itemtype, branchcode => $branchcode, + onsite_checkout => $onsite_checkout, rules => [ 'lengthunit', 'firstremind', diff --git a/installer/data/mysql/fix_unclosed_nonaccruing_fines_bug17135.pl b/installer/data/mysql/fix_unclosed_nonaccruing_fines_bug17135.pl index b3e4b06f37..271abc1e14 100755 --- a/installer/data/mysql/fix_unclosed_nonaccruing_fines_bug17135.pl +++ b/installer/data/mysql/fix_unclosed_nonaccruing_fines_bug17135.pl @@ -157,8 +157,9 @@ sub Bug_17135_fix { ( $control eq 'ItemHomeLibrary' ) ? $overdue->{homebranch} : ( $control eq 'PatronLibrary' ) ? $patron->branchcode : $overdue->{branchcode}; + my $onsite_checkout = $overdue->{onsite_checkout}; - my ($amount) = CalcFine( $overdue, $patron->categorycode, $branchcode, $datedue, $today ); + my ($amount) = CalcFine( $overdue, $patron->categorycode, $branchcode, $onsite_checkout, $datedue, $today ); ### Warn("CalcFine() returned '$amount'"); last if ($amount > 0); ## accruing fine, skip closing diff --git a/misc/cronjobs/fines.pl b/misc/cronjobs/fines.pl index 9f61c0f598..6d41cad23c 100755 --- a/misc/cronjobs/fines.pl +++ b/misc/cronjobs/fines.pl @@ -145,7 +145,7 @@ for my $overdue ( @{$overdues} ) { my ( $amount, $unitcounttotal, $unitcount ) = CalcFine( $overdue, $patron->categorycode, - $branchcode, $datedue, $today ); + $branchcode, $overdue->{onsite_checkout}, $datedue, $today ); # Don't update the fine if today is a holiday. # This ensures that dropbox mode will remove the correct amount of fine. diff --git a/misc/cronjobs/staticfines.pl b/misc/cronjobs/staticfines.pl index 1e7d287702..2dbb542d5f 100755 --- a/misc/cronjobs/staticfines.pl +++ b/misc/cronjobs/staticfines.pl @@ -193,6 +193,7 @@ for ( my $i = 0 ; $i < scalar(@$data) ; $i++ ) { $data->[$i], $patron->categorycode, $branchcode, + $data->[$i]->{onsite_checkout}, $datedue, $today, ); diff --git a/t/db_dependent/Circulation.t b/t/db_dependent/Circulation.t index d6e441d808..c575741174 100755 --- a/t/db_dependent/Circulation.t +++ b/t/db_dependent/Circulation.t @@ -564,7 +564,7 @@ subtest "CanBookBeRenewed tests" => sub { my $passeddatedue1 = AddIssue($renewing_borrower, $item_7->barcode, $five_weeks_ago); is (defined $passeddatedue1, 1, "Item with passed date due checked out, due date: " . $passeddatedue1->date_due); - my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower->{categorycode}, $branch, $five_weeks_ago, $now ); + my ( $fine ) = CalcFine( $item_7->unblessed, $renewing_borrower->{categorycode}, $branch, undef, $five_weeks_ago, $now ); C4::Overdues::UpdateFine( { issue_id => $passeddatedue1->id(), @@ -2537,7 +2537,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, undef, $ten_days_ago, $now ); UpdateFine( { @@ -2575,7 +2575,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, undef, $one_day_ago, $now ); UpdateFine( { @@ -2600,7 +2600,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, undef, $two_days_ago, $now ); UpdateFine( { @@ -2667,7 +2667,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, undef, $one_day_ago, $now ); UpdateFine( { @@ -2870,7 +2870,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, $issue->onsite_checkout, $ten_days_ago, $now ); UpdateFine( { @@ -2953,7 +2953,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, $issue->onsite_checkout, $ten_days_ago, $now ); UpdateFine( { @@ -3035,7 +3035,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, $issue->onsite_checkout, $ten_days_ago, $now ); UpdateFine( { @@ -3133,7 +3133,7 @@ subtest 'AddReturn | is_overdue' => sub { # Fake fines cronjob on this checkout my ($fine) = - CalcFine( $item, $patron->categorycode, $library->{branchcode}, + CalcFine( $item, $patron->categorycode, $library->{branchcode}, $issue->onsite_checkout, $ten_days_ago, $now ); UpdateFine( { diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t index 0c235d0bee..89f11bdb04 100755 --- a/t/db_dependent/Circulation/CalcDateDue.t +++ b/t/db_dependent/Circulation/CalcDateDue.t @@ -54,9 +54,9 @@ my $dateexpiry = '2013-01-01'; my $borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; my $start_date = DateTime->new({year => 2013, month => 2, day => 9}); -my $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +my $date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, $dateexpiry . 'T23:59:00', 'date expiry'); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout, 1 ); #Set syspref ReturnBeforeExpiry = 1 and useDaysMode != 'Days' @@ -65,7 +65,7 @@ t::lib::Mocks::mock_preference('useDaysMode', 'noDays'); $borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; $start_date = DateTime->new({year => 2013, month => 2, day => 9}); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, $dateexpiry . 'T23:59:00', 'date expiry with useDaysMode to noDays'); # Let's add a special holiday on 2013-01-01. With ReturnBeforeExpiry and @@ -78,7 +78,7 @@ $calendar->insert_single_holiday( title =>'holidayTest', description => 'holidayDesc' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, '2012-12-31T23:59:00', 'date expiry should be 2013-01-01 -1 day'); $calendar->insert_single_holiday( day => 31, @@ -87,11 +87,11 @@ $calendar->insert_single_holiday( title =>'holidayTest', description => 'holidayDesc' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, '2012-12-30T23:59:00', 'date expiry should be 2013-01-01 -2 day'); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout, 1 ); #Set syspref ReturnBeforeExpiry = 0 and useDaysMode = 'Days' @@ -100,10 +100,10 @@ t::lib::Mocks::mock_preference('useDaysMode', 'Days'); $borrower = {categorycode => 'B', dateexpiry => $dateexpiry}; $start_date = DateTime->new({year => 2013, month => 2, day => 9}); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, '2013-02-' . (9 + $issuelength) . 'T23:59:00', "date expiry ( 9 + $issuelength )"); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout, 1 ); is($date, '2013-02-' . (9 + $renewalperiod) . 'T23:59:00', "date expiry ( 9 + $renewalperiod )"); @@ -117,10 +117,10 @@ t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 0); t::lib::Mocks::mock_preference('useDaysMode', 'Dayweek'); # No closed day interfering, so we should get the regular due date -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, '2013-02-' . (9 + $issuelength) . 'T23:59:00', "useDaysMode = Dayweek, no closed days, issue date expiry ( start + $issuelength )"); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout, 1 ); is($date, '2013-02-' . (9 + $renewalperiod) . 'T23:59:00', "useDaysMode = Dayweek, no closed days, renewal date expiry ( start + $renewalperiod )"); # Now let's add a closed day on the expected renewal date, it should @@ -133,7 +133,7 @@ $calendar->insert_single_holiday( title =>'DayweekTest1', description => 'DayweekTest1' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout ); is($date, '2013-02-' . (9 + $issuelength + 1) . 'T23:59:00', "useDaysMode = Dayweek, closed on due date, 10 day loan (should not trigger 7 day roll forward), issue date expiry ( start + $issuelength + 1 )"); # Remove the holiday we just created $calendar->delete_holiday( @@ -150,7 +150,7 @@ $calendar->insert_single_holiday( title =>'DayweekTest2', description => 'DayweekTest2' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $borrower, $onsite_checkout, 1 ); is($date, '2013-02-' . (9 + $renewalperiod + 1) . 'T23:59:00', "useDaysMode = Dayweek, closed on due date, 5 day renewal (should not trigger 7 day roll forward), renewal date expiry ( start + $renewalperiod + 1 )"); # Remove the holiday we just created $calendar->delete_holiday( @@ -193,7 +193,7 @@ $calendar->insert_single_holiday( title =>'DayweekTest3', description => 'DayweekTest3' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout ); my $issue_should_add = $dayweek_issuelength + 7; my $dayweek_issue_expected = $start_date->add( days => $issue_should_add ); is($date, $dayweek_issue_expected->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, closed on due date, 14 day loan (should trigger 7 day roll forward), issue date expiry ( start + $issue_should_add )"); @@ -213,7 +213,7 @@ $calendar->insert_single_holiday( title => 'DayweekTest4', description => 'DayweekTest4' ); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout, 1 ); my $renewal_should_add = $dayweek_renewalperiod + 7; my $dayweek_renewal_expected = $start_date->add( days => $renewal_should_add ); is($date, $dayweek_renewal_expected->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, closed on due date, 7 day renewal (should trigger 7 day roll forward), renewal date expiry ( start + $renewal_should_add )"); @@ -254,12 +254,12 @@ $calendar->insert_single_holiday( description => 'DayweekTest7' ); # For issues... -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout ); $dayweek_issue_expected = $start_date->add( days => $issue_should_add ); is($date, $expected_rolled_date->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, closed on due date and two subequent due dates, 14 day loan (should trigger 2 x 7 day roll forward), issue date expiry ( start + 28 )"); # ...and for renewals... $start_date = DateTime->new({year => 2013, month => 2, day => 9}); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout, 1 ); $dayweek_issue_expected = $start_date->add( days => $renewal_should_add ); is($date, $expected_rolled_date->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, closed on due date and three subsequent due dates, 7 day renewal (should trigger 3 x 7 day roll forward), issue date expiry ( start + 28 )"); # Remove the holidays we just created @@ -294,12 +294,12 @@ $calendar->insert_week_day_holiday( description => "Closed on Saturdays" ); # For issues... -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout ); $dayweek_issue_expected = $start_date->add( days => $dayweek_issuelength + 1 ); is($date, $dayweek_issue_expected->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, due on Saturday, closed on Saturdays, 14 day loan (should trigger 1 day roll forward), issue date expiry ( start + 15 )"); # ...and for renewals... $start_date = DateTime->new({year => 2013, month => 2, day => 9}); -$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, 1 ); +$date = C4::Circulation::CalcDateDue( $start_date, $itemtype, $branchcode, $dayweek_borrower, $onsite_checkout, 1 ); $dayweek_renewal_expected = $start_date->add( days => $dayweek_renewalperiod + 1 ); is($date, $dayweek_renewal_expected->strftime('%F') . 'T23:59:00', "useDaysMode = Dayweek, due on Saturday, closed on Saturdays, 7 day renewal (should trigger 1 day roll forward), issue date expiry ( start + 8 )"); # Remove the holiday we just created diff --git a/t/db_dependent/Circulation/CalcFine.t b/t/db_dependent/Circulation/CalcFine.t index c443cd16a9..4cc46b9677 100755 --- a/t/db_dependent/Circulation/CalcFine.t +++ b/t/db_dependent/Circulation/CalcFine.t @@ -94,7 +94,7 @@ subtest 'Test basic functionality' => sub { day => 30, ); - my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); + my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, undef, $start_dt, $end_dt ); is( $amount, 29, 'Amount is calculated correctly' ); @@ -143,13 +143,13 @@ subtest 'Test cap_fine_to_replacement_price' => sub { } ); - my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); + my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, undef, $start_dt, $end_dt ); is( int($amount), 5, 'Amount is calculated correctly' ); # Use default replacement cost (useDefaultReplacementCost) is item's replacement price is 0 $item->replacementprice(0)->store; - ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); + ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, undef, $start_dt, $end_dt ); is( int($amount), 6, 'Amount is calculated correctly' ); teardown(); @@ -189,11 +189,11 @@ subtest 'Test cap_fine_to_replacement_pricew with overduefinescap' => sub { day => 30, ); - my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); + my ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, undef, $start_dt, $end_dt ); is( int($amount), 3, 'Got the lesser of overduefinescap and replacement price where overduefinescap < replacement price' ); Koha::CirculationRules->set_rule({ rule_name => 'overduefinescap', rule_value => 6, branchcode => undef, onsite_checkout => undef, categorycode => undef, itemtype => undef }); - ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, $start_dt, $end_dt ); + ($amount) = CalcFine( $item->unblessed, $patron->{categorycode}, $branch->{branchcode}, undef, $start_dt, $end_dt ); is( int($amount), 5, 'Get the lesser of overduefinescap and replacement price where overduefinescap > replacement price' ); teardown(); diff --git a/t/db_dependent/Circulation/GetHardDueDate.t b/t/db_dependent/Circulation/GetHardDueDate.t index 70cab35f5e..b441780b75 100755 --- a/t/db_dependent/Circulation/GetHardDueDate.t +++ b/t/db_dependent/Circulation/GetHardDueDate.t @@ -11,7 +11,7 @@ use Koha::Library; use t::lib::TestBuilder; -use Test::More tests => 9; +use Test::More tests => 10; BEGIN { use_ok('C4::Circulation'); @@ -241,7 +241,7 @@ Koha::CirculationRules->set_rules( $sampleissuingrule3_holds ); is_deeply( C4::Circulation::GetLoanLength( $samplecat->{categorycode}, - $sampleitemtype1, $samplebranch1->{branchcode} + $sampleitemtype1, $samplebranch1->{branchcode}, 0 ), { issuelength => 5, lengthunit => 'days', renewalperiod => 5 }, "GetLoanLength" @@ -269,6 +269,11 @@ is_deeply( ); #NOTE : is that really what is expected? is_deeply( C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1, $samplebranch1->{branchcode} ), + $default, + "With only three parameters, GetLoanLength returns hardcoded values" +); #NOTE : is that really what is expected? +is_deeply( + C4::Circulation::GetLoanLength( $samplecat->{categorycode}, $sampleitemtype1, $samplebranch1->{branchcode}, 0 ), { issuelength => 5, renewalperiod => 5, @@ -279,7 +284,7 @@ is_deeply( #Test GetHardDueDate my @hardduedate = C4::Circulation::GetHardDueDate( $samplecat->{categorycode}, - $sampleitemtype1, $samplebranch1->{branchcode} ); + $sampleitemtype1, $samplebranch1->{branchcode}, 0 ); is_deeply( \@hardduedate, [ diff --git a/t/db_dependent/Fines.t b/t/db_dependent/Fines.t index f132f1e78b..571eefa97e 100755 --- a/t/db_dependent/Fines.t +++ b/t/db_dependent/Fines.t @@ -37,11 +37,11 @@ ok( $issuingrule, 'Issuing rule created' ); my $period_start = dt_from_string('2000-01-01'); my $period_end = dt_from_string('2000-01-05'); -my ( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +my ( $fine ) = CalcFine( {}, q{}, q{}, q{}, $period_start, $period_end ); is( $fine, 0, '4 days overdue, charge period 7 days, charge at end of interval gives fine of $0' ); $period_end = dt_from_string('2000-01-10'); -( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +( $fine ) = CalcFine( {}, q{}, q{}, q{}, $period_start, $period_end ); is( $fine, 1, '9 days overdue, charge period 7 days, charge at end of interval gives fine of $1' ); # Test charging fine at the *beginning* of each charge period @@ -58,9 +58,9 @@ $issuingrule = Koha::CirculationRules->set_rules( ); $period_end = dt_from_string('2000-01-05'); -( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +( $fine ) = CalcFine( {}, q{}, q{}, q{}, $period_start, $period_end ); is( $fine, 1, '4 days overdue, charge period 7 days, charge at start of interval gives fine of $1' ); $period_end = dt_from_string('2000-01-10'); -( $fine ) = CalcFine( {}, q{}, q{}, $period_start, $period_end ); +( $fine ) = CalcFine( {}, q{}, q{}, q{}, $period_start, $period_end ); is( $fine, 2, '9 days overdue, charge period 7 days, charge at start of interval gives fine of $2' ); -- 2.17.1