From ce89120213add246b05d00b78de29b29a2a7b45e Mon Sep 17 00:00:00 2001 From: Pedro Amorim Date: Wed, 6 Aug 2025 15:17:15 +0000 Subject: [PATCH] Bug 40529: (QA follow-up): Remove hold as hold group target when reverted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If a hold that is a hold group target, i.e. it's 'waiting' or 'in transit', is reverted it needs to remove itself as its own hold group target Tests added here as well: prove t/db_dependent/Koha/Hold.t Signed-off-by: Anneli Österman --- C4/Reserves.pm | 2 ++ Koha/Hold.pm | 23 +++++++++++++++ Koha/Schema/Result/HoldGroupTargetHold.pm | 29 ++++++++++++------- .../bug_40529_-_hold_group_table_target.pl | 1 + installer/data/mysql/kohastructure.sql | 1 + t/db_dependent/Koha/Hold.t | 11 +++++-- 6 files changed, 55 insertions(+), 12 deletions(-) diff --git a/C4/Reserves.pm b/C4/Reserves.pm index aca7116ad3..5e02661d5c 100644 --- a/C4/Reserves.pm +++ b/C4/Reserves.pm @@ -2204,6 +2204,8 @@ sub RevertWaitingStatus { _FixPriority( { biblionumber => $hold->biblionumber } ); + $hold->remove_as_hold_group_target(); + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( { biblio_ids => [ $hold->biblionumber ] } ) if C4::Context->preference('RealTimeHoldsQueue'); diff --git a/Koha/Hold.pm b/Koha/Hold.pm index a4fadda4f4..37bf0a257b 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -219,6 +219,29 @@ sub set_as_hold_group_target { } } +=head3 remove_as_hold_group_target + +$self->remove_as_hold_group_target; + +Removes this hold as the target of its hold group if it is currently set as such, +and if the DisplayAddHoldGroups system preference is enabled. + +=cut + +sub remove_as_hold_group_target { + my ($self) = @_; + + if ( $self->hold_group + && $self->hold_group->target_hold_id eq $self->reserve_id + && C4::Context->preference("DisplayAddHoldGroups") ) + { + $self->_result->find_related( + 'hold_group_target_hold', + { hold_group_id => $self->hold_group->hold_group_id, reserve_id => $self->reserve_id } + )->delete(); + } +} + =head3 is_hold_group_target $self->is_hold_group_target; diff --git a/Koha/Schema/Result/HoldGroupTargetHold.pm b/Koha/Schema/Result/HoldGroupTargetHold.pm index b23ea3146b..388fecace4 100644 --- a/Koha/Schema/Result/HoldGroupTargetHold.pm +++ b/Koha/Schema/Result/HoldGroupTargetHold.pm @@ -36,7 +36,7 @@ foreign key, linking this to the hold_groups table data_type: 'integer' is_foreign_key: 1 - is_nullable: 1 + is_nullable: 0 foreign key, linking this to the reserves table @@ -51,9 +51,23 @@ __PACKAGE__->add_columns( is_nullable => 0, }, "reserve_id", - { data_type => "integer", is_foreign_key => 1, is_nullable => 1 }, + { data_type => "integer", is_foreign_key => 1, is_nullable => 0 }, ); +=head1 PRIMARY KEY + +=over 4 + +=item * L + +=item * L + +=back + +=cut + +__PACKAGE__->set_primary_key("hold_group_id", "reserve_id"); + =head1 UNIQUE CONSTRAINTS =head2 C @@ -109,17 +123,12 @@ __PACKAGE__->belongs_to( "reserve", "Koha::Schema::Result::Reserve", { reserve_id => "reserve_id" }, - { - is_deferrable => 1, - join_type => "LEFT", - on_delete => "CASCADE", - on_update => "RESTRICT", - }, + { is_deferrable => 1, on_delete => "CASCADE", on_update => "RESTRICT" }, ); -# Created by DBIx::Class::Schema::Loader v0.07051 @ 2025-07-25 13:10:48 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:xgRnzTXH+Q+YxYLKQeGe+g +# Created by DBIx::Class::Schema::Loader v0.07051 @ 2025-08-06 15:04:39 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:UD9tr/ceHggVl1rRrimtVg # You can replace this text with custom code or comments, and it will be preserved on regeneration diff --git a/installer/data/mysql/atomicupdate/bug_40529_-_hold_group_table_target.pl b/installer/data/mysql/atomicupdate/bug_40529_-_hold_group_table_target.pl index 4d06435136..bff6bd7504 100755 --- a/installer/data/mysql/atomicupdate/bug_40529_-_hold_group_table_target.pl +++ b/installer/data/mysql/atomicupdate/bug_40529_-_hold_group_table_target.pl @@ -14,6 +14,7 @@ return { CREATE TABLE hold_group_target_holds ( `hold_group_id` int unsigned NOT NULL UNIQUE COMMENT 'foreign key, linking this to the hold_groups table', `reserve_id` int(11) DEFAULT NULL UNIQUE COMMENT 'foreign key, linking this to the reserves table', + PRIMARY KEY (`hold_group_id`, `reserve_id`), CONSTRAINT `hold_group_target_holds_ibfk_1` FOREIGN KEY (`hold_group_id`) REFERENCES `hold_groups` (`hold_group_id`) ON DELETE CASCADE, CONSTRAINT `hold_group_target_holds_ibfk_2` FOREIGN KEY (`reserve_id`) REFERENCES `reserves` (`reserve_id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index b70c81abbd..99245fb677 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -5083,6 +5083,7 @@ DROP TABLE IF EXISTS `hold_group_target_holds`; CREATE TABLE hold_group_target_holds ( `hold_group_id` int unsigned NOT NULL UNIQUE COMMENT 'foreign key, linking this to the hold_groups table', `reserve_id` int(11) DEFAULT NULL UNIQUE COMMENT 'foreign key, linking this to the reserves table', + PRIMARY KEY (`hold_group_id`, `reserve_id`), CONSTRAINT `hold_group_target_holds_ibfk_1` FOREIGN KEY (`hold_group_id`) REFERENCES `hold_groups` (`hold_group_id`) ON DELETE CASCADE, CONSTRAINT `hold_group_target_holds_ibfk_2` FOREIGN KEY (`reserve_id`) REFERENCES `reserves` (`reserve_id`) ON DELETE CASCADE ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t index cb48d1331b..e1254bf5ba 100755 --- a/t/db_dependent/Koha/Hold.t +++ b/t/db_dependent/Koha/Hold.t @@ -34,7 +34,7 @@ use Koha::DateUtils qw(dt_from_string); use Koha::Holds; use Koha::Libraries; use Koha::Old::Holds; -use C4::Reserves qw( AddReserve ModReserveAffect ); +use C4::Reserves qw( AddReserve ModReserveAffect RevertWaitingStatus); use C4::Circulation qw( AddReturn ); my $schema = Koha::Database->new->schema; @@ -1389,7 +1389,7 @@ subtest 'is_hold_group_target, cleanup_hold_group and set_as_hold_group_target t }; subtest '_Findgroupreserve in the context of hold groups' => sub { - plan tests => 17; + plan tests => 19; $schema->storage->txn_begin; @@ -1490,6 +1490,13 @@ subtest '_Findgroupreserve in the context of hold groups' => sub { is( $hold->is_hold_group_target, 1, 'First hold is the hold group target' ); + C4::Reserves::RevertWaitingStatus( { itemnumber => $item->itemnumber } ); + is( $hold->is_hold_group_target, 0, 'First hold is no longer the hold group target' ); + is( $hold->hold_group->target_hold_id, undef, 'Hold group no longer has a target' ); + + AddReturn( $item->barcode, $library_1->{branchcode} ); + ModReserveAffect( $item->itemnumber, undef, $do_transfer, $reserve_id ); + my @reserves_with_target_hold = C4::Reserves::_Findgroupreserve( $item->biblionumber, $item->id, 0, [] ); is( -- 2.39.5