Bug 11401: QA followup

1) Be more careful when checking the NorwegianPatronDBEnable syspref.

Before:
if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {

After:
if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {

This should avoid complaints if the syspref is not initialized.

2) Fix some empty =head2 POD sections

3) Fix some indentation in patrons.pref, to make xt/yaml_valid.t happy

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
I couldn't find any regressions with adding, editing and deleting members.

Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

diff --git a/C4/Members.pm b/C4/Members.pm
index d528259..d88ea45 100644 (file)
--- a/C4/Members.pm
+++ b/C4/Members.pm
@@ -44,7 +44,7 @@ use Text::Unaccent qw( unac_string );
 use Koha::AuthUtils qw(hash_password);
 use Koha::Database;
 use Module::Load;
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     load Koha::NorwegianPatronDB, qw( NLUpdateHashedPIN NLEncryptPIN NLSync );
 }
 
@@ -790,7 +790,7 @@ sub ModMember {
         if ($data{password} eq '****' or $data{password} eq '') {
             delete $data{password};
         } else {
-            if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+            if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
                 # Update the hashed PIN in borrower_sync.hashed_pin, before Koha hashes it
                 NLUpdateHashedPIN( $data{'borrowernumber'}, $data{password} );
             }
@@ -816,7 +816,7 @@ sub ModMember {
 
         # If NorwegianPatronDBEnable is enabled, we set syncstatus to something that a
         # cronjob will use for syncing with NL
-        if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+        if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
             my $borrowersync = Koha::Database->new->schema->resultset('BorrowerSync')->find({
                 'synctype'       => 'norwegianpatrondb',
                 'borrowernumber' => $data{'borrowernumber'}
@@ -884,7 +884,7 @@ sub AddMember {
 
     # If NorwegianPatronDBEnable is enabled, we set syncstatus to something that a
     # cronjob will use for syncing with NL
-    if ( exists $data{'borrowernumber'} && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+    if ( exists $data{'borrowernumber'} && C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
         Koha::Database->new->schema->resultset('BorrowerSync')->create({
             'borrowernumber' => $data{'borrowernumber'},
             'synctype'       => 'norwegianpatrondb',

diff --git a/Koha/NorwegianPatronDB.pm b/Koha/NorwegianPatronDB.pm
index 51955eb..c38d687 100644 (file)
--- a/Koha/NorwegianPatronDB.pm
+++ b/Koha/NorwegianPatronDB.pm
@@ -39,7 +39,7 @@ But in scripts that are also used by others (like e.g. moremember.pl), it will
 be polite to only load the module at runtime, if it is needed:
 
   use Module::Load;
-  if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+  if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
       load Koha::NorwegianPatronDB, qw( NLGetSyncDataFromBorrowernumber );
   }
 

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
index b245d1a..aa567c3 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/patrons.pref
@@ -139,8 +139,8 @@ Patrons:
          - pref: CardnumberLength
          - "characters long. The length can be a single number to specify an exact length, a range separated by a comma (i.e., 'Min,Max'), or a maximum with no minimum (i.e., ',Max')."
          - "If 'cardnumber' is included in the BorrowerMandatoryField list, the minimum length, if not specified here, defaults to one."
-    Norwegian patron database:
-    -
+    "Norwegian patron database":
+     -
          - pref: NorwegianPatronDBEnable
            choices:
                yes: Enable
@@ -148,13 +148,13 @@ Patrons:
          - the ability to communicate with the Norwegian national patron database via the
          - pref: NorwegianPatronDBEndpoint
          - endpoint.
-    -
+     -
          - Communicate with the Norwegian national patron database using the username
          - pref: NorwegianPatronDBUsername
          - and the password
          - pref: NorwegianPatronDBPassword
          - . You can get these from "Base Bibliotek", which is maintained by the Norwegian National Library.
-    -
+     -
          - pref: NorwegianPatronDBSearchNLAfterLocalHit
            choices:
                yes: Do

diff --git a/members/deletemem.pl b/members/deletemem.pl
index 36aaf5d..85c0166 100755 (executable)
--- a/members/deletemem.pl
+++ b/members/deletemem.pl
@@ -32,7 +32,7 @@ use C4::Members;
 use C4::Branch; # GetBranches
 use C4::VirtualShelves (); #no import
 use Module::Load;
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     load Koha::NorwegianPatronDB, qw( NLMarkForDeletion NLSync );
 }
 
@@ -56,7 +56,7 @@ my $member       = $input->param('member');
 # out. If "deletelocal" is set to "true", or not set to anything normal
 # deletion will be done.
 my $deletelocal  = $input->param('deletelocal')  eq 'false' ? 0 : 1; # Deleting locally is the default
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     if ( $input->param('deleteremote') eq 'true' ) {
         # Mark for deletion, then try a live sync
         NLMarkForDeletion( $member );

diff --git a/members/memberentry.pl b/members/memberentry.pl
index a9773d6..7a0329f 100755 (executable)
--- a/members/memberentry.pl
+++ b/members/memberentry.pl
@@ -44,7 +44,7 @@ use C4::Form::MessagingPreferences;
 use Koha::Borrower::Debarments;
 use Koha::DateUtils;
 use Module::Load;
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     load Koha::NorwegianPatronDB, qw( NLGetSyncDataFromBorrowernumber );
 }
 
@@ -421,7 +421,7 @@ if ((!$nok) and $nodouble and ($op eq 'insert' or $op eq 'save')){
             C4::Form::MessagingPreferences::handle_form_action($input, { borrowernumber => $borrowernumber }, $template, 1, $newdata{'categorycode'});
         }
         # Try to do the live sync with the Norwegian national patron database, if it is enabled
-        if ( exists $data{'borrowernumber'} && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+        if ( exists $data{'borrowernumber'} && C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
             NLSync({ 'borrowernumber' => $borrowernumber });
         }
        } elsif ($op eq 'save'){ 
@@ -479,7 +479,7 @@ if ($op eq "modify")  {
         $template->param( categorycode => $borrower_data->{'categorycode'} );
     }
     # Add sync data to the user data
-    if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+    if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
         my $sync = NLGetSyncDataFromBorrowernumber( $borrowernumber );
         if ( $sync ) {
             $template->param(

diff --git a/members/moremember.pl b/members/moremember.pl
index d056580..3fc92ca 100755 (executable)
--- a/members/moremember.pl
+++ b/members/moremember.pl
@@ -54,7 +54,7 @@ use List::MoreUtils qw/uniq/;
 use C4::Members::Attributes qw(GetBorrowerAttributes);
 use Koha::Borrower::Debarments qw(GetDebarments IsDebarred);
 use Module::Load;
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     load Koha::NorwegianPatronDB, qw( NLGetSyncDataFromBorrowernumber );
 }
 #use Smart::Comments;
@@ -295,7 +295,7 @@ else {
 }
 
 # Add sync data to the user data
-if ( C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
+if ( C4::Context->preference('NorwegianPatronDBEnable') && C4::Context->preference('NorwegianPatronDBEnable') == 1 ) {
     my $sync = NLGetSyncDataFromBorrowernumber( $borrowernumber );
     if ( $sync ) {
         $data->{'sync'}       = $sync->sync;

diff --git a/misc/cronjobs/nl-sync-from-koha.pl b/misc/cronjobs/nl-sync-from-koha.pl
index f62fd7a..81e3218 100644 (file)
--- a/misc/cronjobs/nl-sync-from-koha.pl
+++ b/misc/cronjobs/nl-sync-from-koha.pl
@@ -46,6 +46,8 @@ This script performs the following steps:
 
 =head2 Check sysprefs
 
+Check that the necessary sysprefs are set before proceeding.
+
 =cut
 
 my $check_result = NLCheckSysprefs();
@@ -62,6 +64,18 @@ unless ( $run ) {
 
 =head2 Find patrons that need to be synced
 
+Patrons with either of these statuses:
+
+=over 4
+
+=item * edited
+
+=item * new
+
+=item * deleted
+
+=back
+
 =cut
 
 my @needs_sync = Koha::Database->new->schema->resultset('BorrowerSync')->search({
@@ -78,6 +92,8 @@ my @needs_sync = Koha::Database->new->schema->resultset('BorrowerSync')->search(
 
 =head2 Do the actual sync
 
+Data is synced to NL with NLSync.
+
 =cut
 
 my $sync_success = 0;
@@ -106,6 +122,8 @@ foreach my $borrower ( @needs_sync ) {
 
 =head2 Summarize if verbose mode is enabled
 
+Specify -v on the command line to get a summary of the syncing operations.
+
 =cut
 
 if ( $verbose ) {