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

Sponsored-by: Westminster City Council <https://www.westminster.gov.uk/>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit is contained in:
Matt Blenkinsop 2026-03-06 16:24:45 +00:00 committed by Lucas Gass
parent da92eaa4d3
commit bce9aad974
No known key found for this signature in database
GPG Key ID: 60B447398B0D36CA
5 changed files with 202 additions and 66 deletions

View File

@ -3963,6 +3963,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

View File

@ -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);
}
}

View File

@ -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') ) {

View File

@ -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 {
@ -633,67 +633,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 ) = @_;

View File

@ -3941,3 +3941,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;
};