From 81826a7af7c57bdfbe4f7443bfbc0ace7de6a1c5 Mon Sep 17 00:00:00 2001 From: Alex Buckley Date: Wed, 10 Aug 2022 00:54:20 +0000 Subject: [PATCH] Bug 21159: Update item location on checkout This patchset shifts the logic for changing the item location on checkin (controlled by the UpdateItemLocationOnCheckin system preference) to a new subroutine in Koha/Items.pm That subroutine logic is shared with the UpdateItemLocationOnCheckout system preference. Test plan: 1. Apply patches, update databases and restart services 2. Set the following system preferences: - UpdateItemLocationOnCheckin: FIC: PROC - UpdateItemLocationOnCheckout: PROC: FIC 3. Checkout an an item with items.location = 'PROC'. Observe it's location is changed to 'FIC' 4. Return the item. Observe it's location is changed to 'PROC' 5. Change UpdateItemLocationOnCheckout to: PROC: _BLANK_ 6. Issue the item with items.location = 'PROC' and confirm it's location is blanked on checkout 7. Issue and return an item with a different location e.g. 'REF' (don't use 'CART' as this is blanked by bug 14576 on checkout). Observe the location does not change on issue or return. 8. Run unit tests sudo koha-shell kohadev prove t/db_dependent/Circulation/issue.t Sponsored-by: Toi Ohomai Institute of Technology, New Zealand Signed-off-by: David Nind Signed-off-by: Katrin Fischer Signed-off-by: Tomas Cohen Arazi --- C4/Circulation.pm | 70 +++---------------------- Koha/Item.pm | 84 ++++++++++++++++++++++++++++++ t/db_dependent/Circulation/issue.t | 79 +++++++++++++++++++++++++++- 3 files changed, 169 insertions(+), 64 deletions(-) diff --git a/C4/Circulation.pm b/C4/Circulation.pm index 34ff9542e7..3d4a3efa90 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1727,6 +1727,9 @@ sub AddIssue { CartToShelf( $item_object->itemnumber ); } + # Update item location + $item_object->update_item_location( 'checkout' ); + if ( C4::Context->preference('UpdateTotalIssuesOnCirc') ) { UpdateTotalIssues( $item_object->biblionumber, 1, undef, { skip_holds_queue => 1 } ); } @@ -2195,69 +2198,10 @@ sub AddReturn { my $borrowernumber = $patron ? $patron->borrowernumber : undef; # we don't know if we had a borrower or not my $patron_unblessed = $patron ? $patron->unblessed : {}; - my $update_loc_rules = C4::Context->yaml_preference('UpdateItemLocationOnCheckin'); - if ($update_loc_rules) { - if ( defined $update_loc_rules->{_ALL_} ) { - if ( $update_loc_rules->{_ALL_} eq '_PERM_' ) { - $update_loc_rules->{_ALL_} = $item->permanent_location; - } - if ( $update_loc_rules->{_ALL_} eq '_BLANK_' ) { - $update_loc_rules->{_ALL_} = ''; - } - if ( - ( - defined $item->location - && $item->location ne $update_loc_rules->{_ALL_} - ) - || ( !defined $item->location - && $update_loc_rules->{_ALL_} ne "" ) - ) - { - $messages->{'ItemLocationUpdated'} = - { from => $item->location, to => $update_loc_rules->{_ALL_} }; - $item->location( $update_loc_rules->{_ALL_} )->store( - { - log_action => 0, - skip_record_index => 1, - skip_holds_queue => 1 - } - ); - } - } - else { - foreach my $key ( keys %$update_loc_rules ) { - if ( $update_loc_rules->{$key} eq '_PERM_' ) { - $update_loc_rules->{$key} = $item->permanent_location; - } - elsif ( $update_loc_rules->{$key} eq '_BLANK_' ) { - $update_loc_rules->{$key} = ''; - } - if ( - ( - defined $item->location - && $item->location eq $key - && $item->location ne $update_loc_rules->{$key} - ) - || ( $key eq '_BLANK_' - && ( !defined $item->location || $item->location eq '' ) - && $update_loc_rules->{$key} ne '' ) - ) - { - $messages->{'ItemLocationUpdated'} = { - from => $item->location, - to => $update_loc_rules->{$key} - }; - $item->location( $update_loc_rules->{$key} )->store( - { - log_action => 0, - skip_record_index => 1, - skip_holds_queue => 1 - } - ); - last; - } - } - } + # Update item location + my $loc_messages = $item->update_item_location( 'checkin' ); + foreach my $loc_msg_key ( keys %$loc_messages ) { + $messages->{ $loc_msg_key } = $loc_messages->{ $loc_msg_key }; } my $yaml = C4::Context->preference('UpdateNotForLoanStatusOnCheckin'); diff --git a/Koha/Item.pm b/Koha/Item.pm index 484de1a054..21931144b9 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -2355,6 +2355,90 @@ sub strings_map { return $strings; } +=head3 update_item_location + + $item->update_item_location( $action ); + +Update the item location on checkin or checkout. + +=cut + +sub update_item_location { + my ( $self, $action ) = @_; + + my ($update_loc_rules, $messages); + if ( $action eq 'checkin' ) { + $update_loc_rules = C4::Context->yaml_preference('UpdateItemLocationOnCheckin'); + } else { + $update_loc_rules = C4::Context->yaml_preference('UpdateItemLocationOnCheckout'); + } + + if ($update_loc_rules) { + if ( defined $update_loc_rules->{_ALL_} ) { + if ( $update_loc_rules->{_ALL_} eq '_PERM_' ) { + $update_loc_rules->{_ALL_} = $self->permanent_location; + } + if ( $update_loc_rules->{_ALL_} eq '_BLANK_' ) { + $update_loc_rules->{_ALL_} = ''; + } + if ( + ( + defined $self->location + && $self->location ne $update_loc_rules->{_ALL_} + ) + || ( !defined $self->location + && $update_loc_rules->{_ALL_} ne "" ) + ) + { + $messages->{'ItemLocationUpdated'} = + { from => $self->location, to => $update_loc_rules->{_ALL_} }; + $self->location( $update_loc_rules->{_ALL_} )->store( + { + log_action => 0, + skip_record_index => 1, + skip_holds_queue => 1 + } + ); + } + } + else { + foreach my $key ( keys %$update_loc_rules ) { + if ( $update_loc_rules->{$key} eq '_PERM_' ) { + $update_loc_rules->{$key} = $self->permanent_location; + } + elsif ( $update_loc_rules->{$key} eq '_BLANK_' ) { + $update_loc_rules->{$key} = ''; + } + if ( + ( + defined $self->location + && $self->location eq $key + && $self->location ne $update_loc_rules->{$key} + ) + || ( $key eq '_BLANK_' + && ( !defined $self->location || $self->location eq '' ) + && $update_loc_rules->{$key} ne '' ) + ) + { + $messages->{'ItemLocationUpdated'} = { + from => $self->location, + to => $update_loc_rules->{$key} + }; + $self->location( $update_loc_rules->{$key} )->store( + { + log_action => 0, + skip_record_index => 1, + skip_holds_queue => 1 + } + ); + last; + } + } + } + } + return $messages; +} + =head3 _type =cut diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index cb862a45dc..60131b77fc 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -17,7 +17,7 @@ use Modern::Perl; -use Test::More tests => 53; +use Test::More tests => 66; use DateTime::Duration; use t::lib::Mocks; @@ -641,5 +641,82 @@ AddReturn( 'barcode_6', $branchcode_1 ); $item = Koha::Items->find( $itemnumber4 ); ok( $item->notforloan eq 0, q{UpdateNotForLoanStatusOnCheckout does not update notforloan value from 0 with setting "-1: 0"} ); +# Bug 21159 - Update item shelving location on checkout +my $itemnumber5 = Koha::Item->new( + { + biblionumber => $biblionumber, + barcode => 'barcode_7', + itemcallnumber => 'callnumber3', + homebranch => $branchcode_1, + holdingbranch => $branchcode_1, + location => 'FIC', + itype => $itemtype + }, +)->store->itemnumber; + +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', q{} ); +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckin', q{} ); +AddIssue( $patron_1, 'barcode_7'); +my $item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq 'FIC', 'UpdateItemLocationOnCheckout does not modify value when not enabled' ); + +#--- +AddReturn( 'barcode_7', $branchcode_1 ); +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', 'FIC: GEN' ); +$log_count_before = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +is( $item5->location, 'GEN', q{UpdateItemLocationOnCheckout updates location value from 'FIC' to 'GEN' with setting "FIC: GEN"} ); +is( $item5->permanent_location, 'GEN', q{UpdateItemLocationOnCheckout updates permanent_location value from 'FIC' to 'GEN' with setting "FIC: GEN"} ); +$log_count_after = $schema->resultset('ActionLog')->search({module => 'CATALOGUING'})->count(); +is($log_count_before, $log_count_after, "Change from UpdateNotForLoanStatusOnCheckout is not logged"); +AddReturn( 'barcode_7', $branchcode_1 ); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq 'GEN', q{UpdateItemLocationOnCheckout does not update location value from 'GEN' with setting "FIC: GEN"} ); + +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', '_ALL_: CART' ); +AddReturn( 'barcode_7', $branchcode_1 ); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq 'CART', q{UpdateItemLocationOnCheckout updates location value from 'GEN' with setting "_ALL_: CART"} ); +ok( $item5->permanent_location eq 'GEN', q{UpdateItemLocationOnCheckout does not update permanent_location value from 'GEN' with setting "_ALL_: CART"} ); + +$item5->location('GEN')->store; +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', "GEN: _BLANK_\n_BLANK_: PROC\nPROC: _PERM_" ); +AddReturn( 'barcode_7', $branchcode_1 ); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq '', q{UpdateItemLocationOnCheckout updates location value from 'GEN' to '' with setting "GEN: _BLANK_"} ); +AddReturn( 'barcode_7', $branchcode_1 ); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq 'PROC' , q{UpdateItemLocationOnCheckout updates location value from '' to 'PROC' with setting "_BLANK_: PROC"} ); +ok( $item5->permanent_location eq '' , q{UpdateItemLocationOnCheckout does not update permanent_location value from '' to 'PROC' with setting "_BLANK_: PROC"} ); +AddReturn( 'barcode_7', $branchcode_1 ); +AddIssue( $patron_1, 'barcode_7' ); +$item5 = Koha::Items->find( $itemnumber5 ); +ok( $item5->location eq '' , q{UpdateItemLocationOnCheckout updates location value from 'PROC' to '' with setting "PROC: _PERM_" } ); +ok( $item5->permanent_location eq '' , q{UpdateItemLocationOnCheckout does not update permanent_location from '' with setting "PROC: _PERM_" } ); + +# Bug 28472 +my $itemnumber6 = Koha::Item->new( + { + biblionumber => $biblionumber, + barcode => 'barcode_8', + itemcallnumber => 'callnumber5', + homebranch => $branchcode_1, + holdingbranch => $branchcode_1, + location => undef, + itype => $itemtype + } +)->store->itemnumber; + +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', '_ALL_: CART' ); +AddIssue( $patron_1, 'barcode_8' ); +my $item6 = Koha::Items->find( $itemnumber6 ); +is( $item6->location, 'CART', q{UpdateItemLocationOnCheckout updates location value from NULL (i.e. the item has no shelving location set) to 'CART' with setting "_ALL_: CART"} ); + + #End transaction $schema->storage->txn_rollback; -- 2.39.5