Bugzilla – Attachment 101137 Details for
Bug 20443
Move C4::Members::Attributes to Koha namespace
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 20443: Remove extended_attributes_merge
Bug-20443-Remove-extendedattributesmerge.patch (text/plain), 9.65 KB, created by
Jonathan Druart
on 2020-03-20 14:47:53 UTC
(
hide
)
Description:
Bug 20443: Remove extended_attributes_merge
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 14:47:53 UTC
Size:
9.65 KB
patch
obsolete
>From 5bb15672b1bc8fd5419851e8e70261d9aba9fd49 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Tue, 17 Jul 2018 09:50:10 -0300 >Subject: [PATCH] Bug 20443: Remove extended_attributes_merge > >TODO We need tests here! > >Signed-off-by: Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Members/Attributes.pm | 46 +----------------- > Koha/Patron/Attributes.pm | 28 +++++++++++ > Koha/Patrons/Import.pm | 3 +- > t/Members_Attributes.t | 98 --------------------------------------- > 4 files changed, 32 insertions(+), 143 deletions(-) > delete mode 100755 t/Members_Attributes.t > >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index 76cd6a9b32..127ad4e735 100644 >--- a/C4/Members/Attributes.pm >+++ b/C4/Members/Attributes.pm >@@ -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; >diff --git a/Koha/Patron/Attributes.pm b/Koha/Patron/Attributes.pm >index 47ba594f74..9af07c0228 100644 >--- a/Koha/Patron/Attributes.pm >+++ b/Koha/Patron/Attributes.pm >@@ -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 >diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm >index 9d76aeeec8..38c6eb69bd 100644 >--- a/Koha/Patrons/Import.pm >+++ b/Koha/Patrons/Import.pm >@@ -290,7 +290,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? >diff --git a/t/Members_Attributes.t b/t/Members_Attributes.t >deleted file mode 100755 >index 205d6117f1..0000000000 >--- a/t/Members_Attributes.t >+++ /dev/null >@@ -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)"); >-} >- >-- >2.20.1
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 20443
:
101122
|
101123
|
101124
|
101125
|
101126
|
101127
|
101128
|
101129
|
101130
|
101131
|
101132
|
101133
|
101134
|
101135
|
101136
|
101137
|
101138
|
101139
|
101140
|
101141
|
101142
|
101143
|
101144
|
101145
|
101146
|
101147
|
101148
|
101149
|
101150
|
101209
|
101210
|
101211
|
101212
|
101213
|
101214
|
101215
|
101216
|
101217
|
101218
|
101219
|
101220
|
101221
|
101222
|
101223
|
101224
|
101225
|
101226
|
101227
|
101228
|
101229
|
101230
|
101231
|
101232
|
101233
|
101234
|
101235
|
101236
|
101237
|
101472
|
104519