From d056479bad1981d2157fb9e370f00dedca6dd832 Mon Sep 17 00:00:00 2001 From: Kyle M Hall Date: Tue, 17 Feb 2015 09:29:39 -0500 Subject: [PATCH] Bug 13717 - Add ability to move old patron attributes when importing new patron data Some libraries would like to move the value of a patron attribute to a different attribute when updating patrons via the import patrons tool. Test Plan: 1) Enable patron attributes 2) Create 2 attributes TEST and TEST_PREV 3) Create a patron with a TEST attribute 4) Create a patrons CSV file with that patron in it, containing a new TEST attribute 5) Go to tool/import patrons 6) Select your CSV file 7) At the bottom of the form is the to/from selector for attributes, choose TEST to be 'from', and TEST_PREV to be 'to' 8) Submit the form 9) Observe the patron record, note the previous value for TEST has now been moved to an instance of TEST_PREV --- C4/Members/Attributes.pm | 22 ++++++++++- Koha/Borrower/Import.pm | 8 ++- .../prog/en/modules/tools/import_borrowers.tt | 36 +++++++++++++++++ misc/import_borrowers.pl | 17 +++++--- t/Members_Attributes.t | 41 +++++++++++++++++++- tools/import_borrowers.pl | 11 +++++- 6 files changed, 121 insertions(+), 14 deletions(-) diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm index f0ec07f..c2531de 100644 --- a/C4/Members/Attributes.pm +++ b/C4/Members/Attributes.pm @@ -330,9 +330,12 @@ Caches results of C4::Members::AttributeTypes::GetAttributeTypes_hashref(1) for sub extended_attributes_merge { my $old = shift or return; my $new = shift or return $old; - my $keep = @_ ? shift : 0; + my $keep = shift; + my $move = shift; + $AttributeTypes or $AttributeTypes = C4::Members::AttributeTypes::GetAttributeTypes_hashref(1); my @merged = @$old; + foreach my $att (@$new) { unless ($att->{code}) { warn "Cannot merge element: no 'code' defined"; @@ -347,7 +350,22 @@ sub extended_attributes_merge { } push @merged, $att; } - return [( sort {&_sort_by_code($a,$b)} @merged )]; + + if ($move) { + foreach my $att (@$old) { + if ( $move->{ $att->{code} } ) { + $att->{code} = $move->{ $att->{code} }; + unless ( $AttributeTypes->{ $att->{code} }->{repeatable} ) { + + # filter out any existing attributes of the same code + @merged = grep { $att->{code} ne $_->{code} } @merged; + } + push @merged, $att; + } + } + } + + return [ ( sort { &_sort_by_code( $a, $b ) } @merged ) ]; } sub _sort_by_code { diff --git a/Koha/Borrower/Import.pm b/Koha/Borrower/Import.pm index 1041262..f26f8d5 100644 --- a/Koha/Borrower/Import.pm +++ b/Koha/Borrower/Import.pm @@ -21,6 +21,7 @@ use Carp; use Text::CSV; use C4::Members; +use C4::Members::Attributes qw(GetBorrowerAttributes extended_attributes_merge SetBorrowerAttributes); use C4::Branch; sub import_patrons { @@ -30,6 +31,7 @@ sub import_patrons { my $matchpoint = $params->{matchpoint}; my $defaults = $params->{defaults}; my $ext_preserve = $params->{preserve_extended_attributes}; + my $ext_move = $params->{move_extended_attributes}; my $overwrite_cardnumber = $params->{overwrite_cardnumber}; carp("No file handle passed in!") && return unless $handle; @@ -162,7 +164,7 @@ sub import_patrons { push @feedback, { feedback => 1, name => 'attribute string', value => $attr_str }; delete $borrower{patron_attributes} ; # not really a field in borrowers, so we don't want to pass it to ModMember. - $patron_attributes = extended_attributes_code_value_arrayref($attr_str); + $patron_attributes = C4::Members::Attributes::extended_attributes_code_value_arrayref($attr_str); } # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. @@ -275,9 +277,9 @@ sub import_patrons { } } if ($extended) { - if ($ext_preserve) { + if ($ext_preserve || $ext_move) { my $old_attributes = GetBorrowerAttributes($borrowernumber); - $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes ); + $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes, 0, $ext_move ); } push @errors, { unknown_error => 1 } unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes ); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt index 7a4c8af..979b707 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tt @@ -15,6 +15,12 @@ $("#dateenrolled").addClass('datepicker'); $("#dateexpiry").addClass('datepicker'); $("#dateofbirth").addClass('datepicker'); + + $("#clone_move_attributes").click( function(){ + $(".move_attributes").first().clone().prepend(" ").appendTo("#move_attributes"); + + return false; + }); }); @@ -224,6 +230,36 @@ + + [% IF patron_attribute_types %] +
+ + Move existing patron attributes + + + +
    +
  1. + + +
  2. +
  3. + + +
  4. +
+
+ [% END %] [% END %]
diff --git a/misc/import_borrowers.pl b/misc/import_borrowers.pl index 5b8c11b..5cc431e 100755 --- a/misc/import_borrowers.pl +++ b/misc/import_borrowers.pl @@ -30,17 +30,19 @@ my $matchpoint; my $overwrite_cardnumber; my %defaults; my $ext_preserve = 0; +my $ext_move; my $verbose = 0; my $help; GetOptions( - 'c|csv=s' => \$csv_file, - 'm|matchpoint=s' => \$matchpoint, - 'd|default=s' => \%defaults, - 'o|overwrite' => \$overwrite_cardnumber, - 'p|preserve-extended-atributes' => \$ext_preserve, - 'v|verbose+' => \$verbose, - 'h|help|?' => \$help, + 'c|csv=s' => \$csv_file, + 'm|matchpoint=s' => \$matchpoint, + 'd|default=s' => \%defaults, + 'o|overwrite' => \$overwrite_cardnumber, + 'p|preserve-extended-atributes' => \$ext_preserve, + 'ma|move_extended_attributes=s%' => $ext_move, + 'v|verbose+' => \$verbose, + 'h|help|?' => \$help, ); print_help() if ( $help || !$csv_file || !$matchpoint ); @@ -55,6 +57,7 @@ my $return = Koha::Patrons::Import::import_patrons( matchpoint => $matchpoint, overwrite_cardnumber => $overwrite_cardnumber, preserve_extended_attributes => $ext_preserve, + move_extended_attributes => $ext_move, } ); diff --git a/t/Members_Attributes.t b/t/Members_Attributes.t index aafd9a9..caa189b 100755 --- a/t/Members_Attributes.t +++ b/t/Members_Attributes.t @@ -5,7 +5,7 @@ use strict; use warnings; -use Test::More tests => 11; +use Test::More tests => 23; BEGIN { use_ok('C4::Members::Attributes', qw(:all)); @@ -23,6 +23,26 @@ INIT { 'code' => 'grade', 'unique_id' => '0' }, + 'grade_prev' => { + 'opac_display' => '0', + 'staff_searchable' => '1', + 'description' => 'Previous grade level', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'grade_prev', + 'unique_id' => '0' + }, + 'grade_prev_r' => { + 'opac_display' => '0', + 'staff_searchable' => '1', + 'description' => 'Previous grade level', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '1', + 'code' => 'grade_prev_r', + 'unique_id' => '0' + }, 'deanslist' => { 'opac_display' => '0', 'staff_searchable' => '1', @@ -102,3 +122,22 @@ foreach my $test (@merge_tests) { ok($merged = extended_attributes_merge($old, $new, 1), "extended_attributes_merge(\$old, \$new, 1)"); } +my $old = extended_attributes_code_value_arrayref("grade:02,grade_prev:01"); +my $new = extended_attributes_code_value_arrayref("grade:03"); +my $merged = extended_attributes_merge($old, $new, 0, { "grade" => "grade_prev" }); +is( scalar @$merged, 2, "Merge has two attributes" ); +is( $merged->[0]->{code}, 'grade', "First attribute has code of 'grade'" ); +is( $merged->[0]->{value}, '03', "First attribute has value of '03'" ); +is( $merged->[1]->{code}, 'grade_prev', "Second attribute has code of 'grade_prev'" ); +is( $merged->[1]->{value}, '02', "Second attribute has value of '02'" ); + +$old = extended_attributes_code_value_arrayref("grade:02,grade_prev_r:01"); +$new = extended_attributes_code_value_arrayref("grade:03"); +$merged = extended_attributes_merge($old, $new, 0, { "grade" => "grade_prev_r" }); +is( scalar @$merged, 3, "Merge has three attributes" ); +is( $merged->[0]->{code}, 'grade', "First attribute has code of 'grade'" ); +is( $merged->[0]->{value}, '03', "First attribute has value of '03'" ); +is( $merged->[1]->{code}, 'grade_prev_r', "Second attribute has code of 'grade_prev'" ); +is( $merged->[1]->{value}, '01', "Second attribute has value of '01'" ); +is( $merged->[2]->{code}, 'grade_prev_r', "Third attribute has code of 'grade_prev'" ); +is( $merged->[2]->{value}, '02', "Third attribute has value of '02'" ); diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index ace288c..1ed6f7a 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -110,6 +110,11 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { my $handle = $input->upload('uploadborrowers'); my %defaults = $input->Vars; + my @move_attribute_from = $input->param('move_attribute_from'); + my @move_attribute_to = $input->param('move_attribute_to'); + my %move; + @move{@move_attribute_from} = @move_attribute_to; + my $return = Koha::Patrons::Import::import_patrons( { file => $handle, @@ -117,6 +122,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { matchpoint => $matchpoint, overwrite_cardnumber => $input->param('overwrite_cardnumber'), preserve_extended_attributes => $input->param('ext_preserve') || 0, + move_extended_attributes => \%move, } ); @@ -157,7 +163,10 @@ else { { code => "patron_attribute_" . $attr_type->code(), description => $attr_type->description() }; } } - $template->param( matchpoints => \@matchpoints ); + $template->param( + matchpoints => \@matchpoints, + patron_attribute_types => [C4::Members::AttributeTypes::GetAttributeTypes()], + ); } } -- 1.7.2.5