Koha-community/Koha
13
7
Fork 0
Code Issues Releases Wiki Activity

Bug 28220: prevent patron to be created if attributes not stored

Browse source 
Using a transaction

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This commit is contained in:
Jonathan Druart 2021-05-06 10:35:16 +02:00
parent 6e769cc004
commit fa100d8e5e
2 changed files with 59 additions and 52 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

92
Koha/Patrons/Import.pm
View file

@ -331,7 +331,7 @@ sub import_patrons {
}
if ($extended) {
if ($ext_preserve) {
$patron_attributes = $patron->extended_attributes->merge_with( $patron_attributes );
$patron_attributes = $patron->extended_attributes->merge_and_replace_with( $patron_attributes );
}
eval {
# We do not want to filter by branch, maybe we should?
@ -354,51 +354,55 @@ sub import_patrons {
);
}
else {
my $patron = eval {
Koha::Patron->new(\%borrower)->store;
};
unless ( $@ ) {
$borrowernumber = $patron->id;
try {
$schema->storage->txn_do(sub {
my $patron = Koha::Patron->new(\%borrower)->store;
$borrowernumber = $patron->id;
if ( $patron->is_debarred ) {
AddDebarment(
{
borrowernumber => $patron->borrowernumber,
expiration => $patron->debarred,
comment => $patron->debarredcomment,
}
);
}
if ($extended) {
# FIXME Hum, we did not filter earlier and now we do?
$patron->extended_attributes->filter_by_branch_limitations->delete;
$patron->extended_attributes($patron_attributes);
}
if ($set_messaging_prefs) {
C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
{
borrowernumber => $patron->borrowernumber,
categorycode => $patron->categorycode,
}
);
}
$imported++;
push @imported_borrowers, $patron->borrowernumber; #for patronlist
push(
@feedback,
{
feedback => 1,
name => 'lastimported',
value => $patron->surname . ' / ' . $patron->borrowernumber,
if ( $patron->is_debarred ) {
AddDebarment(
{
borrowernumber => $patron->borrowernumber,
expiration => $patron->debarred,
comment => $patron->debarredcomment,
}
);
}
);
}
else {
if ($extended) {
# FIXME Hum, we did not filter earlier and now we do?
$patron->extended_attributes->filter_by_branch_limitations->delete;
$patron->extended_attributes($patron_attributes);
}
if ($set_messaging_prefs) {
C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
{
borrowernumber => $patron->borrowernumber,
categorycode => $patron->categorycode,
}
);
}
$imported++;
push @imported_borrowers, $patron->borrowernumber; #for patronlist
push(
@feedback,
{
feedback => 1,
name => 'lastimported',
value => $patron->surname . ' / ' . $patron->borrowernumber,
}
);
});
} catch {
$invalid++;
push @errors, { unknown_error => 1 };
if ( $_->isa('Koha::Exceptions::Patron::Attribute::UniqueIDConstraint') ) {
my $patron_id = defined $matchpoint ? $borrower{$matchpoint} : $matchpoint_attr_type;
push @errors, { patron_attribute_unique_id_constraint => 1, patron_id => $patron_id, attribute => $_->attribute };
} else {
push @errors, { unknown_error => 1 };
}
push(
@errors,
{
@ -406,7 +410,7 @@ sub import_patrons {
value => $borrower{'surname'} . ' / Create patron',
}
);
}
};
}
# Add a guarantor if we are given a relationship

19
t/db_dependent/Koha/Patrons/Import.t
View file

@ -845,7 +845,7 @@ subtest 'test_format_dates' => sub {
subtest 'patron_attributes' => sub {
plan tests => 4;
plan tests => 6;
t::lib::Mocks::mock_preference('ExtendedPatronAttributes', 1);
@ -875,6 +875,8 @@ subtest 'patron_attributes' => sub {
my $non_existent_attribute_type_code = $non_existent_attribute_type->code;
$non_existent_attribute_type->delete;
our $cardnumber = "1042";
# attributes is { code => \@attributes }
sub build_csv {
my ($attributes) = @_;
@ -882,7 +884,7 @@ subtest 'patron_attributes' => sub {
my $csv_headers = 'cardnumber,surname,firstname,branchcode,categorycode,patron_attributes';
my @attributes_str = map { my $code = $_; map { sprintf "%s:%s", $code, $_ } @{ $attributes->{$code} } } keys %$attributes;
my $attributes_str = join ',', @attributes_str;
my $csv_line = sprintf '1042,John,D,MPL,PT,"%s"', $attributes_str;
my $csv_line = sprintf '%s,John,D,MPL,PT,"%s"', $cardnumber, $attributes_str;
my $filename = make_csv( $temp_dir, $csv_headers, $csv_line );
open( my $fh, "<:encoding(utf8)", $filename ) or die "cannot open $filename: $!";
return $fh;
@ -899,7 +901,7 @@ subtest 'patron_attributes' => sub {
is( $result->{imported}, 1 );
my $patron = Koha::Patrons->find({cardnumber => "1042"});
my $patron = Koha::Patrons->find({cardnumber => $cardnumber});
compare_patron_attributes($patron->extended_attributes->unblessed, { %$attributes } );
$patron->delete;
}
@ -918,12 +920,13 @@ subtest 'patron_attributes' => sub {
};
my $fh = build_csv({ %$attributes });
throws_ok {
$patrons_import->import_patrons({file => $fh});
}
'Koha::Exceptions::Patron::Attribute::UniqueIDConstraint';
my $result = $patrons_import->import_patrons({file => $fh, matchpoint => 'cardnumber'});
my $error = $result->{errors}->[0];
is( $error->{patron_attribute_unique_id_constraint}, 1 );
is( $error->{patron_id}, $cardnumber );
is( $error->{attribute}->code, $unique_attribute_type->code );
my $patron = Koha::Patrons->find({cardnumber => "1042"});
my $patron = Koha::Patrons->find({cardnumber => $cardnumber});
is( $patron, undef );
}