From 7055d4a27088a6a90f83c951974d9b8ba43ef7e9 Mon Sep 17 00:00:00 2001 From: Robin Sheat Date: Thu, 27 Mar 2014 17:44:23 +1300 Subject: [PATCH] Bug 11998 - move most syspref db access into Context.pm There was a lot of direct database access in systempreferences.pl. This moves a lot of it into C4::Context so that it'll interact with caching better, and to centralise common functions and logging in one place. Test plan: * Make sure that creating, editing, deleting local use preferences works * Make sure that other system preferences still function normally --- C4/Context.pm | 76 ++++++++++++++++++++++++++++++++++++---------- admin/systempreferences.pl | 61 ++++++++----------------------------- 2 files changed, 73 insertions(+), 64 deletions(-) diff --git a/C4/Context.pm b/C4/Context.pm index 0c48296..5249cab 100644 --- a/C4/Context.pm +++ b/C4/Context.pm @@ -102,6 +102,7 @@ use ZOOM; use XML::Simple; use C4::Boolean; use C4::Debug; +use C4::Log qw/ logaction /; use POSIX (); use DateTime::TimeZone; use Module::Load::Conditional qw(can_load); @@ -623,37 +624,80 @@ sub clear_syspref_cache { =head2 set_preference - C4::Context->set_preference( $variable, $value ); + C4::Context->set_preference( $variable, $value, [ $explanation, $type, $options ] ); This updates a preference's value both in the systempreferences table and in -the sysprefs cache. +the sysprefs cache. If the optional parameters are provided, then the query +becomes a create. It won't update the parameters (except value) for an existing +preference. =cut sub set_preference { - my $self = shift; - my $var = lc(shift); - my $value = shift; + my ( $self, $var, $value, $expl, $type, $options ) = @_; + $var = lc($var); my $dbh = C4::Context->dbh or return 0; - my $type = $dbh->selectrow_array( "SELECT type FROM systempreferences WHERE variable = ?", {}, $var ); - - $value = 0 if ( $type && $type eq 'YesNo' && $value eq '' ); + my $db_type = $dbh->selectrow_array( + "SELECT type FROM systempreferences WHERE variable = ?", + {}, $var ); - my $sth = $dbh->prepare( " - INSERT INTO systempreferences - ( variable, value ) - VALUES( ?, ? ) - ON DUPLICATE KEY UPDATE value = VALUES(value) - " ); + $value = 0 if ( $db_type && $db_type eq 'YesNo' && $value eq '' ); - if($sth->execute( $var, $value )) { - $syspref_cache{$var} = [time, $value]; + my ( $query, @params, $logtype ); + if ( !defined($db_type) ) { + $query = ' +INSERT INTO systempreferences + ( variable, value, explanation, type, options ) +VALUES( ?, ?, ?, ?, ? )'; + @params = ( $var, $value, $expl, $type, $options ); + $logtype = 'ADD'; + } + else { + $query = ' +INSERT INTO systempreferences + ( variable, value ) +VALUES( ?, ? ) +ON DUPLICATE KEY UPDATE value = VALUES(value)'; + @params = ( $var, $value ); + $logtype = 'MODIFY'; + } + my $sth = $dbh->prepare($query); + if ( $sth->execute(@params) ) { + $syspref_cache{$var} = [ time, $value ]; + logaction( 'SYSTEMPREFERENCE', $logtype, undef, $var . " | " . $value ); } $sth->finish; } +=head2 delete_preference + + C4::Context->delete_preference( $variable ); + +This deletes a system preference from the database. Returns a true value on +success. Failure means there was an issue with the database, not that there +was no syspref of the name. + +=cut + +sub delete_preference { + my ( $self, $var ) = @_; + + # We want to log the previous value + my $val = C4::Context->preference($var); + my $dbh = C4::Context->dbh or die "Unable to talk to the database"; + my $sth = $dbh->prepare("DELETE FROM systempreferences WHERE variable=?"); + my $res = $sth->execute($var); + if ($res) { + delete $syspref_cache{$var}; + logaction( 'SYSTEMPREFERENCE', 'DELETE', undef, + $var . " | " . ( $var // 'undefined' ) ); + return 1; + } + warn "Unable to delete syspref: " . $sth->errstr; + return 0; +} # AUTOLOAD # This implements C4::Config->foo, and simply returns # C4::Context->config("foo"), as described in the documentation for diff --git a/admin/systempreferences.pl b/admin/systempreferences.pl index d41f2a7..d043ea3 100755 --- a/admin/systempreferences.pl +++ b/admin/systempreferences.pl @@ -50,7 +50,6 @@ use C4::Context; use C4::Koha; use C4::Languages qw(getTranslatedLanguages); use C4::ClassSource; -use C4::Log; use C4::Output; use YAML::Syck qw( Dump LoadFile ); @@ -269,24 +268,8 @@ if ( $op eq 'update_and_reedit' ) { ); # we show only the TMPL_VAR names $op } } - my $dbh = C4::Context->dbh; - my $query = "select * from systempreferences where variable=?"; - my $sth = $dbh->prepare($query); - $sth->execute( $input->param('variable') ); - if ( $sth->rows ) { - unless ( C4::Context->config('demo') ) { - my $sth = $dbh->prepare("update systempreferences set value=?,explanation=?,type=?,options=? where variable=?"); - $sth->execute( $value, $input->param('explanation'), $input->param('variable'), $input->param('preftype'), $input->param('prefoptions') ); - logaction( 'SYSTEMPREFERENCE', 'MODIFY', undef, $input->param('variable') . " | " . $value ); - } - } else { - unless ( C4::Context->config('demo') ) { - my $sth = $dbh->prepare("insert into systempreferences (variable,value,explanation) values (?,?,?,?,?)"); - $sth->execute( $input->param('variable'), $input->param('value'), $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions') ); - logaction( 'SYSTEMPREFERENCE', 'ADD', undef, $input->param('variable') . " | " . $input->param('value') ); - } - } - + my $variable = $input->param('variable'); + C4::Context->set_preference($variable, $value) unless C4::Context->config('demo'); } ################## ADD_FORM ################################## @@ -315,13 +298,14 @@ if ( $op eq 'add_form' ) { ################## ADD_VALIDATE ################################## # called by add_form, used to insert/modify data in DB } elsif ( $op eq 'add_validate' ) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("select * from systempreferences where variable=?"); - $sth->execute( $input->param('variable') ); - # to handle multiple values my $value; + my $variable = $input->param('variable'); + my $expl = $input->param('explanation'); + my $type = $input->param('preftype'); + my $options = $input->param('prefoptions'); + # handle multiple value strings (separated by ',') my $params = $input->Vars; if ( defined $params->{'value'} ) { @@ -338,49 +322,30 @@ if ( $op eq 'add_form' ) { } } - if ( $input->param('preftype') eq 'Upload' ) { + if ( $type eq 'Upload' ) { my $lgtfh = $input->upload('value'); $value = join '', <$lgtfh>; $value = encode_base64($value); } - if ( $sth->rows ) { - unless ( C4::Context->config('demo') ) { - my $sth = $dbh->prepare("update systempreferences set value=?,explanation=?,type=?,options=? where variable=?"); - $sth->execute( $value, $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions'), $input->param('variable') ); - logaction( 'SYSTEMPREFERENCE', 'MODIFY', undef, $input->param('variable') . " | " . $value ); - } - } else { - unless ( C4::Context->config('demo') ) { - my $sth = $dbh->prepare("insert into systempreferences (variable,value,explanation,type,options) values (?,?,?,?,?)"); - $sth->execute( $input->param('variable'), $value, $input->param('explanation'), $input->param('preftype'), $input->param('prefoptions') ); - logaction( 'SYSTEMPREFERENCE', 'ADD', undef, $input->param('variable') . " | " . $value ); - } - } + C4::Context->set_preference( $variable, $value, $expl, $type, $options ) + unless C4::Context->config('demo'); print "Content-Type: text/html\n\n"; exit; ################## DELETE_CONFIRM ################################## # called by default form, used to confirm deletion of data in DB } elsif ( $op eq 'delete_confirm' ) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("select variable,value,explanation,type,options from systempreferences where variable=?"); - $sth->execute($searchfield); - my $data = $sth->fetchrow_hashref; + my $value = C4::Context->preference($searchfield); $template->param( searchfield => $searchfield, - Tvalue => $data->{'value'}, + Tvalue => $value, ); # END $OP eq DELETE_CONFIRM ################## DELETE_CONFIRMED ################################## # called by delete_confirm, used to effectively confirm deletion of data in DB } elsif ( $op eq 'delete_confirmed' ) { - my $dbh = C4::Context->dbh; - my $sth = $dbh->prepare("delete from systempreferences where variable=?"); - $sth->execute($searchfield); - my $logstring = $searchfield . " | " . $Tvalue; - logaction( 'SYSTEMPREFERENCE', 'DELETE', undef, $logstring ); - + C4::Context->delete_preference($searchfield); # END $OP eq DELETE_CONFIRMED ################## DEFAULT ################################## } else { # DEFAULT -- 1.8.3.2