Bug 40517: Move creation of hold group into Patron class. Add exceptions

Refactor: Update controller logic to make use of patron->create_hold_group

This ensures consistency when creating a new hold group for a patron,
regardless if on the Staff UI or REST API

Signed-off-by: Anneli <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Pedro Amorim 2025-07-16 12:52:39 +00:00 committed by Lucas Gass
parent 9690b4b95e
commit 07275db04f
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
4 changed files with 144 additions and 15 deletions

View File

@ -0,0 +1,64 @@
package Koha::Exceptions::HoldGroup;
# This file is part of Koha.
#
# Koha is free software; you can redistribute it and/or modify it
# under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 3 of the License, or
# (at your option) any later version.
#
# Koha is distributed in the hope that it will be useful, but
# WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
# GNU General Public License for more details.
#
# You should have received a copy of the GNU General Public License
# along with Koha; if not, see <http://www.gnu.org/licenses>.
use Modern::Perl;
use Koha::Exception;
use Exception::Class (
'Koha::Exceptions::HoldGroup' => {
isa => 'Koha::Exception',
},
'Koha::Exceptions::HoldGroup::HoldDoesNotExist' => {
isa => 'Koha::Exceptions::HoldGroup',
description => "One or more holds do not exist",
fields => ['hold_ids'],
},
'Koha::Exceptions::HoldGroup::HoldDoesNotBelongToPatron' => {
isa => 'Koha::Exceptions::HoldGroup',
description => "One or more holds do not belong to patron",
fields => ['hold_ids'],
},
'Koha::Exceptions::HoldGroup::HoldHasAlreadyBeenFound' => {
isa => 'Koha::Exceptions::HoldGroup',
description => "One or more holds have already been found",
fields => ['barcodes'],
},
'Koha::Exceptions::HoldGroup::HoldAlreadyBelongsToHoldGroup' => {
isa => 'Koha::Exceptions::HoldGroup',
description => "One or more holds already belong to a hold group",
fields => ['hold_ids'],
},
);
=head1 NAME
Koha::Exceptions::HoldGroup - Base class for HoldGroup exceptions
=head1 Exceptions
=head2 Koha::Exceptions::HoldGroup
Generic HoldGroup exception
=head2 Koha::Exceptions::HoldGroup::HoldDoesNotExist
Exception to be used when one or more provided holds do not exist.
=cut
1;

View File

@ -45,6 +45,7 @@ use Koha::DateUtils qw( dt_from_string );
use Koha::Encryption;
use Koha::Exceptions;
use Koha::Exceptions::Password;
use Koha::Exceptions::HoldGroup;
use Koha::Holds;
use Koha::HoldGroups;
use Koha::ILL::Requests;
@ -1903,6 +1904,72 @@ sub hold_groups {
return Koha::HoldGroups->_new_from_dbic($hold_group_rs);
}
=head3 create_hold_group
my $hold_group = $patron->create_hold_group
Creates and returns a hold group given a list of hold ids
If force_grouped is supplied, the hold group will be created even if the holds are already grouped
=cut
sub create_hold_group {
my ( $self, $hold_ids, $force_grouped ) = @_;
my @undef_holds;
foreach my $hold_id (@$hold_ids) {
my $hold = Koha::Holds->find($hold_id);
push @undef_holds, $hold_id unless $hold;
}
Koha::Exceptions::HoldGroup::HoldDoesNotExist->throw(
hold_ids => \@undef_holds,
) if (@undef_holds);
my @not_own_holds;
foreach my $hold_id (@$hold_ids) {
my $hold = Koha::Holds->find($hold_id);
push @not_own_holds, $hold_id unless $hold->borrowernumber eq $self->borrowernumber;
}
Koha::Exceptions::HoldGroup::HoldDoesNotBelongToPatron->throw(
hold_ids => \@not_own_holds,
) if (@not_own_holds);
my @already_found_holds;
foreach my $hold_id (@$hold_ids) {
my $hold = Koha::Holds->find($hold_id);
push @already_found_holds, $hold->item->barcode if $hold->found;
}
Koha::Exceptions::HoldGroup::HoldHasAlreadyBeenFound->throw(
barcodes => \@already_found_holds,
) if (@already_found_holds);
my @already_in_group_holds;
foreach my $hold_id (@$hold_ids) {
my $hold = Koha::Holds->find($hold_id);
push @already_in_group_holds, $hold_id if $hold->hold_group_id;
}
Koha::Exceptions::HoldGroup::HoldAlreadyBelongsToHoldGroup->throw(
hold_ids => \@already_in_group_holds,
) if @already_in_group_holds && !$force_grouped;
my $hold_group_rs = $self->_result->create_related(
'hold_groups',
{}
);
foreach my $hold_id (@$hold_ids) {
my $hold = Koha::Holds->find($hold_id);
$hold->hold_group_id( $hold_group_rs->hold_group_id )->store;
}
return Koha::HoldGroup->_new_from_dbic($hold_group_rs);
}
=head3 curbside_pickups
my $curbside_pickups = $patron->curbside_pickups;

View File

@ -195,7 +195,7 @@ if ( $op eq 'cud-place_reserve' ) {
}
my @failed_holds;
my $hold_group;
my @successful_hold_ids;
while (@selectedItems) {
my $biblioNum = shift(@selectedItems);
my $itemNum = shift(@selectedItems);
@ -292,13 +292,6 @@ if ( $op eq 'cud-place_reserve' ) {
# Here we actually do the reserveration. Stage 3.
if ($canreserve) {
if ($add_to_hold_group) {
#NOTE: We wait to create a hold group until we know that we can actually place a hold/reserve
if ( !$hold_group ) {
$hold_group = Koha::HoldGroup->new->store;
}
}
my $reserve_id = AddReserve(
{
branchcode => $branch,
@ -313,17 +306,19 @@ if ( $op eq 'cud-place_reserve' ) {
found => undef,
itemtype => $itemtype,
item_group_id => $item_group_id,
hold_group_id => $hold_group ? $hold_group->id : undef,
}
);
if ($reserve_id) {
++$reserve_cnt;
push @successful_hold_ids, $reserve_id;
} else {
push @failed_holds, 'not_placed';
}
}
}
$patron->create_hold_group( \@successful_hold_ids ) if $add_to_hold_group;
print $query->redirect( "/cgi-bin/koha/opac-user.pl?"
. ( @failed_holds ? "failed_holds=" . join( '|', @failed_holds ) : q|| )
. "&opac-user-holds=1" );

View File

@ -69,9 +69,7 @@ foreach my $bibnum (@holdable_bibs) {
if ( $op eq 'cud-placerequest' && $patron ) {
my %failed_holds;
if ($hold_group_param) {
$hold_group = Koha::HoldGroup->new->store;
}
my @successful_hold_ids;
foreach my $biblionumber ( keys %bibinfos ) {
@ -95,7 +93,7 @@ if ( $op eq 'cud-placerequest' && $patron ) {
if ( $can_item_be_reserved eq 'OK'
|| ( $can_item_be_reserved ne 'itemAlreadyOnHold' && $can_override ) )
{
AddReserve(
my $reserve_id = AddReserve(
{
branchcode => $item_pickup_location,
borrowernumber => $patron->borrowernumber,
@ -113,6 +111,7 @@ if ( $op eq 'cud-placerequest' && $patron ) {
}
);
push @successful_hold_ids, $reserve_id;
$hold_priority++;
} else {
@ -124,7 +123,7 @@ if ( $op eq 'cud-placerequest' && $patron ) {
} elsif ( @biblionumbers > 1 || $multi_holds ) {
my $bibinfo = $bibinfos{$biblionumber};
if ( $can_override || CanBookBeReserved( $patron->borrowernumber, $biblionumber )->{status} eq 'OK' ) {
AddReserve(
my $reserve_id = AddReserve(
{
branchcode => $bibinfo->{pickup},
borrowernumber => $patron->borrowernumber,
@ -141,13 +140,14 @@ if ( $op eq 'cud-placerequest' && $patron ) {
hold_group_id => $hold_group ? $hold_group->id : undef,
}
);
push @successful_hold_ids, $reserve_id;
}
} else {
# place a request on 1st available
for ( my $i = 0 ; $i < $holds_to_place_count ; $i++ ) {
if ( $can_override || CanBookBeReserved( $patron->borrowernumber, $biblionumber )->{status} eq 'OK' ) {
AddReserve(
my $reserve_id = AddReserve(
{
branchcode => $branch,
borrowernumber => $patron->borrowernumber,
@ -165,11 +165,14 @@ if ( $op eq 'cud-placerequest' && $patron ) {
hold_group_id => $hold_group ? $hold_group->id : undef,
}
);
push @successful_hold_ids, $reserve_id;
}
}
}
}
$patron->create_hold_group( \@successful_hold_ids ) if $hold_group_param;
my $redirect_url = URI->new("request.pl");
my @failed_hold_msgs = ();