From f34501305d1d9020165fdbf0ed22179e5619dac7 Mon Sep 17 00:00:00 2001 From: Lucas Gass Date: Mon, 5 May 2025 22:16:02 +0000 Subject: [PATCH] Bug 31698: Disallow moving waiting or intransit holds Signed-off-by: kristi --- Koha/Hold.pm | 71 +++++++++++++++++++ .../prog/en/includes/holds_table.inc | 12 +++- .../prog/en/modules/reserve/request.tt | 13 ++-- t/db_dependent/Koha/Hold.t | 30 +++++++- 4 files changed, 119 insertions(+), 7 deletions(-) diff --git a/Koha/Hold.pm b/Koha/Hold.pm index c44d46a56a9..7b8681e2fee 100644 --- a/Koha/Hold.pm +++ b/Koha/Hold.pm @@ -178,6 +178,77 @@ sub delete { return $deleted; } +=head3 move_hold + +$hold->move_hold(); + +=cut + +sub move_hold { + my ( $self, $args ) = @_; + + my $original = $self; + my $original_biblionumber = $self->biblionumber; + + my $found = $original->found // ''; + + if ( $found eq 'W' ) { + return { success => 0, error => 'Cannot move a waiting hold' }; + } + + if ( $found eq 'T' ) { + return { success => 0, error => 'Cannot move a hold in transit' }; + } + + my $patron = Koha::Patrons->find( { borrowernumber => $self->borrowernumber } ); + return { success => 0, error => 'Missing patron or target' } unless $patron; + + my ( $new_biblionumber, $new_itemnumber, $item, $canReserve ); + + if ( $args->{new_itemnumber} ) { + $item = Koha::Items->find( { itemnumber => $args->{new_itemnumber} } ) + or return { success => 0, error => 'Target item not found' }; + + $new_itemnumber = $item->itemnumber; + $new_biblionumber = $item->biblionumber; + + $canReserve = C4::Reserves::CanItemBeReserved( $patron, $item, $self->branchcode, { ignore_hold_counts => 1 } ); + } elsif ( $args->{new_biblionumber} ) { + $new_biblionumber = $args->{new_biblionumber}; + + $canReserve = C4::Reserves::CanBookBeReserved( + $patron->borrowernumber, $new_biblionumber, $self->branchcode, + { ignore_hold_counts => 1 } + ); + } else { + return { success => 0, error => 'Missing itemnumber or biblionumber' }; + } + + return { success => 0, error => $canReserve->{status} } + unless $canReserve->{status} eq 'OK'; + + my $max = Koha::Holds->search( + { biblionumber => $new_biblionumber }, + { order_by => { -desc => 'priority' }, rows => 1 } + )->next; + my $base_priority = $max ? $max->priority : 0; + my $priority = $base_priority + 1; + my $new_priority = C4::Reserves::_ShiftPriority( $new_biblionumber, $priority ); + + $self->update( + { + itemnumber => $new_itemnumber, # undef for biblio-level is fine + biblionumber => $new_biblionumber, + priority => $new_priority, + } + ); + + C4::Log::logaction( 'HOLDS', 'MODIFY', $self->reserve_id, $self, undef, $original ) + if C4::Context->preference('HoldsLog'); + + return { success => 1, hold => $self }; +} + =head3 set_transfer =cut diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc index 70ff63bf366..1515ea767c4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/holds_table.inc @@ -46,7 +46,17 @@ [%- END -%] [% SET tr_class = hold.suspend ? 'suspend' : '' %] - + diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt index 45682e1c7b1..d2279de70d4 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt @@ -2055,9 +2055,11 @@ let reserve_biblionumber = $(this).data('biblionumber'); let reserve_itemnumber = $(this).data('itemnumber'); let item_level_hold = $(this).data('item_level_hold'); - let error_message = $(this).data('item_level_hold') ? "" : _("Cannot move a waiting or record level hold"); + let item_waiting = $(this).data('waiting'); + let item_intransit = $(this).data('intransit'); + let error_message = $(this).data('item_level_hold') ? "" : _("Cannot move a waiting, in transit, or record level hold"); let found_status = $(this).data('found'); - if ( item_level_hold ) { + if ( item_level_hold && ( !item_waiting || !item_intransit ) ) { $('#move_hold_item_selection table').append(`${reserve_id}Biblionumber: ${reserve_biblionumber} Itemnumber: ${reserve_itemnumber}${error_message}`) } else { $('#move_hold_item_selection table').append(`${reserve_id}Biblionumber: ${reserve_biblionumber} Itemnumber: ${reserve_itemnumber}${error_message}`) @@ -2078,9 +2080,12 @@ let reserve_biblionumber = $(this).data('biblionumber'); let reserve_itemnumber = $(this).data('itemnumber'); let item_level_hold = $(this).data('item_level_hold'); - let error_message = $(this).data('item_level_hold') ? _("Cannot move a waiting or item level hold") : ""; + let item_status = $(this).data('status'); + let item_waiting = $(this).data('waiting'); + let item_intransit = $(this).data('intransit'); + let error_message = $(this).data('item_level_hold') ? _("Cannot move a waiting, in transit, or item level hold") : ""; let found_status = $(this).data('found'); - if ( !item_level_hold ) { + if ( !item_level_hold && ( !item_waiting || !item_intransit ) ) { $('#move_hold_biblio_selection table').append(`${reserve_id}Biblionumber: ${reserve_biblionumber}${error_message}`) } else { $('#move_hold_biblio_selection table').append(`${reserve_id}Biblionumber: ${reserve_biblionumber}${error_message}`) diff --git a/t/db_dependent/Koha/Hold.t b/t/db_dependent/Koha/Hold.t index 96aa4ee940a..e3d5ad31d58 100755 --- a/t/db_dependent/Koha/Hold.t +++ b/t/db_dependent/Koha/Hold.t @@ -1495,7 +1495,7 @@ subtest 'revert_waiting() tests' => sub { }; }; subtest 'move_hold() tests' => sub { - plan tests => 11; + plan tests => 13; $schema->storage->txn_begin; my $patron = Koha::Patron->new( @@ -1638,7 +1638,33 @@ subtest 'move_hold() tests' => sub { is( $logs_2->count, 1, 'Hold modification was logged' ); #disable HoldsLog - t::lib::Mocks::mock_preference( 'HoldsLog', 1 ); + t::lib::Mocks::mock_preference( 'HoldsLog', 0 ); + + my $hold_6 = Koha::Hold->new( + { + borrowernumber => $patron_4->borrowernumber, + biblionumber => $biblio4->biblionumber, + itemnumber => $item_4->itemnumber, + branchcode => 'CPL', + found => 'W', + } + )->store; + + my $result_4 = $hold_6->move_hold( { new_biblionumber => $biblio1->biblionumber } ); + is( $result_4->{error}, 'Cannot move a waiting hold', 'Correct error for waiting hold' ); + + my $hold_7 = Koha::Hold->new( + { + borrowernumber => $patron_4->borrowernumber, + biblionumber => $biblio4->biblionumber, + itemnumber => $item_4->itemnumber, + branchcode => 'CPL', + found => 'T', + } + )->store; + + my $result_5 = $hold_7->move_hold( { new_biblionumber => $biblio1->biblionumber } ); + is( $result_5->{error}, 'Cannot move a hold in transit', 'Correct error for hold instransit' ); $schema->storage->txn_rollback; }; -- 2.39.5