Bug 28217: Prevent several non-repeatable attributes to be merged

When using the patron merge feature it's possible to generate a patron
with several non-repeatable attributes.

This patch prevents that.

Test plan:
Create 2 patron attribute types, one repeatable and one non-repeatable
Create 2 patrons and add them repeatable attributes
Add a non-repeatable attribute to one of them
Merge the 2 patrons
=> It succeeds, the resulting patron has all the repeatable attribute
and the non-repeatable one.

Do it again but this time add non-repeatable to both patrons
Merge them
=> It fails, you should see an error on the UI
  Merge failed! The following error was reported: Tried to add more than one non-repeatable attributes. type=TYPE value=VALUE.

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2021-04-23 15:37:46 +02:00
parent 915c5d13be
commit 92db5e2247
2 changed files with 123 additions and 2 deletions

View File

@ -62,7 +62,6 @@ our $RESULTSET_PATRON_ID_MAPPING = {
Aqbudget => 'budget_owner_id',
Aqbudgetborrower => 'borrowernumber',
ArticleRequest => 'borrowernumber',
BorrowerAttribute => 'borrowernumber',
BorrowerDebarment => 'borrowernumber',
BorrowerFile => 'borrowernumber',
BorrowerModification => 'borrowernumber',
@ -629,6 +628,16 @@ sub merge_with {
# Unbless for safety, the patron will end up being deleted
$results->{merged}->{$patron_id}->{patron} = $patron->unblessed;
my $attributes = $patron->extended_attributes;
my $new_attributes = [
map { { code => $_->code, attribute => $_->attribute } }
$attributes->as_list
];
$attributes->delete; # We need to delete before trying to merge them to prevent exception on unique and repeatable
for my $attribute ( @$new_attributes ) {
$self->add_extended_attribute($attribute);
}
while (my ($r, $field) = each(%$RESULTSET_PATRON_ID_MAPPING)) {
my $rs = $schema->resultset($r)->search({ $field => $patron_id });
$results->{merged}->{ $patron_id }->{updated}->{$r} = $rs->count();

View File

@ -1606,7 +1606,7 @@ subtest 'BorrowersLog tests' => sub {
$schema->storage->txn_rollback;
subtest 'Test Koha::Patrons::merge' => sub {
plan tests => 113;
plan tests => 110;
my $schema = Koha::Database->new()->schema();
@ -1657,6 +1657,118 @@ subtest 'Test Koha::Patrons::merge' => sub {
is( $results, undef, "Anonymous patron cannot have other patrons merged into it" );
is( Koha::Patrons->search( { borrowernumber => $keeper->id } )->count, 1, "Patron from attempted merge with AnonymousPatron still exists" );
subtest 'extended attributes' => sub {
plan tests => 5;
my $keep_patron =
$builder->build_object( { class => 'Koha::Patrons' } );
my $merge_patron =
$builder->build_object( { class => 'Koha::Patrons' } );
my $attribute_type_normal_1 = $builder->build_object(
{
class => 'Koha::Patron::Attribute::Types',
value => { repeatable => 0, unique_id => 0 }
}
);
my $attribute_type_normal_2 = $builder->build_object(
{
class => 'Koha::Patron::Attribute::Types',
value => { repeatable => 0, unique_id => 0 }
}
);
my $attribute_type_repeatable = $builder->build_object(
{
class => 'Koha::Patron::Attribute::Types',
value => { repeatable => 1, unique_id => 0 }
}
);
my $attr_keep = [
{
code => $attribute_type_normal_1->code,
attribute => 'from attr 1'
},
{
code => $attribute_type_repeatable->code,
attribute => 'from attr repeatable'
}
];
my $attr_merge = [
{
code => $attribute_type_normal_2->code,
attribute => 'to attr 2'
},
{
code => $attribute_type_repeatable->code,
attribute => 'to attr repeatable'
},
];
$keep_patron->extended_attributes($attr_keep);
$merge_patron->extended_attributes($attr_merge);
$keep_patron->merge_with( [ $merge_patron->borrowernumber ] );
my $merged_attributes = $keep_patron->extended_attributes;
is( $merged_attributes->count, 4 );
sub compare_attributes {
my ( $got, $expected, $code ) = @_;
is_deeply(
[
sort $got->search( { code => $code } )
->get_column('attribute')
],
$expected
);
}
compare_attributes(
$merged_attributes,
['from attr 1'],
$attribute_type_normal_1->code
);
compare_attributes(
$merged_attributes,
['to attr 2'],
$attribute_type_normal_2->code
);
compare_attributes(
$merged_attributes,
[ 'from attr repeatable', 'to attr repeatable' ],
$attribute_type_repeatable->code
);
# Cleanup
$keep_patron->delete;
$merge_patron->delete;
# Recreate but expect an exception because 2 "normal" attributes will be in the resulting patron
$keep_patron =
$builder->build_object( { class => 'Koha::Patrons' } );
$merge_patron =
$builder->build_object( { class => 'Koha::Patrons' } );
$keep_patron->extended_attributes($attr_keep);
$merge_patron->extended_attributes(
[
@$attr_merge,
{
code => $attribute_type_normal_1->code,
attribute => 'yet another attribute for non-repeatable'
}
]
);
throws_ok {
$keep_patron->merge_with( [ $merge_patron->borrowernumber ] );
}
'Koha::Exceptions::Patron::Attribute::NonRepeatable',
'Exception thrown trying to merge several non-repeatable attributes';
};
t::lib::Mocks::mock_preference( 'AnonymousPatron', '' );
$schema->storage->txn_rollback;
};