Bugzilla – Attachment 194780 Details for
Bug 26355
Allow patrons to self-renew through the OPAC
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
Bug 26355: (follow-up) Fix handling of extended attributes
Bug-26355-follow-up-Fix-handling-of-extended-attri.patch (text/plain), 13.13 KB, created by
Matt Blenkinsop
on 2026-03-06 16:26:46 UTC
(
hide
)
Description:
Bug 26355: (follow-up) Fix handling of extended attributes
Filename:
MIME Type:
Creator:
Matt Blenkinsop
Created:
2026-03-06 16:26:46 UTC
Size:
13.13 KB
patch
obsolete
>From 233dc6019100673cc59ed9dbd7c3e8c8c9bf866d Mon Sep 17 00:00:00 2001 >From: Matt Blenkinsop <matt.blenkinsop@openfifth.co.uk> >Date: Fri, 6 Mar 2026 16:24:45 +0000 >Subject: [PATCH] Bug 26355: (follow-up) Fix handling of extended attributes > >Extended attributes weren't being identified as changes and so need to be checked to see if any of them have changed > >(cherry picked from commit 830a98fe8ce0b0562ffbaed84a3182b89c6840b4) >--- > Koha/Patron.pm | 67 +++++++++++++++ > Koha/REST/V1/Patrons/SelfRenewal.pm | 7 +- > admin/categories.pl | 4 +- > opac/opac-memberentry.pl | 65 +------------- > t/db_dependent/Koha/Patron.t | 127 +++++++++++++++++++++++++++- > 5 files changed, 203 insertions(+), 67 deletions(-) > >diff --git a/Koha/Patron.pm b/Koha/Patron.pm >index e36539ca3c5..b1b52af21f2 100644 >--- a/Koha/Patron.pm >+++ b/Koha/Patron.pm >@@ -3703,6 +3703,73 @@ sub create_expiry_notice_parameters { > return $sending_params; > } > >+=head3 identify_updated_extended_attributes >+ >+Compares an array of updated extended attributes with the stored values to identify changes >+ >+=cut >+ >+sub identify_updated_extended_attributes { >+ my ( $self, $entered_attributes ) = @_; >+ >+ my @patron_attributes = grep { $_->type->opac_editable ? $_ : () } >+ Koha::Patron::Attributes->search( { borrowernumber => $self->borrowernumber } )->as_list; >+ >+ my $patron_attribute_types; >+ foreach my $attr (@patron_attributes) { >+ $patron_attribute_types->{ $attr->code } += 1; >+ } >+ >+ my $passed_attribute_types; >+ foreach my $attr ( @{$entered_attributes} ) { >+ $passed_attribute_types->{ $attr->{code} } += 1; >+ } >+ >+ my @changed_attributes; >+ >+ # Loop through the current patron attributes >+ foreach my $attribute_type ( keys %{$patron_attribute_types} ) { >+ if ( ( $patron_attribute_types->{$attribute_type} // q{} ) ne >+ ( $passed_attribute_types->{$attribute_type} // q{} ) ) >+ { >+ # count differs, overwrite all attributes for given type >+ foreach my $attr ( @{$entered_attributes} ) { >+ push @changed_attributes, $attr >+ if $attr->{code} eq $attribute_type; >+ } >+ } else { >+ >+ # count matches, check values >+ my $changes = 0; >+ foreach my $attr ( grep { $_->code eq $attribute_type } @patron_attributes ) { >+ $changes = 1 >+ unless any { $_->{attribute} eq $attr->attribute } @{$entered_attributes}; >+ last if $changes; >+ } >+ >+ if ($changes) { >+ foreach my $attr ( @{$entered_attributes} ) { >+ push @changed_attributes, $attr >+ if $attr->{code} eq $attribute_type; >+ } >+ } >+ } >+ } >+ >+ # Loop through passed attributes, looking for new ones >+ foreach my $attribute_type ( keys %{$passed_attribute_types} ) { >+ if ( !defined $patron_attribute_types->{$attribute_type} ) { >+ >+ # YAY, new stuff >+ foreach my $attr ( grep { $_->{code} eq $attribute_type } @{$entered_attributes} ) { >+ push @changed_attributes, $attr; >+ } >+ } >+ } >+ >+ return \@changed_attributes; >+} >+ > =head2 Internal methods > > =head3 _type >diff --git a/Koha/REST/V1/Patrons/SelfRenewal.pm b/Koha/REST/V1/Patrons/SelfRenewal.pm >index b28a0edaf9a..5e490132cf6 100644 >--- a/Koha/REST/V1/Patrons/SelfRenewal.pm >+++ b/Koha/REST/V1/Patrons/SelfRenewal.pm >@@ -99,9 +99,12 @@ sub submit { > $changes_detected++; > } > } >- if ($changes_detected) { >+ my $updated_extended_attributes = >+ $patron->identify_updated_extended_attributes($extended_attributes); >+ if ( $changes_detected || scalar(@$updated_extended_attributes) ) { > $changed_fields->{changed_fields} = join ',', keys %$changed_fields; >- $changed_fields->{extended_attributes} = to_json($extended_attributes) if $extended_attributes; >+ $changed_fields->{extended_attributes} = to_json($updated_extended_attributes) >+ if scalar(@$updated_extended_attributes); > $patron->request_modification($changed_fields); > } > } >diff --git a/admin/categories.pl b/admin/categories.pl >index ec2b66e4847..205adaf5391 100755 >--- a/admin/categories.pl >+++ b/admin/categories.pl >@@ -50,7 +50,9 @@ if ( $op eq 'add_form' ) { > my $category = Koha::Patron::Categories->find($categorycode); > $template->param( > category => scalar $category, >- self_renewal_information_messages => parse_renewal_info_message( $category->self_renewal_information_message ) >+ self_renewal_information_messages => scalar $category >+ ? parse_renewal_info_message( $category->self_renewal_information_message ) >+ : [] > ); > > if ( C4::Context->preference('EnhancedMessagingPreferences') ) { >diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl >index 011f3f2fbe3..4b481f96a18 100755 >--- a/opac/opac-memberentry.pl >+++ b/opac/opac-memberentry.pl >@@ -333,13 +333,14 @@ if ( $op eq 'cud-create' ) { > > $template->param( op => 'edit' ); > } else { >+ my $patron = Koha::Patrons->find($borrowernumber); > > # If preferred name is not included but firstname is then set preferred_name to firstname > $borrower{preferred_name} = $borrower{firstname} > if defined $borrower{firstname} && !defined $borrower{preferred_name}; > my %borrower_changes = DelUnchangedFields( $borrowernumber, %borrower ); > $borrower_changes{'changed_fields'} = join ',', keys %borrower_changes; >- my $extended_attributes_changes = FilterUnchangedAttributes( $borrowernumber, $attributes ); >+ my $extended_attributes_changes = $patron->identify_updated_extended_attributes($attributes); > > if ( $borrower_changes{'changed_fields'} || scalar @{$extended_attributes_changes} > 0 ) { > ( $template, $borrowernumber, $cookie ) = get_template_and_user( >@@ -354,7 +355,6 @@ if ( $op eq 'cud-create' ) { > $borrower_changes{borrowernumber} = $borrowernumber; > $borrower_changes{extended_attributes} = to_json($extended_attributes_changes); > >- my $patron = Koha::Patrons->find($borrowernumber); > $patron->request_modification( \%borrower_changes ); > $template->param( borrower => $patron->unblessed ); > } else { >@@ -628,67 +628,6 @@ sub DelEmptyFields { > return %borrower; > } > >-sub FilterUnchangedAttributes { >- my ( $borrowernumber, $entered_attributes ) = @_; >- >- my @patron_attributes = grep { $_->type->opac_editable ? $_ : () } >- Koha::Patron::Attributes->search( { borrowernumber => $borrowernumber } )->as_list; >- >- my $patron_attribute_types; >- foreach my $attr (@patron_attributes) { >- $patron_attribute_types->{ $attr->code } += 1; >- } >- >- my $passed_attribute_types; >- foreach my $attr ( @{$entered_attributes} ) { >- $passed_attribute_types->{ $attr->{code} } += 1; >- } >- >- my @changed_attributes; >- >- # Loop through the current patron attributes >- foreach my $attribute_type ( keys %{$patron_attribute_types} ) { >- if ( ( $patron_attribute_types->{$attribute_type} // q{} ) ne >- ( $passed_attribute_types->{$attribute_type} // q{} ) ) >- { >- # count differs, overwrite all attributes for given type >- foreach my $attr ( @{$entered_attributes} ) { >- push @changed_attributes, $attr >- if $attr->{code} eq $attribute_type; >- } >- } else { >- >- # count matches, check values >- my $changes = 0; >- foreach my $attr ( grep { $_->code eq $attribute_type } @patron_attributes ) { >- $changes = 1 >- unless any { $_->{value} eq $attr->attribute } @{$entered_attributes}; >- last if $changes; >- } >- >- if ($changes) { >- foreach my $attr ( @{$entered_attributes} ) { >- push @changed_attributes, $attr >- if $attr->{code} eq $attribute_type; >- } >- } >- } >- } >- >- # Loop through passed attributes, looking for new ones >- foreach my $attribute_type ( keys %{$passed_attribute_types} ) { >- if ( !defined $patron_attribute_types->{$attribute_type} ) { >- >- # YAY, new stuff >- foreach my $attr ( grep { $_->{code} eq $attribute_type } @{$entered_attributes} ) { >- push @changed_attributes, $attr; >- } >- } >- } >- >- return \@changed_attributes; >-} >- > sub GeneratePatronAttributesForm { > my ( $borrowernumber, $entered_attributes ) = @_; > >diff --git a/t/db_dependent/Koha/Patron.t b/t/db_dependent/Koha/Patron.t >index 7b7774ac016..9b445a70723 100755 >--- a/t/db_dependent/Koha/Patron.t >+++ b/t/db_dependent/Koha/Patron.t >@@ -19,7 +19,7 @@ > > use Modern::Perl; > >-use Test::More tests => 49; >+use Test::More tests => 50; > use Test::NoWarnings; > use Test::Exception; > use Test::Warn; >@@ -3704,3 +3704,128 @@ subtest "create_expiry_notice_parameters" => sub { > is_deeply( $letter_params, $expected_return, 'Letter params generated correctly' ); > $schema->storage->txn_rollback; > }; >+ >+subtest "identify_updated_extended_attributes" => sub { >+ plan tests => 2; >+ >+ $schema->storage->txn_begin; >+ >+ my $patron = $builder->build_object( >+ { >+ class => 'Koha::Patrons', >+ } >+ ); >+ >+ my $unique_attribute_type = $builder->build_object( >+ { >+ class => 'Koha::Patron::Attribute::Types', >+ value => { >+ unique_id => 1, >+ repeatable => 0, >+ is_date => 0, >+ opac_editable => 1 >+ } >+ } >+ ); >+ my $repeatable_attribute_type = $builder->build_object( >+ { >+ class => 'Koha::Patron::Attribute::Types', >+ value => { >+ unique_id => 0, >+ repeatable => 1, >+ is_date => 0, >+ opac_editable => 1 >+ } >+ } >+ ); >+ my $normal_attribute_type = $builder->build_object( >+ { >+ class => 'Koha::Patron::Attribute::Types', >+ value => { >+ unique_id => 0, >+ repeatable => 0, >+ is_date => 0, >+ opac_editable => 1 >+ } >+ } >+ ); >+ >+ my $attributes = [ >+ { >+ attribute => 'my unique attribute 1', >+ code => $unique_attribute_type->code(), >+ }, >+ { >+ attribute => 'my repeatable attribute 1', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my normal attribute 1', >+ code => $normal_attribute_type->code(), >+ } >+ ]; >+ $patron->extended_attributes($attributes); >+ >+ my $changed_attributes = [ >+ { >+ attribute => 'this is new', >+ code => $unique_attribute_type->code(), >+ }, >+ { >+ attribute => 'my repeatable attribute 1', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my normal attribute 1', >+ code => $normal_attribute_type->code(), >+ } >+ ]; >+ >+ my $updated_attributes = $patron->identify_updated_extended_attributes($changed_attributes); >+ is( scalar(@$updated_attributes), 1, "Only the first one was changed" ); >+ >+ $attributes = [ >+ { >+ attribute => 'my unique attribute 1', >+ code => $unique_attribute_type->code(), >+ }, >+ { >+ attribute => 'my repeatable attribute 1', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my repeatable attribute 2', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my normal attribute 1', >+ code => $normal_attribute_type->code(), >+ } >+ ]; >+ >+ $patron->extended_attributes($attributes); >+ >+ $changed_attributes = [ >+ { >+ attribute => 'this is new', >+ code => $unique_attribute_type->code(), >+ }, >+ { >+ attribute => 'this repeatable one is also new', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my repeatable attribute 2', >+ code => $repeatable_attribute_type->code(), >+ }, >+ { >+ attribute => 'my normal attribute 1', >+ code => $normal_attribute_type->code(), >+ } >+ ]; >+ >+ $updated_attributes = $patron->identify_updated_extended_attributes($changed_attributes); >+ is( scalar(@$updated_attributes), 3, "Two have now been updated, one of which is repeatable and has two values" ); >+ >+ $schema->storage->txn_rollback; >+}; >-- >2.50.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 26355
:
191781
|
191782
|
191783
|
191784
|
191785
|
191786
|
191787
|
191788
|
191789
|
192440
|
192441
|
192442
|
192443
|
192444
|
192445
|
192446
|
192447
|
192448
|
193065
|
193066
|
193067
|
193068
|
193069
|
193070
|
193071
|
193072
|
193073
|
193074
|
193249
|
193250
|
193251
|
193252
|
193253
|
193521
|
193522
|
193523
|
193524
|
193525
|
193526
|
193527
|
193528
|
193529
|
193581
|
193635
|
193654
|
193699
|
193756
|
193757
|
193758
|
193759
|
193760
|
193761
|
193762
|
193763
|
193764
|
193765
|
193921
|
194435
|
194464
|
194465
|
194466
|
194467
|
194468
|
194469
|
194470
|
194471
|
194472
|
194473
|
194474
|
194753
|
194754
|
194755
|
194756
|
194757
|
194758
|
194759
|
194760
|
194761
|
194763
|
194764
|
194765
| 194780