Skip to content

Commit ccfc657

Browse files
joubumrenvoize
joubu
authored and
mrenvoize
committed
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> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
1 parent a1e3a79 commit ccfc657

File tree

18 files changed

+157
-147
lines changed

18 files changed

+157
-147
lines changed

‎C4/Auth_with_ldap.pm

+5-1
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,11 @@ sub checkpw_ldap {
239239
next;
240240
}
241241
if (C4::Members::Attributes::CheckUniqueness($code, $borrower{$code}, $borrowernumber)) {
242-
C4::Members::Attributes::UpdateBorrowerAttribute($borrowernumber, {code => $code, attribute => $borrower{$code}});
242+
my $patron = Koha::Patrons->find($borrowernumber);
243+
if ( $patron ) { # Should not be needed, but we are in C4::Auth LDAP...
244+
my $attribute = Koha::Patron::Attribute->new({code => $code, attribute => $borrower{$code}});
245+
$patron->extended_attributes([$attribute]);
246+
}
243247
} else {
244248
warn "ERROR_extended_unique_id_failed $code $borrower{$code}";
245249
}

‎C4/ILSDI/Services.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -506,7 +506,7 @@ sub GetPatronInfo {
506506
if ( $show_attributes && $show_attributes eq "1" ) {
507507
# FIXME Regression expected here, we do not retrieve the same field as previously
508508
# Waiting for answer on bug 14257 comment 15
509-
my $attrs = $patron->get_extended_attributes->search({ opac_display => 1 })->unblessed;
509+
my $attrs = $patron->extended_attributes->search({ opac_display => 1 })->unblessed;
510510
$borrower->{'attributes'} = $attrs;
511511
}
512512

‎C4/Letters.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -853,7 +853,7 @@ sub _parseletter {
853853
if ($table eq 'borrowers' && $letter->{content}) {
854854
my $patron = Koha::Patrons->find( $values->{borrowernumber} );
855855
if ( $patron ) {
856-
my $attributes = $patron->get_extended_attributes;
856+
my $attributes = $patron->extended_attributes;
857857
my %attr;
858858
while ( my $attribute = $attributes->next ) {
859859
my $code = $attribute->code;

‎C4/Members.pm

+1-1
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use C4::Reserves;
3333
use C4::Accounts;
3434
use C4::Biblio;
3535
use C4::Letters;
36-
use C4::Members::Attributes qw(SearchIdMatchingAttribute UpdateBorrowerAttribute);
36+
use C4::Members::Attributes qw(SearchIdMatchingAttribute);
3737
use C4::NewsChannels; #get slip news
3838
use DateTime;
3939
use Koha::Database;

‎C4/Members/Attributes.pm

+4-62
Original file line numberDiff line numberDiff line change
@@ -29,8 +29,7 @@ our ($csv, $AttributeTypes);
2929

3030
BEGIN {
3131
@ISA = qw(Exporter);
32-
@EXPORT_OK = qw(CheckUniqueness SetBorrowerAttributes
33-
UpdateBorrowerAttribute
32+
@EXPORT_OK = qw(CheckUniqueness
3433
extended_attributes_code_value_arrayref extended_attributes_merge
3534
SearchIdMatchingAttribute);
3635
%EXPORT_TAGS = ( all => \@EXPORT_OK );
@@ -112,71 +111,14 @@ sub CheckUniqueness {
112111
return ($count == 0);
113112
}
114113

115-
=head2 SetBorrowerAttributes
116-
117-
SetBorrowerAttributes($borrowernumber, [ { code => 'CODE', value => 'value' }, ... ] );
118-
119-
Set patron attributes for the patron identified by C<$borrowernumber>,
120-
replacing any that existed previously.
121-
122-
=cut
123-
124-
sub SetBorrowerAttributes {
125-
my $borrowernumber = shift;
126-
my $attr_list = shift;
127-
my $no_branch_limit = shift // 0;
128-
129-
my $dbh = C4::Context->dbh;
130-
131-
my $attributes = Koha::Patrons->find($borrowernumber)->get_extended_attributes;
132-
133-
unless ( $no_branch_limit ) {
134-
$attributes = $attributes->filter_by_branch_limitations;
135-
}
136-
$attributes->delete;
137-
138-
my $sth = $dbh->prepare("INSERT INTO borrower_attributes (borrowernumber, code, attribute)
139-
VALUES (?, ?, ?)");
140-
foreach my $attr (@$attr_list) {
141-
$sth->execute($borrowernumber, $attr->{code}, $attr->{value});
142-
if ($sth->err) {
143-
warn sprintf('Database returned the following error: %s', $sth->errstr);
144-
return; # bail immediately on errors
145-
}
146-
}
147-
return 1; # borrower attributes successfully set
148-
}
149-
150-
=head2 UpdateBorrowerAttribute
151-
152-
UpdateBorrowerAttribute($borrowernumber, $attribute );
153-
154-
Update a borrower attribute C<$attribute> for the patron identified by C<$borrowernumber>,
155-
156-
=cut
157-
158-
sub UpdateBorrowerAttribute {
159-
my ( $borrowernumber, $attribute ) = @_;
160-
161-
Koha::Patrons->find($borrowernumber)->get_extended_attributes->search({ 'me.code' => $attribute->{code} })->filter_by_branch_limitations->delete;
162-
163-
my $dbh = C4::Context->dbh;
164-
my $query = "INSERT INTO borrower_attributes SET attribute = ?, code = ?, borrowernumber = ?";
165-
my @params = ( $attribute->{attribute}, $attribute->{code}, $borrowernumber );
166-
my $sth = $dbh->prepare( $query );
167-
168-
$sth->execute( @params );
169-
}
170-
171-
172114
=head2 extended_attributes_code_value_arrayref
173115
174116
my $patron_attributes = "homeroom:1150605,grade:01,extradata:foobar";
175117
my $aref = extended_attributes_code_value_arrayref($patron_attributes);
176118
177-
Takes a comma-delimited CSV-style string argument and returns the kind of data structure that SetBorrowerAttributes wants,
119+
Takes a comma-delimited CSV-style string argument and returns the kind of data structure that Koha::Patron->extended_attributes wants,
178120
namely a reference to array of hashrefs like:
179-
[ { code => 'CODE', value => 'value' }, { code => 'CODE2', value => 'othervalue' } ... ]
121+
[ { code => 'CODE', attribute => 'value' }, { code => 'CODE2', attribute => 'othervalue' } ... ]
180122
181123
Caches Text::CSV parser object for efficiency.
182124
@@ -190,7 +132,7 @@ sub extended_attributes_code_value_arrayref {
190132
# TODO: error handling (check $ok)
191133
return [
192134
sort {&_sort_by_code($a,$b)}
193-
map { map { my @arr = split /:/, $_, 2; { code => $arr[0], value => $arr[1] } } $_ }
135+
map { map { my @arr = split /:/, $_, 2; { code => $arr[0], attribute => $arr[1] } } $_ }
194136
@list
195137
];
196138
# nested map because of split

‎Koha/Patron.pm

+29-4
Original file line numberDiff line numberDiff line change
@@ -1438,16 +1438,41 @@ sub generate_userid {
14381438
return $self;
14391439
}
14401440

1441-
=head3 get_extended_attributes
1441+
=head3 add_extended_attribute
14421442
1443-
my $attributes = $patron->get_extended_attributes
1443+
=cut
1444+
1445+
sub add_extended_attribute {
1446+
my ($self, $attribute) = @_;
1447+
$attribute->{borrowernumber} = $self->borrowernumber;
1448+
return Koha::Patron::Attribute->new($attribute)->store;
1449+
}
1450+
1451+
=head3 extended_attributes
14441452
14451453
Return object of Koha::Patron::Attributes type with all attributes set for this patron
14461454
1455+
Or setter FIXME
1456+
14471457
=cut
14481458

1449-
sub get_extended_attributes {
1450-
my ( $self ) = @_;
1459+
sub extended_attributes {
1460+
my ( $self, $attributes ) = @_;
1461+
if ($attributes) { # setter
1462+
my $schema = $self->_result->result_source->schema;
1463+
$schema->txn_do(
1464+
sub {
1465+
# Remove the existing one
1466+
$self->extended_attributes->filter_by_branch_limitations->delete;
1467+
1468+
# Insert the new ones
1469+
for my $attribute (@$attributes) {
1470+
$self->_result->create_related('borrower_attributes', $attribute);
1471+
}
1472+
}
1473+
);
1474+
}
1475+
14511476
my $rs = $self->_result->borrower_attributes;
14521477
# We call search to use the filters in Koha::Patron::Attributes->search
14531478
return Koha::Patron::Attributes->_new_from_dbic($rs)->search;

‎Koha/Patrons/Import.pm

+14-5
Original file line numberDiff line numberDiff line change
@@ -299,11 +299,18 @@ sub import_patrons {
299299
}
300300
if ($extended) {
301301
if ($ext_preserve) {
302-
my $old_attributes = $patron->get_extended_attributes->as_list;
302+
my $old_attributes = $patron->extended_attributes->as_list;
303303
$patron_attributes = extended_attributes_merge( $old_attributes, $patron_attributes );
304304
}
305-
push @errors, { unknown_error => 1 }
306-
unless SetBorrowerAttributes( $borrower{'borrowernumber'}, $patron_attributes, 'no_branch_limit' );
305+
eval {
306+
# We do not want to filter by branch, maybe we should?
307+
Koha::Patrons->find($borrowernumber)->extended_attributes->delete;
308+
$patron->extended_attributes($patron_attributes);
309+
};
310+
if ($@) {
311+
# FIXME This is not an unknown error, we can do better here
312+
push @errors, { unknown_error => 1 };
313+
}
307314
}
308315
$overwritten++;
309316
push(
@@ -332,7 +339,9 @@ sub import_patrons {
332339
}
333340

334341
if ($extended) {
335-
SetBorrowerAttributes( $patron->borrowernumber, $patron_attributes );
342+
# FIXME Hum, we did not filter earlier and now we do?
343+
$patron->extended_attributes->filter_by_branch_limitations->delete;
344+
$patron->extended_attributes($patron_attributes);
336345
}
337346

338347
if ($set_messaging_prefs) {
@@ -460,7 +469,7 @@ sub set_column_keys {
460469
461470
my $patron_attributes = set_patron_attributes($extended, $borrower{patron_attributes}, $feedback);
462471
463-
Returns a reference to array of hashrefs data structure as expected by SetBorrowerAttributes.
472+
Returns a reference to array of hashrefs data structure as expected by Koha::Patron->extended_attributes
464473
465474
=cut
466475

‎koha-tmpl/intranet-tmpl/prog/en/includes/circ-menu.inc

+1-1
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@
7171
[% END %]
7272

7373
[% IF Koha.Preference('ExtendedPatronAttributes') %]
74-
[% FOREACH extendedattribute IN patron.get_extended_attributes %]
74+
[% FOREACH extendedattribute IN patron.extended_attributes %]
7575
[% IF ( extendedattribute.type.display_checkout ) %] [%# FIXME We should filter in the line above %]
7676
[% IF ( extendedattribute.attribute ) %] [%# FIXME Why that? why not if == 0? %]
7777
<li class="patronattribute">

‎members/memberentry.pl

+5-3
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,8 @@ BEGIN
480480
}
481481

482482
if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
483-
C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes);
483+
$patron->extended_attributes->filter_by_branch_limitations->delete;
484+
$patron->extended_attributes($extended_patron_attributes);
484485
}
485486
if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
486487
C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'});
@@ -550,7 +551,8 @@ BEGIN
550551

551552
add_guarantors( $patron, $input );
552553
if (C4::Context->preference('ExtendedPatronAttributes') and $input->param('setting_extended_patron_attributes')) {
553-
C4::Members::Attributes::SetBorrowerAttributes($borrowernumber, $extended_patron_attributes);
554+
$patron->extended_attributes->filter_by_branch_limitations->delete;
555+
$patron->extended_attributes($extended_patron_attributes);
554556
}
555557
if (C4::Context->preference('EnhancedMessagingPreferences') and $input->param('setting_messaging_prefs')) {
556558
C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template);
@@ -871,7 +873,7 @@ sub patron_attributes_form {
871873
return;
872874
}
873875
my $patron = Koha::Patrons->find($borrowernumber); # Already fetched but outside of this sub
874-
my @attributes = $patron->get_extended_attributes->as_list; # FIXME Must be improved!
876+
my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
875877
my @classes = uniq( map {$_->type->class} @attributes );
876878
@classes = sort @classes;
877879

‎members/moremember.pl

+1-1
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ BEGIN
130130
);
131131

132132
if (C4::Context->preference('ExtendedPatronAttributes')) {
133-
my @attributes = $patron->get_extended_attributes->as_list; # FIXME Must be improved!
133+
my @attributes = $patron->extended_attributes->as_list; # FIXME Must be improved!
134134
my @classes = uniq( map {$_->type->class} @attributes );
135135
@classes = sort @classes;
136136

‎opac/opac-memberentry.pl

+2-1
Original file line numberDiff line numberDiff line change
@@ -213,7 +213,8 @@
213213
my $patron = Koha::Patron->new( \%borrower )->store;
214214
Koha::Patron::Consent->new({ borrowernumber => $patron->borrowernumber, type => 'GDPR_PROCESSING', given_on => $consent_dt })->store if $consent_dt;
215215
if ( $patron ) {
216-
C4::Members::Attributes::SetBorrowerAttributes( $patron->borrowernumber, $attributes );
216+
$patron->extended_attributes->filter_by_branch_limitations->delete;
217+
$patron->extended_attributes($attributes);
217218
if ( C4::Context->preference('EnhancedMessagingPreferences') ) {
218219
C4::Form::MessagingPreferences::handle_form_action(
219220
$cgi,

‎t/db_dependent/Auth_with_ldap.t

+1-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ subtest 'checkpw_ldap tests' => sub {
206206

207207
C4::Auth_with_ldap::checkpw_ldap( $dbh, 'hola', password => 'hey' );
208208
ok(
209-
Koha::Patrons->find($borrower->{borrowernumber})->get_extended_attributes->count,
209+
Koha::Patrons->find($borrower->{borrowernumber})->extended_attributes->count,
210210
'Extended attributes are not deleted'
211211
);
212212

‎t/db_dependent/Koha/Patron/Modifications.t

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ subtest 'approve tests' => sub {
174174
);
175175
is( $patron->firstname, 'Kyle',
176176
'Patron modification set the right firstname' );
177-
my $patron_attributes = $patron->get_extended_attributes;
177+
my $patron_attributes = $patron->extended_attributes;
178178
my $attribute_1 = $patron_attributes->next;
179179
is( $attribute_1->code,
180180
'CODE_1', 'Patron modification correctly saved attribute code' );

‎t/db_dependent/Koha/Patrons.t

+11-9
Original file line numberDiff line numberDiff line change
@@ -2019,7 +2019,7 @@ subtest 'anonymize' => sub {
20192019
};
20202020
$schema->storage->txn_rollback;
20212021

2022-
subtest 'get_extended_attributes' => sub {
2022+
subtest 'extended_attributes' => sub {
20232023
plan tests => 10;
20242024
my $schema = Koha::Database->new->schema;
20252025
$schema->storage->txn_begin;
@@ -2069,17 +2069,19 @@ subtest 'get_extended_attributes' => sub {
20692069
}
20702070
];
20712071

2072-
my $extended_attributes = $patron_1->get_extended_attributes;
2073-
is( ref($extended_attributes), 'Koha::Patron::Attributes', 'Koha::Patron->get_extended_attributes must return a Koha::Patron::Attribute set' );
2072+
my $extended_attributes = $patron_1->extended_attributes;
2073+
is( ref($extended_attributes), 'Koha::Patron::Attributes', 'Koha::Patron->extended_attributes must return a Koha::Patron::Attribute set' );
20742074
is( $extended_attributes->count, 0, 'There should not be attribute yet');
20752075

2076-
C4::Members::Attributes::SetBorrowerAttributes($patron_1->borrowernumber, $attributes_for_1);
2077-
C4::Members::Attributes::SetBorrowerAttributes($patron_2->borrowernumber, $attributes_for_2);
2076+
$patron_1->extended_attributes->filter_by_branch_limitations->delete;
2077+
$patron_2->extended_attributes->filter_by_branch_limitations->delete;
2078+
$patron_1->extended_attributes($attributes_for_1);
2079+
$patron_2->extended_attributes($attributes_for_2);
20782080

2079-
my $extended_attributes_for_1 = $patron_1->get_extended_attributes;
2081+
my $extended_attributes_for_1 = $patron_1->extended_attributes;
20802082
is( $extended_attributes_for_1->count, 3, 'There should be 3 attributes now for patron 1');
20812083

2082-
my $extended_attributes_for_2 = $patron_2->get_extended_attributes;
2084+
my $extended_attributes_for_2 = $patron_2->extended_attributes;
20832085
is( $extended_attributes_for_2->count, 2, 'There should be 2 attributes now for patron 2');
20842086

20852087
my $attribute_12 = $extended_attributes_for_2->search({ code => $attribute_type1->code });
@@ -2095,11 +2097,11 @@ subtest 'get_extended_attributes' => sub {
20952097
# Test branch limitations
20962098
set_logged_in_user($patron_2);
20972099
# Return all
2098-
$extended_attributes_for_1 = $patron_1->get_extended_attributes;
2100+
$extended_attributes_for_1 = $patron_1->extended_attributes;
20992101
is( $extended_attributes_for_1->count, 3, 'There should be 2 attributes for patron 1, the limited one should be returned');
21002102

21012103
# Return filtered
2102-
$extended_attributes_for_1 = $patron_1->get_extended_attributes->filter_by_branch_limitations;
2104+
$extended_attributes_for_1 = $patron_1->extended_attributes->filter_by_branch_limitations;
21032105
is( $extended_attributes_for_1->count, 2, 'There should be 2 attributes for patron 1, the limited one should be returned');
21042106

21052107
# Not filtered

‎t/db_dependent/Koha/Patrons/Import.t

+2-2
Original file line numberDiff line numberDiff line change
@@ -565,10 +565,10 @@ subtest 'test_set_patron_attributes' => sub {
565565
# Then ...
566566
ok($result_3, 'Got some data back from set patron attributes');
567567
is($result_3->[0]->{code}, 'grade', 'Got the expected first code from set patron attributes');
568-
is($result_3->[0]->{value}, '01', 'Got the expected first value from set patron attributes');
568+
is($result_3->[0]->{attribute}, '01', 'Got the expected first value from set patron attributes');
569569

570570
is($result_3->[1]->{code}, 'homeroom', 'Got the expected second code from set patron attributes');
571-
is($result_3->[1]->{value}, 1150605, 'Got the expected second value from set patron attributes');
571+
is($result_3->[1]->{attribute}, 1150605, 'Got the expected second value from set patron attributes');
572572

573573
is(scalar @feedback_3, 1, 'Got the expected 1 array size from set patron attributes with extended user');
574574
is($feedback_3[0]->{feedback}, 1, 'Got the expected second feedback from set patron attributes with extended user');

0 commit comments

Comments
 (0)