Attachment 32890 Details for Bug 11401 – Bug 11401 - QA followup

[patch] Bug 11401 - QA followup

Bug-11401---QA-followup.patch (text/plain), 10.35 KB, created by Magnus Enger on 2014-10-29 11:37:50 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Magnus Enger

Created: 2014-10-29 11:37:50 UTC

Size: 10.35 KB

patch

obsolete

>From e08d60bc53f065f5164f5ae14a87bf9c6a795991 Mon Sep 17 00:00:00 2001
>From: Magnus Enger <digitalutvikling@gmail.com>
>Date: Wed, 29 Oct 2014 10:26:11 +0100
>Subject: [PATCH] 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
>---
> C4/Members.pm                                        |    8 ++++----
> Koha/NorwegianPatronDB.pm                            |    2 +-
> .../prog/en/modules/admin/preferences/patrons.pref   |    8 ++++----
> members/deletemem.pl                                 |    4 ++--
> members/memberentry.pl                               |    6 +++---
> members/moremember.pl                                |    4 ++--
> misc/cronjobs/nl-sync-from-koha.pl                   |   18 ++++++++++++++++++
> 7 files changed, 34 insertions(+), 16 deletions(-)
>
>diff --git a/C4/Members.pm b/C4/Members.pm
>index b754aeb..577c738 100644
>--- 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 );
> }
> 
>@@ -789,7 +789,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} );
>             }
>@@ -815,7 +815,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'}
>@@ -883,7 +883,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
>--- 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
>--- 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
>--- 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
>--- 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
>--- 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
>--- 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 ) {
>-- 
>1.7.10.4

Actions: View | Diff | Splinter Review

Attachments on bug 11401: 30752 | 31710 | 31711 | 31815 | 31816 | 32546 | 32607 | 32888 | 32889 | 32890 | 32949 | 32950 | 32951 | 32952 | 33528 | 33570 | 33582