From 876add59def37b61f34b5576be422d3b429b8240 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 --- 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 baab76fef6b..74569244fb7 100644 --- a/C4/Circulation.pm +++ b/C4/Circulation.pm @@ -1684,6 +1684,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 } ); } @@ -2152,69 +2155,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 5bcc917afa5..4a9775b3854 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -2171,6 +2171,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 cb862a45dc0..60131b77fc2 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.20.1