Bugzilla – Attachment 184641 Details for
Bug 40517
Allow grouping existing holds
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 40517: Move creation of hold group into Patron class. Add exceptions
Bug-40517-Move-creation-of-hold-group-into-Patron-.patch (text/plain), 10.23 KB, created by
Pedro Amorim
on 2025-07-25 14:38:31 UTC
(
hide
)
Description:
Bug 40517: Move creation of hold group into Patron class. Add exceptions
Filename:
MIME Type:
Creator:
Pedro Amorim
Created:
2025-07-25 14:38:31 UTC
Size:
10.23 KB
patch
obsolete
>From bd6dc389042c5221dbfac32dba1a6bf707a460a0 Mon Sep 17 00:00:00 2001 >From: Pedro Amorim <pedro.amorim@openfifth.co.uk> >Date: Wed, 16 Jul 2025 12:52:39 +0000 >Subject: [PATCH] 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 >--- > Koha/Exceptions/HoldGroup.pm | 64 ++++++++++++++++++++++++++++++++++ > Koha/Patron.pm | 67 ++++++++++++++++++++++++++++++++++++ > opac/opac-reserve.pl | 13 +++---- > reserve/placerequest.pl | 15 ++++---- > 4 files changed, 144 insertions(+), 15 deletions(-) > create mode 100644 Koha/Exceptions/HoldGroup.pm > >diff --git a/Koha/Exceptions/HoldGroup.pm b/Koha/Exceptions/HoldGroup.pm >new file mode 100644 >index 00000000000..8ceea1a8e70 >--- /dev/null >+++ b/Koha/Exceptions/HoldGroup.pm >@@ -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; >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index dda09bff4a2..1f70c473e9c 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -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; >@@ -1843,6 +1844,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; >diff --git a/opac/opac-reserve.pl b/opac/opac-reserve.pl >index 0ba22c25f32..a00ad63fa61 100755 >--- a/opac/opac-reserve.pl >+++ b/opac/opac-reserve.pl >@@ -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" ); >diff --git a/reserve/placerequest.pl b/reserve/placerequest.pl >index 00e3e3a64b7..cd096b80bfa 100755 >--- a/reserve/placerequest.pl >+++ b/reserve/placerequest.pl >@@ -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 = (); > >-- >2.39.5
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40517
:
184640
| 184641 |
184642
|
184643
|
184644
|
184645
|
184646