Bugzilla – Attachment 162097 Details for
Bug 6796
Overnight checkouts taking into account opening and closing hours
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 6796: (follow-up) QA test tool fixes
Bug-6796-follow-up-QA-test-tool-fixes.patch (text/plain), 18.52 KB, created by
Martin Renvoize (ashimema)
on 2024-02-13 10:40:18 UTC
(
hide
)
Description:
Bug 6796: (follow-up) QA test tool fixes
Filename:
MIME Type:
Creator:
Martin Renvoize (ashimema)
Created:
2024-02-13 10:40:18 UTC
Size:
18.52 KB
patch
obsolete
>From 471d67d0f6e2c93bf4bf94a85f6bfe673e8ae314 Mon Sep 17 00:00:00 2001 >From: Aleisha Amohia <aleishaamohia@hotmail.com> >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 <samalau@gmail.com> >Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> >--- > 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 5d564e1caed..6805bee2a61 100644 >--- a/C4/Circulation.pm >+++ b/C4/Circulation.pm >@@ -3895,20 +3895,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 ); > } > >@@ -3916,20 +3919,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} ); > } >@@ -3940,23 +3947,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 d51cf67764b..3b621b4f0e8 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 cc207641f95..d8452725af7 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/branches.tt >@@ -303,21 +303,21 @@ > [% IF opening_hours # Existing library %] > [% daycount = 0 %] > [% FOREACH hr IN opening_hours %] >- <tr id="hours_[% daycount %]"> >+ <tr id="hours_[% daycount | html %]"> > <td> > [% PROCESS dayname day=daycount %] >- <input type="hidden" value="[% daycount %]" name="day"> >+ <input type="hidden" value="[% daycount | html %]" name="day"> > </td> > <td> > [% IF hr.day == daycount && hr.open_time %] >- <input type="time" value="[% hr.open_time %]" name="open_time"> >+ <input type="time" value="[% hr.open_time | html %]" name="open_time"> > [% ELSE %] > <input type="time" value="" name="open_time"> > [% END %] > </td> > <td> > [% IF hr.day == daycount && hr.close_time %] >- <input type="time" value="[% hr.close_time %]" name="close_time"> >+ <input type="time" value="[% hr.close_time | html %]" name="close_time"> > [% ELSE %] > <input type="time" value="" name="close_time"> > [% END %] >@@ -327,10 +327,10 @@ > [% END %] > [% ELSE # New library %] > [% FOREACH daycount IN [0..6] %] >- <tr id="hours_[% day %]"> >+ <tr id="hours_[% day | html %]"> > <td> > [% PROCESS dayname day=daycount %] >- <input type="hidden" value="[% daycount %]" name="day"> >+ <input type="hidden" value="[% daycount | html %]" name="day"> > </td> > <td> > <input type="time" value="" name="open_time"> >diff --git a/t/db_dependent/Circulation/CalcDateDue.t b/t/db_dependent/Circulation/CalcDateDue.t >index 54bde9fe957..f4c232b9dd3 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.43.0
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=162097">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 6796
:
124669
|
124670
|
124671
|
124672
|
134323
|
134324
|
134325
|
134326
|
134327
|
145669
|
145670
|
145671
|
145672
|
145673
|
146988
|
146989
|
146990
|
146991
|
146992
|
150442
|
150443
|
150444
|
150445
|
150446
|
150447
|
150448
|
150477
|
150478
|
153252
|
153253
|
153254
|
153255
|
153256
|
153257
|
153258
|
153259
|
153260
|
153261
|
153271
|
153272
|
153273
|
153274
|
153275
|
153276
|
153277
|
153278
|
153279
|
153280
|
156772
|
156773
|
156774
|
156775
|
156776
|
156777
|
156778
|
156779
|
156780
|
161970
|
161971
|
161972
|
161973
|
161974
|
161975
|
161976
|
161977
|
161978
|
161979
|
161980
|
161986
|
161987
|
161988
|
161989
|
162043
|
162044
|
162045
|
162046
|
162047
|
162048
|
162049
|
162050
|
162051
|
162052
|
162053
|
162054
|
162055
|
162056
|
162090
|
162091
|
162092
|
162093
|
162094
|
162095
|
162096
|
162097
|
162098
|
162099
|
162100
|
162101
|
162102
|
162103
|
162104
|
164511
|
164512
|
164513
|
164514
|
164515
|
164516
|
164517
|
164518
|
164538
|
164539
|
164540
|
164541
|
164542
|
164543
|
164544
|
164545
|
164546
|
164547
|
164548
|
164549
|
164550
|
164551
|
164552