From 68095f8609ad7f2478a72110b58f079b49a82c95 Mon Sep 17 00:00:00 2001 From: Katrin Fischer Date: Wed, 1 Nov 2023 11:10:24 +0000 Subject: [PATCH] Bug 21159: (QA follow-up) Add say to database update, perltidy, fix tests * Perltidies to pass the QA script * Adds missing say statement to the database update * Makes sure the tests pass on a database, where UpdateItemLocaton* system preferences are not empty Signed-off-by: Katrin Fischer --- Koha/Item.pm | 20 ++--- ...dd_UpdateItemLocationOnCheckout_syspref.pl | 11 ++- t/db_dependent/Circulation/issue.t | 85 +++++++++++++------ 3 files changed, 75 insertions(+), 41 deletions(-) diff --git a/Koha/Item.pm b/Koha/Item.pm index 4a9775b385..7003e1415b 100644 --- a/Koha/Item.pm +++ b/Koha/Item.pm @@ -2171,6 +2171,7 @@ sub strings_map { return $strings; } + =head3 update_item_location $item->update_item_location( $action ); @@ -2182,7 +2183,7 @@ Update the item location on checkin or checkout. sub update_item_location { my ( $self, $action ) = @_; - my ($update_loc_rules, $messages); + my ( $update_loc_rules, $messages ); if ( $action eq 'checkin' ) { $update_loc_rules = C4::Context->yaml_preference('UpdateItemLocationOnCheckin'); } else { @@ -2198,16 +2199,13 @@ sub update_item_location { $update_loc_rules->{_ALL_} = ''; } if ( - ( - defined $self->location - && $self->location ne $update_loc_rules->{_ALL_} - ) + ( 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_} }; + { from => $self->location, to => $update_loc_rules->{_ALL_} }; $self->location( $update_loc_rules->{_ALL_} )->store( { log_action => 0, @@ -2216,13 +2214,11 @@ sub update_item_location { } ); } - } - else { + } 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_' ) { + } elsif ( $update_loc_rules->{$key} eq '_BLANK_' ) { $update_loc_rules->{$key} = ''; } if ( @@ -2234,7 +2230,7 @@ sub update_item_location { || ( $key eq '_BLANK_' && ( !defined $self->location || $self->location eq '' ) && $update_loc_rules->{$key} ne '' ) - ) + ) { $messages->{'ItemLocationUpdated'} = { from => $self->location, diff --git a/installer/data/mysql/atomicupdate/bug_21159-add_UpdateItemLocationOnCheckout_syspref.pl b/installer/data/mysql/atomicupdate/bug_21159-add_UpdateItemLocationOnCheckout_syspref.pl index cf7e255ef4..5147bb4036 100755 --- a/installer/data/mysql/atomicupdate/bug_21159-add_UpdateItemLocationOnCheckout_syspref.pl +++ b/installer/data/mysql/atomicupdate/bug_21159-add_UpdateItemLocationOnCheckout_syspref.pl @@ -1,12 +1,15 @@ use Modern::Perl; return { - bug_number => "21159", + bug_number => "21159", description => "Add new system preference UpdateItemLocationOnCheckout", - up => sub { + up => sub { my ($args) = @_; - my ($dbh, $out) = @$args{qw(dbh out)}; + my ( $dbh, $out ) = @$args{qw(dbh out)}; - $dbh->do(q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('UpdateItemLocationOnCheckout', '', 'NULL', 'This is a list of value pairs.\n Examples:\n\nPROC: FIC - causes an item in the Processing Center location to be updated into the Fiction location on check out.\nFIC: GEN - causes an item in the Fiction location to be updated into the General stacks location on check out.\n_BLANK_:FIC - causes an item that has no location to be updated into the Fiction location on check out.\nFIC: _BLANK_ - causes an item in location FIC to be updated to a blank location on check out.\n_ALL_:FIC - causes all items to be updated into the Fiction location on check out.\nPROC: _PERM_ - causes an item that is in the Processing Center to be updated to it''s permanent location.\n\nGeneral rule: if the location value on the left matches the item''s current location, it will be updated to match the location value on the right.\nNote: PROC and CART are special values, for these locations only can location and permanent_location differ, in all other cases an update will affect both. Items in the CART location will be returned to their permanent location on checkout.\n\nThe special term _BLANK_ may be used on either side of a value pair to update or remove the location from items with no location assigned.\nThe special term _ALL_ is used on the left side of the colon (:) to affect all items.\nThe special term _PERM_ is used on the right side of the colon (:) to return items to their permanent location.', 'Free') }); + $dbh->do( + q{INSERT IGNORE INTO systempreferences (variable,value,options,explanation,type) VALUES ('UpdateItemLocationOnCheckout', '', 'NULL', 'This is a list of value pairs.\n Examples:\n\nPROC: FIC - causes an item in the Processing Center location to be updated into the Fiction location on check out.\nFIC: GEN - causes an item in the Fiction location to be updated into the General stacks location on check out.\n_BLANK_:FIC - causes an item that has no location to be updated into the Fiction location on check out.\nFIC: _BLANK_ - causes an item in location FIC to be updated to a blank location on check out.\n_ALL_:FIC - causes all items to be updated into the Fiction location on check out.\nPROC: _PERM_ - causes an item that is in the Processing Center to be updated to it''s permanent location.\n\nGeneral rule: if the location value on the left matches the item''s current location, it will be updated to match the location value on the right.\nNote: PROC and CART are special values, for these locations only can location and permanent_location differ, in all other cases an update will affect both. Items in the CART location will be returned to their permanent location on checkout.\n\nThe special term _BLANK_ may be used on either side of a value pair to update or remove the location from items with no location assigned.\nThe special term _ALL_ is used on the left side of the colon (:) to affect all items.\nThe special term _PERM_ is used on the right side of the colon (:) to return items to their permanent location.', 'Free') } + ); + say $out "Added new system preference 'UpdateItemLocationOnCheckout'"; }, }; diff --git a/t/db_dependent/Circulation/issue.t b/t/db_dependent/Circulation/issue.t index 60131b77fc..61f0ec6e37 100755 --- a/t/db_dependent/Circulation/issue.t +++ b/t/db_dependent/Circulation/issue.t @@ -480,6 +480,9 @@ my $itemnumber2 = Koha::Item->new( } )->store->itemnumber; +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckout', q{} ); +t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckin', q{} ); + t::lib::Mocks::mock_preference( 'UpdateItemLocationOnCheckin', q{} ); AddReturn( 'barcode_4', $branchcode_1 ); my $item2 = Koha::Items->find( $itemnumber2 ); @@ -655,49 +658,79 @@ my $itemnumber5 = Koha::Item->new( )->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 ); +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(); +$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"); +$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"} ); +$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 = 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_"} ); +$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"} ); +$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_" } ); +$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( @@ -714,9 +747,11 @@ my $itemnumber6 = Koha::Item->new( 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"} ); - +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.30.2