Attachment 37785 Details for Bug 13399 – Bug 13399: Missing validation on authorised value descriptions

[patch] Bug 13399: Missing validation on authorised value descriptions

Bug-13399-Missing-validation-on-authorised-value-d.patch (text/plain), 8.76 KB, created by Mark Tompsett on 2015-04-14 02:00:25 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mark Tompsett

Created: 2015-04-14 02:00:25 UTC

Size: 8.76 KB

patch

obsolete

>From d6a6fc81eae5e242305f10c0e8883c96572b945c 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&amp;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..af74a60 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 &quot;[% duplicate_value %]&quot; for category &quot;[% duplicate_category %]&quot; &mdash; 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">
>     [% FOREACH value IN tab_list.values %]
>     [% IF ( value == tab_list.default ) %]
>         <option value="[% value %]" selected="selected">[% value %]</option>
>-- 
>1.9.1

Actions: View | Diff | Splinter Review

Attachments on bug 13399: 36242 | 37532 | 37783 | 37784 | 37785