From 233dc6019100673cc59ed9dbd7c3e8c8c9bf866d Mon Sep 17 00:00:00 2001 From: Matt Blenkinsop 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