<commit_before>Various fixes to NL-synchronization

* Check if the patron has a PIN-code, before attempting
  to decrypt it. (Some libraries do not use PIN-codes.)

* Force country in address to be 'no'

* Derive sex from personal number.
  Must be present, either 'M' or 'F'.

* Don't sync patrons without cardnumber

* Only sync patrons belonging to categories: 'B', 'V', 'ANS', 'ADMIN'

* Change syncstatus 'edited' when new patron is trying to sync, but
  NL reports that it allready exists.

* Set syncstatus to 'new' when NL reports that patron does not exit.

* Set correct syncstatus 'synced' when an edited patron was successfully synced.

* Re-sync patrons which have been created as new, but allready exitsed with the same fnr in NL.

* Add standard message preferences according to patron category

* Fix re-sync of imported patron from NL

* Retry fixing bad data with up to 3 calls to NL

* Make sure 'server_tid' is stored in borrower_sync.lastsync after successfull sync.

* Turn of sync if patron is missing fnr

* Turn of sync if patron is allready deleted in NL

* Return early if NLSync is called on a patron without a row in borrower_sync

* Don't try syncing when password hash length is not 32


diff --git a/Koha/NorwegianPatronDB.pm b/Koha/NorwegianPatronDB.pm
index 237ddd7..3ba9873 100644
--- a/Koha/NorwegianPatronDB.pm
+++ b/Koha/NorwegianPatronDB.pm
@@ -74,6 +74,7 @@ our %EXPORT_TAGS = ( all => [qw(
         NLGetFirstname
         NLGetSurname
         NLGetSyncDataFromBorrowernumber
+        NLFnrToSex
 )] );
 Exporter::export_ok_tags('all');

@@ -230,6 +231,19 @@ sub NLSync {
         }
     }

+    # NLSync should never have been called, this patron has no row in borrower_sync
+    return unless $sync;
+
+    if (length($sync->hashed_pin) != 32) {
+        # NL password is assumed to be a hashed 4-digit PIN-code. The hash length should
+        # be excactly 32.
+
+        # Turn off any further syncing
+        $sync->update( { 'sync' => 0 } );
+
+        return;
+    }
+
     my $client = SOAP::Lite
         ->on_action( sub { return '""';})
         ->uri( $nl_uri )
@@ -237,33 +251,135 @@ sub NLSync {

     my $cardnumber = SOAP::Data->name( 'lnr' => $patron->cardnumber );

-    # Call the appropriate method based on syncstatus
+    # We will now perform the SOAP call. After that we must carefully
+    # inspect the response and take actions depending
+    # on its contents. There are several situations we have to handle.
+    # NL is very picky about what it accepts, and issues might stem
+    # from missing or bad data or inconsistency with existing data in NL.
+    #
+    # We might need to perform multiple extra SOAP calls to fix the
+    # issue, so in order to not risk enter into an infinite loop, we
+    # limit the number of times we try to fix things before we give
+    # up and return the last response.
+
+    my $max_tries = 3;
+    my $ok;
     my $response;
-    if ( $sync->syncstatus eq 'edited' || $sync->syncstatus eq 'new' ) {
-        my $soap_patron = _koha_patron_to_soap( $patron );
-        if ( $sync->syncstatus eq 'edited' ) {
-            $response = $client->endre( $cardnumber, $soap_patron );
-        } elsif ( $sync->syncstatus eq 'new' ) {
-            $response = $client->nyPost( $soap_patron );
+
+    for (my $i=1; $i<=$max_tries; $i++) {
+        # Call the appropriate method based on syncstatus
+        if ( $sync->syncstatus eq 'edited' || $sync->syncstatus eq 'new' ) {
+            my $soap_patron = _koha_patron_to_soap( $patron );
+            if ( $sync->syncstatus eq 'edited' ) {
+                $response = $client->endre( $cardnumber, $soap_patron );
+            } elsif ( $sync->syncstatus eq 'new' ) {
+                $response = $client->nyPost( $soap_patron );
+            }
+        } elsif ( $sync->syncstatus eq 'delete' ) {
+            $response = $client->slett( $cardnumber );
         }
-    }
-    if ( $sync->syncstatus eq 'delete' ) {
-        $response = $client->slett( $cardnumber );
+
+        ($response, $ok) = _responseOK($sync, $response, $patron, $client);
+        last if $ok;
     }

-    # Update the sync data according to the results
-    if ( $response->{'status'} && $response->{'status'} == 1 ) {
+    return $response;
+}
+
+sub _responseOK {
+    my ( $sync, $response, $patron, $client ) = @_;
+
+    # Handle successfull operation
+    if ( $response->result->{'status'} && $response->result->{'status'} == 1 ) {
         if ( $sync->syncstatus eq 'delete' ) {
             # Turn off any further syncing
             $sync->update( { 'sync' => 0 } );
         }
-        # Update the syncstatus to 'synced'
-        $sync->update( { 'syncstatus' => 'synced' } );
-        # Update the 'synclast' attribute with the "server time" ("server_tid") returned by the method
-        $sync->update( { 'lastsync' => $response->{'server_tid'} } );
+        $sync->update( { 'syncstatus' => 'synced', 'lastsync' => $response->result->{'server_tid'} } );
+        return ($response, 1);
+    }
+
+    # Handle error special cases
+    my $cause = $response->result->{'melding'};
+    return ($response, 1) unless $cause;
+
+    if ($cause =~ /lnr finnes allerede i registeret/) {
+        # Because of staff mistakes, patrons are sometimes registered with existing cardnumber
+        # without checking if they allready exist in NL.
+        # We set syncstatus to 'edited', so that it might be correctly synced next time.
+        # The lastsync value is arbritary, and will be corrected on next iteration
+        my $now = `date +"%Y-%m-%dT%T"`;
+        chomp($now);
+        $sync->update( { 'syncstatus' => 'edited', 'lastsync' => $now } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /lnr finnes ikke i registeret/) {
+        # Make sure syncstatus is 'new'
+        $sync->update( { 'syncstatus' => 'new' } );
+        return ($response, 0);
     }
-    return $response;

+    if ($cause =~ /Mangler verdi for sist_endret/) {
+        # Set an arbritary value to missing laststync, it will be corrected on next iteration
+        my $now = `date +"%Y-%m-%dT%T"`;
+        chomp($now);
+        $sync->update( { 'lastsync' => $now } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /SIST_ENDRET/) {
+        # NL has different timestamp; returnin an error like this:
+        #  endre: Uoverensstemmelse med gjeldende data i basen:
+        #  SIST_ENDRET = 2016-11-12T14:05:22 != '2016-11-07T15:14:30'
+        # Save the timestamp as in NL, to be corrected in next sync
+        my $i = index($cause, 'SIST_ENDRET');
+        $sync->update( { 'lastsync' => substr($cause, $i + 14, 19) } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /Person med gitt fnr finnes allerede i registeret med nummer/) {
+        # Patron was registered as new, withouth checking for existing patron in NL using the given fnr
+        # We attemt to sync the new cardnumber to NL.
+        # Reference error message:
+        #   Person med gitt fnr finnes allerede i registeret med nummer: N001981874
+        my $old_lnr = substr($cause, 69);
+        my $soap_patron = _koha_patron_to_soap($patron);
+        my $cardnumber = SOAP::Data->name( 'lnr' => $old_lnr );
+
+        # We also need sist_endret value as found in NL:
+        my $old_res = NLSearch($old_lnr);;
+        my $old_patron = $old_res->result()->{'respons_poster'}[0];
+        my $sist_endret = $old_patron->{'sist_endret'};
+        $sync->update( { 'lastsync' => $sist_endret });
+
+        my $new_lnr = $patron->cardnumber;
+        say "attempting to update cardnumber from '$old_lnr' to '$new_lnr'";
+        $response = $client->endre( $cardnumber, $soap_patron );
+
+        return _responseOK($sync, $response, $patron, $client);
+    }
+
+    if ($cause =~ /Person med gitt fnr finnes allerede i registeret/) {
+        # TODO patron exists with Høgskolekort. Don't know what to do with it.
+        # Needs to be fixed manually by NL maintainer.
+        return ($response, 1);
+    }
+
+    if ($cause =~ /fnr_hash mangler/) {
+        # Missing required personal number. Turn of NL-sync
+        $sync->update( { 'sync' => 0 } );
+        return ($response, 1);
+    }
+
+    if ($cause =~ /slettet tidligere/) {
+        $sync->update( { 'sync' => 0 } );<commit_msg>NL: remove debug print<commit_after>Various fixes to NL-synchronization

* Check if the patron has a PIN-code, before attempting
  to decrypt it. (Some libraries do not use PIN-codes.)

* Force country in address to be 'no'

* Derive sex from personal number.
  Must be present, either 'M' or 'F'.

* Don't sync patrons without cardnumber

* Only sync patrons belonging to categories: 'B', 'V', 'ANS', 'ADMIN'

* Change syncstatus 'edited' when new patron is trying to sync, but
  NL reports that it allready exists.

* Set syncstatus to 'new' when NL reports that patron does not exit.

* Set correct syncstatus 'synced' when an edited patron was successfully synced.

* Re-sync patrons which have been created as new, but allready exitsed with the same fnr in NL.

* Add standard message preferences according to patron category

* Fix re-sync of imported patron from NL

* Retry fixing bad data with up to 3 calls to NL

* Make sure 'server_tid' is stored in borrower_sync.lastsync after successfull sync.

* Turn of sync if patron is missing fnr

* Turn of sync if patron is allready deleted in NL

* Return early if NLSync is called on a patron without a row in borrower_sync

* Don't try syncing when password hash length is not 32


diff --git a/Koha/NorwegianPatronDB.pm b/Koha/NorwegianPatronDB.pm
index 237ddd7..3ba9873 100644
--- a/Koha/NorwegianPatronDB.pm
+++ b/Koha/NorwegianPatronDB.pm
@@ -74,6 +74,7 @@ our %EXPORT_TAGS = ( all => [qw(
         NLGetFirstname
         NLGetSurname
         NLGetSyncDataFromBorrowernumber
+        NLFnrToSex
 )] );
 Exporter::export_ok_tags('all');

@@ -230,6 +231,19 @@ sub NLSync {
         }
     }

+    # NLSync should never have been called, this patron has no row in borrower_sync
+    return unless $sync;
+
+    if (length($sync->hashed_pin) != 32) {
+        # NL password is assumed to be a hashed 4-digit PIN-code. The hash length should
+        # be excactly 32.
+
+        # Turn off any further syncing
+        $sync->update( { 'sync' => 0 } );
+
+        return;
+    }
+
     my $client = SOAP::Lite
         ->on_action( sub { return '""';})
         ->uri( $nl_uri )
@@ -237,33 +251,135 @@ sub NLSync {

     my $cardnumber = SOAP::Data->name( 'lnr' => $patron->cardnumber );

-    # Call the appropriate method based on syncstatus
+    # We will now perform the SOAP call. After that we must carefully
+    # inspect the response and take actions depending
+    # on its contents. There are several situations we have to handle.
+    # NL is very picky about what it accepts, and issues might stem
+    # from missing or bad data or inconsistency with existing data in NL.
+    #
+    # We might need to perform multiple extra SOAP calls to fix the
+    # issue, so in order to not risk enter into an infinite loop, we
+    # limit the number of times we try to fix things before we give
+    # up and return the last response.
+
+    my $max_tries = 3;
+    my $ok;
     my $response;
-    if ( $sync->syncstatus eq 'edited' || $sync->syncstatus eq 'new' ) {
-        my $soap_patron = _koha_patron_to_soap( $patron );
-        if ( $sync->syncstatus eq 'edited' ) {
-            $response = $client->endre( $cardnumber, $soap_patron );
-        } elsif ( $sync->syncstatus eq 'new' ) {
-            $response = $client->nyPost( $soap_patron );
+
+    for (my $i=1; $i<=$max_tries; $i++) {
+        # Call the appropriate method based on syncstatus
+        if ( $sync->syncstatus eq 'edited' || $sync->syncstatus eq 'new' ) {
+            my $soap_patron = _koha_patron_to_soap( $patron );
+            if ( $sync->syncstatus eq 'edited' ) {
+                $response = $client->endre( $cardnumber, $soap_patron );
+            } elsif ( $sync->syncstatus eq 'new' ) {
+                $response = $client->nyPost( $soap_patron );
+            }
+        } elsif ( $sync->syncstatus eq 'delete' ) {
+            $response = $client->slett( $cardnumber );
         }
-    }
-    if ( $sync->syncstatus eq 'delete' ) {
-        $response = $client->slett( $cardnumber );
+
+        ($response, $ok) = _responseOK($sync, $response, $patron, $client);
+        last if $ok;
     }

-    # Update the sync data according to the results
-    if ( $response->{'status'} && $response->{'status'} == 1 ) {
+    return $response;
+}
+
+sub _responseOK {
+    my ( $sync, $response, $patron, $client ) = @_;
+
+    # Handle successfull operation
+    if ( $response->result->{'status'} && $response->result->{'status'} == 1 ) {
         if ( $sync->syncstatus eq 'delete' ) {
             # Turn off any further syncing
             $sync->update( { 'sync' => 0 } );
         }
-        # Update the syncstatus to 'synced'
-        $sync->update( { 'syncstatus' => 'synced' } );
-        # Update the 'synclast' attribute with the "server time" ("server_tid") returned by the method
-        $sync->update( { 'lastsync' => $response->{'server_tid'} } );
+        $sync->update( { 'syncstatus' => 'synced', 'lastsync' => $response->result->{'server_tid'} } );
+        return ($response, 1);
+    }
+
+    # Handle error special cases
+    my $cause = $response->result->{'melding'};
+    return ($response, 1) unless $cause;
+
+    if ($cause =~ /lnr finnes allerede i registeret/) {
+        # Because of staff mistakes, patrons are sometimes registered with existing cardnumber
+        # without checking if they allready exist in NL.
+        # We set syncstatus to 'edited', so that it might be correctly synced next time.
+        # The lastsync value is arbritary, and will be corrected on next iteration
+        my $now = `date +"%Y-%m-%dT%T"`;
+        chomp($now);
+        $sync->update( { 'syncstatus' => 'edited', 'lastsync' => $now } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /lnr finnes ikke i registeret/) {
+        # Make sure syncstatus is 'new'
+        $sync->update( { 'syncstatus' => 'new' } );
+        return ($response, 0);
     }
-    return $response;

+    if ($cause =~ /Mangler verdi for sist_endret/) {
+        # Set an arbritary value to missing laststync, it will be corrected on next iteration
+        my $now = `date +"%Y-%m-%dT%T"`;
+        chomp($now);
+        $sync->update( { 'lastsync' => $now } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /SIST_ENDRET/) {
+        # NL has different timestamp; returnin an error like this:
+        #  endre: Uoverensstemmelse med gjeldende data i basen:
+        #  SIST_ENDRET = 2016-11-12T14:05:22 != '2016-11-07T15:14:30'
+        # Save the timestamp as in NL, to be corrected in next sync
+        my $i = index($cause, 'SIST_ENDRET');
+        $sync->update( { 'lastsync' => substr($cause, $i + 14, 19) } );
+        return ($response, 0);
+    }
+
+    if ($cause =~ /Person med gitt fnr finnes allerede i registeret med nummer/) {
+        # Patron was registered as new, withouth checking for existing patron in NL using the given fnr
+        # We attemt to sync the new cardnumber to NL.
+        # Reference error message:
+        #   Person med gitt fnr finnes allerede i registeret med nummer: N001981874
+        my $old_lnr = substr($cause, 69);
+        my $soap_patron = _koha_patron_to_soap($patron);
+        my $cardnumber = SOAP::Data->name( 'lnr' => $old_lnr );
+
+        # We also need sist_endret value as found in NL:
+        my $old_res = NLSearch($old_lnr);;
+        my $old_patron = $old_res->result()->{'respons_poster'}[0];
+        my $sist_endret = $old_patron->{'sist_endret'};
+        $sync->update( { 'lastsync' => $sist_endret });
+
+        my $new_lnr = $patron->cardnumber;
+        #say "attempting to update cardnumber from '$old_lnr' to '$new_lnr'";
+        $response = $client->endre( $cardnumber, $soap_patron );
+
+        return _responseOK($sync, $response, $patron, $client);
+    }
+
+    if ($cause =~ /Person med gitt fnr finnes allerede i registeret/) {
+        # TODO patron exists with Høgskolekort. Don't know what to do with it.
+        # Needs to be fixed manually by NL maintainer.
+        return ($response, 1);
+    }
+
+    if ($cause =~ /fnr_hash mangler/) {
+        # Missing required personal number. Turn of NL-sync
+        $sync->update( { 'sync' => 0 } );
+        return ($response, 1);
+    }
+
+    if ($cause =~ /slettet tidligere/) {
+        $sync->update( { 'sync' => 0 } );
{'fraction_non_alphanumeric': 0.13845600575401582, 'fraction_numerical': 0.013066410932630064, 'mean_word_length': 2.6130359462970985, 'pattern_counts': {'":': 0, '<': 5, '<?xml version=': 0, '>': 139, 'https://': 0, 'lorem ipsum': 0, 'www.': 0, 'xml': 0}, 'pii_count': 0, 'substrings_counts': 0, 'word_list_counts': {'cursed_substrings.json': 2, 'profanity_word_list.json': 0, 'sexual_word_list.json': 0, 'zh_pornsignals.json': 0}}
{'dir': 'git-commits-cleaned', 'id': '3418774', 'n_tokens_mistral': 5660, 'n_tokens_neox': 5387, 'n_words': 1773}