From 592ab10fe5085bddb078cf019a6fb5ce9a92dc73 Mon Sep 17 00:00:00 2001 From: Jonathan Druart Date: Thu, 11 Feb 2021 12:18:31 +0100 Subject: [PATCH] Bug 22824: Remove C4::Boolean, true_p, boolean_preference, etc. It seems that we don't really need all this overhead. YesNo must be a boolean and contain 1 or 0. Signed-off-by: Kyle M Hall Signed-off-by: Julian Maurice Signed-off-by: Martin Renvoize Signed-off-by: Jonathan Druart Signed-off-by: Jonathan Druart --- C4/Auth.pm | 2 +- C4/Boolean.pm | 105 ------------------------------------ C4/Context.pm | 8 --- admin/systempreferences.pl | 2 +- debian/templates/plack.psgi | 1 - misc/admin/koha-preferences | 37 ++----------- opac/opac-memberentry.pl | 4 +- t/Boolean.t | 39 -------------- 8 files changed, 7 insertions(+), 191 deletions(-) delete mode 100644 C4/Boolean.pm delete mode 100755 t/Boolean.t diff --git a/C4/Auth.pm b/C4/Auth.pm index 8469a204ae..8368dd82ff 100644 --- a/C4/Auth.pm +++ b/C4/Auth.pm @@ -1190,7 +1190,7 @@ sub checkauth { $register_name = $register->name if ($register); } my $branches = { map { $_->branchcode => $_->unblessed } Koha::Libraries->search }; - if ( $type ne 'opac' and C4::Context->boolean_preference('AutoLocation') ) { + if ( $type ne 'opac' and C4::Context->preference('AutoLocation') ) { # we have to check they are coming from the right ip range my $domain = $branches->{$branchcode}->{'branchip'}; diff --git a/C4/Boolean.pm b/C4/Boolean.pm deleted file mode 100644 index 7ef9fc0674..0000000000 --- a/C4/Boolean.pm +++ /dev/null @@ -1,105 +0,0 @@ -package C4::Boolean; - -#package to handle Boolean values in the parameters table -# Note: This is just a utility module; it should not be instantiated. - - -# Copyright 2003 Katipo Communications -# -# This file is part of Koha. -# -# Koha is free software; you can redistribute it and/or modify it -# under the terms of the GNU General Public License as published by -# the Free Software Foundation; either version 3 of the License, or -# (at your option) any later version. -# -# Koha is distributed in the hope that it will be useful, but -# WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with Koha; if not, see . - -use strict; -use warnings; - -use Carp; -use base qw(Exporter); - -our @EXPORT_OK = qw( true_p); - -=head1 NAME - -C4::Boolean - Convenience function to handle boolean values -in the parameter table - -=head1 SYNOPSIS - - use C4::Boolean; - -=head1 DESCRIPTION - -In the parameter table, there are various Boolean values that -variously require a 0/1, no/yes, false/true, or off/on values. -This module aims to provide scripts a means to interpret these -Boolean values in a consistent way which makes common sense. - -=head1 FUNCTIONS - -=over 2 - -=cut - -use constant INVALID_BOOLEAN_STRING_EXCEPTION => - q{The given value does not seem to be interpretable as a Boolean value}; - -our %strings = ( - '0' => 0, '1' => 1, # C - '-1' => 1, # BASIC - 'nil' => 0, 't' => 1, # LISP - 'false' => 0, 'true' => 1, # Pascal - 'off' => 0, 'on' => 1, - 'no' => 0, 'yes' => 1, - 'n' => 0, 'y' => 1, -); - -=item true_p - - if ( C4::Boolean::true_p(C4::Context->preference("IndependentBranches")) ) { - ... - } - -Tries to interpret the passed string as a Boolean value. Returns -the value if the string can be interpreted as such; otherwise an -exception is thrown. - -=cut - -sub true_p { - my $x = shift; - my $it; - if (!defined $x || ref $x ) { - carp INVALID_BOOLEAN_STRING_EXCEPTION; - return; - } - $x = lc $x; - $x =~ s/\s//g; - if (defined $strings{$x}) { - $it = $strings{$x}; - } else { - carp INVALID_BOOLEAN_STRING_EXCEPTION; - } - return $it; -} - -1; -__END__ - -=back - -=head1 AUTHOR - -Koha Development Team - -=cut diff --git a/C4/Context.pm b/C4/Context.pm index f6406fd7ba..06bdf08e44 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -99,7 +99,6 @@ use POSIX (); use YAML::XS; use ZOOM; -use C4::Boolean; use C4::Debug; use Koha::Caches; use Koha::Config::SysPref; @@ -427,13 +426,6 @@ sub preference { return $value; } -sub boolean_preference { - my $self = shift; - my $var = shift; # The system preference to return - my $it = preference($self, $var); - return defined($it)? C4::Boolean::true_p($it): undef; -} - =head2 yaml_preference Retrieves the required system preference value, and converts it diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl index c203385584..7d2e1936b2 100755 --- a/admin/systempreferences.pl +++ b/admin/systempreferences.pl @@ -114,7 +114,7 @@ sub GetPrefParams { $params->{'type_choice'} = 1; } elsif ( $data->{'type'} eq 'YesNo' ) { $params->{'type_yesno'} = 1; - $data->{'value'} = C4::Context->boolean_preference( $data->{'variable'} ); + $data->{'value'} = C4::Context->preference( $data->{'variable'} ); if ( defined( $data->{'value'} ) and $data->{'value'} eq '1' ) { $params->{'value_yes'} = 1; } else { diff --git a/debian/templates/plack.psgi b/debian/templates/plack.psgi index a322114192..89cd6ef48f 100644 --- a/debian/templates/plack.psgi +++ b/debian/templates/plack.psgi @@ -26,7 +26,6 @@ use Plack::Request; use Mojo::Server::PSGI; # Pre-load libraries -use C4::Boolean; use C4::Koha; use C4::Languages; use C4::Letters; diff --git a/misc/admin/koha-preferences b/misc/admin/koha-preferences index 24c0384cac..085844fef2 100755 --- a/misc/admin/koha-preferences +++ b/misc/admin/koha-preferences @@ -20,7 +20,6 @@ use Modern::Perl; use Koha::Script; -use C4::Boolean; use C4::Context; use C4::Debug; use C4::Log; @@ -56,27 +55,6 @@ sub _debug { print STDERR $message . "\n" if ( $C4::Debug::debug ); } -sub _normalize_value { - my ($row) = @_; - - # AFAIK, there are no sysprefs where NULL has a different meaning than ''. - return ( $row->{'value'} || '' ) unless ( $row->{'type'} eq 'YesNo' ); - - # - # In theory, YesNo sysprefs should always contain 1/0. - # In practice, however, they don't. - # - Yogi Berra - # - my $bool_value = eval { - return C4::Boolean::true_p( $row->{'value'} ) ? 1 : 0; - }; - if ( $@ ) { - return 0; - } else { - return $bool_value; - } -} - sub _set_preference { my ( $preference, $value ) = @_; @@ -89,7 +67,7 @@ sub GetPreferences { my $dbh = C4::Context->dbh; return { - map { $_->{'variable'}, _normalize_value($_) } + map { $_->{'variable'}, $_ } @{ $dbh->selectall_arrayref( " SELECT variable, value, type @@ -113,11 +91,6 @@ sub SetPreferences { exit 2 if ( scalar( @$current_state ) != scalar( keys %preferences ) ); - foreach my $row ( @$current_state ) { - # YAML::Syck encodes no as '', so deal with that - $preferences{$row->{'variable'}} = $preferences{$row->{'variable'}} eq '' ? 0 : C4::Boolean::true_p( $preferences{$row->{'variable'}} ) if ( $row->{'type'} eq 'YesNo' ); - } - # Iterate through again, now that we've checked all of the YesNo sysprefs foreach my $row ( @$current_state ) { @@ -151,9 +124,7 @@ sub _fetch_preference { sub GetPreference { my ( $preference ) = @_; - my $row = _fetch_preference( $preference ); - - return _normalize_value( $row ); + return _fetch_preference( $preference ); } sub SetPreference { @@ -161,9 +132,7 @@ sub SetPreference { my $row = _fetch_preference( $preference ); - $value = C4::Boolean::true_p($value) ? 1 : 0 if ( $row->{'type'} eq 'YesNo' ); - - exit 3 if ( $value eq $row->{'value'} ); + exit 3 if ( $value eq $row->{'value'} ); #FIXME exit?? _set_preference( $preference, $value ); } diff --git a/opac/opac-memberentry.pl b/opac/opac-memberentry.pl index 9ca03ef3dd..9e359521ac 100755 --- a/opac/opac-memberentry.pl +++ b/opac/opac-memberentry.pl @@ -163,7 +163,7 @@ if ( $action eq 'create' ) { } else { if ( - C4::Context->boolean_preference( + C4::Context->preference( 'PatronSelfRegistrationVerifyByEmail') ) { @@ -401,7 +401,7 @@ sub GetMandatoryFields { if ( $action eq 'create' || $action eq 'new' ) { $mandatory_fields{'email'} = 1 - if C4::Context->boolean_preference( + if C4::Context->preference( 'PatronSelfRegistrationVerifyByEmail'); } diff --git a/t/Boolean.t b/t/Boolean.t deleted file mode 100755 index 2c5434bd3f..0000000000 --- a/t/Boolean.t +++ /dev/null @@ -1,39 +0,0 @@ - -use Modern::Perl; - -use Test::More tests => 22; -use Test::Warn; - -BEGIN { use_ok( 'C4::Boolean', qw( true_p ) ); } - -is( true_p('0'), '0', 'recognizes \'0\' as false' ); -is( true_p('nil'), '0', 'recognizes \'nil\' as false' ); -is( true_p('false'), '0', 'recognizes \'false\' as false' ); -is( true_p('off'), '0', 'recognizes \'off\' as false' ); -is( true_p('no'), '0', 'recognizes \'no\' as false' ); -is( true_p('n'), '0', 'recognizes \'n\' as false' ); -is( true_p('NO'), '0', 'verified case insensitivity' ); - -is( true_p('1'), '1', 'recognizes \'1\' as true' ); -is( true_p('-1'), '1', 'recognizes \'-1\' as true' ); -is( true_p('t'), '1', 'recognizes \'t\' as true' ); -is( true_p('true'), '1', 'recognizes \'true\' as true' ); -is( true_p('on'), '1', 'recognizes \'on\' as true' ); -is( true_p('yes'), '1', 'recognizes \'yes\' as true' ); -is( true_p('y'), '1', 'recognizes \'y\' as true' ); -is( true_p('YES'), '1', 'verified case insensitivity' ); - -my $result; -warning_like { $result = true_p(undef) } - qr/^The given value does not seem to be interpretable as a Boolean value/, - 'Invalid boolean (undef) raises warning'; -is( $result, undef, 'recognizes undefined as not boolean' ); -warning_like { $result = true_p('foo') } - qr/^The given value does not seem to be interpretable as a Boolean value/, - 'Invalid boolean (\'foo\') raises warning'; -is( $result, undef, 'recognizes \'foo\' as not boolean' ); -warning_like { $result = true_p([]) } - qr/^The given value does not seem to be interpretable as a Boolean value/, - 'Invalid boolean (reference) raises warning'; -is( $result, undef, 'recognizes a reference as not a boolean' ); - -- 2.39.5