From 189a823b7329454639d77c4361bdb748787568f2 Mon Sep 17 00:00:00 2001 From: Maryse Simard Date: Tue, 18 Jun 2024 09:40:32 -0400 Subject: [PATCH] Bug 17983: Add minutes loan to DiscreteCalendar This is an additional feature to our new Discrete Calendar. Theses patches add the possibility to choose the loan unit as minutes in circulations rules. Test plan: - Apply patch of Bugzilla 17015 - Apply this patch. - Run test t/db_dependent/minutes_loan.t - Create new item type for the new rule. - Create circulation rule and fill the fields, make sure the unit is set to : minutes, choose the item type of the one created above.- - Checkout an item that follow the circulation rule and set due date in the past. - Run fines.pl. - Check if the correct amount was calculated. - Note: make sure to factor in the fine grace period if set and the system preference FinesIncludeGracePeriod.fixed patch tests and minor adjustments --- C4/Circulation.pm | 19 ++ C4/Overdues.pm | 12 + Koha/DiscreteCalendar.pm | 96 +++++++ .../prog/en/modules/admin/smart-rules.tt | 3 + t/db_dependent/MinuteLoans.t | 267 ++++++++++++++++++ 5 files changed, 397 insertions(+) create mode 100755 t/db_dependent/MinuteLoans.t diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 208b211c83..49a8550995 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3968,6 +3968,8 @@ sub CalcDateDue { # due time doesn't conflict with library open hours, don't need to check $datedue->add( hours => $loanlength->{$length_key} ); } + } elsif ( $loanlength->{lengthunit} eq 'minutes' ) { + $datedue->add( minutes => $loanlength->{$length_key} ); } else { # days $datedue->add( days => $loanlength->{$length_key} ); $datedue->set_hour(23); @@ -3999,6 +4001,9 @@ sub CalcDateDue { $dur = DateTime::Duration->new( hours => $loanlength->{$length_key} ); } } + elsif ( $loanlength->{lengthunit} eq 'minutes' ) { + $dur = DateTime::Duration->new( minutes => $loanlength->{$length_key} ); + } else { # days $dur = DateTime::Duration->new( days => $loanlength->{$length_key} ); } @@ -4048,6 +4053,20 @@ sub CalcDateDue { } } } + + #check if it's minutes or hourly loan and set due date to close hour, if the current due date is passed closing hours. + if ($loanlength->{lengthunit} eq 'hours' || $loanlength->{lengthunit} eq 'minutes') { + my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branch, days_mode => $daysmode }); + my $dateInfo = $calendar->get_date_info($datedue); + my $close = dt_from_string($dateInfo->{date} ." ". $dateInfo->{close_hour}, 'iso', C4::Context->tz()); + + my $close_datetime = $datedue->clone()->set(hour => $close->hour(), minute=> $close->minute()); + + if (DateTime->compare($datedue, $close_datetime) > 0) { + $datedue = $close_datetime->clone(); + } + } + return $datedue; } diff --git a/C4/Overdues.pm b/C4/Overdues.pm index b6183fa503..a844a91c1b 100644 --- a/C4/Overdues.pm +++ b/C4/Overdues.pm @@ -333,6 +333,18 @@ sub get_chargeable_units { } return $charge_duration->in_units('hours'); } + elsif($unit eq 'minutes'){ + if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { + my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branchcode }); + $charge_duration = $calendar->open_minutes_between( $date_due, $date_returned ); + } else { + $charge_duration = $date_returned->delta_ms( $date_due ); + } + if($charge_duration == 0 && $charge_duration * 60 > 0) { + return 1; + } + return $charge_duration; + } else { # days if(C4::Context->preference('finesCalendar') eq 'noFinesWhenClosed') { my $calendar = Koha::DiscreteCalendar->new({ branchcode => $branchcode }); diff --git a/Koha/DiscreteCalendar.pm b/Koha/DiscreteCalendar.pm index 178e2b1dfe..d6e8e9c379 100644 --- a/Koha/DiscreteCalendar.pm +++ b/Koha/DiscreteCalendar.pm @@ -1255,6 +1255,75 @@ sub open_hours_between { return ($working_hours_between->next()->get_column('hours_between') - $not_used_hours->next()->get_column('not_used_hours')); } +=head2 open_minutes_between + + $minutes = $calendar->open_minutes_between($start_date, $end_date) + +Returns the number of minutes between C<$start_date> and C<$end_date>, taking into +account the opening hours of the library. + +=cut + +sub open_minutes_between { + my ($self, $start_date, $end_date) = @_; + my $branchcode = $self->{branchcode}; + my $schema = Koha::Database->new->schema; + my $dtf = DateTime::Format::Strptime->new(pattern => "%F %T"); + $start_date = $dtf->format_datetime($start_date); + $end_date = $dtf->format_datetime($end_date); + + my $working_minutes_between = $schema->resultset('DiscreteCalendar')->search( + { + branchcode => $branchcode, + is_opened => 1, + }, + { + select => \['sum(time_to_sec(timediff(close_hour, open_hour)) / 60)'], + as => [qw /minutes_between/], + where => \['date BETWEEN DATE(?) AND DATE(?)', $start_date, $end_date] + } + ); + + my $loan_day = $schema->resultset('DiscreteCalendar')->search( + { + branchcode => $branchcode, + }, + { + order_by => \[ 'ABS(DATEDIFF(date, ?))', $start_date ], + rows => 1, + } + ); + + my $return_day = $schema->resultset('DiscreteCalendar')->search( + { + branchcode => $branchcode, + }, + { + order_by => \[ 'ABS(DATEDIFF(date, ?))', $end_date ], + rows => 1, + } + ); + + #Capture the time portion of the date + $start_date =~ /\s(.*)/; + my $loan_date_time = $1; + $end_date =~ /\s(.*)/; + my $return_date_time = $1; + + my $not_used_minutes = $schema->resultset('DiscreteCalendar')->search( + { + branchcode => $branchcode, + is_opened => 1, + }, + { + select => \[ '(time_to_sec(timediff(?, ?)) + time_to_sec(timediff(?, ?)) ) / 60', $return_day->next()->close_hour(), $return_date_time, $loan_date_time, $loan_day->next()->open_hour()], + as => [qw /not_used_minutes/], + } + ); + + return ($working_minutes_between->next()->get_column('minutes_between') || 0 ) - ( $not_used_minutes->next()->get_column('not_used_minutes') || 0); +} + =head2 addDuration my $dt = $calendar->addDuration($date, $dur, $unit) @@ -1281,6 +1350,8 @@ sub addDuration { my $return_by_hour = 10; $dt = $self->addHours($startdate, $add_duration, $return_by_hour); + } elsif($unit eq 'minutes') { + $dt = $self->addMinutes($startdate, $add_duration); } else { # days $dt = $self->addDays($startdate, $add_duration); @@ -1323,6 +1394,31 @@ sub addHours { return $base_date; } +=head2 addMinutes + + $end = $calendar->addMinutes($date, $dur) + +Add C<$dur> minutes to C<$start> date. + +=cut + +sub addMinutes { + my ( $self, $startdate, $minutes_duration) = @_; + my $base_date = $startdate->clone(); + + $base_date->add_duration($minutes_duration); + + my $dateInfo = $self->get_date_info($base_date); + + my $close_datetime = dt_from_string($dateInfo->{date} ." ". $dateInfo->{close_hour}, 'iso'); + + if(DateTime->compare($base_date, $close_datetime) > 0) { + return $close_datetime; + } + + return $base_date; +} + =head2 addDays $date = $calendar->addDays($start, $duration) diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt index 26d44e54c6..1418828eff 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt @@ -271,6 +271,8 @@ Days [% ELSIF ( lengthunit == 'hours') %] Hours + [% ELSIF ( lengthunit == 'minutes') %] + Minutes [% ELSE %] Undefined [% END %] @@ -460,6 +462,7 @@ diff --git a/t/db_dependent/MinuteLoans.t b/t/db_dependent/MinuteLoans.t new file mode 100755 index 0000000000..b143d5d68d --- /dev/null +++ b/t/db_dependent/MinuteLoans.t @@ -0,0 +1,267 @@ +#!/usr/bin/env perl + +use strict; +use warnings; + +use DateTime; +use DateTime::Duration; +use Test::More tests => 10; +use Test::MockModule; + +use C4::Circulation; +use C4::Overdues; +use Koha::DateUtils qw( dt_from_string ); +use Koha::DiscreteCalendar; + +use t::lib::Mocks; +use t::lib::TestBuilder; + +my $dbh = C4::Context->dbh(); +my $today = DateTime->today; +my $schema = Koha::Database->new->schema; +$schema->storage->txn_begin; + +my $builder = t::lib::TestBuilder->new(); +my $branch = $builder->build({ source => 'Branch' })->{ branchcode }; + +my $calendar = Koha::DiscreteCalendar->new( { branchcode => $branch } ); + +####################################### +# Add minutes and open minutes between +####################################### + +my $start_date = dt_from_string->set( hour => 14, minute => 15, second => 0); +my $end_date = dt_from_string->set( hour => 14, minute => 20, second => 0); + +is($calendar->open_minutes_between($start_date, $end_date), 5, "Item returned 5 minutes late"); + +#Adding 10 minutes +my $ten_mins_duration = DateTime::Duration->new( minutes => 10); +is($calendar->addDuration($start_date, $ten_mins_duration, 'minutes' ), + dt_from_string->set( hour => 14, minute => 25, second => 0), + 'Added 10 minutes to loan' ); + +#Adding 10 minutes, passed closing hour +$start_date = dt_from_string->set( hour => 16, minute => 51, second => 0 ); +is($calendar->addDuration($start_date, $ten_mins_duration, 'minutes' ), + dt_from_string->set( hour => 17, minute => 0, second => 0), + 'Added 10 minutes, due date passed closing hours, set due date to same day at close hour' ); + +#Item returned next open day after a holiday. +my $open_minutes_today = DateTime->today->add(hours =>11, minutes => 10); +my $open_minutes_tomorrow = DateTime->today->add(days =>1); +my $open_minutes_day_after_tomorrow = DateTime->today->add(days => 2, hours =>11); + +$calendar->edit_holiday({ + title => "Single holiday Today", + holiday_type => $Koha::DiscreteCalendar::HOLIDAYS->{EXCEPTION}, + start_date => $open_minutes_tomorrow, + end_date =>$open_minutes_tomorrow +}); + +is($calendar->open_minutes_between($open_minutes_today, $open_minutes_day_after_tomorrow), + 530, + "Correct open minutes, with a holiday in between"); + +###################### +# DueDate calculation +###################### + +#Set syspref ReturnBeforeExpiry = 1 and useDaysMode = 'Days' +t::lib::Mocks::mock_preference('ReturnBeforeExpiry', 1); +t::lib::Mocks::mock_preference('useDaysMode', 'Days'); + +my $issuelength = 25; +my $renewalperiod = 5; +my $lengthunit = 'minutes'; +my $dateexpiry = DateTime->today->add(years => 1); + +my $mock_loan_length = [ + ['issuelength', 'renewalperiod', 'lengthunit'], + [$issuelength, $renewalperiod, $lengthunit] +]; + +my $categorycode = $builder->build({ source => 'Category' })->{categorycode}; +my $patron = $builder->build_object({ + class => 'Koha::Patrons', + value => { + categorycode => $categorycode, + branchcode => $branch, + dateexpiry => $dateexpiry, + }, +}); +my $itemtype = $builder->build_object( { class => 'Koha::ItemTypes' } )->itemtype; +$dbh->do("INSERT INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES('$branch', '$categorycode', '$itemtype', 'issuelength', '$issuelength')"); +$dbh->do("INSERT INTO circulation_rules (branchcode, categorycode, itemtype, rule_name, rule_value) VALUES('$branch', '$categorycode', '$itemtype', 'lengthunit', '$lengthunit')"); + + +my $date = C4::Circulation::CalcDateDue( $today, $itemtype, $branch, $patron ); +is($date, DateTime->today->add(minutes => $issuelength), "Due date calculated correctly"); + +#passed closing hour +$issuelength = 1300; +Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => $branch, + itemtype => $itemtype, + rules => { + issuelength => $issuelength, + } + } +); + +$date = C4::Circulation::CalcDateDue( $today, $itemtype, $branch, $patron ); +is($date, DateTime->today->add(hours =>17), "Due date passed close hour, item due at close hour"); + +############################# +# Chargeable minutes between +############################ + +$issuelength = 25; +Koha::CirculationRules->set_rules( + { + categorycode => $categorycode, + branchcode => $branch, + itemtype => $itemtype, + rules => { + issuelength => $issuelength, + } + } +); + +my $date_due = DateTime->today; +my $date_returned = DateTime->today->add(minutes => 40); + +my $chargeable_units = C4::Overdues::get_chargeable_units('minutes', $date_due, $date_returned, $branch); +is($chargeable_units, 40, "40 minutes of use"); + +###################### +# Fines calculation +##################### + +my $biblio = $builder->build_sample_biblio(); + +my $item = $builder->build( + { + source => 'Item', + value => { + biblionumber => $biblio->biblionumber, + homebranch => $branch, + holdingbranch => $branch, + replacementprice => '5.00', + }, + } +); +my $rule = $builder->schema->resultset('CirculationRule')->search({ + branchcode => undef, + categorycode => undef, + itemtype => undef, +}); +$rule->delete_all if $rule; +# FinesIncludeGracePeriod included +t::lib::Mocks::mock_preference('FinesIncludeGracePeriod', 1); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'fine', + rule_value => '1', + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'lengthunit', + rule_value => 'minutes', + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'finedays', + rule_value => 0, + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'firstremind', + rule_value => 5, + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'chargeperiod', + rule_value => 1, + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'overduefinescap', + rule_value => 9000, + }, + } +); +$builder->build( + { + source => 'CirculationRule', + value => { + branchcode => undef, + categorycode => undef, + itemtype => undef, + rule_name => 'cap_fine_to_replacement_price', + rule_value => 0, + }, + } +); + +my $start_dt = dt_from_string->set( hour => 15, minute => 0, second => 0); +my $end_dt = dt_from_string->set( hour => 15, minute => 4, second => 0); + +my @amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch, $start_dt, $end_dt ); +is($amount[0], 0, "No fine when under the fine grace period"); + +$end_dt = dt_from_string->set( hour => 15, minute => 6, second => 0); +@amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch, $start_dt, $end_dt ); +is($amount[0], 6, "6\$ fine for 6 minutes delay, fine grace period included"); + +# FinesIncludeGracePeriod not included +t::lib::Mocks::mock_preference('FinesIncludeGracePeriod', 0); + +@amount = C4::Overdues::CalcFine( $item, $patron->{categorycode}, $branch, $start_dt, $end_dt ); +is($amount[0], 1, "1\$ fine for 6 minutes delay, fine grace period not included"); + +$schema->storage->txn_rollback; + +1; -- 2.34.1