From 8dfb0adc8c4ce5b75a8d945b58836fb7e41cb685 Mon Sep 17 00:00:00 2001 From: Nick Clemens Date: Fri, 28 Feb 2020 13:23:44 +0000 Subject: [PATCH] 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 Signed-off-by: Jonathan Druart Signed-off-by: Martin Renvoize --- Koha/Item.pm | 7 ++----- .../Bug_24759_cleanup_OpacRenewalBranch.perl | 16 ++++++++++++++++ installer/data/mysql/sysprefs.sql | 2 +- .../prog/en/modules/admin/preferences/opac.pref | 2 +- t/db_dependent/Koha/Item.t | 10 +++------- 5 files changed, 23 insertions(+), 14 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/Bug_24759_cleanup_OpacRenewalBranch.perl diff --git a/Koha/Item.pm b/Koha/Item.pm index 2b5239e611..148f3d6ab6 100644 --- 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 index 0000000000..f447939027 --- /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 ba95509549..d1aa527448 100644 --- 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 9c9b2b7f00..f653396792 100644 --- 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 51bdc8fb21..116d9458e2 100644 --- 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"); -- 2.39.5