From 4154e4d7baecf54da0da64cea847fc63ad6d74a3 Mon Sep 17 00:00:00 2001 From: David Gustafsson Date: Thu, 13 Mar 2025 19:36:03 +0100 Subject: [PATCH] Bug 26744: (follow up) Preserve order when adding patron attributes --- Koha/Patron.pm | 84 +++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 38 deletions(-) diff --git a/Koha/Patron.pm b/Koha/Patron.pm index bd3572d92c4..47e5706faf9 100644 --- a/Koha/Patron.pm +++ b/Koha/Patron.pm @@ -2305,11 +2305,9 @@ sub extended_attributes { $attribute->validate_type(); } - # Sort new attributes by code - @new_attributes = sort { $a->attribute cmp $b->attribute } @new_attributes; - # Stash changes after - for my $attribute ( values @new_attributes ) { + # Sort attributes to ensure order is the same as for current attributes retrieved above + for my $attribute ( sort { $a->attribute cmp $b->attribute } @new_attributes ) { my $repeatable = $attribute->type->repeatable ? 1 : 0; $attribute_changes{$repeatable}->{ $attribute->code }->{after} //= []; push( @@ -2327,6 +2325,7 @@ sub extended_attributes { $schema->txn_do( sub { my $all_changes = {}; + my @changed_attributes_codes; while ( my ( $repeatable, $changes ) = each %attribute_changes ) { while ( my ( $code, $change ) = each %{$changes} ) { $change->{before} //= []; @@ -2338,6 +2337,8 @@ sub extended_attributes { $change->{after} = @{ $change->{after} } ? $change->{after}->[0] : ''; } + push @changed_attributes_codes, $code; + # Remove existing $self->extended_attributes->filter_by_branch_limitations->search( { @@ -2345,49 +2346,56 @@ sub extended_attributes { } )->delete; - # Add possible new attribute values - for my $attribute (@new_attributes) { - $attribute->store() if ( $attribute->code eq $code ); - } if ( C4::Context->preference("BorrowersLog") ) { $all_changes->{"attribute.$code"} = $change; } } } } - my $new_types = {}; - for my $attribute ( @{$attributes} ) { - $new_types->{ $attribute->{code} } = 1; - } - # Check globally mandatory types - my $interface = C4::Context->interface; - my $params = { - mandatory => 1, - category_code => [ undef, $self->categorycode ], - 'borrower_attribute_types_branches.b_branchcode' => undef, - }; + if ( @changed_attributes_codes ) { - if ( $interface eq 'opac' ) { - $params->{opac_editable} = 1; - } + # Store changed attributes in the order they where passed in as some tests + # relies on ids being assigned in that order + my $new_types = {}; + for my $new_attribute (@new_attributes) { + if ( grep { $_ eq $new_attribute->code } @changed_attributes_codes ) { + $new_attribute->store(); + } + $new_types->{ $new_attribute->code } = 1; + } - my @required_attribute_types = Koha::Patron::Attribute::Types->search( - $params, - { join => 'borrower_attribute_types_branches' } - )->get_column('code'); - for my $type (@required_attribute_types) { - Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute->throw( - type => $type, - ) if !$new_types->{$type}; - } - if ( %{$all_changes} ) { - logaction( - "MEMBERS", - "MODIFY", - $self->borrowernumber, - to_json( $all_changes, { pretty => 1, canonical => 1 } ) - ); + # Check globally mandatory types + my $interface = C4::Context->interface; + my $params = { + mandatory => 1, + category_code => [ undef, $self->categorycode ], + 'borrower_attribute_types_branches.b_branchcode' => undef, + }; + + if ( $interface eq 'opac' ) { + $params->{opac_editable} = 1; + } + + my @required_attribute_types = Koha::Patron::Attribute::Types->search( + $params, + { join => 'borrower_attribute_types_branches' } + )->get_column('code'); + + for my $type (@required_attribute_types) { + Koha::Exceptions::Patron::MissingMandatoryExtendedAttribute->throw( + type => $type, + ) if !$new_types->{$type}; + } + + if ( %{$all_changes} ) { + logaction( + "MEMBERS", + "MODIFY", + $self->borrowernumber, + to_json( $all_changes, { pretty => 1, canonical => 1 } ) + ); + } } } ); -- 2.34.1