From 3c9ada510d23d14b4675c95d479a0c3c3362d5d3 Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Mon, 8 Mar 2021 13:43:11 +0000 Subject: [PATCH] Bug 27281: Update LostItem to use Koha::Item[::Transfer] methods This patch updates C4::Circulation::LostItem to use the Koha::Item and Koha::Item::Transfer methods to cancel transfers when an item is marked as lost. Test plan 1/ Confirm t/db_dependant/Circulation.t passes prior to applying the patches 2/ Apply the patch and run updatedatabase.pl 3/ Confirm that t/db_dependant/Circulation.t still passes 4/ Signoff --- C4/Circulation.pm | 12 ++++--- Koha/Item.pm | 32 +++++++++++++++++++ Koha/Schema/Result/Branchtransfer.pm | 7 ++-- .../data/mysql/atomicupdate/bug_27281.perl | 28 ++++++++++++++++ installer/data/mysql/kohastructure.sql | 2 +- 5 files changed, 73 insertions(+), 8 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug_27281.perl diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 03fe5c78f2..be8ebd8ac8 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -3912,11 +3912,15 @@ sub LostItem{ MarkIssueReturned($borrowernumber,$itemnumber,undef,$patron->privacy) if $mark_returned; } - #When item is marked lost automatically cancel its outstanding transfers and set items holdingbranch to the transfer source branch (frombranch) - if (my ( $datesent,$frombranch,$tobranch ) = GetTransfers($itemnumber)) { - Koha::Items->find($itemnumber)->holdingbranch($frombranch)->store({ skip_record_index => $params->{skip_record_index} }); + # When an item is marked as lost, we should automatically cancel its outstanding transfers + # and set items holdingbranch to the transfer source branch (frombranch). + my $item = Koha::Items->find($itemnumber); + my $transfers = $item->get_transfers; + while (my $transfer = $transfers->next) { + $item->holdingbranch($transfer->frombranch) if $transfer->in_transit; + $transfer->cancel({ reason => 'ItemLost', force => 1 }); } - my $transferdeleted = DeleteTransfer($itemnumber); + $item->store({skip_record_index => $params->{skip_record_index}}); } sub GetOfflineOperations { diff --git a/Koha/Item.pm b/Koha/Item.pm index 7bbc87b95b..6b551917ec 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -497,6 +497,38 @@ sub get_transfer { return Koha::Item::Transfer->_new_from_dbic($transfer_rs); } +=head3 get_transfers + + my $transfer = $item->get_transfers; + +Return the list of outstanding transfers (i.e requested but not yet cancelled +or recieved). + +Note: Transfers are retrieved in a Modified FIFO (First In First Out) order +whereby the most recently sent, but not received, transfer will be returned +first if it exists, otherwise requests are in oldest to newest request order. + +This allows for transfers to queue, which is the case for stock rotation and +rotating collections where a manual transfer may need to take precedence but +we still expect the item to end up at a final location eventually. + +=cut + +sub get_transfers { + my ($self) = @_; + my $transfer_rs = $self->_result->branchtransfers->search( + { + datearrived => undef, + datecancelled => undef + }, + { + order_by => + [ { -desc => 'datesent' }, { -asc => 'daterequested' } ], + } + ); + return Koha::Item::Transfers->_new_from_dbic($transfer_rs); +} + =head3 last_returned_by Gets and sets the last borrower to return an item. diff --git a/Koha/Schema/Result/Branchtransfer.pm b/Koha/Schema/Result/Branchtransfer.pm index b4bf37b55c..2994efd70c 100644 --- a/Koha/Schema/Result/Branchtransfer.pm +++ b/Koha/Schema/Result/Branchtransfer.pm @@ -111,7 +111,7 @@ what triggered the transfer =head2 cancellation_reason data_type: 'enum' - extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve"]} + extra: {list => ["Manual","StockrotationAdvance","StockrotationRepatriation","ReturnToHome","ReturnToHolding","RotatingCollection","Reserve","LostReserve","CancelReserve","ItemLost"]} is_nullable: 1 what triggered the transfer cancellation @@ -203,6 +203,7 @@ __PACKAGE__->add_columns( "Reserve", "LostReserve", "CancelReserve", + "ItemLost", ], }, is_nullable => 1, @@ -269,8 +270,8 @@ __PACKAGE__->belongs_to( ); -# Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-03-03 13:47:35 -# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:pL0dO9OUwImy2rv7Md3AyA +# Created by DBIx::Class::Schema::Loader v0.07046 @ 2021-03-08 13:36:32 +# DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:HU4Pt8CfD6lX87BjK1g3Bw sub koha_object_class { 'Koha::Item::Transfer'; diff --git a/installer/data/mysql/atomicupdate/bug_27281.perl b/installer/data/mysql/atomicupdate/bug_27281.perl new file mode 100644 index 0000000000..0d18397a99 --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_27281.perl @@ -0,0 +1,28 @@ +$DBversion = 'XXX'; # will be replaced by the RM +if( CheckVersion( $DBversion ) ) { + + # Add 'LostItem' to reserves cancellation_reason enum + $dbh->do( + qq{ + ALTER TABLE + `branchtransfers` + MODIFY COLUMN + `cancellation_reason` enum( + 'Manual', + 'StockrotationAdvance', + 'StockrotationRepatriation', + 'ReturnToHome', + 'ReturnToHolding', + 'RotatingCollection', + 'Reserve', + 'LostReserve', + 'CancelReserve', + 'ItemLost' + ) + AFTER `comments` + } + ); + + # Always end with this (adjust the bug info) + NewVersion( $DBversion, 27281, "Add 'ItemLost' to cancellation_reason enum"); +} diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql index ae247e67c8..9c31cb8cac 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -1531,7 +1531,7 @@ CREATE TABLE `branchtransfers` ( `tobranch` varchar(10) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '' COMMENT 'the branch the transfer was going to', `comments` longtext COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'any comments related to the transfer', `reason` enum('Manual','StockrotationAdvance','StockrotationRepatriation','ReturnToHome','ReturnToHolding','RotatingCollection','Reserve','LostReserve','CancelReserve') COLLATE utf8mb4_unicode_ci DEFAULT NULL COMMENT 'what triggered the transfer', - `cancellation_reason` ENUM('Manual', 'StockrotationAdvance', 'StockrotationRepatriation', 'ReturnToHome', 'ReturnToHolding', 'RotatingCollection', 'Reserve', 'LostReserve', 'CancelReserve') default NULL COMMENT 'what triggered the transfer cancellation', + `cancellation_reason` ENUM('Manual', 'StockrotationAdvance', 'StockrotationRepatriation', 'ReturnToHome', 'ReturnToHolding', 'RotatingCollection', 'Reserve', 'LostReserve', 'CancelReserve', 'ItemLost') default NULL COMMENT 'what triggered the transfer cancellation', PRIMARY KEY (`branchtransfer_id`), KEY `frombranch` (`frombranch`), KEY `tobranch` (`tobranch`), -- 2.20.1