Bugzilla – Attachment 101124 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 GetBorrowerAttributeValue to Koha::Patron->get_extended_attribute_value
Bug-20443-Move-GetBorrowerAttributeValue-to-KohaPa.patch (text/plain), 13.44 KB, created by
Jonathan Druart
on 2020-03-20 14:47:04 UTC
(
hide
)
Description:
Bug 20443: Move GetBorrowerAttributeValue to Koha::Patron->get_extended_attribute_value
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 14:47:04 UTC
Size:
13.44 KB
patch
obsolete
>From 8b0fd5142ffb4eb7c01c74bf715d7b8c47fdb943 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Thu, 12 Jul 2018 17:20:57 -0300 >Subject: [PATCH] Bug 20443: Move GetBorrowerAttributeValue to > Koha::Patron->get_extended_attribute_value > >We want to retrieve a specific patron's attribute for a given patron. >We then add a new method to Koha::Patron. > >This patch add a getter method ->get_extended_attribute_value >to use the DBIx::Class relation > >Signed-off-by: Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Members/Attributes.pm | 24 +-------- > Koha/Patron.pm | 24 +++++++++ > opac/opac-user.pl | 5 +- > t/db_dependent/Auth_with_ldap.t | 6 ++- > t/db_dependent/Koha/Patrons.t | 76 +++++++++++++++++++++++++++++ > t/db_dependent/Members/Attributes.t | 33 ++++++------- > 6 files changed, 123 insertions(+), 45 deletions(-) > >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index cf9051d754..f2e78c96a4 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 GetBorrowerAttributeValue CheckUniqueness SetBorrowerAttributes >+ @EXPORT_OK = qw(GetBorrowerAttributes CheckUniqueness SetBorrowerAttributes > DeleteBorrowerAttribute UpdateBorrowerAttribute > extended_attributes_code_value_arrayref extended_attributes_merge > SearchIdMatchingAttribute); >@@ -95,28 +95,6 @@ sub GetBorrowerAttributes { > return \@results; > } > >-=head2 GetBorrowerAttributeValue >- >- my $value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber, $attribute_code); >- >-Retrieve the value of an extended attribute C<$attribute_code> associated with the >-patron specified by C<$borrowernumber>. >- >-=cut >- >-sub GetBorrowerAttributeValue { >- my $borrowernumber = shift; >- my $code = shift; >- >- my $dbh = C4::Context->dbh(); >- my $query = "SELECT attribute >- FROM borrower_attributes >- WHERE borrowernumber = ? >- AND code = ?"; >- my $value = $dbh->selectrow_array($query, undef, $borrowernumber, $code); >- return $value; >-} >- > =head2 SearchIdMatchingAttribute > > my $matching_borrowernumbers = C4::Members::Attributes::SearchIdMatchingAttribute($filter); >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 8b8b6d5ec3..c8d0981773 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1557,6 +1557,30 @@ sub add_guarantor { > )->store(); > } > >+=head3 get_extended_attribute_value >+ >+my $attribute_value = $patron->get_extended_attribute_value( $code ); >+ >+Return the attribute's value for the code passed in parameter. >+ >+It not exist it returns undef >+ >+Note that this will not work for repeatable attribute types. >+ >+Maybe you certainly not want to use this method, it is actually only used for SHOW_BARCODE >+(which should be a real patron's attribute (not extended) >+ >+=cut >+ >+sub get_extended_attribute_value { >+ my ( $self, $code ) = @_; >+ my $rs = $self->_result->borrower_attributes; >+ return unless $rs; >+ my $attribute = $rs->search( { code => $code } ); >+ return unless $attribute->count; >+ return $attribute->next->attribute; >+} >+ > =head3 to_api > > my $json = $patron->to_api; >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index 96e9bf42fb..c360363731 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -28,7 +28,6 @@ use C4::External::BakerTaylor qw( image_url link_url ); > use C4::Reserves; > use C4::Members; > use C4::Members::AttributeTypes; >-use C4::Members::Attributes qw/GetBorrowerAttributeValue/; > use C4::Output; > use C4::Biblio; > use C4::Items; >@@ -287,10 +286,10 @@ $template->param( canrenew => $canrenew ); > $template->param( OVERDUES => \@overdues ); > $template->param( overdues_count => $overdues_count ); > >-my $show_barcode = Koha::Patron::Attribute::Types->search( >+my $show_barcode = Koha::Patron::Attribute::Types->search( # FIXME we should not need this search > { code => ATTRIBUTE_SHOW_BARCODE } )->count; > if ($show_barcode) { >- my $patron_show_barcode = GetBorrowerAttributeValue($borrowernumber, ATTRIBUTE_SHOW_BARCODE); >+ my $patron_show_barcode = $patron->get_extended_attribute_value(ATTRIBUTE_SHOW_BARCODE); > undef $show_barcode if defined($patron_show_barcode) && !$patron_show_barcode; > } > $template->param( show_barcode => 1 ) if $show_barcode; >diff --git a/t/db_dependent/Auth_with_ldap.t b/t/db_dependent/Auth_with_ldap.t >index e0c3c4532c..f267a072cc 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -117,6 +117,8 @@ $builder->build( > } > ); > >+my $patron = Koha::Patrons->find($borrower->{borrowernumber}); >+ > # C4::Auth_with_ldap needs several stuff set first ^^^ > use_ok('C4::Auth_with_ldap'); > can_ok( >@@ -212,13 +214,13 @@ subtest 'checkpw_ldap tests' => sub { > 'Extended attributes are not deleted' > ); > >- is( C4::Members::Attributes::GetBorrowerAttributeValue($borrower->{borrowernumber}, $attr_type2->{code}), 'BAR', 'Mapped attribute is BAR' ); >+ is( $patron->get_extended_attribute_value( $attr_type2->{code} ), 'BAR', 'Mapped attribute is BAR' ); > $auth->unmock('update_local'); > $auth->unmock('ldap_entry_2_hash'); > > $update = 0; > $desired_count_result = 0; # user auth problem >- Koha::Patrons->find( $borrower->{borrowernumber} )->delete; >+ $patron->delete; > reload_ldap_module(); > is( > C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' ), >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index 662b8524db..f02611d98d 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -2018,3 +2018,79 @@ subtest 'anonymize' => sub { > is( $patron2->firstname, undef, 'First name patron2 cleared' ); > }; > $schema->storage->txn_rollback; >+ >+subtest 'get_extended_attributes' => sub { >+ plan tests => 7; >+ my $schema = Koha::Database->new->schema; >+ $schema->storage->txn_begin; >+ >+ my $patron_1 = $builder->build_object({class=> 'Koha::Patrons'}); >+ my $patron_2 = $builder->build_object({class=> 'Koha::Patrons'}); >+ >+ set_logged_in_user($patron_1); >+ >+ my $attribute_type1 = C4::Members::AttributeTypes->new('my code1', 'my description1'); >+ $attribute_type1->unique_id(1); >+ $attribute_type1->store(); >+ >+ my $attribute_type2 = C4::Members::AttributeTypes->new('my code2', 'my description2'); >+ $attribute_type2->opac_display(1); >+ $attribute_type2->staff_searchable(1); >+ $attribute_type2->store(); >+ >+ my $new_library = $builder->build( { source => 'Branch' } ); >+ my $attribute_type_limited = C4::Members::AttributeTypes->new('my code3', 'my description3'); >+ $attribute_type_limited->branches([ $new_library->{branchcode} ]); >+ $attribute_type_limited->store; >+ >+ my $attributes_for_1 = [ >+ { >+ value => 'my attribute1', >+ code => $attribute_type1->code(), >+ }, >+ { >+ value => 'my attribute2', >+ code => $attribute_type2->code(), >+ }, >+ { >+ value => 'my attribute limited', >+ code => $attribute_type_limited->code(), >+ } >+ ]; >+ >+ my $attributes_for_2 = [ >+ { >+ value => 'my attribute12', >+ code => $attribute_type1->code(), >+ }, >+ { >+ value => 'my attribute limited 2', >+ code => $attribute_type_limited->code(), >+ } >+ ]; >+ >+ 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' ); >+ 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); >+ >+ my $extended_attributes_for_1 = $patron_1->get_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; >+ 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 }); >+ is( $attribute_12->next->attribute, 'my attribute12', 'search by code should return the correct attribute' ); >+ >+ $attribute_12 = $patron_2->get_extended_attribute_value( $attribute_type1->code ); >+ is( $attribute_12, 'my attribute12', 'Koha::Patron->get_extended_attribute_value should return the correct attribute value' ); >+ >+ # TODO - What about multiple? >+ 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' ); >+ >+ $schema->storage->txn_rollback; >+}; >diff --git a/t/db_dependent/Members/Attributes.t b/t/db_dependent/Members/Attributes.t >index b30291d470..775a28d71d 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 => 48; >+use Test::More tests => 46; > > use_ok('C4::Members::Attributes'); > >@@ -71,9 +71,11 @@ $attribute_type_limited->branches([ $new_library->{branchcode} ]); > $attribute_type_limited->store; > > my $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, 0, 'GetBorrowerAttributes returns the correct number of borrower attributes' ); >+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 $attributes = [ > { >@@ -131,19 +133,17 @@ is( @$borrower_attributes, 3, 'SetBorrowerAttributes should not have removed the > #$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' ); > >-my $attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue(); >-is( $attribute_value, undef, 'GetBorrowerAttributeValue without arguments returns undef' ); >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber); >-is( $attribute_value, undef, 'GetBorrowerAttributeValue without the attribute code returns undef' ); >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue(undef, $attributes->[0]->{code}); >-is( $attribute_value, undef, 'GetBorrowerAttributeValue with a undef borrower number returns undef' ); >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber, 'my invalid code'); >-is( $attribute_value, undef, 'GetBorrowerAttributeValue with an invalid code retuns undef' ); >+$patron = Koha::Patrons->find($borrowernumber); >+my $extended_attributes = $patron->get_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_value('my invalid code'); >+is( $attribute_value, undef, 'non existent attribute should undef'); > >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber, $attributes->[0]->{code}); >-is( $attribute_value, $attributes->[0]->{value}, 'GetBorrowerAttributeValue returns the correct attribute value' ); >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber, $attributes->[1]->{code}); >-is( $attribute_value, $attributes->[1]->{value}, 'GetBorrowerAttributeValue returns the correct attribute value' ); >+$attribute_value = $patron->get_extended_attribute_value($attributes->[0]->{code}); >+is( $attribute_value, $attributes->[0]->{value}, 'get_extended_attribute_value returns the correct attribute value' ); >+$attribute_value = $patron->get_extended_attribute_value($attributes->[1]->{code}); >+is( $attribute_value, $attributes->[1]->{value}, 'get_extended_attribute_value returns the correct attribute value' ); > > > my $attribute = { >@@ -166,7 +166,6 @@ $check_uniqueness = C4::Members::Attributes::CheckUniqueness(undef, $attribute-> > is( $check_uniqueness, 0, 'CheckUniqueness without the argument code returns false' ); > $check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code'); > is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns false' ); >-$attribute_value = C4::Members::Attributes::GetBorrowerAttributeValue($borrowernumber, $attributes->[1]->{code}); > $check_uniqueness = C4::Members::Attributes::CheckUniqueness('my invalid code', $attribute->{attribute}); > is( $check_uniqueness, 0, 'CheckUniqueness with an invalid argument code returns fale' ); > $check_uniqueness = C4::Members::Attributes::CheckUniqueness($attribute->{code}, 'new value'); >-- >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