From e312c1eeea45c2f8c2f109f357856c1b7ff67a63 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Mon, 13 Feb 2023 16:37:51 +0000 Subject: [PATCH] Bug 31187: Preserve permanent_location if it is mapped to a field in the editor To test: 1 - Map permanent_location to a marc field 2 - Expose that field in the item editor 3 - Cataloging -> Batch item modification 4 - Enter the barcode for an item 5 - Change the location of the item and retain the permanent location on saving 6 - note that permanent location is set to locatoin 7 - Apply patch 8 - Repeat 9 - Note permanent location is retained Signed-off-by: Andrew Fuerste-Henry Squashed and amended to fix modified count: Bug 31187: (follow-up) Fix whitespace Bug 31187: Make sure to not count twice --- Koha/Items.pm | 10 +++++++ t/db_dependent/Koha/Items/BatchUpdate.t | 36 ++++++++++++++++++++++++- tools/batchMod.pl | 6 ++++- 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/Koha/Items.pm b/Koha/Items.pm index 3f3ba3fb4f..c7cdf41124 100644 --- a/Koha/Items.pm +++ b/Koha/Items.pm @@ -295,6 +295,16 @@ sub batch_update { my $old = $old_values->{$attribute}; my $new = $new_values->{$attribute}; + if ( $attribute eq 'permanent_location' && $new eq '' ) { + + # In the case where permanent_location is exposed we need + # to preserve the original value when none is passed + # the script will send a blank value to indicate that it was + # included in the form + $new_values->{permanent_location} = $old; + $item->make_column_dirty('permanent_location'); + next; # We aren't modifying, we are preserving, so skip the increment + } $modified++ if ( defined $old xor defined $new ) || ( defined $old && defined $new && $new ne $old ); diff --git a/t/db_dependent/Koha/Items/BatchUpdate.t b/t/db_dependent/Koha/Items/BatchUpdate.t index af6e5b6c43..8c728f11ab 100755 --- a/t/db_dependent/Koha/Items/BatchUpdate.t +++ b/t/db_dependent/Koha/Items/BatchUpdate.t @@ -16,7 +16,7 @@ # along with Koha; if not, see . use Modern::Perl; -use Test::More tests=> 8; +use Test::More tests=> 9; use Test::Warn; use utf8; @@ -156,6 +156,40 @@ subtest 'blank' => sub { }; +subtest 'permanent_location' => sub { + + # This is a special case as some libraries add this field in the frameworks + # to allow explicitly setting a temporary location. + # When mapped an empty value is submitted in the form with the key of permanent_location + + plan tests => 2; + + $items->batch_update( + { + new_values => { + 'permanent_location' => 'perm', + 'location' => 'loc' + } + } + )->reset; + + $item = $item->get_from_storage; + is( $item->permanent_location, 'perm', 'Updated as expected when value passed'); + + $items->batch_update( + { + new_values => { + 'permanent_location' => '', + 'location' => 'new_loc' + } + } + )->reset; + + $item = $item->get_from_storage; + is( $item->permanent_location, 'perm', 'Permanent location not updated when mapped, so key present, but no value passed '); + +}; + subtest 'regex' => sub { plan tests => 12; diff --git a/tools/batchMod.pl b/tools/batchMod.pl index 6c252ff849..9f2bb73507 100755 --- a/tools/batchMod.pl +++ b/tools/batchMod.pl @@ -155,7 +155,11 @@ if ( $op eq "action" ) { my @v = grep { $_ ne "" } uniq $input->multi_param($cgi_var_name); - next unless @v; + next unless @v || $attr eq 'permanent_location'; + # If permanent_location is present in the form we need to pass it forward + # to indicate that the user may modify this field directly and so + # it needs to be marked as 'dirty' on any update. Otherwise the object always + # sets location to the permanent_location as well $new_item_data->{$attr} = join '|', @v; } -- 2.30.2