Bug 28220: prevent patron to be created if attributes not stored
[koha-ffzg.git] / Koha / Patrons / Import.pm
index 413aae3..13597c3 100644 (file)
@@ -22,6 +22,7 @@ use namespace::clean;
 use Carp;
 use Text::CSV;
 use Encode qw( decode_utf8 );
+use Try::Tiny;
 
 use C4::Members;
 
@@ -71,9 +72,14 @@ sub import_patrons {
     my $defaults             = $params->{defaults};
     my $ext_preserve         = $params->{preserve_extended_attributes};
     my $overwrite_cardnumber = $params->{overwrite_cardnumber};
+    my $overwrite_passwords  = $params->{overwrite_passwords};
+    my $dry_run              = $params->{dry_run};
     my $extended             = C4::Context->preference('ExtendedPatronAttributes');
     my $set_messaging_prefs  = C4::Context->preference('EnhancedMessagingPreferences');
 
+    my $schema = Koha::Database->new->schema;
+    $schema->storage->txn_begin if $dry_run;
+
     my @columnkeys = $self->set_column_keys($extended);
     my @feedback;
     my @errors;
@@ -174,8 +180,14 @@ sub import_patrons {
         elsif ($extended) {
             if ( defined($matchpoint_attr_type) ) {
                 foreach my $attr (@$patron_attributes) {
-                    if ( $attr->{code} eq $matchpoint and $attr->{value} ne '' ) {
-                        my @borrowernumbers = $matchpoint_attr_type->get_patrons( $attr->{value} );
+                    if ( $attr->{code} eq $matchpoint and $attr->{attribute} ne '' ) {
+                        my @borrowernumbers = Koha::Patron::Attributes->search(
+                            {
+                                code      => $matchpoint_attr_type->code,
+                                attribute => $attr->{attribute}
+                            }
+                        )->get_column('borrowernumber');
+
                         $borrowernumber = $borrowernumbers[0] if scalar(@borrowernumbers) == 1;
                         $patron = Koha::Patrons->find( $borrowernumber );
                         last;
@@ -215,15 +227,15 @@ sub import_patrons {
             next LINE;
         }
 
-        my $relationship        = $borrower{relationship};
-        my $guarantor_id        = $borrower{guarantor_id};
-        delete $borrower{relationship};
+        my $guarantor_relationship = $borrower{guarantor_relationship};
+        delete $borrower{guarantor_relationship};
+        my $guarantor_id = $borrower{guarantor_id};
         delete $borrower{guarantor_id};
 
         # Remove warning for int datatype that cannot be null
         # Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1018
         for my $field (
-            qw( privacy privacy_guarantor_fines privacy_guarantor_checkouts anonymized ))
+            qw( privacy privacy_guarantor_fines privacy_guarantor_checkouts anonymized login_attempts ))
         {
             delete $borrower{$field}
               if exists $borrower{$field} and $borrower{$field} eq "";
@@ -249,8 +261,8 @@ sub import_patrons {
                 # use values from extant patron unless our csv file includes this column or we provided a default.
                 # FIXME : You cannot update a field with a  perl-evaluated false value using the defaults.
 
-                # The password is always encrypted, skip it!
-                next if $col eq 'password';
+                # The password is always encrypted, skip it unless we are forcing overwrite!
+                next if $col eq 'password' && !$overwrite_passwords;
 
                 unless ( exists( $csvkeycol{$col} ) || $defaults->{$col} ) {
                     $borrower{$col} = $member->{$col} if ( $member->{$col} );
@@ -295,9 +307,31 @@ sub import_patrons {
                     );
                 }
             }
+            if ($patron->category->category_type ne 'S' && $overwrite_passwords && defined $borrower{password} && $borrower{password} ne ''){
+                try {
+                    $patron->set_password({ password => $borrower{password} });
+                }
+                catch {
+                    if ( $_->isa('Koha::Exceptions::Password::TooShort') ) {
+                        push @errors, { passwd_too_short => 1, borrowernumber => $borrowernumber, length => $_->{length}, min_length => $_->{min_length} };
+                    }
+                    elsif ( $_->isa('Koha::Exceptions::Password::WhitespaceCharacters') ) {
+                        push @errors, { passwd_whitespace => 1, borrowernumber => $borrowernumber } ;
+                    }
+                    elsif ( $_->isa('Koha::Exceptions::Password::TooWeak') ) {
+                        push @errors, { passwd_too_weak => 1, borrowernumber => $borrowernumber } ;
+                    }
+                    elsif ( $_->isa('Koha::Exceptions::Password::Plugin') ) {
+                        push @errors, { passwd_plugin_err => 1, borrowernumber => $borrowernumber } ;
+                    }
+                    else {
+                        push @errors, { passwd_unknown_err => 1, borrowernumber => $borrowernumber } ;
+                    }
+                }
+            }
             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?
@@ -320,50 +354,55 @@ sub import_patrons {
             );
         }
         else {
-            my $patron = eval {
-                Koha::Patron->new(\%borrower)->store;
-            };
-            unless ( $@ ) {
+            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 ( $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 ($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,
+                            }
+                        );
+                    }
 
-                if ($set_messaging_prefs) {
-                    C4::Members::Messaging::SetMessagingPreferencesFromDefaults(
+                    $imported++;
+                    push @imported_borrowers, $patron->borrowernumber; #for patronlist
+                    push(
+                        @feedback,
                         {
-                            borrowernumber => $patron->borrowernumber,
-                            categorycode   => $patron->categorycode,
+                            feedback => 1,
+                            name     => 'lastimported',
+                            value    => $patron->surname . ' / ' . $patron->borrowernumber,
                         }
                     );
-                }
-
-                $imported++;
-                push @imported_borrowers, $patron->borrowernumber; #for patronlist
-                push(
-                    @feedback,
-                    {
-                        feedback => 1,
-                        name     => 'lastimported',
-                        value    => $patron->surname . ' / ' . $patron->borrowernumber,
-                    }
-                );
-            }
-            else {
+                });
+            } 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,
                     {
@@ -371,21 +410,36 @@ sub import_patrons {
                         value => $borrower{'surname'} . ' / Create patron',
                     }
                 );
-            }
+            };
         }
 
         # Add a guarantor if we are given a relationship
         if ( $guarantor_id ) {
-            Koha::Patron::Relationship->new(
+            my $relationship = Koha::Patron::Relationships->find(
                 {
                     guarantee_id => $borrowernumber,
-                    relationship => $relationship,
                     guarantor_id => $guarantor_id,
                 }
-            )->store();
+            );
+
+            if ( $relationship ) {
+                $relationship->relationship( $guarantor_relationship );
+                $relationship->store();
+            }
+            else {
+                Koha::Patron::Relationship->new(
+                    {
+                        guarantee_id => $borrowernumber,
+                        relationship => $guarantor_relationship,
+                        guarantor_id => $guarantor_id,
+                    }
+                )->store();
+            }
         }
     }
 
+    $schema->storage->txn_rollback if $dry_run;
+
     return {
         feedback      => \@feedback,
         errors        => \@errors,
@@ -458,6 +512,7 @@ sub set_column_keys {
 
     my @columnkeys = map { $_ ne 'borrowernumber' ? $_ : () } Koha::Patrons->columns();
     push( @columnkeys, 'patron_attributes' ) if $extended;
+    push( @columnkeys, qw( guarantor_relationship guarantor_id ) );
 
     return @columnkeys;
 }
@@ -487,7 +542,7 @@ sub generate_patron_attributes {
     my $ok   = $csv->parse($string);  # parse field again to get subfields!
     my @list = $csv->fields();
     my @patron_attributes =
-      sort { $a->{code} cmp $b->{code} || $a->{value} cmp $b->{value} }
+      sort { $a->{code} cmp $b->{code} || $a->{attribute} cmp $b->{attribute} }
       map {
         my @arr = split /:/, $_, 2;
         { code => $arr[0], attribute => $arr[1] }