diff --git a/Koha/Calendar.pm b/Koha/Calendar.pm index 5ffebb47ab..645b538700 100644 --- a/Koha/Calendar.pm +++ b/Koha/Calendar.pm @@ -8,6 +8,15 @@ use DateTime::Duration; use C4::Context; use Koha::Caches; use Koha::Exceptions; +use Koha::Exceptions::Calendar; + +# This limit avoids an infinite loop when searching for an open day in an +# always closed library +# The value is arbitrary, but it should be large enough to be able to +# consider there is no open days if we haven't found any with that many +# iterations, and small enough to allow the loop to end quickly +# See next_open_days and prev_open_days +use constant OPEN_DAYS_SEARCH_MAX_ITERATIONS => 5000; sub new { my ( $classname, %options ) = @_; @@ -261,10 +270,18 @@ sub next_open_days { my $base_date = $dt->clone(); $base_date->add(days => $to_add); - while ($self->is_holiday($base_date)) { + my $i = 0; + while ( $self->is_holiday($base_date) && $i < OPEN_DAYS_SEARCH_MAX_ITERATIONS ) { my $add_next = $self->get_push_amt($base_date); $base_date->add(days => $add_next); + ++$i; } + + if ( $self->is_holiday($base_date) ) { + Koha::Exceptions::Calendar::NoOpenDays->throw( + sprintf( 'Unable to find an open day for library %s', $self->{branchcode} ) ); + } + return $base_date; } @@ -282,11 +299,18 @@ sub prev_open_days { $base_date->add(days => $to_sub); - while ($self->is_holiday($base_date)) { + my $i = 0; + while ( $self->is_holiday($base_date) && $i < OPEN_DAYS_SEARCH_MAX_ITERATIONS ) { my $sub_next = $self->get_push_amt($base_date); # Ensure we're subtracting when we need to be $sub_next = $sub_next > 0 ? 0 - $sub_next : $sub_next; $base_date->add(days => $sub_next); + ++$i; + } + + if ( $self->is_holiday($base_date) ) { + Koha::Exceptions::Calendar::NoOpenDays->throw( + sprintf( 'Unable to find an open day for library %s', $self->{branchcode} ) ); } return $base_date; diff --git a/Koha/Exceptions/Calendar.pm b/Koha/Exceptions/Calendar.pm new file mode 100644 index 0000000000..3891c3bced --- /dev/null +++ b/Koha/Exceptions/Calendar.pm @@ -0,0 +1,17 @@ +package Koha::Exceptions::Calendar; + +use Modern::Perl; + +use Koha::Exception; + +use Exception::Class ( + 'Koha::Exceptions::Calendar' => { + isa => 'Koha::Exception', + }, + 'Koha::Exceptions::Calendar::NoOpenDays' => { + isa => 'Koha::Exceptions::Calendar', + description => 'Library has no open days', + }, +); + +1; diff --git a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js index 7c586adeed..d5237792c7 100644 --- a/koha-tmpl/intranet-tmpl/prog/js/checkouts.js +++ b/koha-tmpl/intranet-tmpl/prog/js/checkouts.js @@ -201,6 +201,8 @@ $(document).ready(function() { content += __("Not allowed: patron restricted"); } else if ( data.error == "overdue" ) { content += __("Not allowed: overdue"); + } else if ( data.error == 'no_open_days' ) { + content += __('Unable to find an open day'); } else if ( data.error ) { content += data.error; } else { diff --git a/svc/renew b/svc/renew index 470c02e796..bc4ae53af9 100755 --- a/svc/renew +++ b/svc/renew @@ -76,6 +76,8 @@ if ( $data->{renew_okay} || ( $seen && $data->{error} eq 'too_unseen') ) { } catch { if ( ref($_) eq 'Koha::Exceptions::Checkout::FailedRenewal' ) { $data->{error} = 'renewal_failed'; + } elsif ( ref($_) eq 'Koha::Exceptions::Calendar::NoOpenDays' ) { + $data->{error} = 'no_open_days'; } else { $_->rethrow; } diff --git a/t/db_dependent/Holidays.t b/t/db_dependent/Holidays.t index 607906a169..42c2ce604e 100755 --- a/t/db_dependent/Holidays.t +++ b/t/db_dependent/Holidays.t @@ -17,7 +17,8 @@ use Modern::Perl; -use Test::More tests => 12; +use Test::More tests => 14; +use Test::Exception; use DateTime; use DateTime::TimeZone; @@ -277,5 +278,63 @@ subtest 'copy_to_branch' => sub { }; +subtest 'with a library that is never open' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; + my $calendar = C4::Calendar->new( branchcode => $branchcode ); + foreach my $weekday ( 0 .. 6 ) { + $calendar->insert_week_day_holiday( weekday => $weekday, title => '', description => '' ); + } + + my $now = DateTime->now; + + subtest 'next_open_days should throw an exception' => sub { + my $kcalendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => 'Calendar' ); + throws_ok { $kcalendar->next_open_days( $now, 1 ) } 'Koha::Exceptions::Calendar::NoOpenDays'; + }; + + subtest 'prev_open_days should throw an exception' => sub { + my $kcalendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => 'Calendar' ); + throws_ok { $kcalendar->prev_open_days( $now, 1 ) } 'Koha::Exceptions::Calendar::NoOpenDays'; + }; + + $schema->storage->txn_rollback; +}; + +subtest 'with a library that is *almost* never open' => sub { + plan tests => 2; + + $schema->storage->txn_begin; + + my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode}; + my $calendar = C4::Calendar->new( branchcode => $branchcode ); + foreach my $weekday ( 0 .. 6 ) { + $calendar->insert_week_day_holiday( weekday => $weekday, title => '', description => '' ); + } + + my $now = DateTime->now; + my $open_day_in_the_future = $now->clone()->add( years => 1 ); + my $open_day_in_the_past = $now->clone()->subtract( years => 1 ); + $calendar->insert_exception_holiday( date => $open_day_in_the_future->ymd, title => '', description => '' ); + $calendar->insert_exception_holiday( date => $open_day_in_the_past->ymd, title => '', description => '' ); + + subtest 'next_open_days should find the open day' => sub { + my $kcalendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => 'Calendar' ); + my $next_open_day = $kcalendar->next_open_days( $now, 1 ); + is( $next_open_day->ymd, $open_day_in_the_future->ymd ); + }; + + subtest 'prev_open_days should find the open day' => sub { + my $kcalendar = Koha::Calendar->new( branchcode => $branchcode, days_mode => 'Calendar' ); + my $prev_open_day = $kcalendar->prev_open_days( $now, 1 ); + is( $prev_open_day->ymd, $open_day_in_the_past->ymd ); + }; + + $schema->storage->txn_rollback; +}; + # Clear cache Koha::Caches->get_instance->flush_all;