Bug 41956: Remove visual_hold_group_id

Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Pedro Amorim 2026-04-22 16:02:39 +00:00 committed by Lucas Gass
parent 8dab161af0
commit 5d716d1e28
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
8 changed files with 34 additions and 43 deletions

View File

@ -255,8 +255,8 @@ sub move_hold {
$self->cleanup_hold_group;
Dissociates the last hold from a group if it is the only one remaining and no history exists.
If no active holds remain, the group is deleted unless it has history.
(Koha::Old::Holds), in which case it is archived by clearing its visual ID.
If no active holds remain and no history exists, the group is deleted.
Groups with history are kept to preserve the hold_group_id reference in old_reserves.
=cut
@ -276,12 +276,8 @@ sub cleanup_hold_group {
$active_count = 0;
}
if ( $active_count == 0 ) {
if ( $history_count > 0 ) {
$hold_group->set( { visual_hold_group_id => undef } )->store;
} else {
$hold_group->delete;
}
if ( $active_count == 0 && $history_count == 0 ) {
$hold_group->delete;
}
}

View File

@ -1925,8 +1925,12 @@ Return all of this patron's active hold groups
sub hold_groups {
my ($self) = @_;
my $hold_group_rs = $self->_result->hold_groups->search(
{ visual_hold_group_id => { '!=' => undef } },
{ order_by => 'hold_group_id' }
{ 'reserves.hold_group_id' => { '!=' => undef } },
{
join => 'reserves',
distinct => 1,
order_by => 'me.hold_group_id',
}
);
return Koha::HoldGroups->_new_from_dbic($hold_group_rs);
}
@ -1986,12 +1990,6 @@ sub create_hold_group {
hold_ids => \@already_in_group_holds,
) if @already_in_group_holds && !$force_grouped;
my @existing_ids = grep { defined } $self->_result->hold_groups->get_column('visual_hold_group_id')->all;
my $next_available_visual_hold_group_id = 1;
while ( grep { $_ == $next_available_visual_hold_group_id } @existing_ids ) {
$next_available_visual_hold_group_id++;
}
my @holds_to_group = Koha::Holds->search( { reserve_id => { -in => $hold_ids } } )->as_list;
my %cleanup_map;
@ -2003,8 +2001,7 @@ sub create_hold_group {
my $hold_group = Koha::HoldGroup->new(
{
borrowernumber => $self->borrowernumber,
visual_hold_group_id => $next_available_visual_hold_group_id
borrowernumber => $self->borrowernumber,
}
);
$hold_group->store( { holds => \@holds_to_group } );

View File

@ -38,13 +38,6 @@ __PACKAGE__->table("hold_groups");
foreign key, linking this to the borrowers table
=head2 visual_hold_group_id
data_type: 'integer'
is_nullable: 1
visual ID for this hold group, in the context of the related patron
=cut
__PACKAGE__->add_columns(
@ -57,8 +50,6 @@ __PACKAGE__->add_columns(
},
"borrowernumber",
{ data_type => "integer", is_foreign_key => 1, is_nullable => 1 },
"visual_hold_group_id",
{ data_type => "integer", is_nullable => 1 },
);
=head1 PRIMARY KEY
@ -141,8 +132,8 @@ __PACKAGE__->has_many(
);
# Created by DBIx::Class::Schema::Loader v0.07051 @ 2025-11-03 19:54:07
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:o9/2a3gatQnmc7nTYRpUPQ
# Created by DBIx::Class::Schema::Loader v0.07051 @ 2026-04-22 16:00:06
# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:I9NhEMijQQcaKqEzwVoBzQ
# You can replace this text with custom code or comments, and it will be preserved on regeneration

View File

@ -7,9 +7,6 @@ properties:
borrowernumber:
type: integer
description: Patron this hold group belongs to
visual_hold_group_id:
type: integer
description: Visual ID for this hold group, in the context of the related patron
target_hold_id:
type: integer
description: Target hold ID of this hold group

View File

@ -0,0 +1,16 @@
use Modern::Perl;
use Koha::Installer::Output qw(say_warning say_success say_info);
return {
bug_number => "d41956",
description => "Remove visual_hold_group_id column from hold_groups",
up => sub {
my ($args) = @_;
my ( $dbh, $out ) = @$args{qw(dbh out)};
if ( column_exists( 'hold_groups', 'visual_hold_group_id' ) ) {
$dbh->do(q{ALTER TABLE hold_groups DROP COLUMN visual_hold_group_id});
say_success( $out, "Removed column 'visual_hold_group_id' from hold_groups" );
}
},
};

View File

@ -3562,7 +3562,6 @@ DROP TABLE IF EXISTS `hold_groups`;
CREATE TABLE `hold_groups` (
`hold_group_id` int(10) unsigned NOT NULL AUTO_INCREMENT,
`borrowernumber` int(11) DEFAULT NULL COMMENT 'foreign key, linking this to the borrowers table',
`visual_hold_group_id` int(11) DEFAULT NULL COMMENT 'visual ID for this hold group, in the context of the related patron',
PRIMARY KEY (`hold_group_id`),
KEY `hold_groups_borrowernumber` (`borrowernumber`),
CONSTRAINT `hold_groups_ibfk_1` FOREIGN KEY (`borrowernumber`) REFERENCES `borrowers` (`borrowernumber`) ON DELETE CASCADE

View File

@ -114,7 +114,6 @@ while ( my $h = $holds_rs->next() ) {
reservenotes => $h->reservenotes(),
itemtype_limit => $itemtype_limit,
hold_group_id => $h->hold_group_id,
visual_hold_group_id => $h->hold_group ? $h->hold_group->visual_hold_group_id : q{},
is_hold_group_target => $h->is_hold_group_target,
reservedate_formatted => $h->reservedate()
? output_pref( { dt => dt_from_string( $h->reservedate() ), dateonly => 1 } )

View File

@ -3586,9 +3586,9 @@ subtest 'reset_2fa() tests' => sub {
$schema->storage->txn_rollback;
};
subtest "create_hold_group, hold_groups, visual_hold_group_id tests" => sub {
subtest "create_hold_group, hold_groups tests" => sub {
plan tests => 19;
plan tests => 16;
$schema->storage->txn_begin;
@ -3640,17 +3640,15 @@ subtest "create_hold_group, hold_groups, visual_hold_group_id tests" => sub {
is( $patron_hold_groups->count, 1, 'Patron has one hold group' );
my $hold_group = $patron->hold_groups->as_list->[0];
is( $hold_group->visual_hold_group_id, 1, 'Visual hold group id is 1' );
is( $hold_group->hold_group_id, $hold1->get_from_storage->hold_group_id, 'hold1 added to hold_group' );
is( $hold_group->hold_group_id, $hold2->get_from_storage->hold_group_id, 'hold2 added to hold_group' );
is( $hold_group->hold_group_id, $hold3->get_from_storage->hold_group_id, 'hold3 added to hold_group' );
is( $hold_group->hold_group_id, $hold1->get_from_storage->hold_group_id, 'hold1 added to hold_group' );
is( $hold_group->hold_group_id, $hold2->get_from_storage->hold_group_id, 'hold2 added to hold_group' );
is( $hold_group->hold_group_id, $hold3->get_from_storage->hold_group_id, 'hold3 added to hold_group' );
# Create 2nd hold group
my $hg_to_be_kept = $patron->create_hold_group( [ $hold4->reserve_id, $hold5->reserve_id ] );
is( $patron_hold_groups->count, 2, 'Patron has two hold groups' );
my $second_hold_group = $patron->hold_groups->as_list->[1];
is( $second_hold_group->visual_hold_group_id, 2, 'Visual hold group id is 2' );
is(
$second_hold_group->hold_group_id, $hold4->get_from_storage->hold_group_id,
'hold4 added to second hold_group'
@ -3676,8 +3674,6 @@ subtest "create_hold_group, hold_groups, visual_hold_group_id tests" => sub {
# Create 3rd hold group
$patron->create_hold_group( [ $hold5->reserve_id, $hold6->reserve_id ], 1 );
my $third_hold_group = $patron->hold_groups->as_list->[0];
is( $third_hold_group->visual_hold_group_id, 1, 'Visual hold group id is 1' );
t::lib::Mocks::mock_preference( 'HoldsLog', 1 );
my $new_hg = $patron->create_hold_group( [ $hold6->reserve_id, $hold5->reserve_id ], 1 );