From d0645cc8b1c0c39c80efbf5027211555847dad1c Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Wed, 5 Aug 2026 08:53:40 +0100 Subject: [PATCH] Bug 42310: (QA follow-up) Restore in-memory holiday cache sync in C4::Calendar MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Pedro Amorim --- C4/Calendar.pm | 19 ++++++++++++-- t/db_dependent/Holidays.t | 55 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 71 insertions(+), 3 deletions(-) diff --git a/C4/Calendar.pm b/C4/Calendar.pm index 3e92b0130b..8b067bd080 100644 --- a/C4/Calendar.pm +++ b/C4/Calendar.pm @@ -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; } diff --git a/t/db_dependent/Holidays.t b/t/db_dependent/Holidays.t index 28fcd3a362..4572a668dc 100755 --- a/t/db_dependent/Holidays.t +++ b/t/db_dependent/Holidays.t @@ -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;