From 3b0eda0b975792c119a6852fd67c46f06a56d840 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Fri, 5 Jan 2024 16:08:44 +0000 Subject: [PATCH] Bug 35100: Various fixes 1) Don't automagically always set a transfer to in transit on checkin.. wait for the user to actually confirm that's the case 2) New transfers triggered by a hold should take precidence, so hide transfers for any other reason from display 3) Update get_transfer and get_transfers to ensure ordering isn't lost when prefetch is used and add tests for this Signed-off-by: Nick Clemens Signed-off-by: Martin Renvoize --- C4/Circulation.pm | 31 ++- C4/Items.pm | 2 +- Koha/Item.pm | 16 +- Koha/Schema/Result/Item.pm | 3 +- circ/returns.pl | 4 + .../prog/en/includes/item-status.inc | 4 +- .../prog/en/modules/circ/returns.tt | 5 +- t/db_dependent/Circulation.t | 35 ++- t/db_dependent/Koha/Item.t | 251 +++++++++++++++++- 9 files changed, 315 insertions(+), 36 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 70ccd7fb54d..1b6459b180e 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -2368,16 +2368,15 @@ sub AddReturn { if ( $transfer->tobranch eq $branch ) { $transfer->receive; $messages->{'TransferArrived'} = $transfer->frombranch; + # validTransfer=1 allows us returning the item back if the reserve is cancelled $validTransfer = 1 if $transfer->reason eq 'Reserve'; - } - else { + } else { $messages->{'TransferTrigger'} = $transfer->reason; if ( $transfer->frombranch eq $branch ) { - $transfer->transit; - $messages->{'WasTransfered'} = $transfer->tobranch; - } - else { + $messages->{'TransferTo'} = $transfer->tobranch; + $messages->{'WasTransfered'} = $transfer->id; + } else { $messages->{'WrongTransfer'} = $transfer->tobranch; $messages->{'WrongTransferItem'} = $item->itemnumber; } @@ -2527,15 +2526,21 @@ sub AddReturn { and not $messages->{RecallNeedsTransfer} ) { my $BranchTransferLimitsType = C4::Context->preference("BranchTransferLimitsType") eq 'itemtype' ? 'effective_itemtype' : 'ccode'; - if (C4::Context->preference("AutomaticItemReturn" ) or - (C4::Context->preference("UseBranchTransferLimits") and - ! IsBranchTransferAllowed($branch, $returnbranch, $item->$BranchTransferLimitsType ) - )) { - ModItemTransfer($item->itemnumber, $branch, $returnbranch, $transfer_trigger, { skip_record_index => 1 }); - $messages->{'WasTransfered'} = $returnbranch; + if ( + C4::Context->preference("AutomaticItemReturn") + or ( C4::Context->preference("UseBranchTransferLimits") + and !IsBranchTransferAllowed( $branch, $returnbranch, $item->$BranchTransferLimitsType ) ) + ) + { + my $transfer = ModItemTransfer( + $item->itemnumber, $branch, $returnbranch, $transfer_trigger, + { skip_record_index => 1 } + ); + $messages->{'TransferTo'} = $returnbranch; + $messages->{'WasTransfered'} = $transfer->id; $messages->{'TransferTrigger'} = $transfer_trigger; } else { - $messages->{'NeedsTransfer'} = $returnbranch; + $messages->{'NeedsTransfer'} = $returnbranch; $messages->{'TransferTrigger'} = $transfer_trigger; } } diff --git a/C4/Items.pm b/C4/Items.pm index 3041d8f13e7..174e7efa5c8 100644 --- a/C4/Items.pm +++ b/C4/Items.pm @@ -386,7 +386,7 @@ sub ModItemTransfer { $transfer->transit({ skip_record_index => $params->{skip_record_index} }); } - return; + return $transfer; } =head2 ModDateLastSeen diff --git a/Koha/Item.pm b/Koha/Item.pm index ebfc05a1026..ac6cd1f16c1 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -799,8 +799,13 @@ we still expect the item to end up at a final location eventually. sub get_transfer { my ($self) = @_; - my $transfer = $self->_result->current_branchtransfers->next; - return Koha::Item::Transfer->_new_from_dbic($transfer) if $transfer; + my $transfers = $self->_result->current_branchtransfers->search( + undef, + { 'order_by' => [ { '-desc' => 'datesent' }, { '-asc' => 'daterequested' } ] } + ); + + my $transfer = $transfers->next; + return Koha::Item::Transfer->_new_from_dbic($transfer) if $transfer; } =head3 transfer @@ -844,9 +849,12 @@ we still expect the item to end up at a final location eventually. sub get_transfers { my ($self) = @_; - my $transfer_rs = $self->_result->current_branchtransfers; + my $transfers_rs = $self->_result->current_branchtransfers->search( + undef, + { 'order_by' => [ { '-desc' => 'datesent' }, { '-asc' => 'daterequested' } ] } + ); - return Koha::Item::Transfers->_new_from_dbic($transfer_rs); + return Koha::Item::Transfers->_new_from_dbic($transfers_rs); } =head3 last_returned_by diff --git a/Koha/Schema/Result/Item.pm b/Koha/Schema/Result/Item.pm index dcbd2275a82..f6e9972ba3a 100644 --- a/Koha/Schema/Result/Item.pm +++ b/Koha/Schema/Result/Item.pm @@ -1057,8 +1057,7 @@ __PACKAGE__->has_many( "$args->{foreign_alias}.datearrived" => undef, "$args->{foreign_alias}.datecancelled" => undef, }; - }, - { order_by => [ { -desc => 'datesent' }, { -asc => 'daterequested' } ] } + } ); # Relationship with bundled items diff --git a/circ/returns.pl b/circ/returns.pl index 72e004a7957..c84fd048696 100755 --- a/circ/returns.pl +++ b/circ/returns.pl @@ -542,6 +542,7 @@ if ( $messages->{'WasTransfered'} ) { $template->param( found => 1, transfer => $messages->{'WasTransfered'}, + transferto => $messages->{'TransferTo'}, trigger => $messages->{'TransferTrigger'}, itemnumber => $itemnumber, ); @@ -720,6 +721,9 @@ foreach my $code ( keys %$messages ) { elsif ( $code eq 'WasTransfered' ) { ; # FIXME... anything to do here? } + elsif ( $code eq 'TransferTo' ) { + ; # Handled above, along with WasTransfered + } elsif ( $code eq 'withdrawn' ) { $err{withdrawn} = 1; } diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/item-status.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/item-status.inc index 11adad376f3..3ea1ed7c719 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/item-status.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/item-status.inc @@ -43,9 +43,9 @@ [% END %] [% END %] -[% IF ( item.notforloan || item.effective_itemtype.notforloan ) %] +[% IF ( item.is_notforloan ) %] [% SET itemavailable = 0 %] - [% SET notforloan = item.notforloan || item.effective_itemtype.notforloan %] + [% SET notforloan = item.is_notforloan %] [% notforloan_description = AuthorisedValues.GetDescriptionByKohaField( kohafield => 'items.notforloan', authorised_value => notforloan ) %] [% IF notforloan_description %] [% notforloan_description | html %] diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt index 7091e5ef8e7..e282dbd2d26 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/returns.tt @@ -122,7 +122,7 @@ [% END # /IF hold_auto_filled %] - [% IF ( trigger ) %] + [% IF ( trigger && !transfertodo ) %]

Reason for transfer

@@ -754,7 +754,7 @@