Bugzilla – Attachment 101126 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: Move GetBorrowerAttributes to Koha::Patron->extended_attributes
Bug-20443-Move-GetBorrowerAttributes-to-KohaPatron.patch (text/plain), 35.90 KB, created by
Jonathan Druart
on 2020-03-20 14:47:12 UTC
(
hide
)
Description:
Bug 20443: Move GetBorrowerAttributes to Koha::Patron->extended_attributes
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 14:47:12 UTC
Size:
35.90 KB
patch
obsolete
>From b49e667b690b251ef3b3f845c38c81916c87c0f2 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 12 Jul 2018 18:26:58 -0300 >Subject: [PATCH] Bug 20443: Move GetBorrowerAttributes to > Koha::Patron->extended_attributes > >The GetBorrowerAttributes subroutine return the attributes for a given >patron. > >Using get_extended_attributes we can acchieve it easily. The problematic >here is to restore the method's name (value vs attribute, >value_description vs description of the authorised value, as well as >display_checkout that should not be a method of Attribute, but >Attribute::Type instead) > >value_description was used when the attribute types were attached to an >authorised value category. To avoid the necessary test in template and >controller there is now a $attribute->description method that will >display either the attribute's value OR the value of the authorised >value when needed. We should certainly use this one from few other >places. > >Notes: >* This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes. >It will be renamed with Koha::Patron->extended_attributes in ones of the next >patches when it will become a setter as well. >* GetBorrowerAttributes did not care about the library limits, we still >do not >* The opac_only flag was not used outside of test, we drop it off. >* To maintain the existing behavior we add a default order-by clause to >the search method [code, attribute] >* From C4::Letters::_parseletter we always display the staff description >of the AV, There is now a FIXME to warn about it >* FIXMEs are not regressions, existing behaviors must be kept >* TODO add a new check to bug 21010 to search for inconsistencies in >patron's attributes attached to non-existent authorised values >* One test has been updated in Modifications.t, order_by is now >by default set to ['code', 'attribute'] > >Signed-off-by: Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Letters.pm | 11 ++- > C4/Members/Attributes.pm | 51 +--------- > Koha/Patron.pm | 13 ++- > Koha/Patron/Attribute.pm | 47 ++++++++- > Koha/Patron/Attributes.pm | 1 + > Koha/Patrons/Import.pm | 2 +- > .../prog/en/includes/circ-menu.inc | 10 +- > .../prog/en/modules/members/moremember.tt | 8 +- > .../prog/en/modules/tools/modborrowers.tt | 3 +- > members/memberentry.pl | 7 +- > members/moremember.pl | 9 +- > opac/opac-memberentry.pl | 1 - > t/db_dependent/Auth_with_ldap.t | 6 +- > t/db_dependent/Koha/Patron/Modifications.t | 21 ++-- > t/db_dependent/Koha/Patrons.t | 11 ++- > t/db_dependent/Members/Attributes.t | 97 ++++++++----------- > tools/modborrowers.pl | 29 +++--- > 17 files changed, 161 insertions(+), 166 deletions(-) > >diff --git a/C4/Letters.pm b/C4/Letters.pm >index 7c5d470bb0..a5bb9ead64 100644 >--- a/C4/Letters.pm >+++ b/C4/Letters.pm >@@ -28,7 +28,6 @@ use Template; > use Module::Load::Conditional qw(can_load); > > use C4::Members; >-use C4::Members::Attributes qw(GetBorrowerAttributes); > use C4::Log; > use C4::SMS; > use C4::Debug; >@@ -852,11 +851,13 @@ sub _parseletter { > } > > if ($table eq 'borrowers' && $letter->{content}) { >- if ( my $attributes = GetBorrowerAttributes($values->{borrowernumber}) ) { >+ my $patron = Koha::Patrons->find( $values->{borrowernumber} ); >+ if ( $patron ) { >+ my $attributes = $patron->get_extended_attributes; > my %attr; >- foreach (@$attributes) { >- my $code = $_->{code}; >- my $val = $_->{value_description} || $_->{value}; >+ while ( my $attribute = $attributes->next ) { >+ my $code = $attribute->code; >+ my $val = $attribute->description; # FIXME - we always display intranet description here! > $val =~ s/\p{P}(?=$)//g if $val; > next unless $val gt ''; > $attr{$code} ||= []; >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index f2e78c96a4..96fb139410 100644 >--- a/C4/Members/Attributes.pm >+++ b/C4/Members/Attributes.pm >@@ -29,7 +29,7 @@ our ($csv, $AttributeTypes); > > BEGIN { > @ISA = qw(Exporter); >- @EXPORT_OK = qw(GetBorrowerAttributes CheckUniqueness SetBorrowerAttributes >+ @EXPORT_OK = qw(CheckUniqueness SetBorrowerAttributes > DeleteBorrowerAttribute UpdateBorrowerAttribute > extended_attributes_code_value_arrayref extended_attributes_merge > SearchIdMatchingAttribute); >@@ -43,58 +43,9 @@ C4::Members::Attributes - manage extend patron attributes > =head1 SYNOPSIS > > use C4::Members::Attributes; >- my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); > > =head1 FUNCTIONS > >-=head2 GetBorrowerAttributes >- >- my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber[, $opac_only]); >- >-Retrieve an arrayref of extended attributes associated with the >-patron specified by C<$borrowernumber>. Each entry in the arrayref >-is a hashref containing the following keys: >- >-code (attribute type code) >-description (attribute type description) >-value (attribute value) >-value_description (attribute value description (if associated with an authorised value)) >- >-If the C<$opac_only> parameter is present and has a true value, only the attributes >-marked for OPAC display are returned. >- >-=cut >- >-sub GetBorrowerAttributes { >- my $borrowernumber = shift; >- my $opac_only = @_ ? shift : 0; >- >- my $dbh = C4::Context->dbh(); >- my $query = "SELECT code, description, attribute, lib, display_checkout, category_code, class >- FROM borrower_attributes >- JOIN borrower_attribute_types USING (code) >- LEFT JOIN authorised_values ON (category = authorised_value_category AND attribute = authorised_value) >- WHERE borrowernumber = ?"; >- $query .= "\nAND opac_display = 1" if $opac_only; >- $query .= "\nORDER BY code, attribute"; >- my $sth = $dbh->prepare_cached($query); >- $sth->execute($borrowernumber); >- my @results = (); >- while (my $row = $sth->fetchrow_hashref()) { >- push @results, { >- code => $row->{'code'}, >- description => $row->{'description'}, >- value => $row->{'attribute'}, >- value_description => $row->{'lib'}, >- display_checkout => $row->{'display_checkout'}, >- category_code => $row->{'category_code'}, >- class => $row->{'class'}, >- } >- } >- $sth->finish; >- return \@results; >-} >- > =head2 SearchIdMatchingAttribute > > my $matching_borrowernumbers = C4::Members::Attributes::SearchIdMatchingAttribute($filter); >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index c8d0981773..35b666cc29 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1438,20 +1438,19 @@ sub generate_userid { > return $self; > } > >-=head3 attributes >+=head3 get_extended_attributes > >-my $attributes = $patron->attributes >+my $attributes = $patron->get_extended_attributes > > Return object of Koha::Patron::Attributes type with all attributes set for this patron > > =cut > >-sub attributes { >+sub get_extended_attributes { > my ( $self ) = @_; >- return Koha::Patron::Attributes->search({ >- borrowernumber => $self->borrowernumber, >- branchcode => $self->branchcode, >- }); >+ 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; > } > > =head3 lock >diff --git a/Koha/Patron/Attribute.pm b/Koha/Patron/Attribute.pm >index 0a44cbf49b..5dd63e952f 100644 >--- a/Koha/Patron/Attribute.pm >+++ b/Koha/Patron/Attribute.pm >@@ -64,9 +64,54 @@ sub type { > > my $self = shift; > >- return Koha::Patron::Attribute::Types->find( $self->code ); >+ return scalar Koha::Patron::Attribute::Types->find( $self->code ); > } > >+=head3 >+ >+my $authorised_value = $attribute->authorised_value; >+ >+Return the Koha::AuthorisedValue object of this attribute when one is attached. >+ >+Return undef if this attribute is not attached to an authorised value >+ >+=cut >+ >+sub authorised_value { >+ my ($self) = @_; >+ >+ return unless $self->type->authorised_value_category; >+ >+ my $av = Koha::AuthorisedValues->search( >+ { >+ category => $self->type->authorised_value_category, >+ authorised_value => $self->attribute, >+ } >+ ); >+ return unless $av->count; # Data inconsistency >+ return $av->next; >+} >+ >+=head3 >+ >+my $description = $patron_attribute->description; >+ >+Return the value of this attribute or the description of the authorised value (when attached). >+ >+This method must be called when the authorised value's description must be >+displayed instead of the code. >+ >+=cut >+ >+sub description { >+ my ( $self) = @_; >+ if ( $self->type->authorised_value_category ) { >+ return $self->authorised_value->lib; >+ } >+ return $self->attribute; >+} >+ >+ > =head2 Internal methods > > =head3 _check_repeatable >diff --git a/Koha/Patron/Attributes.pm b/Koha/Patron/Attributes.pm >index 587189ff4d..0cb6875daf 100644 >--- a/Koha/Patron/Attributes.pm >+++ b/Koha/Patron/Attributes.pm >@@ -60,6 +60,7 @@ sub search { > }, > } : {}; > $attributes //= {}; >+ unless ( exists $attributes->{order_by} ) { $attributes->{order_by} = ['code', 'attribute'] } > return $self->SUPER::search( { %$params, %$or }, { %$attributes, %$join } ); > } > >diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm >index 91ab8fbc4c..f1473678ce 100644 >--- a/Koha/Patrons/Import.pm >+++ b/Koha/Patrons/Import.pm >@@ -290,7 +290,7 @@ sub import_patrons { > } > if ($extended) { > if ($ext_preserve) { >- my $old_attributes = GetBorrowerAttributes($borrowernumber); >+ my $old_attributes = $patron->get_extended_attributes->as_list; > $patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes ); > } > push @errors, { unknown_error => 1 } >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 95ace5e248..fbb4a2b24a 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc >@@ -71,9 +71,13 @@ > [% END %] > > [% IF Koha.Preference('ExtendedPatronAttributes') %] >- [% FOREACH extendedattribute IN patron.attributes %] >- [% IF ( extendedattribute.display_checkout ) %] >- <li class="patronattribute"><span class="patronattributelabel">[% extendedattribute.type_description | html %]</span> : [% IF ( extendedattribute.value_description ) %][% extendedattribute.value_description | html %][% ELSE %][% extendedattribute.attribute | html %][% END %]</li> >+ [% FOREACH extendedattribute IN patron.get_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"> >+ <span class="patronattributelabel">[% extendedattribute.type.description | html %]</span>: [% extendedattribute.description | html %] >+ </li> >+ [% END %] > [% END %] > [% END %] > [% END %] >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt >index ab3e89abf7..4953249861 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt >@@ -381,12 +381,8 @@ > <ol> > [% FOREACH item IN attribute.items %] > <li> >- <span class="label">[% item.description | html %]: </span> >- [% IF ( item.value_description ) %] >- [% item.value_description | html %] >- [% ELSE %] >- [% item.value| html | html_line_break %] >- [% END %] >+ <span class="label">[% item.type.description | html %]: </span> >+ [% item.description | html_line_break %] > </li> > [% END %] > </ol> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt >index adcb860fb3..9147d522f9 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/modborrowers.tt >@@ -191,7 +191,8 @@ > <td>[% borrower.debarredcomment | html %]</td> > [% FOREACH pa IN borrower.patron_attributes %] > [% IF ( pa.code ) %] >- <td>[% pa.code | html %]=[% pa.value | html %]</td> >+ [%# Replace pa.attribute with pa.description if we prefer to display the description %] >+ <td>[% pa.code | html %]=[% pa.attribute | html %]</td> > [% ELSE %] > <td></td> > [% END %] >diff --git a/members/memberentry.pl b/members/memberentry.pl >index 902beeb0e8..8bc4af09ed 100755 >--- a/members/memberentry.pl >+++ b/members/memberentry.pl >@@ -857,13 +857,14 @@ sub patron_attributes_form { > $template->param(no_patron_attribute_types => 1); > return; > } >- my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >- my @classes = uniq( map {$_->{class}} @$attributes ); >+ 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 @classes = uniq( map {$_->type->class} @attributes ); > @classes = sort @classes; > > # map patron's attributes into a more convenient structure > my %attr_hash = (); >- foreach my $attr (@$attributes) { >+ foreach my $attr (@attributes) { > push @{ $attr_hash{$attr->{code}} }, $attr; > } > >diff --git a/members/moremember.pl b/members/moremember.pl >index f083203d4a..b14148821a 100755 >--- a/members/moremember.pl >+++ b/members/moremember.pl >@@ -35,7 +35,6 @@ use C4::Output; > use C4::Members::AttributeTypes; > use C4::Form::MessagingPreferences; > use List::MoreUtils qw/uniq/; >-use C4::Members::Attributes qw(GetBorrowerAttributes); > use Koha::Patron::Debarments qw(GetDebarments); > use Koha::Patron::Messages; > use Koha::DateUtils; >@@ -131,15 +130,15 @@ $template->param( > ); > > if (C4::Context->preference('ExtendedPatronAttributes')) { >- my $attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >- my @classes = uniq( map {$_->{class}} @$attributes ); >+ my @attributes = $patron->get_extended_attributes->as_list; # FIXME Must be improved! >+ my @classes = uniq( map {$_->type->class} @attributes ); > @classes = sort @classes; > > my @attributes_loop; > for my $class (@classes) { > my @items; >- for my $attr (@$attributes) { >- push @items, $attr if $attr->{class} eq $class >+ for my $attr (@attributes) { >+ push @items, $attr if $attr->type->class eq $class > } > my $av = Koha::AuthorisedValues->search({ category => 'PA_CLASS', authorised_value => $class }); > my $lib = $av->count ? $av->next->lib : $class; >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index e92af011a1..3d7770070c 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -26,7 +26,6 @@ use String::Random qw( random_string ); > use C4::Auth; > use C4::Output; > use C4::Members; >-use C4::Members::Attributes qw( GetBorrowerAttributes ); > use C4::Form::MessagingPreferences; > use Koha::AuthUtils; > use Koha::Patrons; >diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t >index f267a072cc..5fd03efd51 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -206,11 +206,7 @@ subtest 'checkpw_ldap tests' => sub { > > C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ); > ok( >- @{ >- C4::Members::Attributes::GetBorrowerAttributes( >- $borrower->{borrowernumber} >- ) >- }, >+ Koha::Patrons->find($borrower->{borrowernumber})->get_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 65edba477f..ce279adc6d 100755 >--- a/t/db_dependent/Koha/Patron/Modifications.t >+++ b/t/db_dependent/Koha/Patron/Modifications.t >@@ -30,7 +30,6 @@ use Try::Tiny; > > use C4::Context; > use C4::Members; >-use C4::Members::Attributes qw( GetBorrowerAttributes ); > use Koha::Patrons; > use Koha::Patron::Attribute; > >@@ -175,14 +174,16 @@ subtest 'approve tests' => sub { > ); > is( $patron->firstname, 'Kyle', > 'Patron modification set the right firstname' ); >- my @patron_attributes = GetBorrowerAttributes( $patron->borrowernumber ); >- is( $patron_attributes[0][0]->{code}, >+ my $patron_attributes = $patron->get_extended_attributes; >+ my $attribute_1 = $patron_attributes->next; >+ is( $attribute_1->code, > 'CODE_1', 'Patron modification correctly saved attribute code' ); >- is( $patron_attributes[0][0]->{value}, >+ is( $attribute_1->attribute, > 'VALUE_1', 'Patron modification correctly saved attribute value' ); >- is( $patron_attributes[0][1]->{code}, >+ my $attribute_2 = $patron_attributes->next; >+ is( $attribute_2->code, > 'CODE_2', 'Patron modification correctly saved attribute code' ); >- is( $patron_attributes[0][1]->{value}, >+ is( $attribute_2->attribute, > 0, 'Patron modification correctly saved attribute with value 0, not confused with delete' ); > > # Create a new Koha::Patron::Modification, skip extended_attributes to >@@ -219,7 +220,7 @@ subtest 'approve tests' => sub { > )->store(); > ok( $patron_modification->approve, > 'Patron modification correctly approved' ); >- @patron_attributes >+ my @patron_attributes > = map { $_->unblessed } > Koha::Patron::Attributes->search( > { borrowernumber => $patron->borrowernumber } ); >@@ -232,12 +233,12 @@ subtest 'approve tests' => sub { > is( $patron_attributes[1]->{code}, > 'CODE_2', 'Attribute updated correctly (code)' ); > is( $patron_attributes[1]->{attribute}, >- 'Tomasito', 'Attribute updated correctly (attribute)' ); >+ 'None', 'Attribute updated correctly (attribute)' ); > > is( $patron_attributes[2]->{code}, > 'CODE_2', 'Attribute updated correctly (code)' ); > is( $patron_attributes[2]->{attribute}, >- 'None', 'Attribute updated correctly (attribute)' ); >+ 'Tomasito', 'Attribute updated correctly (attribute)' ); > > my $empty_code_json = '[{"code":"CODE_2","value":""}]'; > $verification_token = md5_hex( time() . {} . rand() . {} . $$ ); >@@ -251,7 +252,7 @@ subtest 'approve tests' => sub { > )->store(); > ok( $patron_modification->approve, > 'Patron modification correctly approved' ); >- @patron_attributes >+ $patron_attributes > = map { $_->unblessed } > Koha::Patron::Attributes->search( > { borrowernumber => $patron->borrowernumber } ); >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index f02611d98d..146bdbfb95 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -2020,7 +2020,7 @@ subtest 'anonymize' => sub { > $schema->storage->txn_rollback; > > subtest 'get_extended_attributes' => sub { >- plan tests => 7; >+ plan tests => 9; > my $schema = Koha::Database->new->schema; > $schema->storage->txn_begin; > >@@ -2092,5 +2092,14 @@ subtest 'get_extended_attributes' => sub { > my $non_existent = $patron_2->get_extended_attribute_value( 'not_exist' ); > is( $non_existent, undef, 'Koha::Patron->get_extended_attribute_value must return undef if the attribute does not exist' ); > >+ # Test branch limitations >+ set_logged_in_user($patron_2); >+ C4::Members::Attributes::SetBorrowerAttributes($patron_1->borrowernumber, $attributes_for_1); >+ $extended_attributes_for_1 = $patron_1->get_extended_attributes; >+ is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should not be returned'); >+ >+ my $limited_value = $patron_1->get_extended_attribute_value( $attribute_type_limited->code ); >+ is( $limited_value, undef, ); >+ > $schema->storage->txn_rollback; > }; >diff --git a/t/db_dependent/Members/Attributes.t b/t/db_dependent/Members/Attributes.t >index 775a28d71d..33e8b3d3e9 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 => 46; >+use Test::More tests => 41; > > use_ok('C4::Members::Attributes'); > >@@ -70,12 +70,9 @@ my $attribute_type_limited = C4::Members::AttributeTypes->new('my code3', 'my de > $attribute_type_limited->branches([ $new_library->{branchcode} ]); > $attribute_type_limited->store; > >-my $borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes(); >-ok(1); >-#is( @$borrower_attributes, 0, 'GetBorrowerAttributes without the borrower number returns an empty array' ); > $patron = Koha::Patrons->find($borrowernumber); >-$borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 0, 'GetBorrowerAttributes returns the correct number of borrower attributes' ); >+my $borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 0, 'get_extended_attributes returns the correct number of borrower attributes' ); > > my $attributes = [ > { >@@ -92,28 +89,18 @@ my $attributes = [ > } > ]; > >-my $set_borrower_attributes = C4::Members::Attributes::SetBorrowerAttributes(); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes(); >-is( @$borrower_attributes, 0, 'SetBorrowerAttributes without arguments does not add borrower attributes' ); >- >-$set_borrower_attributes = C4::Members::Attributes::SetBorrowerAttributes($borrowernumber); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes(); >-is( @$borrower_attributes, 0, 'SetBorrowerAttributes without the attributes does not add borrower attributes' ); >- >-$set_borrower_attributes = C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $attributes); >+my $set_borrower_attributes = C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $attributes); > is( $set_borrower_attributes, 1, 'SetBorrowerAttributes returns the success code' ); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes(); >-is( @$borrower_attributes, 0, 'GetBorrowerAttributes without the borrower number returns an empty array' ); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'GetBorrowerAttributes returns the correct number of borrower attributes' ); >-is( $borrower_attributes->[0]->{code}, $attributes->[0]->{code}, 'SetBorrowerAttributes stores the correct code correctly' ); >-is( $borrower_attributes->[0]->{description}, $attribute_type1->description(), 'SetBorrowerAttributes stores the field description correctly' ); >-is( $borrower_attributes->[0]->{value}, $attributes->[0]->{value}, 'SetBorrowerAttributes stores the field value correctly' ); >-is( $borrower_attributes->[1]->{code}, $attributes->[1]->{code}, 'SetBorrowerAttributes stores the field code correctly' ); >-is( $borrower_attributes->[1]->{description}, $attribute_type2->description(), 'SetBorrowerAttributes stores the field description correctly' ); >-is( $borrower_attributes->[1]->{value}, $attributes->[1]->{value}, 'SetBorrowerAttributes stores the field value correctly' ); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'GetBorrowerAttributes returns the correct number of borrower attributes' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'get_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' ); >+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' ); > > $attributes = [ > { >@@ -126,8 +113,8 @@ $attributes = [ > } > ]; > C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $attributes); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'SetBorrowerAttributes should not have removed the attributes limited to another branch' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'SetBorrowerAttributes 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'); >@@ -151,11 +138,12 @@ my $attribute = { > code => $attribute_type1->code(), > }; > C4::Members::Attributes::UpdateBorrowerAttribute($borrowernumber, $attribute); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'UpdateBorrowerAttribute does not change the number of borrower attributes' ); >-is( $borrower_attributes->[0]->{code}, $attribute->{code}, 'UpdateBorrowerAttribute updates the field code correctly' ); >-is( $borrower_attributes->[0]->{description}, $attribute_type1->description(), 'UpdateBorrowerAttribute updates the field description correctly' ); >-is( $borrower_attributes->[0]->{value}, $attribute->{attribute}, 'UpdateBorrowerAttribute updates the field value correctly' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'UpdateBorrowerAttribute 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' ); > > > my $check_uniqueness = C4::Members::Attributes::CheckUniqueness(); >@@ -187,30 +175,31 @@ for my $attr( split(' ', $attributes->[1]->{value}) ) { > > > C4::Members::Attributes::DeleteBorrowerAttribute(); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'DeleteBorrowerAttribute without arguments deletes nothing' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'DeleteBorrowerAttribute without arguments deletes nothing' ); > C4::Members::Attributes::DeleteBorrowerAttribute($borrowernumber); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'DeleteBorrowerAttribute without the attribute deletes nothing' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'DeleteBorrowerAttribute without the attribute deletes nothing' ); > C4::Members::Attributes::DeleteBorrowerAttribute(undef, $attribute); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 3, 'DeleteBorrowerAttribute with a undef borrower number deletes nothing' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 3, 'DeleteBorrowerAttribute with a undef borrower number deletes nothing' ); > > C4::Members::Attributes::DeleteBorrowerAttribute($borrowernumber, $attribute); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 2, 'DeleteBorrowerAttribute deletes a borrower attribute' ); >-is( $borrower_attributes->[0]->{code}, $attributes->[1]->{code}, 'DeleteBorrowerAttribute deletes the correct entry'); >-is( $borrower_attributes->[0]->{description}, $attribute_type2->description(), 'DeleteBorrowerAttribute deletes the correct entry'); >-is( $borrower_attributes->[0]->{value}, $attributes->[1]->{value}, 'DeleteBorrowerAttribute deletes the correct entry'); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 2, 'DeleteBorrowerAttribute deletes a borrower attribute' ); >+$attr_0 = $borrower_attributes->next; >+is( $attr_0->code, $attributes->[1]->{code}, 'DeleteBorrowerAttribute deletes the correct entry'); >+is( $attr_0->type->description, $attribute_type2->description(), 'DeleteBorrowerAttribute deletes the correct entry'); >+is( $attr_0->attribute, $attributes->[1]->{value}, 'DeleteBorrowerAttribute deletes the correct entry'); > > C4::Members::Attributes::DeleteBorrowerAttribute($borrowernumber, $attributes->[1]); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 1, 'DeleteBorrowerAttribute deletes a borrower attribute' ); >+$borrower_attributes = $patron->get_extended_attributes; >+is( $borrower_attributes->count, 1, 'DeleteBorrowerAttribute deletes a borrower attribute' ); > > # Regression tests for bug 16504 > t::lib::Mocks::mock_userenv({ branchcode => $new_library->{branchcode} }); >-my $another_patron = $builder->build( >- { source => 'Borrower', >+my $another_patron = $builder->build_object( >+ { class => 'Koha::Patrons', > value => { > firstname => 'my another firstname', > surname => 'my another surname', >@@ -232,8 +221,8 @@ $attributes = [ > code => $attribute_type_limited->code(), > } > ]; >-C4::Members::Attributes::SetBorrowerAttributes($another_patron->{borrowernumber}, $attributes); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($another_patron->{borrowernumber}); >-is( @$borrower_attributes, 3, 'SetBorrowerAttributes should have added the 3 attributes for another patron'); >-$borrower_attributes = C4::Members::Attributes::GetBorrowerAttributes($borrowernumber); >-is( @$borrower_attributes, 1, 'SetBorrowerAttributes should not have removed the attributes of other patrons' ); >+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' ); >diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl >index 1a08a8ffbd..ecc0e67794 100755 >--- a/tools/modborrowers.pl >+++ b/tools/modborrowers.pl >@@ -92,11 +92,12 @@ if ( $op eq 'show' ) { > my $patron = Koha::Patrons->find( { cardnumber => $cardnumber } ); > if ( $patron ) { > if ( $logged_in_user->can_see_patron_infos( $patron ) ) { >- $patron = $patron->unblessed; >- $patron->{patron_attributes} = C4::Members::Attributes::GetBorrowerAttributes( $patron->{borrowernumber} ); >- $max_nb_attr = scalar( @{ $patron->{patron_attributes} } ) >- if scalar( @{ $patron->{patron_attributes} } ) > $max_nb_attr; >- push @borrowers, $patron; >+ my $borrower = $patron->unblessed; >+ my $attributes = $patron->get_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; >+ push @borrowers, $borrower; > } else { > push @notfoundcardnumbers, $cardnumber; > } >@@ -107,7 +108,7 @@ if ( $op eq 'show' ) { > > # Just for a correct display > for my $borrower ( @borrowers ) { >- my $length = scalar( @{ $borrower->{patron_attributes} } ); >+ my $length = $borrower->{patron_attributes_count}; > push @{ $borrower->{patron_attributes} }, {} for ( $length .. $max_nb_attr - 1); > } > >@@ -373,7 +374,7 @@ if ( $op eq 'do' ) { > } > } > >- my $borrower_categorycode = Koha::Patrons->find( $borrowernumber )->categorycode; >+ my $patron = Koha::Patrons->find( $borrowernumber ); > my $i=0; > for ( @attributes ) { > next unless $_; >@@ -382,7 +383,7 @@ if ( $op eq 'do' ) { > $attribute->{attribute} = $attr_values[$i]; > my $attr_type = C4::Members::AttributeTypes->fetch( $_ ); > # If this borrower is not in the category of this attribute, we don't want to modify this attribute >- ++$i and next if $attr_type->{category_code} and $attr_type->{category_code} ne $borrower_categorycode; >+ ++$i and next if $attr_type->{category_code} and $attr_type->{category_code} ne $patron->category_code; > my $valuename = "attr" . $i . "_value"; > if ( grep { $_ eq $valuename } @disabled ) { > # The attribute is disabled, we remove it for this borrower ! >@@ -407,11 +408,13 @@ if ( $op eq 'do' ) { > for my $borrowernumber ( @borrowernumbers ) { > my $patron = Koha::Patrons->find( $borrowernumber ); > if ( $patron ) { >- $patron = $patron->unblessed; >- $patron->{patron_attributes} = C4::Members::Attributes::GetBorrowerAttributes( $patron->{borrowernumber} ); >- $max_nb_attr = scalar( @{ $patron->{patron_attributes} } ) >- if scalar( @{ $patron->{patron_attributes} } ) > $max_nb_attr; >- push @borrowers, $patron; >+ my $category_description = $patron->category->description; >+ my $borrower = $patron->unblessed; >+ $borrower->{category_description} = $category_description; >+ my $attributes = $patron->get_extended_attributes; >+ $borrower->{patron_attributes} = $attributes->as_list; >+ $max_nb_attr = $attributes->count if $attributes->count > $max_nb_attr; >+ push @borrowers, $borrower; > } > } > my @patron_attributes_option; >-- >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