Bug 42310: (QA follow-up) Reject in-place date changes on SingleClosure/Exception

store() only flushes the _holidays cache on insert, on the assumption that
updates only ever touch title/description. That assumption held for every
current caller (ModSingleholiday/ModExceptionholiday only ever change those
two fields), but nothing enforced it - a future caller updating date in
place would silently desync the cache for up to its TTL, with no error.

Make the assumption explicit: reject an update that changes date, rather
than silently allowing it. Callers that need to move a closure/exception to
a different date must delete and re-add.

Test plan:
1) prove t/db_dependent/Koha/Library/Calendar.t t/db_dependent/Calendar.t
   t/db_dependent/Holidays.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 2026-08-05 09:02:27 +01:00 committed by Pedro Amorim
parent 1399f8af42
commit 004fe4de21
No known key found for this signature in database
GPG Key ID: 06B75BA7F3894D55
3 changed files with 51 additions and 1 deletions

View File

@ -21,6 +21,7 @@ use Modern::Perl;
use Koha::Caches;
use Koha::Database;
use Koha::Exceptions;
use base qw(Koha::Object);
@ -38,12 +39,20 @@ Overloaded store method that clears the holidays cache on insert.
Only flushes on insert (new exception), not on title/description updates,
because the holidays cache only stores dates and their open/closed status.
An update that changes C<date> in place would desync the cache with no
warning, so that is rejected outright - callers must delete and re-add
instead.
=cut
sub store {
my ($self) = @_;
Koha::Exceptions::BadParameter->throw(
error => 'Cannot change date on an existing closure exception; delete and re-add instead',
parameter => 'date'
) if $self->in_storage && $self->_result->is_column_changed('date');
my $flush = !$self->in_storage;
$self = $self->SUPER::store;

View File

@ -21,6 +21,7 @@ use Modern::Perl;
use Koha::Caches;
use Koha::Database;
use Koha::Exceptions;
use base qw(Koha::Object);
@ -38,12 +39,20 @@ Overloaded store method that clears the holidays cache on insert.
Only flushes on insert (new closure), not on title/description updates,
because the holidays cache only stores dates and their open/closed status.
An update that changes C<date> in place would desync the cache with no
warning, so that is rejected outright - callers must delete and re-add
instead.
=cut
sub store {
my ($self) = @_;
Koha::Exceptions::BadParameter->throw(
error => 'Cannot change date on an existing single closure; delete and re-add instead',
parameter => 'date'
) if $self->in_storage && $self->_result->is_column_changed('date');
my $flush = !$self->in_storage;
$self = $self->SUPER::store;

View File

@ -19,7 +19,7 @@
use Modern::Perl;
use Test::More tests => 7;
use Test::More tests => 8;
use Test::Exception;
use Test::NoWarnings;
@ -339,3 +339,35 @@ subtest 'add_weekly_closure validates weekday' => sub {
$schema->storage->txn_rollback;
};
subtest 'SingleClosure/Exception store() rejects an in-place date change' => sub {
plan tests => 4;
$schema->storage->txn_begin;
my $library = $builder->build_object( { class => 'Koha::Libraries' } );
my $calendar = Koha::Library::Calendar->new( branchcode => $library->branchcode );
my $single = $calendar->add_single_closure( { date => '2027-08-01', title => 'Staff day', description => '' } );
throws_ok {
$single->date('2027-08-02')->store;
}
'Koha::Exceptions::BadParameter', 'SingleClosure->store dies when date is changed in place';
$single->discard_changes;
$single->title('Staff day (renamed)')->store;
is( $single->title, 'Staff day (renamed)', 'SingleClosure->store still allows a title-only update' );
my $exception = $calendar->add_exception( { date => '2027-08-03', title => 'Special open', description => '' } );
throws_ok {
$exception->date('2027-08-04')->store;
}
'Koha::Exceptions::BadParameter', 'Exception->store dies when date is changed in place';
$exception->discard_changes;
$exception->title('Special open (renamed)')->store;
is( $exception->title, 'Special open (renamed)', 'Exception->store still allows a title-only update' );
$schema->storage->txn_rollback;
};