From b9d16c6572207f821ebd3e549cacf60fb03b2e6d Mon Sep 17 00:00:00 2001
From: Mark Tompsett <mtompset@hotmail.com>
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() {
<div class="dialog alert">Could not add value "[% duplicate_value %]" for category "[% duplicate_category %]" — value already present.
</div>
[% END %]
+[% IF ( BadLib ) %]
+<div class="dialog alert">Staff client authorized value missing description</div>
+[% END %]
+[% IF ( BadLibOpac ) %]
+<div class="dialog alert">OPAC authorized value missing description</div>
+[% END %]
<form action="/cgi-bin/koha/admin/authorised_values.pl" method="post" id="category">
<label for="searchfield">Show category: </label>
- <select name="searchfield" id="searchfield" size="1">
+ <select name="searchfield" id="searchfield" value="[% tab_list.default %]">
[% FOREACH value IN tab_list.values %]
[% IF ( value == tab_list.default ) %]
<option value="[% value %]" selected="selected">[% value %]</option>
--
1.9.1