From: Joe Atzberger <[email protected]> Essentially, this patch provides the option to overwrite only matching Extended Attributes, instead of all of them, treating the ext. fields more like normal fields.
Several functions added to Members::Attributes with corresponding tests. [ LL ref. 342 ] Signed-off-by: Galen Charlton <[email protected]> --- C4/Members/AttributeTypes.pm | 22 +++-- C4/Members/Attributes.pm | 107 ++++++++++++++++++-- .../prog/en/modules/tools/import_borrowers.tmpl | 27 ++++- t/Members_Attributes.t | 110 ++++++++++++++++++++ tools/import_borrowers.pl | 35 +++--- 5 files changed, 263 insertions(+), 38 deletions(-) create mode 100755 t/Members_Attributes.t diff --git a/C4/Members/AttributeTypes.pm b/C4/Members/AttributeTypes.pm index 4eb6990..93c67a5 100644 --- a/C4/Members/AttributeTypes.pm +++ b/C4/Members/AttributeTypes.pm @@ -60,28 +60,34 @@ $attr_type = C4::Members::AttributeTypes->delete($code); =over 4 -my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes(); +my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes($all_fields); =back Returns an array of hashrefs of each attribute type defined in the database. The array is sorted by code. Each hashref contains -the following fields: +at least the following fields: code description +If $all_fields is true, then each hashref also contains the other fields from borrower_attribute_types. + =cut sub GetAttributeTypes { + my $all = @_ ? shift : 0; + my $select = $all ? '*' : 'code, description'; my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare('SELECT code, description FROM borrower_attribute_types ORDER by code'); + my $sth = $dbh->prepare("SELECT $select FROM borrower_attribute_types ORDER by code"); $sth->execute(); - my @results = (); - while (my $row = $sth->fetchrow_hashref) { - push @results, $row; - } - return @results; + my $results = $sth->fetchall_arrayref({}); + return @$results; +} + +sub GetAttributeTypes_hashref { + my %hash = map {$_->{code} => $_} GetAttributeTypes(@_); + return \%hash; } =head1 METHODS diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm index e012320..88c6c09 100644 --- a/C4/Members/Attributes.pm +++ b/C4/Members/Attributes.pm @@ -18,25 +18,34 @@ package C4::Members::Attributes; # Suite 330, Boston, MA 02111-1307 USA use strict; +use warnings; + +use Text::CSV; # Don't be tempted to use Text::CSV::Unicode -- even in binary mode it fails. use C4::Context; use C4::Members::AttributeTypes; -use vars qw($VERSION); +use vars qw($VERSION @ISA @EXPORT_OK @EXPORT %EXPORT_TAGS); +our ($csv, $AttributeTypes); BEGIN { # set the version for version checking - $VERSION = 3.00; + $VERSION = 3.01; + @ISA = qw(Exporter); + @EXPORT_OK = qw(GetBorrowerAttributes CheckUniqueness SetBorrowerAttributes + extended_attributes_code_value_arrayref extended_attributes_merge); + %EXPORT_TAGS = ( all => \...@export_ok ); } =head1 NAME -C4::Members::Attribute - manage extend patron attributes +C4::Members::Attributes - manage extend patron attributes =head1 SYNOPSIS =over 4 -my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); + use C4::Members::Attributes; + my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); =back @@ -96,7 +105,7 @@ sub GetBorrowerAttributes { =over 4 -my $ok = CheckUniqueness($code, $value[, $borrowernumber]); + my $ok = CheckUniqueness($code, $value[, $borrowernumber]); =back @@ -137,7 +146,6 @@ sub CheckUniqueness { $sth->execute($code, $value); } my ($count) = $sth->fetchrow_array; - $sth->finish(); return ($count == 0); } @@ -145,7 +153,7 @@ sub CheckUniqueness { =over 4 -SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value', password => 'password' }, ... ] ); + SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value', password => 'password' }, ... ] ); =back @@ -170,6 +178,91 @@ sub SetBorrowerAttributes { } } +=head2 extended_attributes_code_value_arrayref + +=over 4 + + my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar"; + my $aref = extended_attributes_code_value_arrayref($patron_attributes); + +=back + +Takes a comma-delimited CSV-style string argument and returns the kind of data structure that SetBorrowerAttributes wants, +namely a reference to array of hashrefs like: + [ { code => 'CODE', value => 'value' }, { code => 'CODE2', value => 'othervalue' } ... ] + +Caches Text::CSV parser object for efficiency. + +=cut + +sub extended_attributes_code_value_arrayref { + my $string = shift or return; + $csv or $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode + my $ok = $csv->parse($string); # parse field again to get subfields! + my @list = $csv->fields(); + # TODO: error handling (check $ok) + return [ + sort {&_sort_by_code($a,$b)} + map { map { my @arr = split /:/, $_, 2; { code => $arr[0], value => $arr[1] } } $_ } + @list + ]; + # nested map because of split +} + +=head2 extended_attributes_merge + +=over 4 + + my $old_attributes = extended_attributes_code_value_arrayref("homeroom:224,grade:04,deanslist:2007,deanslist:2008,somedata:xxx"); + my $new_attributes = extended_attributes_code_value_arrayref("homeroom:115,grade:05,deanslist:2009,extradata:foobar"); + my $merged = extended_attributes_merge($patron_attributes, $new_attributes, 1); + + # assuming deanslist is a repeatable code, value same as: + # $merged = extended_attributes_code_value_arrayref("homeroom:115,grade:05,deanslist:2007,deanslist:2008,deanslist:2009,extradata:foobar,somedata:xxx"); + +=back + +Takes three arguments. The first two are references to array of hashrefs, each like: + [ { code => 'CODE', value => 'value' }, { code => 'CODE2', value => 'othervalue' } ... ] + +The third option specifies whether repeatable codes are clobbered or collected. True for non-clobber. + +Returns one reference to (merged) array of hashref. + +Caches results of C4::Members::AttributeTypes::GetAttributeTypes_hashref(1) for efficiency. + +=cut + +sub extended_attributes_merge { + my $old = shift or return; + my $new = shift or return $old; + my $keep = @_ ? shift : 0; + $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"; + next; + } + unless ($AttributeTypes->{$att->{code}}) { + warn "Cannot merge element: unrecognized code = '$att->{code}'"; + next; + } + unless ($AttributeTypes->{$att->{code}}->{repeatable} and $keep) { + @merged = grep {$att->{code} ne $_->{code}} @merged; # filter out any existing attributes of the same code + } + push @merged, $att; + } + return [( sort {&_sort_by_code($a,$b)} @merged )]; +} + +sub _sort_by_code { + my ($x, $y) = @_; + defined ($x->{code}) or return -1; + defined ($y->{code}) or return 1; + return $x->{code} cmp $y->{code} || $x->{value} cmp $y->{value}; +} + =head1 AUTHOR Koha Development Team <[email protected]> diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl index 47dc3a0..8d88ce6 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/import_borrowers.tmpl @@ -114,12 +114,27 @@ <!-- /TMPL_LOOP --> </ol></fieldset> <fieldset class="rows"> - <legend>If matching record is already in the borrowers table:</legend><ol><li class="radio"> - - <input type="radio" id="overwrite_cardnumberno" name="overwrite_cardnumber" value="0" checked="checked" /><label for="overwrite_cardnumberno">Ignore this one, keep the existing one</label></li> -<li class="radio"> - <input type="radio" id="overwrite_cardnumberyes" name="overwrite_cardnumber" value="1" /><label for="overwrite_cardnumberyes">Overwrite the existing one with this</label> - </li></ol></fieldset> + <legend>If matching record is already in the borrowers table:</legend> + <ol><li class="radio"> + <input type="radio" id="overwrite_cardnumberno" name="overwrite_cardnumber" value="0" checked="checked" /><label for="overwrite_cardnumberno">Ignore this one, keep the existing one</label> + </li> + <li class="radio"> + <input type="radio" id="overwrite_cardnumberyes" name="overwrite_cardnumber" value="1" /><label for="overwrite_cardnumberyes">Overwrite the existing one with this</label> + </li> + </ol> + </fieldset> + <!-- TMPL_IF NAME="ExtendedPatronAttributes" --> + <fieldset class="rows"> + <legend>Extended Attributes</legend> + <ol><li class="radio"> + <input type="radio" id="ext_preserve_0" name="ext_preserve" value="0" checked="checked" /><label for="ext_preserve_0">Replace all Extended Attributes</label> + </li> + <li class="radio"> + <input type="radio" id="ext_preserve_1" name="ext_preserve" value="1" /><label for="ext_preserve_1">Replace only included Extended Attributes</label> + </li> + </ol> + </fieldset> + <!-- /TMPL_IF --> <fieldset class="action"><input type="submit" value="Import" /></fieldset> </form> <!-- /TMPL_IF --> diff --git a/t/Members_Attributes.t b/t/Members_Attributes.t new file mode 100755 index 0000000..a60a988 --- /dev/null +++ b/t/Members_Attributes.t @@ -0,0 +1,110 @@ +#!/usr/bin/perl +# +# + +use strict; +use warnings; + +use Test::More tests => 11; + +BEGIN { + use_ok('C4::Members::Attributes', qw(:all)); +} + +INIT { + $C4::Members::Attributes::AttributeTypes = { + 'grade' => { + 'opac_display' => '1', + 'staff_searchable' => '1', + 'description' => 'Grade level', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'grade', + 'unique_id' => '0' + }, + 'deanslist' => { + 'opac_display' => '0', + 'staff_searchable' => '1', + 'description' => 'Deans List (annual)', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '1', + 'code' => 'deanslist', + 'unique_id' => '0' + }, + 'somedata' => { + 'opac_display' => '0', + 'staff_searchable' => '0', + 'description' => 'Some Ext. Attribute', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'somedata', + 'unique_id' => '0' + }, + 'extradata' => { + 'opac_display' => '0', + 'staff_searchable' => '0', + 'description' => 'Another Ext. Attribute', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'extradata', + 'unique_id' => '0' + }, + 'school_id' => { + 'opac_display' => '1', + 'staff_searchable' => '1', + 'description' => 'School ID Number', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'school_id', + 'unique_id' => '1' + }, + 'homeroom' => { + 'opac_display' => '1', + 'staff_searchable' => '1', + 'description' => 'Homeroom', + 'password_allowed' => '0', + 'authorised_value_category' => '', + 'repeatable' => '0', + 'code' => 'homeroom', + 'unique_id' => '0' + } + }; # This is important to prevent extended_attributes_merge from touching DB. +} + + +my @merge_tests = ( + { + line1 => "homeroom:501", + line2 => "grade:01", + merge => "homeroom:501,grade:01", + }, + { + line1 => "homeroom:224,grade:04,deanslist:2008,deanslist:2007,somedata:xxx", + line2 => "homeroom:115,grade:05,deanslist:2009,extradata:foobar", + merge => "homeroom:115,grade:05,deanslist:2008,deanslist:2007,deanslist:2009,extradata:foobar,somedata:xxx", + }, +); + +can_ok('C4::Members::Attributes', qw(extended_attributes_merge extended_attributes_code_value_arrayref)); + +ok(ref($C4::Members::Attributes::AttributeTypes) eq 'HASH', '$C4::Members::Attributes::AttributeTypes is a hashref'); + +diag scalar(@merge_tests) . " tests for extended_attributes_merge"; + +foreach my $test (@merge_tests) { + my ($old, $new, $merged); + ok($old = extended_attributes_code_value_arrayref($test->{line1}), "extended_attributes_code_value_arrayref('$test->{line1}')"); + foreach (@$old) { diag "old attribute: $_->{code} = $_->{value}"; } + ok($new = extended_attributes_code_value_arrayref($test->{line2}), "extended_attributes_code_value_arrayref('$test->{line2}')"); + foreach (@$new) { diag "new attribute: $_->{code} = $_->{value}"; } + ok($merged = extended_attributes_merge($old, $new), "extended_attributes_merge(\$old, \$new)"); + foreach (@$merged) { diag "merge (overwrite) attribute: $_->{code} = $_->{value}"; } + ok($merged = extended_attributes_merge($old, $new, 1), "extended_attributes_merge(\$old, \$new, 1)"); + foreach (@$merged) { diag "merge (preserve) attribute: $_->{code} = $_->{value}"; } +} + diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl index 0385275..f987fa3 100755 --- a/tools/import_borrowers.pl +++ b/tools/import_borrowers.pl @@ -42,7 +42,7 @@ use C4::Dates qw(format_date_in_iso); use C4::Context; use C4::Branch qw(GetBranchName); use C4::Members; -use C4::Members::Attributes; +use C4::Members::Attributes qw(:all); use C4::Members::AttributeTypes; use C4::Members::Messaging; @@ -64,7 +64,7 @@ if ($extended) { my $columnkeystpl = [ map { {'key' => $_} } grep {$_ ne 'borrowernumber' && $_ ne 'cardnumber'} @columnkeys ]; # ref. to array of hashrefs. my $input = CGI->new(); -my $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode +our $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode # push @feedback, {feedback=>1, name=>'backend', value=>$csv->backend, backend=>$csv->backend}; my ( $template, $loggedinuser, $cookie ) = get_template_and_user({ @@ -125,6 +125,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $csvkeycol{$keycol} = $col++; } #warn($borrowerline); + my $ext_preserve = $input->param('ext_preserve') || 0; if ($extended) { $matchpoint_attr_type = C4::Members::AttributeTypes->fetch($matchpoint); } @@ -189,14 +190,10 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { # The first 25 errors are enough. Keeping track of 30,000+ would destroy performance. next LINE; } - my @attrs; if ($extended) { my $attr_str = $borrower{patron_attributes}; - delete $borrower{patron_attributes}; - my $ok = $csv->parse($attr_str); - my @list = $csv->fields(); - # FIXME error handling - $patron_attributes = [ map { map { my @arr = split /:/, $_, 2; { code => $arr[0], value => $arr[1] } } $_ } @list ]; + 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); } # Popular spreadsheet applications make it difficult to force date outputs to be zero-padded, but we require it. foreach (qw(dateofbirth dateenrolled dateexpiry)) { @@ -239,20 +236,24 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { next LINE; } $borrower{'borrowernumber'} = $borrowernumber; - for my $col ( keys %borrower) { - # use values from extant patron unless our csv file includes this column or we provided a default. - # FIXME : You cannot update a field with a perl-evaluated false value using the defaults. - unless(exists($csvkeycol{$col}) || $defaults{$col}) { - $borrower{$col} = $member->{$col} if($member->{$col}) ; + for my $col (keys %borrower) { + # use values from extant patron unless our csv file includes this column or we provided a default. + # FIXME : You cannot update a field with a perl-evaluated false value using the defaults. + unless(exists($csvkeycol{$col}) || $defaults{$col}) { + $borrower{$col} = $member->{$col} if($member->{$col}) ; + } } - } unless (ModMember(%borrower)) { $invalid++; $template->param('lastinvalid'=>$borrower{'surname'}.' / '.$borrowernumber); next LINE; } if ($extended) { - C4::Members::Attributes::SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes); + if ($ext_preserve) { + my $old_attributes = GetBorrowerAttributes($borrowernumber); + $patron_attributes = extended_attributes_merge($old_attributes, $patron_attributes); #TODO: expose repeatable options in template + } + SetBorrowerAttributes($borrower{'borrowernumber'}, $patron_attributes); } $overwritten++; $template->param('lastoverwritten'=>$borrower{'surname'}.' / '.$borrowernumber); @@ -264,7 +265,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { } if ($borrowernumber = AddMember(%borrower)) { if ($extended) { - C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $patron_attributes); + SetBorrowerAttributes($borrowernumber, $patron_attributes); } if ($set_messaging_prefs) { C4::Members::Messaging::SetMessagingPreferencesFromDefaults({ borrowernumber => $borrowernumber, @@ -273,7 +274,7 @@ if ( $uploadborrowers && length($uploadborrowers) > 0 ) { $imported++; $template->param('lastimported'=>$borrower{'surname'}.' / '.$borrowernumber); } else { - $invalid++; # was just "$invalid", I assume incrementing was the point --atz + $invalid++; $template->param('lastinvalid'=>$borrower{'surname'}.' / AddMember'); } } -- 1.5.6.5 _______________________________________________ Koha-patches mailing list [email protected] http://lists.koha.org/mailman/listinfo/koha-patches
