From 41edb937dfe59983323c70bab23a7a935cb81556 Mon Sep 17 00:00:00 2001 From: Aleisha Amohia Date: Mon, 1 May 2023 22:10:44 +0000 Subject: [PATCH] Bug 6796: (follow-up) QA test tool fixes Sponsored-by: Catalyst IT Sponsored-by: Auckland University of Technology Signed-off-by: Sam Lau Signed-off-by: Martin Renvoize Signed-off-by: Katrin Fischer --- C4/Circulation.pm | 36 +++++---- admin/branches.pl | 29 ++++--- .../prog/en/modules/admin/branches.tt | 12 +-- t/db_dependent/Circulation/CalcDateDue.t | 78 ++++++++++++------- 4 files changed, 95 insertions(+), 60 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index c10c2f4c30..f4bf52c8dd 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3891,20 +3891,23 @@ sub CalcDateDue { my $considerlibraryhours = C4::Context->preference('ConsiderLibraryHoursInCirculation'); # starter vars so don't do calculations directly to $datedue - my $potential_datedue = $datedue->clone; - my $library_close = $datedue->clone; - my $dayofweek = $datedue->day_of_week - 1; + my $potential_datedue = $datedue->clone; + my $library_close = $datedue->clone; + my $dayofweek = $datedue->day_of_week - 1; my $tomorrow_dayofweek = $dayofweek + 1; + # If it's Sunday and tomorrow would be == 7, make tomorrow 0 (Days are stored as 0-6) if ( $tomorrow_dayofweek > 6 ) { $tomorrow_dayofweek = 0; } - my $todayhours = Koha::Library::Hours->find({ library_id => $branch, day => $dayofweek }); - my @close = undef; - my $tomorrowhours = Koha::Library::Hours->find({ library_id => $branch, day => $dayofweek+1 }); # get open hours of next day + my $todayhours = Koha::Library::Hours->find( { library_id => $branch, day => $dayofweek } ); + my @close = undef; + my $tomorrowhours = Koha::Library::Hours->find( { library_id => $branch, day => $tomorrow_dayofweek } ) + ; # get open hours of next day my @open = undef; - if ( $todayhours->close_time and $tomorrowhours->open_time ) { - @close = split( ":", $todayhours->close_time ); - $library_close = $library_close->set( hour => $close[0], minute => $close[1] ); - $potential_datedue = $potential_datedue->add( hours => $loanlength->{$length_key} ); # datedue without consideration for open hours + if ( $considerlibraryhours ne 'ignore' and $todayhours->close_time and $tomorrowhours->open_time ) { + @close = split( ":", $todayhours->close_time ); + $library_close = $library_close->set( hour => $close[0], minute => $close[1] ); + $potential_datedue = $potential_datedue->add( hours => $loanlength->{$length_key} ) + ; # datedue without consideration for open hours @open = split( ":", $tomorrowhours->open_time ); } @@ -3912,20 +3915,24 @@ sub CalcDateDue { if ( $daysmode eq 'Days' ) { # ignoring calendar if ( $loanlength->{lengthunit} eq 'hours' ) { - if ( $potential_datedue > $library_close and $todayhours->close_time and $tomorrowhours->open_time ) { + if ( $considerlibraryhours ne 'ignore' and $potential_datedue > $library_close and $todayhours->close_time and $tomorrowhours->open_time ) { if ( $considerlibraryhours eq 'close' ) { + # datedue will be after the library closes on that day # shorten loan period to end when library closes $datedue->set( hour => $close[0], minute => $close[1] ); } elsif ( $considerlibraryhours eq 'open' ) { + # datedue will be after the library closes on that day # extend loan period to when library opens following day $datedue->add( days => 1 )->set( hour => $open[0], minute => $open[1] ); } else { + # ignore library open hours $datedue->add( hours => $loanlength->{$length_key} ); } } else { + # due time doesn't conflict with library open hours, don't need to check $datedue->add( hours => $loanlength->{$length_key} ); } @@ -3936,23 +3943,26 @@ sub CalcDateDue { } } else { my $dur; - my $sethours; if ($loanlength->{lengthunit} eq 'hours') { - if ( $potential_datedue > $library_close and $todayhours->close_time and $tomorrowhours->open_time ) { + if ( $considerlibraryhours ne 'ignore' and $potential_datedue > $library_close and $todayhours->close_time and $tomorrowhours->open_time ) { if ( $considerlibraryhours eq 'close' ) { + # datedue will be after the library closes on that day # shorten loan period to end when library closes by hardcoding due time $datedue->set( hour => $close[0], minute => $close[1] ); } elsif ( $considerlibraryhours eq 'open' ) { + # datedue will be after the library closes on that day # extend loan period to when library opens following day by hardcoding due time for next open day $dur = DateTime::Duration->new( days => 1 ); $datedue->set( hour => $open[0], minute => $open[1] ); } else { + # ignore library open hours $dur = DateTime::Duration->new( hours => $loanlength->{$length_key} ); } } else { + # due time doesn't conflict with library open hours, don't need to check $dur = DateTime::Duration->new( hours => $loanlength->{$length_key} ); } diff --git a/admin/branches.pl b/admin/branches.pl index b94325eef2..9a21ce59a7 100755 --- a/admin/branches.pl +++ b/admin/branches.pl @@ -50,11 +50,12 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( ); if ( $op eq 'add_form' ) { - my @opening_hours = Koha::Library::Hours->search({ library_id => $branchcode }, { order_by => { -asc => 'day' } })->as_list; + my @opening_hours = + Koha::Library::Hours->search( { library_id => $branchcode }, { order_by => { -asc => 'day' } } )->as_list; $template->param( - library => Koha::Libraries->find($branchcode), - smtp_servers => Koha::SMTP::Servers->search, + library => Koha::Libraries->find($branchcode), + smtp_servers => Koha::SMTP::Servers->search, opening_hours => \@opening_hours ); } elsif ( $branchcode && $op eq 'view' ) { @@ -120,11 +121,11 @@ if ( $op eq 'add_form' ) { } } - my @days = $input->multi_param("day"); - my @open_times = $input->multi_param("open_time"); + my @days = $input->multi_param("day"); + my @open_times = $input->multi_param("open_time"); my @close_times = $input->multi_param("close_time"); - foreach my $day ( @days ) { + foreach my $day (@days) { if ( $open_times[$day] !~ /([0-9]{2}:[0-9]{2})/ ) { $open_times[$day] = undef; } @@ -132,7 +133,8 @@ if ( $op eq 'add_form' ) { $close_times[$day] = undef; } - my $openday = Koha::Library::Hours->find({ library_id => $branchcode, day => $day })->update({ open_time => $open_times[$day], close_time => $close_times[$day] }); + my $openday = Koha::Library::Hours->find( { library_id => $branchcode, day => $day } ) + ->update( { open_time => $open_times[$day], close_time => $close_times[$day] } ); } push @messages, { type => 'message', code => 'success_on_update' }; @@ -173,11 +175,11 @@ if ( $op eq 'add_form' ) { } } - my @days = $input->multi_param("day"); - my @open_times = $input->multi_param("open_time"); + my @days = $input->multi_param("day"); + my @open_times = $input->multi_param("open_time"); my @close_times = $input->multi_param("close_time"); - foreach my $day ( @days ) { + foreach my $day (@days) { if ( $open_times[$day] !~ /([0-9]{2}:[0-9]{2})/ ) { $open_times[$day] = undef; } @@ -185,7 +187,12 @@ if ( $op eq 'add_form' ) { $close_times[$day] = undef; } - my $openday = Koha::Library::Hour->new({ library_id => $branchcode, day => $day, open_time => $open_times[$day], close_time => $close_times[$day] })->store; + my $openday = Koha::Library::Hour->new( + { + library_id => $branchcode, day => $day, open_time => $open_times[$day], + close_time => $close_times[$day] + } + )->store; } push @messages, { type => 'message', code => 'success_on_insert' }; diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt index 46f822fb23..ff839f0fce 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt @@ -304,21 +304,21 @@ [% IF opening_hours # Existing library %] [% daycount = 0 %] [% FOREACH hr IN opening_hours %] - + [% PROCESS dayname day=daycount %] - + [% IF hr.day == daycount && hr.open_time %] - + [% ELSE %] [% END %] [% IF hr.day == daycount && hr.close_time %] - + [% ELSE %] [% END %] @@ -328,10 +328,10 @@ [% END %] [% ELSE # New library %] [% FOREACH daycount IN [0..6] %] - + [% PROCESS dayname day=daycount %] - + diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t index 54bde9fe95..f4c232b9dd 100755 --- a/t/db_dependent/Circulation/CalcDateDue.t +++ b/t/db_dependent/Circulation/CalcDateDue.t @@ -8,7 +8,7 @@ use DBI; use DateTime; use t::lib::Mocks; use t::lib::TestBuilder; -use C4::Calendar qw( new insert_single_holiday delete_holiday insert_week_day_holiday ); +use C4::Calendar qw( new insert_single_holiday delete_holiday insert_week_day_holiday ); use Koha::DateUtils qw( dt_from_string ); use Koha::Library::Hours; use Koha::CirculationRules; @@ -362,79 +362,97 @@ is( $date->ymd, $renewed_date->ymd, 'Renewal period of "" should trigger fallove my $library1 = $builder->build( { source => 'Branch' } ); Koha::CirculationRules->set_rules( { - categorycode => $categorycode, + categorycode => $borrower->categorycode, itemtype => $itemtype, branchcode => $library1->{branchcode}, rules => { - issuelength => 3, # loan period is 3 hours - lengthunit => 'hours', - daysmode => '', + issuelength => 3, # loan period is 3 hours + lengthunit => 'hours', + daysmode => '', } } ); -my $open = DateTime->new( year => 2023, month => 5, day => 1, hour => 10 )->hms; +my $open = DateTime->new( year => 2023, month => 5, day => 1, hour => 10 )->hms; my $close = DateTime->new( year => 2023, month => 5, day => 1, hour => 16 )->hms; -my $now = DateTime->new( year => 2023, month => 5, day => 1, hour => 14 ); +my $now = DateTime->new( year => 2023, month => 5, day => 1, hour => 14 ); + +foreach ( 0 .. 6 ) { -foreach (0..6) { # library opened 4 hours ago and closes in 2 hours. - Koha::Library::Hour->new({ day => $_, library_id => $library1->{branchcode}, open_time => $open, close_time => $close })->store; + Koha::Library::Hour->new( + { day => $_, library_id => $library1->{branchcode}, open_time => $open, close_time => $close } )->store; } # ignore calendar -t::lib::Mocks::mock_preference('useDaysMode', 'Days'); -t::lib::Mocks::mock_preference('ConsiderLibraryHoursInCirculation', 'close'); +t::lib::Mocks::mock_preference( 'useDaysMode', 'Days' ); +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'close' ); + # shorten loan period $date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); my $expected_duetime = $now->clone->add( hours => 2 ); -is( $date, $expected_duetime, "Loan period was shortened because ConsiderLibraryHoursInCirculation is set to close time" ); +is( + $date, $expected_duetime, + "Loan period was shortened because ConsiderLibraryHoursInCirculation is set to close time" +); + +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); -t::lib::Mocks::mock_preference('ConsiderLibraryHoursWhenIssuing', 'open'); # extend loan period -$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); +$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); $expected_duetime = $now->clone->add( days => 1 )->subtract( hours => 4 ); -is( $date, $expected_duetime, "Loan period was extended because ConsiderLibraryHoursInCirculation is set to open time" ); +is( + $date, $expected_duetime, + "Loan period was extended because ConsiderLibraryHoursInCirculation is set to open time" +); my $holiday_tomorrow = $now->clone->add( days => 1 ); # consider calendar my $library1_calendar = C4::Calendar->new( branchcode => $library1->{branchcode} ); $library1_calendar->insert_single_holiday( - day => $holiday_tomorrow->day, - month => $holiday_tomorrow->month, - year => $holiday_tomorrow->year, - title => 'testholiday', - description => 'testholiday' + day => $holiday_tomorrow->day, + month => $holiday_tomorrow->month, + year => $holiday_tomorrow->year, + title => 'testholiday', + description => 'testholiday' ); Koha::CirculationRules->set_rules( { - categorycode => $categorycode, + categorycode => $patron_category->categorycode, itemtype => $itemtype, branchcode => $library1->{branchcode}, rules => { - issuelength => 18, # loan period must cross over into tomorrow - lengthunit => 'hours', + issuelength => 18, # loan period must cross over into tomorrow + lengthunit => 'hours', } } ); -t::lib::Mocks::mock_preference('useDaysMode', 'Calendar'); -t::lib::Mocks::mock_preference('ConsiderLibraryHoursInCirculation', 'close'); +t::lib::Mocks::mock_preference( 'useDaysMode', 'Calendar' ); +t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'close' ); + # shorten loan period -$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); -$expected_duetime = $now->clone->add( days => 2, hours => 2 ); -is( $date, $expected_duetime, "Loan period was shortened (but considers the holiday) because ConsiderLibraryHoursInCirculation is set to close time" ); +$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); +$expected_duetime = $now->clone->add( hours => 2 ); +is( + $date, $expected_duetime, + "Loan period was shortened (but considers the holiday) because ConsiderLibraryHoursInCirculation is set to close time" +); t::lib::Mocks::mock_preference( 'ConsiderLibraryHoursInCirculation', 'open' ); + # extend loan period -$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); +$date = C4::Circulation::CalcDateDue( $now, $itemtype, $library1->{branchcode}, $borrower ); $expected_duetime = $now->clone->add( days => 2 )->subtract( hours => 4 ); -is( $date, $expected_duetime, "Loan period was extended (but considers the holiday) because ConsiderLibraryHoursInCirculation is set to open time" ); +is( + $date, $expected_duetime, + "Loan period was extended (but considers the holiday) because ConsiderLibraryHoursInCirculation is set to open time" +); $cache->clear_from_cache($key); $schema->storage->txn_rollback; -- 2.39.5