Bug 42310: (QA follow-up) Restore in-memory holiday cache sync in C4::Calendar
insert_single_holiday and insert_exception_holiday delegate to
Koha::Library::Calendar::SingleClosure/Exception, but stopped updating
$self->{single_holidays}/{exception_holidays} on the C4::Calendar object
itself, unlike the sibling insert_week_day_holiday/insert_day_month_holiday
methods which still do. isHoliday() reads exactly those hashes, so a date
just inserted via insert_single_holiday/insert_exception_holiday would read
back as "not a holiday" for the rest of that object's lifetime - reachable
from tools/newHolidays.pl and tools/exceptionHolidays.pl, which loop over
several dates on one $calendar object, checking isHoliday() before each
insert to skip duplicates.
While adding a regression test, also found that _init() has the same class
of bug on the DB-load path: it builds the single/exception lookup keys from
`split(/-/, $row->{date})`, which keeps the zero-padding from the DATE
string (e.g. "06"), while isHoliday() numifies its lookup key (e.g. "6").
Any single/exception holiday with a single-digit month or day was therefore
invisible to isHoliday() even on a freshly reloaded C4::Calendar object.
Both call sites now numify year/month/day before building the hash key.
Test plan:
1) prove t/db_dependent/Holidays.t t/db_dependent/Calendar.t
t/db_dependent/Koha/Library/Calendar.t t/db_dependent/HoldsQueue.t
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This commit is contained in:
Martin Renvoize
Pedro Amorim
parent
e56aff8dfc
commit
d0645cc8b1
|
|
@ -104,7 +104,7 @@ sub _init {
|
|||
$exceptions_sth->execute($branch);
|
||||
my %exception_holidays;
|
||||
while ( my $row = $exceptions_sth->fetchrow_hashref ) {
|
||||
my ( $year, $month, $day ) = split( /-/, $row->{date} );
|
||||
my ( $year, $month, $day ) = map { $_ + 0 } split( /-/, $row->{date} );
|
||||
$exception_holidays{"$year/$month/$day"}{title} = $row->{title};
|
||||
$exception_holidays{"$year/$month/$day"}{description} = $row->{description};
|
||||
$exception_holidays{"$year/$month/$day"}{date} = $row->{date};
|
||||
|
|
@ -115,7 +115,7 @@ sub _init {
|
|||
$single_sth->execute($branch);
|
||||
my %single_holidays;
|
||||
while ( my $row = $single_sth->fetchrow_hashref ) {
|
||||
my ( $year, $month, $day ) = split( /-/, $row->{date} );
|
||||
my ( $year, $month, $day ) = map { $_ + 0 } split( /-/, $row->{date} );
|
||||
$single_holidays{"$year/$month/$day"}{title} = $row->{title};
|
||||
$single_holidays{"$year/$month/$day"}{description} = $row->{description};
|
||||
$single_holidays{"$year/$month/$day"}{date} = $row->{date};
|
||||
|
|
@ -274,11 +274,18 @@ sub insert_single_holiday {
|
|||
unless ($date) {
|
||||
$date = sprintf( ISO_DATE_FORMAT, $options{year}, $options{month}, $options{day} );
|
||||
}
|
||||
@options{qw(year month day)} = map { $_ + 0 } ( $date =~ m/(\d+)-(\d+)-(\d+)/ )
|
||||
unless $options{day};
|
||||
|
||||
Koha::Library::Calendar->new( branchcode => $self->{branchcode} )->add_single_closure(
|
||||
{
|
||||
date => $date, title => $options{title}, description => $options{description},
|
||||
}
|
||||
);
|
||||
|
||||
$self->{'single_holidays'}->{"$options{year}/$options{month}/$options{day}"}{title} = $options{title};
|
||||
$self->{'single_holidays'}->{"$options{year}/$options{month}/$options{day}"}{description} = $options{description};
|
||||
|
||||
return $self;
|
||||
}
|
||||
|
||||
|
|
@ -311,11 +318,19 @@ sub insert_exception_holiday {
|
|||
unless ($date) {
|
||||
$date = sprintf( ISO_DATE_FORMAT, $options{year}, $options{month}, $options{day} );
|
||||
}
|
||||
@options{qw(year month day)} = map { $_ + 0 } ( $date =~ m/(\d+)-(\d+)-(\d+)/ )
|
||||
unless $options{day};
|
||||
|
||||
Koha::Library::Calendar->new( branchcode => $self->{branchcode} )->add_exception(
|
||||
{
|
||||
date => $date, title => $options{title}, description => $options{description},
|
||||
}
|
||||
);
|
||||
|
||||
$self->{'exception_holidays'}->{"$options{year}/$options{month}/$options{day}"}{title} = $options{title};
|
||||
$self->{'exception_holidays'}->{"$options{year}/$options{month}/$options{day}"}{description} =
|
||||
$options{description};
|
||||
|
||||
return $self;
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -18,7 +18,7 @@
|
|||
use Modern::Perl;
|
||||
|
||||
use Test::NoWarnings;
|
||||
use Test::More tests => 15;
|
||||
use Test::More tests => 16;
|
||||
use Test::Exception;
|
||||
|
||||
use DateTime;
|
||||
|
|
@ -386,5 +386,58 @@ subtest 'with a library that is *almost* never open' => sub {
|
|||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
subtest 'insert_single_holiday/insert_exception_holiday update the in-memory cache' => sub {
|
||||
plan tests => 4;
|
||||
|
||||
$schema->storage->txn_begin;
|
||||
|
||||
my $branchcode = $builder->build( { source => 'Branch' } )->{branchcode};
|
||||
my $calendar = C4::Calendar->new( branchcode => $branchcode );
|
||||
|
||||
my $single_day = dt_from_string()->add( days => 10 );
|
||||
$calendar->insert_single_holiday(
|
||||
day => $single_day->day,
|
||||
month => $single_day->month,
|
||||
year => $single_day->year,
|
||||
title => 'Single',
|
||||
description => '',
|
||||
);
|
||||
is(
|
||||
$calendar->isHoliday( $single_day->day, $single_day->month, $single_day->year ), 1,
|
||||
'isHoliday sees a single holiday just inserted on the same C4::Calendar object'
|
||||
);
|
||||
|
||||
# Close every weekday so exception_day is a holiday until the exception is added.
|
||||
foreach my $weekday ( 0 .. 6 ) {
|
||||
$calendar->insert_week_day_holiday( weekday => $weekday, title => '', description => '' );
|
||||
}
|
||||
|
||||
# insert_exception_holiday called with 'date' rather than day/month/year,
|
||||
# exercising the date-string derivation path.
|
||||
my $exception_day = dt_from_string()->add( days => 20 );
|
||||
$calendar->insert_exception_holiday(
|
||||
date => $exception_day->ymd,
|
||||
title => 'Exception',
|
||||
description => '',
|
||||
);
|
||||
is(
|
||||
$calendar->isHoliday( $exception_day->day, $exception_day->month, $exception_day->year ), 0,
|
||||
'isHoliday sees an exception holiday just inserted on the same C4::Calendar object, overriding the weekday closure'
|
||||
);
|
||||
|
||||
# A second, freshly instantiated C4::Calendar object should agree.
|
||||
my $reloaded = C4::Calendar->new( branchcode => $branchcode );
|
||||
is(
|
||||
$reloaded->isHoliday( $single_day->day, $single_day->month, $single_day->year ), 1,
|
||||
'A freshly instantiated C4::Calendar also sees the single holiday'
|
||||
);
|
||||
is(
|
||||
$reloaded->isHoliday( $exception_day->day, $exception_day->month, $exception_day->year ), 0,
|
||||
'A freshly instantiated C4::Calendar also sees the exception holiday'
|
||||
);
|
||||
|
||||
$schema->storage->txn_rollback;
|
||||
};
|
||||
|
||||
# Clear cache
|
||||
Koha::Caches->get_instance->flush_all;
|
||||
|
|
|
|||
Loading…
Reference in New Issue