Bugzilla – Attachment 101217 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 UpdateBorrowerAttribute and SetBorrowerAttributes
Bug-20443-Remove-UpdateBorrowerAttribute-and-SetBo.patch (text/plain), 35.86 KB, created by
Jonathan Druart
on 2020-03-20 15:05:27 UTC
(
hide
)
Description:
Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttributes
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 15:05:27 UTC
Size:
35.86 KB
patch
obsolete
>From 5ff053bb1605c1d351e1ab70580dbf031bbdbaab Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 13 Jul 2018 14:11:44 -0300 >Subject: [PATCH] Bug 20443: Remove UpdateBorrowerAttribute and > SetBorrowerAttributes > >This patch replace Koha::Patron->get_extended_attributes with >->extended_attributes >It's now a getter a setter method. > >It permits to replace UpdateBorrowerAttribute and use >create_related from DBIx::Class > >Notes: >* We face the same variable names difference than in a previous patch >(value vs attribute) > >Bug 20443: Remove SetBorrowerAttributes > >squash + RM get_extended_attributes > > RM get_extended_attributes > >SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute > >Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Auth_with_ldap.pm | 6 +- > C4/ILSDI/Services.pm | 2 +- > C4/Letters.pm | 2 +- > C4/Members.pm | 2 +- > C4/Members/Attributes.pm | 66 +------------ > Koha/Patron.pm | 33 ++++++- > Koha/Patrons/Import.pm | 19 +++- > .../prog/en/includes/circ-menu.inc | 2 +- > members/memberentry.pl | 8 +- > members/moremember.pl | 2 +- > opac/opac-memberentry.pl | 3 +- > t/db_dependent/Auth_with_ldap.t | 2 +- > t/db_dependent/Koha/Patron/Modifications.t | 2 +- > t/db_dependent/Koha/Patrons.t | 20 ++-- > t/db_dependent/Koha/Patrons/Import.t | 4 +- > t/db_dependent/Members/Attributes.t | 92 ++++++++++--------- > t/db_dependent/Utils/Datatables_Members.t | 29 ++++-- > tools/modborrowers.pl | 10 +- > 18 files changed, 157 insertions(+), 147 deletions(-) > >diff --git a/C4/Auth_with_ldap.pm b/C4/Auth_with_ldap.pm >index 09b2b3f66b..3cff983bf7 100644 >--- a/C4/Auth_with_ldap.pm >+++ b/C4/Auth_with_ldap.pm >@@ -239,7 +239,11 @@ sub checkpw_ldap { > next; > } > if (C4::Members::Attributes::CheckUniqueness($code, $borrower{$code}, $borrowernumber)) { >- C4::Members::Attributes::UpdateBorrowerAttribute($borrowernumber, {code => $code, attribute => $borrower{$code}}); >+ my $patron = Koha::Patrons->find($borrowernumber); >+ if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP... >+ my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}}); >+ $patron->extended_attributes([$attribute]); >+ } > } else { > warn "ERROR_extended_unique_id_failed $code $borrower{$code}"; > } >diff --git a/C4/ILSDI/Services.pm b/C4/ILSDI/Services.pm >index 0416f5c82e..29cb58a772 100644 >--- a/C4/ILSDI/Services.pm >+++ b/C4/ILSDI/Services.pm >@@ -506,7 +506,7 @@ sub GetPatronInfo { > if ( $show_attributes && $show_attributes eq "1" ) { > # FIXME Regression expected here, we do not retrieve the same field as previously > # Waiting for answer on bug 14257 comment 15 >- my $attrs = $patron->get_extended_attributes->search({ opac_display => 1 })->unblessed; >+ my $attrs = $patron->extended_attributes->search({ opac_display => 1 })->unblessed; > $borrower->{'attributes'} = $attrs; > } > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index a5bb9ead64..9e583cd5ae 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -853,7 +853,7 @@ sub _parseletter { > if ($table eq 'borrowers' && $letter->{content}) { > my $patron = Koha::Patrons->find( $values->{borrowernumber} ); > if ( $patron ) { >- my $attributes = $patron->get_extended_attributes; >+ my $attributes = $patron->extended_attributes; > my %attr; > while ( my $attribute = $attributes->next ) { > my $code = $attribute->code; >diff --git a/C4/Members.pm b/C4/Members.pm >index eb5dbfb4e2..9355436c33 100644 >--- a/C4/Members.pm >+++ b/C4/Members.pm >@@ -33,7 +33,7 @@ use C4::Reserves; > use C4::Accounts; > use C4::Biblio; > use C4::Letters; >-use C4::Members::Attributes qw(SearchIdMatchingAttribute UpdateBorrowerAttribute); >+use C4::Members::Attributes qw(SearchIdMatchingAttribute); > use C4::NewsChannels; #get slip news > use DateTime; > use Koha::Database; >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index 6f5001d12c..c48828c67a 100644 >--- a/C4/Members/Attributes.pm >+++ b/C4/Members/Attributes.pm >@@ -29,8 +29,7 @@ our ($csv, $AttributeTypes); > > BEGIN { > @ISA = qw(Exporter); >- @EXPORT_OK = qw(CheckUniqueness SetBorrowerAttributes >- UpdateBorrowerAttribute >+ @EXPORT_OK = qw(CheckUniqueness > extended_attributes_code_value_arrayref extended_attributes_merge > SearchIdMatchingAttribute); > %EXPORT_TAGS = ( all => \@EXPORT_OK ); >@@ -112,71 +111,14 @@ sub CheckUniqueness { > return ($count == 0); > } > >-=head2 SetBorrowerAttributes >- >- SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value' }, ... ] ); >- >-Set patron attributes for the patron identified by C<$borrowernumber>, >-replacing any that existed previously. >- >-=cut >- >-sub SetBorrowerAttributes { >- my $borrowernumber = shift; >- my $attr_list = shift; >- my $no_branch_limit = shift // 0; >- >- my $dbh = C4::Context->dbh; >- >- my $attributes = Koha::Patrons->find($borrowernumber)->get_extended_attributes; >- >- unless ( $no_branch_limit ) { >- $attributes = $attributes->filter_by_branch_limitations; >- } >- $attributes->delete; >- >- my $sth = $dbh->prepare("INSERT INTO borrower_attributes (borrowernumber, code, attribute) >- VALUES (?, ?, ?)"); >- foreach my $attr (@$attr_list) { >- $sth->execute($borrowernumber, $attr->{code}, $attr->{value}); >- if ($sth->err) { >- warn sprintf('Database returned the following error: %s', $sth->errstr); >- return; # bail immediately on errors >- } >- } >- return 1; # borrower attributes successfully set >-} >- >-=head2 UpdateBorrowerAttribute >- >- UpdateBorrowerAttribute($borrowernumber, $attribute ); >- >-Update a borrower attribute C<$attribute> for the patron identified by C<$borrowernumber>, >- >-=cut >- >-sub UpdateBorrowerAttribute { >- my ( $borrowernumber, $attribute ) = @_; >- >- Koha::Patrons->find($borrowernumber)->get_extended_attributes->search({ 'me.code' => $attribute->{code} })->filter_by_branch_limitations->delete; >- >- my $dbh = C4::Context->dbh; >- my $query = "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?"; >- my @params = ( $attribute->{attribute}, $attribute->{code}, $borrowernumber ); >- my $sth = $dbh->prepare( $query ); >- >- $sth->execute( @params ); >-} >- >- > =head2 extended_attributes_code_value_arrayref > > my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar"; > my $aref = extended_attributes_code_value_arrayref($patron_attributes); > >-Takes a comma-delimited CSV-style string argument and returns the kind of data structure that SetBorrowerAttributes wants, >+Takes a comma-delimited CSV-style string argument and returns the kind of data structure that Koha::Patron->extended_attributes wants, > namely a reference to array of hashrefs like: >- [ { code => 'CODE', value => 'value' }, { code => 'CODE2', value => 'othervalue' } ... ] >+ [ { code => 'CODE', attribute => 'value' }, { code => 'CODE2', attribute => 'othervalue' } ... ] > > Caches Text::CSV parser object for efficiency. > >@@ -190,7 +132,7 @@ sub extended_attributes_code_value_arrayref { > # TODO: error handling (check $ok) > return [ > sort {&_sort_by_code($a,$b)} >- map { map { my @arr = split /:/, $_, 2; { code => $arr[0], value => $arr[1] } } $_ } >+ map { map { my @arr = split /:/, $_, 2; { code => $arr[0], attribute => $arr[1] } } $_ } > @list > ]; > # nested map because of split >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index d7bea28b4f..9a06db38c1 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1438,16 +1438,41 @@ sub generate_userid { > return $self; > } > >-=head3 get_extended_attributes >+=head3 add_extended_attribute > >-my $attributes = $patron->get_extended_attributes >+=cut >+ >+sub add_extended_attribute { >+ my ($self, $attribute) = @_; >+ $attribute->{borrowernumber} = $self->borrowernumber; >+ return Koha::Patron::Attribute->new($attribute)->store; >+} >+ >+=head3 extended_attributes > > Return object of Koha::Patron::Attributes type with all attributes set for this patron > >+Or setter FIXME >+ > =cut > >-sub get_extended_attributes { >- my ( $self ) = @_; >+sub extended_attributes { >+ my ( $self, $attributes ) = @_; >+ if ($attributes) { # setter >+ my $schema = $self->_result->result_source->schema; >+ $schema->txn_do( >+ sub { >+ # Remove the existing one >+ $self->extended_attributes->filter_by_branch_limitations->delete; >+ >+ # Insert the new ones >+ for my $attribute (@$attributes) { >+ $self->_result->create_related('borrower_attributes', $attribute); >+ } >+ } >+ ); >+ } >+ > my $rs = $self->_result->borrower_attributes; > # We call search to use the filters in Koha::Patron::Attributes->search > return Koha::Patron::Attributes->_new_from_dbic($rs)->search; >diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm >index f1473678ce..60243cf2d8 100644 >--- a/Koha/Patrons/Import.pm >+++ b/Koha/Patrons/Import.pm >@@ -290,11 +290,18 @@ sub import_patrons { > } > if ($extended) { > if ($ext_preserve) { >- my $old_attributes = $patron->get_extended_attributes->as_list; >+ my $old_attributes = $patron->extended_attributes->as_list; > $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes ); > } >- push @errors, { unknown_error => 1 } >- unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes, 'no_branch_limit' ); >+ eval { >+ # We do not want to filter by branch, maybe we should? >+ Koha::Patrons->find($borrowernumber)->extended_attributes->delete; >+ $patron->extended_attributes($patron_attributes); >+ }; >+ if ($@) { >+ # FIXME This is not an unknown error, we can do better here >+ push @errors, { unknown_error => 1 }; >+ } > } > $overwritten++; > push( >@@ -323,7 +330,9 @@ sub import_patrons { > } > > if ($extended) { >- SetBorrowerAttributes( $patron->borrowernumber, $patron_attributes ); >+ # FIXME Hum, we did not filter earlier and now we do? >+ $patron->extended_attributes->filter_by_branch_limitations->delete; >+ $patron->extended_attributes($patron_attributes); > } > > if ($set_messaging_prefs) { >@@ -451,7 +460,7 @@ sub set_column_keys { > > my $patron_attributes = set_patron_attributes($extended, $borrower{patron_attributes}, $feedback); > >-Returns a reference to array of hashrefs data structure as expected by SetBorrowerAttributes. >+Returns a reference to array of hashrefs data structure as expected by Koha::Patron->extended_attributes > > =cut > >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc >index fbb4a2b24a..7b3494268b 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc >@@ -71,7 +71,7 @@ > [% END %] > > [% IF Koha.Preference('ExtendedPatronAttributes') %] >- [% FOREACH extendedattribute IN patron.get_extended_attributes %] >+ [% FOREACH extendedattribute IN patron.extended_attributes %] > [% IF ( extendedattribute.type.display_checkout ) %] [%# FIXME We should filter in the line above %] > [% IF ( extendedattribute.attribute ) %] [%# FIXME Why that? why not if == 0? %] > <li class="patronattribute"> >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 8bc4af09ed..4b3a0c0f13 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -480,7 +480,8 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ > } > > if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { >- C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); >+ $patron->extended_attributes->filter_by_branch_limitations->delete; >+ $patron->extended_attributes($extended_patron_attributes); > } > if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { > C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'}); >@@ -550,7 +551,8 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){ > > add_guarantors( $patron, $input ); > if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) { >- C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes); >+ $patron->extended_attributes->filter_by_branch_limitations->delete; >+ $patron->extended_attributes($extended_patron_attributes); > } > if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) { > C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template); >@@ -858,7 +860,7 @@ sub patron_attributes_form { > return; > } > my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub >- my @attributes = $patron->get_extended_attributes->as_list; # FIXME Must be improved! >+ my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved! > my @classes = uniq( map {$_->type->class} @attributes ); > @classes = sort @classes; > >diff --git a/members/moremember.pl b/members/moremember.pl >index b14148821a..ea00da74f8 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -130,7 +130,7 @@ $template->param( > ); > > if (C4::Context->preference('ExtendedPatronAttributes')) { >- my @attributes = $patron->get_extended_attributes->as_list; # FIXME Must be improved! >+ my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved! > my @classes = uniq( map {$_->type->class} @attributes ); > @classes = sort @classes; > >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index 3d7770070c..4f3c1de98b 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -213,7 +213,8 @@ if ( $action eq 'create' ) { > my $patron = Koha::Patron->new( \%borrower )->store; > Koha::Patron::Consent->new({ borrowernumber => $patron->borrowernumber, type => 'GDPR_PROCESSING', given_on => $consent_dt })->store if $consent_dt; > if ( $patron ) { >- C4::Members::Attributes::SetBorrowerAttributes( $patron->borrowernumber, $attributes ); >+ $patron->extended_attributes->filter_by_branch_limitations->delete; >+ $patron->extended_attributes($attributes); > if ( C4::Context->preference('EnhancedMessagingPreferences') ) { > C4::Form::MessagingPreferences::handle_form_action( > $cgi, >diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t >index 001f2a8c27..5871b87c52 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -206,7 +206,7 @@ subtest 'checkpw_ldap tests' => sub { > > C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ); > ok( >- Koha::Patrons->find($borrower->{borrowernumber})->get_extended_attributes->count, >+ Koha::Patrons->find($borrower->{borrowernumber})->extended_attributes->count, > 'Extended attributes are not deleted' > ); > >diff --git a/t/db_dependent/Koha/Patron/Modifications.t b/t/db_dependent/Koha/Patron/Modifications.t >index ce279adc6d..3378ccdc0e 100755 >--- a/t/db_dependent/Koha/Patron/Modifications.t >+++ b/t/db_dependent/Koha/Patron/Modifications.t >@@ -174,7 +174,7 @@ subtest 'approve tests' => sub { > ); > is( $patron->firstname, 'Kyle', > 'Patron modification set the right firstname' ); >- my $patron_attributes = $patron->get_extended_attributes; >+ my $patron_attributes = $patron->extended_attributes; > my $attribute_1 = $patron_attributes->next; > is( $attribute_1->code, > 'CODE_1', 'Patron modification correctly saved attribute code' ); >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 2a39023d48..69331d9be9 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -2019,7 +2019,7 @@ subtest 'anonymize' => sub { > }; > $schema->storage->txn_rollback; > >-subtest 'get_extended_attributes' => sub { >+subtest 'extended_attributes' => sub { > plan tests => 10; > my $schema = Koha::Database->new->schema; > $schema->storage->txn_begin; >@@ -2069,17 +2069,19 @@ subtest 'get_extended_attributes' => sub { > } > ]; > >- my $extended_attributes = $patron_1->get_extended_attributes; >- is( ref($extended_attributes), 'Koha::Patron::Attributes', 'Koha::Patron->get_extended_attributes must return a Koha::Patron::Attribute set' ); >+ my $extended_attributes = $patron_1->extended_attributes; >+ is( ref($extended_attributes), 'Koha::Patron::Attributes', 'Koha::Patron->extended_attributes must return a Koha::Patron::Attribute set' ); > is( $extended_attributes->count, 0, 'There should not be attribute yet'); > >- C4::Members::Attributes::SetBorrowerAttributes($patron_1->borrowernumber, $attributes_for_1); >- C4::Members::Attributes::SetBorrowerAttributes($patron_2->borrowernumber, $attributes_for_2); >+ $patron_1->extended_attributes->filter_by_branch_limitations->delete; >+ $patron_2->extended_attributes->filter_by_branch_limitations->delete; >+ $patron_1->extended_attributes($attributes_for_1); >+ $patron_2->extended_attributes($attributes_for_2); > >- my $extended_attributes_for_1 = $patron_1->get_extended_attributes; >+ my $extended_attributes_for_1 = $patron_1->extended_attributes; > is( $extended_attributes_for_1->count, 3, 'There should be 3 attributes now for patron 1'); > >- my $extended_attributes_for_2 = $patron_2->get_extended_attributes; >+ my $extended_attributes_for_2 = $patron_2->extended_attributes; > is( $extended_attributes_for_2->count, 2, 'There should be 2 attributes now for patron 2'); > > my $attribute_12 = $extended_attributes_for_2->search({ code => $attribute_type1->code }); >@@ -2095,11 +2097,11 @@ subtest 'get_extended_attributes' => sub { > # Test branch limitations > set_logged_in_user($patron_2); > # Return all >- $extended_attributes_for_1 = $patron_1->get_extended_attributes; >+ $extended_attributes_for_1 = $patron_1->extended_attributes; > is( $extended_attributes_for_1->count, 3, 'There should be 2 attributes for patron 1, the limited one should be returned'); > > # Return filtered >- $extended_attributes_for_1 = $patron_1->get_extended_attributes->filter_by_branch_limitations; >+ $extended_attributes_for_1 = $patron_1->extended_attributes->filter_by_branch_limitations; > is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should be returned'); > > # Not filtered >diff --git a/t/db_dependent/Koha/Patrons/Import.t b/t/db_dependent/Koha/Patrons/Import.t >index e908613cc7..fd68c296c4 100644 >--- a/t/db_dependent/Koha/Patrons/Import.t >+++ b/t/db_dependent/Koha/Patrons/Import.t >@@ -562,10 +562,10 @@ subtest 'test_set_patron_attributes' => sub { > # Then ... > ok($result_3, 'Got some data back from set patron attributes'); > is($result_3->[0]->{code}, 'grade', 'Got the expected first code from set patron attributes'); >- is($result_3->[0]->{value}, '01', 'Got the expected first value from set patron attributes'); >+ is($result_3->[0]->{attribute}, '01', 'Got the expected first value from set patron attributes'); > > is($result_3->[1]->{code}, 'homeroom', 'Got the expected second code from set patron attributes'); >- is($result_3->[1]->{value}, 1150605, 'Got the expected second value from set patron attributes'); >+ is($result_3->[1]->{attribute}, 1150605, 'Got the expected second value from set patron attributes'); > > is(scalar @feedback_3, 1, 'Got the expected 1 array size from set patron attributes with extended user'); > is($feedback_3[0]->{feedback}, 1, 'Got the expected second feedback from set patron attributes with extended user'); >diff --git a/t/db_dependent/Members/Attributes.t b/t/db_dependent/Members/Attributes.t >index e5c525dce8..b1b001d432 100644 >--- a/t/db_dependent/Members/Attributes.t >+++ b/t/db_dependent/Members/Attributes.t >@@ -26,7 +26,7 @@ use Koha::Database; > use t::lib::TestBuilder; > use t::lib::Mocks; > >-use Test::More tests => 38; >+use Test::More tests => 39; > > use_ok('C4::Members::Attributes'); > >@@ -71,79 +71,83 @@ $attribute_type_limited->branches([ $new_library->{branchcode} ]); > $attribute_type_limited->store; > > $patron = Koha::Patrons->find($borrowernumber); >-my $borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 0, 'get_extended_attributes returns the correct number of borrower attributes' ); >+my $borrower_attributes = $patron->extended_attributes; >+is( $borrower_attributes->count, 0, 'extended_attributes returns the correct number of borrower attributes' ); > > my $attributes = [ > { >- value => 'my attribute1', >+ attribute => 'my attribute1', > code => $attribute_type1->code(), > }, > { >- value => 'my attribute2', >+ attribute => 'my attribute2', > code => $attribute_type2->code(), > }, > { >- value => 'my attribute limited', >+ attribute => 'my attribute limited', > code => $attribute_type_limited->code(), > } > ]; > >-my $set_borrower_attributes = C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $attributes); >-is( $set_borrower_attributes, 1, 'SetBorrowerAttributes returns the success code' ); >-$borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 3, 'get_extended_attributes returns the correct number of borrower attributes' ); >+$patron->extended_attributes->filter_by_branch_limitations->delete; >+my $set_borrower_attributes = $patron->extended_attributes($attributes); >+is( ref($set_borrower_attributes), 'Koha::Patron::Attributes', ''); >+is( $set_borrower_attributes->count, 3, ''); >+$borrower_attributes = $patron->extended_attributes; >+is( $borrower_attributes->count, 3, 'extended_attributes returns the correct number of borrower attributes' ); > my $attr_0 = $borrower_attributes->next; >-is( $attr_0->code, $attributes->[0]->{code}, 'SetBorrowerAttributes stores the correct code correctly' ); >-is( $attr_0->type->description, $attribute_type1->description(), 'SetBorrowerAttributes stores the field description correctly' ); >-is( $attr_0->attribute, $attributes->[0]->{value}, 'SetBorrowerAttributes stores the field value correctly' ); >+is( $attr_0->code, $attributes->[0]->{code}, 'setter for extended_attributes sestores the correct code correctly' ); >+is( $attr_0->type->description, $attribute_type1->description(), 'setter for extended_attributes stores the field description correctly' ); >+is( $attr_0->attribute, $attributes->[0]->{attribute}, 'setter for extended_attributes stores the field value correctly' ); > my $attr_1 = $borrower_attributes->next; >-is( $attr_1->code, $attributes->[1]->{code}, 'SetBorrowerAttributes stores the field code correctly' ); >-is( $attr_1->type->description, $attribute_type2->description(), 'SetBorrowerAttributes stores the field description correctly' ); >-is( $attr_1->attribute, $attributes->[1]->{value}, 'SetBorrowerAttributes stores the field value correctly' ); >+is( $attr_1->code, $attributes->[1]->{code}, 'setter for extended_attributes stores the field code correctly' ); >+is( $attr_1->type->description, $attribute_type2->description(), 'setter for extended_attributes stores the field description correctly' ); >+is( $attr_1->attribute, $attributes->[1]->{attribute}, 'setter for extended_attributes stores the field value correctly' ); > > $attributes = [ > { >- value => 'my attribute1', >+ attribute => 'my attribute1', > code => $attribute_type1->code(), > }, > { >- value => 'my attribute2', >+ attribute => 'my attribute2', > code => $attribute_type2->code(), > } > ]; >-C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $attributes); >-$borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 3, 'SetBorrowerAttributes should not have removed the attributes limited to another branch' ); >+$patron->extended_attributes->filter_by_branch_limitations->delete; >+$patron->extended_attributes($attributes); >+$borrower_attributes = $patron->extended_attributes; >+is( $borrower_attributes->count, 3, 'setter for extended_attributes should not have removed the attributes limited to another branch' ); > > # TODO This is not implemented yet > #$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber, undef, 'branch_limited'); > #is( @$borrower_attributes, 2, 'GetBorrowerAttributes returns the correct number of borrower attributes filtered on library' ); > > $patron = Koha::Patrons->find($borrowernumber); >-my $extended_attributes = $patron->get_extended_attributes; >+my $extended_attributes = $patron->extended_attributes; > my $attribute_value = $extended_attributes->search({ code => 'my invalid code' }); > is( $attribute_value->count, 0, 'non existent attribute should return empty result set'); > $attribute_value = $patron->get_extended_attribute('my invalid code'); > is( $attribute_value, undef, 'non existent attribute should undef'); > > $attribute_value = $patron->get_extended_attribute($attributes->[0]->{code}); >-is( $attribute_value->attribute, $attributes->[0]->{value}, 'get_extended_attribute returns the correct attribute value' ); >+is( $attribute_value->attribute, $attributes->[0]->{attribute}, 'get_extended_attribute returns the correct attribute value' ); > $attribute_value = $patron->get_extended_attribute($attributes->[1]->{code}); >-is( $attribute_value->attribute, $attributes->[1]->{value}, 'get_extended_attribute returns the correct attribute value' ); >+is( $attribute_value->attribute, $attributes->[1]->{attribute}, 'get_extended_attribute returns the correct attribute value' ); > > > my $attribute = { > attribute => 'my attribute3', > code => $attribute_type1->code(), > }; >-C4::Members::Attributes::UpdateBorrowerAttribute($borrowernumber, $attribute); >-$borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 3, 'UpdateBorrowerAttribute does not change the number of borrower attributes' ); >+$patron->extended_attributes->search({'me.code' => $attribute->{code}})->filter_by_branch_limitations->delete; >+$patron->add_extended_attribute($attribute); >+$borrower_attributes = $patron->extended_attributes; >+is( $borrower_attributes->count, 3, 'delete then add a new attribute does not change the number of borrower attributes' ); > $attr_0 = $borrower_attributes->next; >-is( $attr_0->code, $attribute->{code}, 'UpdateBorrowerAttribute updates the field code correctly' ); >-is( $attr_0->type->description, $attribute_type1->description(), 'UpdateBorrowerAttribute updates the field description correctly' ); >-is( $attr_0->attribute, $attribute->{attribute}, 'UpdateBorrowerAttribute updates the field value correctly' ); >+is( $attr_0->code, $attribute->{code}, 'delete then add a new attribute updates the field code correctly' ); >+is( $attr_0->type->description, $attribute_type1->description(), 'delete then add a new attribute updates the field description correctly' ); >+is( $attr_0->attribute, $attribute->{attribute}, 'delete then add a new attribute updates the field value correctly' ); > > > my $check_uniqueness = C4::Members::Attributes::CheckUniqueness(); >@@ -160,7 +164,7 @@ $check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, > is( $check_uniqueness, 1, 'CheckUniqueness with a new value returns true' ); > $check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', 'new value'); > is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code and a new value returns false' ); >-$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attributes->[1]->{code}, $attributes->[1]->{value}); >+$check_uniqueness = C4::Members::Attributes::CheckUniqueness($attributes->[1]->{code}, $attributes->[1]->{attribute}); > is( $check_uniqueness, 1, 'CheckUniqueness with an attribute unique_id=0 returns true' ); > $check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, $attribute->{attribute}); > is( $check_uniqueness, '', 'CheckUniqueness returns false' ); >@@ -168,22 +172,22 @@ is( $check_uniqueness, '', 'CheckUniqueness returns false' ); > > my $borrower_numbers = C4::Members::Attributes::SearchIdMatchingAttribute('attribute1'); > is( @$borrower_numbers, 0, 'SearchIdMatchingAttribute searchs only in attributes with staff_searchable=1' ); >-for my $attr( split(' ', $attributes->[1]->{value}) ) { >+for my $attr( split(' ', $attributes->[1]->{attribute}) ) { > $borrower_numbers = C4::Members::Attributes::SearchIdMatchingAttribute($attr); > is( $borrower_numbers->[0], $borrowernumber, 'SearchIdMatchingAttribute returns the borrower numbers matching' ); > } > > > $patron->get_extended_attribute($attribute->{code})->delete; >-$borrower_attributes = $patron->get_extended_attributes; >+$borrower_attributes = $patron->extended_attributes; > is( $borrower_attributes->count, 2, 'delete attribute by code' ); > $attr_0 = $borrower_attributes->next; > is( $attr_0->code, $attributes->[1]->{code}, 'delete attribute by code'); > is( $attr_0->type->description, $attribute_type2->description(), 'delete attribute by code'); >-is( $attr_0->attribute, $attributes->[1]->{value}, 'delete attribute by code'); >+is( $attr_0->attribute, $attributes->[1]->{attribute}, 'delete attribute by code'); > > $patron->get_extended_attribute($attributes->[1]->{code})->delete; >-$borrower_attributes = $patron->get_extended_attributes; >+$borrower_attributes = $patron->extended_attributes; > is( $borrower_attributes->count, 1, 'delete attribute by code' ); > > # Regression tests for bug 16504 >@@ -199,20 +203,22 @@ my $another_patron = $builder->build_object( > ); > $attributes = [ > { >- value => 'my attribute1', >+ attribute => 'my attribute1', > code => $attribute_type1->code(), > }, > { >- value => 'my attribute2', >+ attribute => 'my attribute2', > code => $attribute_type2->code(), > }, > { >- value => 'my attribute limited', >+ attribute => 'my attribute limited', > code => $attribute_type_limited->code(), > } > ]; >-C4::Members::Attributes::SetBorrowerAttributes($another_patron->borrowernumber, $attributes); >-$borrower_attributes = $another_patron->get_extended_attributes; >-is( $borrower_attributes->count, 3, 'SetBorrowerAttributes should have added the 3 attributes for another patron'); >-$borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 1, 'SetBorrowerAttributes should not have removed the attributes of other patrons' ); >+ >+$another_patron->extended_attributes->filter_by_branch_limitations->delete; >+$another_patron->extended_attributes($attributes); >+$borrower_attributes = $another_patron->extended_attributes; >+is( $borrower_attributes->count, 3, 'setter for extended_attributes should have added the 3 attributes for another patron'); >+$borrower_attributes = $patron->extended_attributes; >+is( $borrower_attributes->count, 1, 'setter for extended_attributes should not have removed the attributes of other patrons' ); >diff --git a/t/db_dependent/Utils/Datatables_Members.t b/t/db_dependent/Utils/Datatables_Members.t >index feca4f88fc..2e76da5615 100644 >--- a/t/db_dependent/Utils/Datatables_Members.t >+++ b/t/db_dependent/Utils/Datatables_Members.t >@@ -26,6 +26,7 @@ use C4::Members::Attributes; > use C4::Members::AttributeTypes; > > use Koha::Library; >+use Koha::Patrons; > use Koha::Patron::Categories; > > use t::lib::Mocks; >@@ -279,15 +280,29 @@ my $attribute_type = C4::Members::AttributeTypes->new( 'ATM_1', 'my attribute ty > $attribute_type->{staff_searchable} = 1; > $attribute_type->store; > >- >-C4::Members::Attributes::SetBorrowerAttributes( >- $john_doe->{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for a common user' } ] >+Koha::Patrons->find( $john_doe->{borrowernumber} )->extended_attributes( >+ [ >+ { >+ code => $attribute_type->{code}, >+ attribute => 'the default value for a common user' >+ } >+ ] > ); >-C4::Members::Attributes::SetBorrowerAttributes( >- $jane_doe->{borrowernumber}, [ { code => $attribute_type->{code}, value => 'the default value for another common user' } ] >+Koha::Patrons->find( $jane_doe->{borrowernumber} )->extended_attributes( >+ [ >+ { >+ code => $attribute_type->{code}, >+ attribute => 'the default value for another common user' >+ } >+ ] > ); >-C4::Members::Attributes::SetBorrowerAttributes( >- $john_smith->{borrowernumber}, [ { code => $attribute_type->{code}, value => 'Attribute which not appears even if contains "Dupont"' } ] >+Koha::Patrons->find( $john_smith->{borrowernumber} )->extended_attributes( >+ [ >+ { >+ code => $attribute_type->{code}, >+ attribute => 'Attribute which not appears even if contains "Dupont"' >+ } >+ ] > ); > > t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 1); >diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl >index e368368979..dff86263c2 100755 >--- a/tools/modborrowers.pl >+++ b/tools/modborrowers.pl >@@ -93,7 +93,7 @@ if ( $op eq 'show' ) { > if ( $patron ) { > if ( $logged_in_user->can_see_patron_infos( $patron ) ) { > my $borrower = $patron->unblessed; >- my $attributes = $patron->get_extended_attributes; >+ my $attributes = $patron->extended_attributes; > $borrower->{patron_attributes} = $attributes->as_list; > $borrower->{patron_attributes_count} = $attributes->count; > $max_nb_attr = $borrower->{patron_attributes_count} if $borrower->{patron_attributes_count} > $max_nb_attr; >@@ -393,7 +393,11 @@ if ( $op eq 'do' ) { > push @errors, { error => $@ } if $@; > } else { > eval { >- C4::Members::Attributes::UpdateBorrowerAttribute( $borrowernumber, $attribute ); >+ # Note: >+ # We should not need to filter by branch, but stay on the safe side >+ # Repeatable are not supported so we can do that - TODO >+ $patron->extended_attributes->search({'me.code' => $attribute->{code}})->filter_by_branch_limitations->delete; >+ $patron->add_extended_attribute($attribute); > }; > push @errors, { error => $@ } if $@; > } >@@ -411,7 +415,7 @@ if ( $op eq 'do' ) { > my $category_description = $patron->category->description; > my $borrower = $patron->unblessed; > $borrower->{category_description} = $category_description; >- my $attributes = $patron->get_extended_attributes; >+ my $attributes = $patron->extended_attributes; > $borrower->{patron_attributes} = $attributes->as_list; > $max_nb_attr = $attributes->count if $attributes->count > $max_nb_attr; > push @borrowers, $borrower; >-- >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