From 39c2d10fe26ed751972b98520bad0e4ef53a6ca3 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 7 Nov 2025 12:43:44 +0000 Subject: [PATCH] Bug 19871: Make exception handling more explicit in add_to_bundle Replace the implicit exception translation call with an explicit safe_do pattern that clearly shows exceptions are rethrown. Changes: - Use $schema->safe_do() wrapper for automatic exception translation - Add explicit $_->rethrow() in catch block for clarity - Follow the same pattern used in Koha::Patron::Modification::approve() - Make control flow more obvious to future maintainers This change maintains identical behavior while improving code clarity by making it explicit that the catch block never returns normally. --- Koha/Item.pm | 72 +++++++++++++++++++++++++++------------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 229ff6695a9..acc9deeeaf7 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -2293,49 +2293,53 @@ sub add_to_bundle { my $BundleNotLoanValue = C4::Context->preference('BundleNotLoanValue'); - try { - $schema->txn_do( - sub { + $schema->txn_do( + sub { + try { + $schema->safe_do( + sub { - Koha::Exceptions::Item::Bundle::BundleIsCheckedOut->throw if $self->checkout; + Koha::Exceptions::Item::Bundle::BundleIsCheckedOut->throw if $self->checkout; - my $checkout = $bundle_item->checkout; - if ($checkout) { - unless ( $options->{force_checkin} ) { - Koha::Exceptions::Item::Bundle::ItemIsCheckedOut->throw(); - } + my $checkout = $bundle_item->checkout; + if ($checkout) { + unless ( $options->{force_checkin} ) { + Koha::Exceptions::Item::Bundle::ItemIsCheckedOut->throw(); + } - my $branchcode = C4::Context->userenv->{'branch'}; - my ($success) = C4::Circulation::AddReturn( $bundle_item->barcode, $branchcode ); + my $branchcode = C4::Context->userenv->{'branch'}; + my ($success) = C4::Circulation::AddReturn( $bundle_item->barcode, $branchcode ); - if ($success) { + if ($success) { - # HoldsQueue doesn't seem to mind bundles, not sure if this is correct, but rebuild for now - Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( - { biblio_ids => [ $self->biblionumber ] } ) - if C4::Context->preference('RealTimeHoldsQueue'); - } else { - Koha::Exceptions::Checkin::FailedCheckin->throw(); - } - } + # HoldsQueue doesn't seem to mind bundles, not sure if this is correct, but rebuild for now + Koha::BackgroundJob::BatchUpdateBiblioHoldsQueue->new->enqueue( + { biblio_ids => [ $self->biblionumber ] } ) + if C4::Context->preference('RealTimeHoldsQueue'); + } else { + Koha::Exceptions::Checkin::FailedCheckin->throw(); + } + } - my $holds = $bundle_item->current_holds; - if ( $holds->count ) { - unless ( $options->{ignore_holds} ) { - Koha::Exceptions::Item::Bundle::ItemHasHolds->throw(); - } - } + my $holds = $bundle_item->current_holds; + if ( $holds->count ) { + unless ( $options->{ignore_holds} ) { + Koha::Exceptions::Item::Bundle::ItemHasHolds->throw(); + } + } - $self->_result->add_to_item_bundles_hosts( { item => $bundle_item->itemnumber } ); + $self->_result->add_to_item_bundles_hosts( { item => $bundle_item->itemnumber } ); - $bundle_item->notforloan($BundleNotLoanValue)->store(); - } - ); - } catch { + $bundle_item->notforloan($BundleNotLoanValue)->store(); + } + ); + } catch { - # Use centralized exception translation instead of duplicated code - $schema->translate_exception($_); - }; + # Propagate any exception (already translated by safe_do if needed) + $_->rethrow(); + }; + } + ); } =head3 remove_from_bundle -- 2.51.1