Bugzilla – Attachment 101129 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 DeleteBorrowerAttribute
Bug-20443-Remove-DeleteBorrowerAttribute.patch (text/plain), 11.84 KB, created by
Jonathan Druart
on 2020-03-20 14:47:23 UTC
(
hide
)
Description:
Bug 20443: Remove DeleteBorrowerAttribute
Filename:
MIME Type:
Creator:
Jonathan Druart
Created:
2020-03-20 14:47:23 UTC
Size:
11.84 KB
patch
obsolete
>From ad08cdf7b68af365f6450efdd4053ed3319d2961 Mon Sep 17 00:00:00 2001 >From: Jonathan Druart <jonathan.druart@bugs.koha-community.org> >Date: Fri, 13 Jul 2018 13:07:05 -0300 >Subject: [PATCH] Bug 20443: Remove DeleteBorrowerAttribute > >Signed-off-by: Signed-off-by: Nick Clemens <nick@bywatersolutions.com> >--- > C4/Members/Attributes.pm | 24 ++----------------- > Koha/Patron.pm | 14 +++++------ > opac/opac-user.pl | 4 ++-- > t/db_dependent/Auth_with_ldap.t | 2 +- > t/db_dependent/Koha/Patrons.t | 16 ++++++------- > t/db_dependent/Members/Attributes.t | 36 +++++++++++------------------ > tools/modborrowers.pl | 2 +- > 7 files changed, 34 insertions(+), 64 deletions(-) > >diff --git a/C4/Members/Attributes.pm b/C4/Members/Attributes.pm >index d52584096c..6f5001d12c 100644 >--- a/C4/Members/Attributes.pm >+++ b/C4/Members/Attributes.pm >@@ -30,7 +30,7 @@ our ($csv, $AttributeTypes); > BEGIN { > @ISA = qw(Exporter); > @EXPORT_OK = qw(CheckUniqueness SetBorrowerAttributes >- DeleteBorrowerAttribute UpdateBorrowerAttribute >+ UpdateBorrowerAttribute > extended_attributes_code_value_arrayref extended_attributes_merge > SearchIdMatchingAttribute); > %EXPORT_TAGS = ( all => \@EXPORT_OK ); >@@ -147,26 +147,6 @@ sub SetBorrowerAttributes { > return 1; # borrower attributes successfully set > } > >-=head2 DeleteBorrowerAttribute >- >- DeleteBorrowerAttribute($borrowernumber, $attribute); >- >-Delete a borrower attribute for the patron identified by C<$borrowernumber> and the attribute code of C<$attribute> >- >-=cut >- >-sub DeleteBorrowerAttribute { >- my ( $borrowernumber, $attribute ) = @_; >- >- my $dbh = C4::Context->dbh; >- my $sth = $dbh->prepare(qq{ >- DELETE FROM borrower_attributes >- WHERE borrowernumber = ? >- AND code = ? >- } ); >- $sth->execute( $borrowernumber, $attribute->{code} ); >-} >- > =head2 UpdateBorrowerAttribute > > UpdateBorrowerAttribute($borrowernumber, $attribute ); >@@ -178,7 +158,7 @@ Update a borrower attribute C<$attribute> for the patron identified by C<$borrow > sub UpdateBorrowerAttribute { > my ( $borrowernumber, $attribute ) = @_; > >- DeleteBorrowerAttribute $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 = ?"; >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index 35b666cc29..d7bea28b4f 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -1556,11 +1556,11 @@ sub add_guarantor { > )->store(); > } > >-=head3 get_extended_attribute_value >+=head3 get_extended_attribute > >-my $attribute_value = $patron->get_extended_attribute_value( $code ); >+my $attribute_value = $patron->get_extended_attribute( $code ); > >-Return the attribute's value for the code passed in parameter. >+Return the attribute for the code passed in parameter. > > It not exist it returns undef > >@@ -1571,13 +1571,13 @@ Maybe you certainly not want to use this method, it is actually only used for SH > > =cut > >-sub get_extended_attribute_value { >- my ( $self, $code ) = @_; >+sub get_extended_attribute { >+ my ( $self, $code, $value ) = @_; > my $rs = $self->_result->borrower_attributes; > return unless $rs; >- my $attribute = $rs->search( { code => $code } ); >+ my $attribute = $rs->search({ code => $code, ( $value ? ( attribute => $value ) : () ) }); > return unless $attribute->count; >- return $attribute->next->attribute; >+ return $attribute->next; > } > > =head3 to_api >diff --git a/opac/opac-user.pl b/opac/opac-user.pl >index c360363731..16a1c0f36d 100755 >--- a/opac/opac-user.pl >+++ b/opac/opac-user.pl >@@ -289,8 +289,8 @@ $template->param( overdues_count => $overdues_count ); > 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 = $patron->get_extended_attribute_value(ATTRIBUTE_SHOW_BARCODE); >- undef $show_barcode if defined($patron_show_barcode) && !$patron_show_barcode; >+ my $patron_show_barcode = $patron->get_extended_attribute(ATTRIBUTE_SHOW_BARCODE); >+ undef $show_barcode if $patron_show_barcode and not $patron_show_barcode->attribute; > } > $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 5fd03efd51..001f2a8c27 100755 >--- a/t/db_dependent/Auth_with_ldap.t >+++ b/t/db_dependent/Auth_with_ldap.t >@@ -210,7 +210,7 @@ subtest 'checkpw_ldap tests' => sub { > 'Extended attributes are not deleted' > ); > >- is( $patron->get_extended_attribute_value( $attr_type2->{code} ), 'BAR', 'Mapped attribute is BAR' ); >+ is( $patron->get_extended_attribute( $attr_type2->{code} )->attribute, 'BAR', 'Mapped attribute is BAR' ); > $auth->unmock('update_local'); > $auth->unmock('ldap_entry_2_hash'); > >diff --git a/t/db_dependent/Koha/Patrons.t b/t/db_dependent/Koha/Patrons.t >index daee4f9b89..2a39023d48 100644 >--- a/t/db_dependent/Koha/Patrons.t >+++ b/t/db_dependent/Koha/Patrons.t >@@ -2085,12 +2085,12 @@ subtest 'get_extended_attributes' => sub { > 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' ); >+ $attribute_12 = $patron_2->get_extended_attribute( $attribute_type1->code ); >+ is( $attribute_12->attribute, 'my attribute12', 'Koha::Patron->get_extended_attribute 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' ); >+ # TODO - What about multiple? POD explains the problem >+ my $non_existent = $patron_2->get_extended_attribute( 'not_exist' ); >+ is( $non_existent, undef, 'Koha::Patron->get_extended_attribute must return undef if the attribute does not exist' ); > > # Test branch limitations > set_logged_in_user($patron_2); >@@ -2103,11 +2103,11 @@ subtest 'get_extended_attributes' => sub { > is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should be returned'); > > # Not filtered >- my $limited_value = $patron_1->get_extended_attribute_value( $attribute_type_limited->code ); >- is( $limited_value, 'my attribute limited', ); >+ my $limited_value = $patron_1->get_extended_attribute( $attribute_type_limited->code ); >+ is( $limited_value->attribute, 'my attribute limited', ); > > ## Do we need a filtered? >- #$limited_value = $patron_1->get_extended_attribute_value( $attribute_type_limited->code ); >+ #$limited_value = $patron_1->get_extended_attribute( $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 33e8b3d3e9..e5c525dce8 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 => 41; >+use Test::More tests => 38; > > use_ok('C4::Members::Attributes'); > >@@ -124,13 +124,13 @@ $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'); >+$attribute_value = $patron->get_extended_attribute('my invalid code'); > is( $attribute_value, undef, 'non existent attribute should undef'); > >-$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' ); >+$attribute_value = $patron->get_extended_attribute($attributes->[0]->{code}); >+is( $attribute_value->attribute, $attributes->[0]->{value}, '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' ); > > > my $attribute = { >@@ -174,27 +174,17 @@ for my $attr( split(' ', $attributes->[1]->{value}) ) { > } > > >-C4::Members::Attributes::DeleteBorrowerAttribute(); >+$patron->get_extended_attribute($attribute->{code})->delete; > $borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 3, 'DeleteBorrowerAttribute without arguments deletes nothing' ); >-C4::Members::Attributes::DeleteBorrowerAttribute($borrowernumber); >-$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 = $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 = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 2, 'DeleteBorrowerAttribute deletes a borrower attribute' ); >+is( $borrower_attributes->count, 2, 'delete attribute by code' ); > $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'); >+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'); > >-C4::Members::Attributes::DeleteBorrowerAttribute($borrowernumber, $attributes->[1]); >+$patron->get_extended_attribute($attributes->[1]->{code})->delete; > $borrower_attributes = $patron->get_extended_attributes; >-is( $borrower_attributes->count, 1, 'DeleteBorrowerAttribute deletes a borrower attribute' ); >+is( $borrower_attributes->count, 1, 'delete attribute by code' ); > > # Regression tests for bug 16504 > t::lib::Mocks::mock_userenv({ branchcode => $new_library->{branchcode} }); >diff --git a/tools/modborrowers.pl b/tools/modborrowers.pl >index ecc0e67794..e368368979 100755 >--- a/tools/modborrowers.pl >+++ b/tools/modborrowers.pl >@@ -388,7 +388,7 @@ if ( $op eq 'do' ) { > if ( grep { $_ eq $valuename } @disabled ) { > # The attribute is disabled, we remove it for this borrower ! > eval { >- C4::Members::Attributes::DeleteBorrowerAttribute( $borrowernumber, $attribute ); >+ $patron->get_extended_attribute($attribute->{code})->delete; > }; > push @errors, { error => $@ } if $@; > } else { >-- >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