gitea
/
Koha-clms
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Bug 20443: Remove extended_attributes_merge 

TODO We need tests here!

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This commit is contained in:
Jonathan Druart 2018-07-17 09:50:10 -03:00 committed by Martin Renvoize
parent 836e950d25
commit bffb16eb70
No known key found for this signature in database
GPG Key ID: 422B469130441A0F
4 changed files with 32 additions and 143 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

46
C4/Members/Attributes.pm
View File

@ -31,7 +31,7 @@ our ($csv, $AttributeTypes);
BEGIN {
@ISA = qw(Exporter);
@EXPORT_OK = qw(
extended_attributes_code_value_arrayref extended_attributes_merge
extended_attributes_code_value_arrayref
);
%EXPORT_TAGS = ( all => \@EXPORT_OK );
}
@ -61,6 +61,7 @@ Caches Text::CSV parser object for efficiency.
sub extended_attributes_code_value_arrayref {
my $string = shift or return;
use Data::Printer colored => 1; warn p $string;
$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();
@ -73,49 +74,6 @@ sub extended_attributes_code_value_arrayref {
# nested map because of split
}
=head2 extended_attributes_merge
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");
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 attribute types for efficiency. # FIXME That is very bad and can cause side-effects undef plack
=cut
sub extended_attributes_merge {
my $old = shift or return;
my $new = shift or return $old;
my $keep = @_ ? shift : 0;
$AttributeTypes or $AttributeTypes = Koha::Patron::Attribute::Types->search->unblessed;
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;

28
Koha/Patron/Attributes.pm
View File

@ -87,6 +87,34 @@ sub filter_by_branch_limitations {
return $self->search( $or, $join );
}
sub merge_with {
my ( $self, $new_attributes ) = @_;
my @merged = $self->as_list;
while ( my ( $attr ) = $new_attributes->next ) {
unless ( $attr->code ) {
warn "Cannot merge element: no 'code' defined";
next;
}
# FIXME Do we need that here or let ->store do the job?
unless ( $attr->type ) {
warn "Cannot merge element: unrecognized code = '$attr->code'";
next;
}
unless ( $attr->type->repeatable ) {
# filter out any existing attributes of the same code
@merged = grep {$attr->code ne $_->code} @merged;
}
push @merged, $attr;
}
# WARNING - we would like to return a set, but $new_attributes is not in storage yet
# Maybe there is something obvious I (JD) am missing
return [ sort { $a->{code} cmp $b->{code} || $a->{attribute} cmp $b->{attribute} } @merged ];
}
=head3 _type
=cut

3
Koha/Patrons/Import.pm
View File

@ -299,7 +299,8 @@ sub import_patrons {
if ($extended) {
if ($ext_preserve) {
my $old_attributes = $patron->extended_attributes->as_list;
$patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes );
$patron_attributes = $patron->extended_attributes->merge_with( $patron_attributes );
}
eval {
# We do not want to filter by branch, maybe we should?

98
t/Members_Attributes.t
View File

@ -1,98 +0,0 @@
#!/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',
'authorised_value_category' => '',
'repeatable' => '0',
'code' => 'grade',
'unique_id' => '0'
},
'deanslist' => {
'opac_display' => '0',
'staff_searchable' => '1',
'description' => 'Deans List (annual)',
'authorised_value_category' => '',
'repeatable' => '1',
'code' => 'deanslist',
'unique_id' => '0'
},
'somedata' => {
'opac_display' => '0',
'staff_searchable' => '0',
'description' => 'Some Ext. Attribute',
'authorised_value_category' => '',
'repeatable' => '0',
'code' => 'somedata',
'unique_id' => '0'
},
'extradata' => {
'opac_display' => '0',
'staff_searchable' => '0',
'description' => 'Another Ext. Attribute',
'authorised_value_category' => '',
'repeatable' => '0',
'code' => 'extradata',
'unique_id' => '0'
},
'school_id' => {
'opac_display' => '1',
'staff_searchable' => '1',
'description' => 'School ID Number',
'authorised_value_category' => '',
'repeatable' => '0',
'code' => 'school_id',
'unique_id' => '1'
},
'homeroom' => {
'opac_display' => '1',
'staff_searchable' => '1',
'description' => 'Homeroom',
'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');
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}')");
ok($new = extended_attributes_code_value_arrayref($test->{line2}), "extended_attributes_code_value_arrayref('$test->{line2}')");
ok($merged = extended_attributes_merge($old, $new), "extended_attributes_merge(\$old, \$new)");
ok($merged = extended_attributes_merge($old, $new, 1), "extended_attributes_merge(\$old, \$new, 1)");
}