Bug 24759: CleanUp OpacRenewalBranch values

We had a unique behvaiour where the syspref was set to string 'NULL'
as opposed to undef, we need to clean that up

To test:
1 - Set OpacRenewalBranch to 'NULL' in staff interface
2 - Renew via opac
3 - Check statistics to ensure branch is blank

Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

diff --git a/Koha/Item.pm b/Koha/Item.pm
index 2b5239e..148f3d6 100644 (file)
--- a/Koha/Item.pm
+++ b/Koha/Item.pm
@@ -715,7 +715,7 @@ sub renewalbranch {
     my $branchcode;
     if ( $interface eq 'opac' ){
         my $renewalbranch = C4::Context->preference('OpacRenewalBranch');
-        if( !defined $renewalbranch ){
+        if( !defined $renewalbranch || $renewalbranch eq 'opacrenew' ){
             $branchcode = 'OPACRenew';
         }
         elsif ( $renewalbranch eq 'itemhomebranch' ) {
@@ -727,11 +727,8 @@ sub renewalbranch {
         elsif ( $renewalbranch eq 'checkoutbranch' ) {
             $branchcode = $self->checkout->branchcode;
         }
-        elsif ( $renewalbranch eq 'NULL' ) {
-            $branchcode = '';
-        }
         else {
-            $branchcode = 'OPACRenew';
+            $branchcode = "";
         }
     } else {
         $branchcode = ( C4::Context->userenv && defined C4::Context->userenv->{branch} )

diff --git a/installer/data/mysql/atomicupdate/Bug_24759_cleanup_OpacRenewalBranch.perl b/installer/data/mysql/atomicupdate/Bug_24759_cleanup_OpacRenewalBranch.perl
new file mode 100644 (file)
index 0000000..f447939
--- /dev/null
+++ b/installer/data/mysql/atomicupdate/Bug_24759_cleanup_OpacRenewalBranch.perl
@@ -0,0 +1,16 @@
+$DBversion = 'XXX';
+if( CheckVersion( $DBversion ) ) {
+    $dbh->do(q{
+        UPDATE systempreferences SET options = 'itemhomebranch|patronhomebranch|checkoutbranch|none' WHERE variable='OpacRenewalBranch'
+    });
+    $dbh->do(q{
+        UPDATE systempreferences SET value = "none" WHERE variable='OpacRenewalBranch'
+        AND value = 'NULL'
+    });
+    $dbh->do(q{
+        UPDATE systempreferences SET value = 'opacrenew' WHERE variable='OpacRenewalBranch'
+        AND value NOT IN ('checkoutbranch','itemhomebranch','opacrenew','patronhomebranch','none')
+    });
+    SetVersion( $DBversion );
+    print "Upgrade to $DBversion done (Bug 24759 - cleanup OpacRenewalBranch)\n";
+}

diff --git a/installer/data/mysql/sysprefs.sql b/installer/data/mysql/sysprefs.sql
index ba95509..d1aa527 100644 (file)
--- a/installer/data/mysql/sysprefs.sql
+++ b/installer/data/mysql/sysprefs.sql
@@ -421,7 +421,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, `
 ('OpacPublic','1',NULL,'Turn on/off public OPAC','YesNo'),
 ('opacreadinghistory','1','','If ON, enables display of Patron Circulation History in OPAC','YesNo'),
 ('OpacRenewalAllowed','0',NULL,'If ON, users can renew their issues directly from their OPAC account','YesNo'),
-('OpacRenewalBranch','checkoutbranch','itemhomebranch|patronhomebranch|checkoutbranch|null','Choose how the branch for an OPAC renewal is recorded in statistics','Choice'),
+('OpacRenewalBranch','checkoutbranch','itemhomebranch|patronhomebranch|checkoutbranch|none','Choose how the branch for an OPAC renewal is recorded in statistics','Choice'),
 ('OpacResetPassword','0','','Shows the ''Forgot your password?'' link in the OPAC','YesNo'),
 ('OPACResultsLibrary', 'homebranch', 'homebranch|holdingbranch', 'Defines whether the OPAC displays the holding or home branch in search results when using XSLT', 'Choice'),
 ('OPACResultsSidebar','','70|10','Define HTML to be included on the search results page, underneath the facets sidebar','Textarea'),

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
index 9c9b2b7..f653396 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/opac.pref
@@ -618,7 +618,7 @@ OPAC:
                   itemhomebranch: "the item's home library"
                   patronhomebranch: "the patron's home library"
                   checkoutbranch: "the library the item was checked out from"
-                  null: "NULL"
+                  none: "NULL"
                   opacrenew: "'OPACRenew'"
             - as branchcode to store in the statistics table.
         -

diff --git a/t/db_dependent/Koha/Item.t b/t/db_dependent/Koha/Item.t
index 51bdc8f..116d945 100644 (file)
--- a/t/db_dependent/Koha/Item.t
+++ b/t/db_dependent/Koha/Item.t
@@ -463,17 +463,13 @@ subtest 'renewalbranch' => sub {
 
     C4::Context->interface( 'opac' );
 
-    t::lib::Mocks::mock_preference('OpacRenewalBranch', '');
-    is( $item->renewalbranch, 'OPACRenew', "If interface opac and OpacRenewalBranch blank, we get the OPACRenew");
-    is( $item->renewalbranch({branch=>'CHICKEN'}), 'OPACRenew', "If interface opac and OpacRenewalBranch blank, we get the OPACRenew even if branch passes");
-
     t::lib::Mocks::mock_preference('OpacRenewalBranch', undef);
     is( $item->renewalbranch, 'OPACRenew', "If interface opac and OpacRenewalBranch undef, we get OPACRenew");
     is( $item->renewalbranch({branch=>'COW'}), 'OPACRenew', "If interface opac and OpacRenewalBranch undef, we get OPACRenew even if branch passed");
 
-    t::lib::Mocks::mock_preference('OpacRenewalBranch', 'NULL');
-    is( $item->renewalbranch, '', "If interface opac and OpacRenewalBranch is string 'NULL', we get blank string");
-    is( $item->renewalbranch({branch=>'COW'}), '', "If interface opac and OpacRenewalBranch is string 'NULL', we get blank string even if branch passed");
+    t::lib::Mocks::mock_preference('OpacRenewalBranch', 'none');
+    is( $item->renewalbranch, '', "If interface opac and OpacRenewalBranch is none, we get blank string");
+    is( $item->renewalbranch({branch=>'COW'}), '', "If interface opac and OpacRenewalBranch is none, we get blank string even if branch passed");
 
     t::lib::Mocks::mock_preference('OpacRenewalBranch', 'checkoutbranch');
     is( $item->renewalbranch, $checkout->branchcode, "If interface opac and OpacRenewalBranch set to checkoutbranch, we get branch of checkout");