From b9d16c6572207f821ebd3e549cacf60fb03b2e6d Mon Sep 17 00:00:00 2001 From: Mark Tompsett Date: Fri, 27 Feb 2015 22:44:50 -0500 Subject: [PATCH] Bug 13399: Missing validation on authorised value descriptions This adds validations which force both the OPAC and Staff Client authorized value descriptions to be filled in. TEST PLAN --------- 1) Login to staff client 2) Koha administration -> Authorized values 3) Change drop-down to 'DEPARTMENT' -- Assuming you have default values installed, and DEPARTMENT exists. 4) Click '+ New authorized value for DEPARTMENT' 5) Enter a 'Authorized value:' and click 'Save' -- This will create it without OPAC and Staff Client descriptions! This is wrong. 6) Apply patch 7) $ ./installer/data/mysql/updatedatabase.pl -- expecting atomic update to run. 8) Refresh staff client page -- the descriptions should now be filled in. 9) Repeat steps 4 and 5 -- This time, you will be given two error messages -- Nothing will be created. 10) Attempt to create a new authorized value with descriptions. -- should save without issue. 11) Run koha qa test tools -- SQL files are unchecked, but steps 7 & 8 confirm validity. -- Should be no issues. --- admin/authorised_values.pl | 38 ++++++++++++--------- ...399-fix-empty-authorised-value-descriptions.sql | 39 ++++++++++++++++++++++ .../prog/en/modules/admin/authorised_values.tt | 8 ++++- 3 files changed, 69 insertions(+), 16 deletions(-) create mode 100644 installer/data/mysql/atomicupdate/bug-13399-fix-empty-authorised-value-descriptions.sql diff --git a/admin/authorised_values.pl b/admin/authorised_values.pl index 422f3a9..953fb8c 100755 --- a/admin/authorised_values.pl +++ b/admin/authorised_values.pl @@ -124,18 +124,25 @@ if ($op eq 'add_form') { $imageurl = '' if $imageurl =~ /removeImage/; my $duplicate_entry = 0; my @branches = $input->param('branches'); + my $lib = $input->param('lib'); + my $lib_opac = $input->param('lib_opac'); + undef $lib if ($lib eq ""); # to insert NULL instead of a blank string + undef $lib_opac if ($lib_opac eq ""); # to insert NULL instead of a blank string + my $category = $input->param('category'); + my $authorised_value; if ( $id ) { # Update my $sth = $dbh->prepare( "SELECT category, authorised_value FROM authorised_values WHERE id = ? "); $sth->execute($id); - my ($category, $authorised_value) = $sth->fetchrow_array(); + ($category, $authorised_value) = $sth->fetchrow_array(); if ( $authorised_value ne $new_authorised_value ) { my $sth = $dbh->prepare_cached( "SELECT COUNT(*) FROM authorised_values " . "WHERE category = ? AND authorised_value = ? and id <> ? "); $sth->execute($new_category, $new_authorised_value, $id); ($duplicate_entry) = $sth->fetchrow_array(); } - unless ( $duplicate_entry ) { + + if ( !$duplicate_entry && defined($lib) && defined($lib_opac)) { my $sth=$dbh->prepare( 'UPDATE authorised_values SET category = ?, authorised_value = ?, @@ -143,10 +150,6 @@ if ($op eq 'add_form') { lib_opac = ?, imageurl = ? WHERE id=?' ); - my $lib = $input->param('lib'); - my $lib_opac = $input->param('lib_opac'); - undef $lib if ($lib eq ""); # to insert NULL instead of a blank string - undef $lib_opac if ($lib_opac eq ""); # to insert NULL instead of a blank string $sth->execute($new_category, $new_authorised_value, $lib, $lib_opac, $imageurl, $id); if ( @branches ) { $sth = $dbh->prepare("DELETE FROM authorised_values_branches WHERE av_id = ?"); @@ -171,14 +174,11 @@ if ($op eq 'add_form') { "WHERE category = ? AND authorised_value = ? "); $sth->execute($new_category, $new_authorised_value); ($duplicate_entry) = $sth->fetchrow_array(); - unless ( $duplicate_entry ) { + + if ( !$duplicate_entry && defined($lib) && defined($lib_opac)) { my $sth=$dbh->prepare( 'INSERT INTO authorised_values ( category, authorised_value, lib, lib_opac, imageurl ) values (?, ?, ?, ?, ?)' ); - my $lib = $input->param('lib'); - my $lib_opac = $input->param('lib_opac'); - undef $lib if ($lib eq ""); # to insert NULL instead of a blank string - undef $lib_opac if ($lib_opac eq ""); # to insert NULL instead of a blank string $sth->execute( $new_category, $new_authorised_value, $lib, $lib_opac, $imageurl ); $id = $dbh->{'mysql_insertid'}; if ( @branches ) { @@ -192,7 +192,6 @@ if ($op eq 'add_form') { $sth->execute($id, $branchcode); } } - my $category = $input->param('category'); print $input->redirect("/cgi-bin/koha/admin/authorised_values.pl?searchfield=$category&offset=$offset"); exit; } @@ -201,9 +200,18 @@ if ($op eq 'add_form') { $template->param(duplicate_category => $new_category, duplicate_value => $new_authorised_value, else => 1); - default_form(); - } - + } + if ( !defined($lib) ) { + $template->param(BadLib => 1, + else => 1); + } + if ( !defined($lib_opac) ) { + $template->param(BadLibOpac => 1, + else => 1); + } + $searchfield=$category; + default_form(); + ################## DELETE_CONFIRM ################################## # called by default form, used to confirm deletion of data in DB } elsif ($op eq 'delete_confirm') { diff --git a/installer/data/mysql/atomicupdate/bug-13399-fix-empty-authorised-value-descriptions.sql b/installer/data/mysql/atomicupdate/bug-13399-fix-empty-authorised-value-descriptions.sql new file mode 100644 index 0000000..19784ef --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug-13399-fix-empty-authorised-value-descriptions.sql @@ -0,0 +1,39 @@ +-- Attempt to correct the OPAC and Staff Client authorised value +-- descriptions from each other. +-- Update staff client authorised values description +UPDATE authorised_values + SET lib=lib_opac + WHERE (lib IS NULL OR lib=''); + +-- Update OPAC authorised values description +UPDATE authorised_values + SET lib_opac=lib + WHERE (lib_opac IS NULL OR lib_opac=''); + +-- However, they might both be blank, so attempt authorised_value +-- Update staff client authorised values description +UPDATE authorised_values + SET lib=authorised_value + WHERE (lib IS NULL OR lib=''); + +-- Update OPAC authorised values description +UPDATE authorised_values + SET lib_opac=authorised_value + WHERE (lib_opac IS NULL OR lib_opac=''); + +-- In the extremely unlikely event they are still blank, attempt +-- category +-- Update staff client authorised values description +UPDATE authorised_values + SET lib=category + WHERE (lib IS NULL OR lib=''); + +-- Update OPAC authorised values description +UPDATE authorised_values + SET lib_opac=category + WHERE (lib_opac IS NULL OR lib_opac=''); + +-- Update authorised values code, in the case it is blank. +UPDATE authorised_values + SET authorised_value=category + WHERE (authorised_value IS NULL OR authorised_value=''); diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt index 7dae71e..0e98352 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/authorised_values.tt @@ -194,9 +194,15 @@ $(document).ready(function() {
Could not add value "[% duplicate_value %]" for category "[% duplicate_category %]" — value already present.
[% END %] +[% IF ( BadLib ) %] +
Staff client authorized value missing description
+[% END %] +[% IF ( BadLibOpac ) %] +
OPAC authorized value missing description
+[% END %]
- [% FOREACH value IN tab_list.values %] [% IF ( value == tab_list.default ) %] -- 1.9.1