From 23265158fcab25f8f5188cea6f07af46812d41cc Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Thu, 16 Jun 2016 09:28:01 -0400
Subject: [PATCH] Bug 16754: Use validation plugin in budgets, planning, and
 contracts

This patch removes the custom javascript form validation from three
templates: Budgets, budget planning, and contracts. Validation via the
validation plugin is done instead.

This patch also removes use of event attributes like onclick and
onchange. Also changed: I added the "Required" text next to required
fields as necessary.

To test, apply the patch and go to Administration -> Budgets.

- Test the budget creation process for adding, editing, and duplicating
  budgets. In all cases:
  - The start date and end date should be required
  - You should be warned if you enter an end date which falls before the
    start date.
  - The description should be required
  - The budget amount should be required to be a number

To test the budget planning page:

Note that much of the JS removed from this template was unused. It
looked for inputs by the name 'est_total' which don't exist.

- When viewing the table of funds for a budget, select a choice from the
  'planning' toolbar menu.
  - Verify that after entering a numeric value in any cell, the
    'remaning' value at the end of the row is correctly updated.
  - Verify that after entering non-numeric data an error is displayed.
  - Verify that clicking the 'auto-fill' button at the end of the row
    works to fill any empty or zero-value fields with the remaning funds
    available.

To test the vendor contracts page:

- Go to Acquisitions -> Vendor -> Contracts
- Add or edit a contract
  - Confirm that name, start date, and end date are required.
  - Confirm that you are warned if you enter an end date which falls
    before the start date.

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
---
 .../prog/en/includes/calendar.inc             |  33 ++++++
 .../prog/en/modules/admin/aqbudgetperiods.tt  | 108 ++++++------------
 .../prog/en/modules/admin/aqcontract.tt       |  71 +++++-------
 .../prog/en/modules/admin/aqplan.tt           |  52 +++------
 4 files changed, 108 insertions(+), 156 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
index 653221e4a9..1d06bca2d2 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/calendar.inc
@@ -125,6 +125,39 @@ jQuery(function($){
     $.datepicker.setDefaults($.datepicker.regional['']);
 });
 
+/*  jQuery Validator plugin custom method
+    This allows you to check that a given date falls after another.
+    It is required that a message be defined.
+
+   Syntax:
+       $("#form_id").validate({
+        rules: {
+            input_name_of_later_date_field: {
+                is_date_after: "#input_id_of_earlier_date_field"
+            },
+        },
+        messages: {
+            input_name_of_later_date_field: {
+                is_date_after: _("Validation error to be shown, i.e. End date must come after start date")
+            }
+        }
+    });
+*/
+
+jQuery.validator.addMethod("is_date_after",
+function(value, element, params) {
+    var from = Date_from_syspref( $(params).val() );
+    var to = Date_from_syspref(value);
+    return to > from;
+});
+
+jQuery.validator.addMethod("date_on_or_after",
+function(value, element, params) {
+    var from = Date_from_syspref( $(params).val() );
+    var to = Date_from_syspref(value);
+    return to >= from;
+});
+
 $(document).ready(function(){
 
 $.datepicker.setDefaults({
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
index 22b6d2ea81..cf7d6b3a52 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqbudgetperiods.tt
@@ -119,13 +119,13 @@
         <div class="col-sm-10 col-sm-push-2">
             <main>
 
-[% UNLESS close_form OR closed%]
+[% UNLESS close_form OR closed OR duplicate_form %]
   [% INCLUDE 'budgets-admin-toolbar.inc' %]
 [% END %]
 
 [% IF ( duplicate_form ) %]
 <h3>Duplicate budget</h3>
-<form action="/cgi-bin/koha/admin/aqbudgetperiods.pl" name="f" method="post">
+<form action="/cgi-bin/koha/admin/aqbudgetperiods.pl" id="add_modify_budget" name="f" method="post">
     <fieldset class="rows">
     <input type="hidden" name="op" value="duplicate_budget" />
     <input type="hidden" name="budget_period_id" value="[% budget_period_id | html %]" />
@@ -135,18 +135,21 @@
     <li>
     <label class="required" for="from">Start date: </label>
     <input type="text" size="10" id="from" name="budget_period_startdate" value="[% budget_period_startdate | $KohaDates %]" class="datepickerfrom" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+        <span class="required">Required</span>
+        <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
     </li>
     <li>
 
     <label class="required" for="to">End date: </label>
     <input type="text" size="10" id="to" name="budget_period_enddate" value="[% budget_period_enddate | $KohaDates %]" class="datepickerto" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+        <span class="required">Required</span>
+        <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
     </li>
 
     <li>
     <label class="required" for="budget_period_description">Description</label>
     <input type="text" id="budget_period_description" name="budget_period_description" value="[% budgetperiod.budget_period_description | html %]" />
+        <span class="required">Required</span>
     </li>
 
     <li>
@@ -175,7 +178,8 @@
     </fieldset>
 
     <fieldset class="action">
-        <input type="button" value="Save Changes"  onclick="CheckDuplicateForm(this.form)" /> <a class="cancel" href="/cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id | html %]">Cancel</a>
+        <input type="submit" value="Save" />
+        <a class="cancel" href="/cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id | html %]">Cancel</a>
     </fieldset>
 
 </form>
@@ -190,7 +194,7 @@
     [% ELSE %]
         <h3>Add budget</h3>
 	[% END %]
-<form action="/cgi-bin/koha/admin/aqbudgetperiods.pl" name="f" method="post">
+<form action="/cgi-bin/koha/admin/aqbudgetperiods.pl" id="add_modify_budget" name="f" method="post">
     <fieldset class="rows">
     <!-- ################################################################################# -->
     <!-- display information about the budget period that must be added or modified -->
@@ -201,13 +205,15 @@
     <li>
     <label class="required" for="from">Start date: </label>
     <input type="text" size="10" id="from" name="budget_period_startdate" value="[% budget_period_startdate | $KohaDates %]" class="datepickerfrom" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+        <span class="required">Required</span>
+        <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
     </li>
     <li>
 
     <label class="required" for="to">End date: </label>
     <input type="text" size="10" id="to" name="budget_period_enddate" value="[% budget_period_enddate | $KohaDates %]" class="datepickerto" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+        <span class="required">Required</span>
+        <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
     </li>
 
     <li>
@@ -216,6 +222,7 @@
         <label class="required" for="budget_period_description">Description: </label>
         <input type="text" id="budget_period_description" name="budget_period_description"
         size="48" maxlength="80" value="[% budget_period_description | html %]" />
+            <span class="required">Required</span>
     </li>
 
     <li>
@@ -245,8 +252,7 @@
     <fieldset class="action">
         <!-- "save changes" button -->
         <!-- ###################################### -->
-        <input type="button" value="Save"  onclick="Check(this.form)" />
-        <!--             <input type="submit" value="Save Changes"  />  -->
+        <input type="submit" value="Save" />
         [% IF ( budget_period_id ) %]
             <a href="/cgi-bin/koha/admin/aqbudgets.pl?budget_period_id=[% budget_period_id | html %]" class="cancel">Cancel</a>
         [% ELSE %]
@@ -504,70 +510,6 @@
     [% END %]
     [% Asset.js("js/acq.js") | $raw %]
     <script>
-    // #################################################################################
-    // Javascript
-    // #################################################################################
-        function CheckDuplicateForm(f){
-            var ok=1;
-            var _alertString="";
-            var alertString="";
-            if(!(isNotNull(f.budget_period_startdate,1))){
-              _alertString += "\n- " + _("Start date missing");
-            }
-            if (!(isNotNull(f.budget_period_enddate,1))){
-              _alertString += "\n- " + _("End date missing");
-            }
-            if( f.budget_period_startdate > f.budget_period_enddate ) {
-              _alertString += "\n- " + _("Start date must be before end date");
-            }
-            if (!(isNotNull(f.budget_period_description,1))){
-              _alertString += "\n- " + _("Budget description missing");
-            }
-
-            if(_alertString.length==0){
-              f.submit();
-            } else {
-              alertString += _("Form not submitted because of the following problem(s)");
-              alertString += "\n-----------------------------------------\n";
-              alertString += _alertString;
-              alert(alertString);
-            }
-        }
-        function Check(f) {
-            var ok=1;
-            var _alertString="";
-            var alertString2;
-
-            if (!(isNotNull(f.budget_period_startdate,1))) {
-                    _alertString += "\n- " + _("Start date missing");
-            }
-
-            if (!(isNotNull(f.budget_period_enddate,1))) {
-                    _alertString += "\n- " + _("End date missing");
-            }
-
-            if ( f.budget_period_startdate >   f.budget_period_enddate )  {
-                    _alertString += "\n- " + _("Start date must be before end date");
-            }
-
-            if (!(isNotNull(f.budget_period_description,1))) {
-                    _alertString += "\n- " + _("Description missing");
-            }
-
-            if (!(isNum(f.budget_period_total))) {
-                    _alertString += "\n- " + _("Amount must be a valid number, or empty");
-            }
-
-            if (_alertString.length==0) {
-                    f.submit();
-            } else {
-                    alertString2  = _("Form not submitted because of the following problem(s)");
-                    alertString2 += "\n------------------------------------------------------------------------------------\n";
-                    alertString2 += _alertString;
-                    alert(alertString2);
-            }
-        }
-
         $(document).ready(function() {
             var tabs = $('#budgetsTabs').tabs();
             [% IF ( tab ) %]
@@ -633,6 +575,24 @@
                 "sPaginationType": "full_numbers"
               }));
             [% END %]
+            $("#add_modify_budget").validate({
+                rules: {
+                    budget_period_startdate: "required",
+                    budget_period_enddate: {
+                        required: true,
+                        date_on_or_after: "#from"
+                    },
+                    budget_period_description: "required",
+                    budget_period_total: {
+                        number: true
+                    }
+                },
+                messages: {
+                    budget_period_enddate: {
+                        date_on_or_after: _("Budget end date must be on or after budget start date")
+                    }
+                }
+            });
         });
     </script>
 [% END %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt
index 8e596f21e8..b25ba7568f 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqcontract.tt
@@ -59,7 +59,7 @@
 [% END %]
 
                 [% IF ( add_form ) %]
-                    <form name="Aform" action="[% script_name | html %]" method="post">
+                    <form name="Aform" action="/cgi-bin/koha/admin/aqcontract.pl" id="add_modify_contract" method="post">
                         <input type="hidden" name="op" value="add_validate" />
                         <input type="hidden" name="booksellerid" value="[% booksellerid | html %]" />
                         <input type="hidden" name="checked" value="0" />
@@ -76,24 +76,27 @@
                                             <input type="hidden" name="is_a_modif" value="1" />
                                         </li>
                                 [% END %]
-                                <li><label for="contractname" class="required">Name: *</label> &nbsp;
+                                <li><label for="contractname" class="required">Name:</label> &nbsp;
                                         <input type="text" name="contractname" id="contractname" size="40" maxlength="80" value="[% contractname | html %]" />
+                                        <span class="required">Required</span>
                                 </li>
                                 <li><label for="contractdescription">Description: </label> &nbsp;
                                         <input type="text" name="contractdescription" id="contractdescription" size="40" maxlength="80" value="[% contractdescription | html %]" />
                                 </li>
-                                <li><label for="from" class="required">Start date: *</label> &nbsp;
+                                <li><label for="from" class="required">Start date:</label> &nbsp;
                                         <input type="text" name="contractstartdate" id="from" value="[% contractstartdate | $KohaDates %]" maxlength="10" size="10" class="datepickerfrom" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+                                        <span class="required">Required</span>
+                                    <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
                                 </li>
-                                <li><label for="to" class="required">End date: *</label> &nbsp;
+                                <li><label for="to" class="required">End date:</label> &nbsp;
                                         <input type="text" name="contractenddate" id="to" value="[% contractenddate | $KohaDates %]" maxlength="10"  size="10" class="datepickerto" />
-				<div class="hint">[% INCLUDE 'date-format.inc' %]</div>
+                                        <span class="required">Required</span>
+                                    <div class="hint">[% INCLUDE 'date-format.inc' %]</div>
                                 </li>
                             </ol>
                         </fieldset>
                         <fieldset class="action">
-                            <input type="button" value="Save" onclick="Check(this.form);" /> <a class="cancel" href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | html %]&amp;contractnumber=[% contractnumber | html %]">Cancel</a>
+                            <input type="submit" value="Save" /> <a class="cancel" href="/cgi-bin/koha/admin/aqcontract.pl?booksellerid=[% booksellerid | html %]&amp;contractnumber=[% contractnumber | html %]">Cancel</a>
                         </fieldset>
                     </form>
             [% END %]
@@ -184,43 +187,23 @@
     [% INCLUDE 'calendar.inc' %]
     [% Asset.js("js/acq.js") | $raw %]
     <script>
-    //<![CDATA[
-    // to check if the data are correctly entered.
-    function Check(ff) {
-        var ok=0;
-        var _alertString=_("Form not submitted because of the following problem(s)");
-        _alertString +="\n-------------------------------------------------------------------\n\n";
-        if (!(isNotNull(ff.contractname,0))){
-            ok=1;
-            _alertString += _("- Name missing") + "\n";
-        }
-        var startDate = Date_from_syspref($("#from").val());
-        var endDate   = Date_from_syspref($("#to").val());
-        if (!parseInt(startDate.getTime())) {
-            ok=1;
-            _alertString += _("- Start date missing or invalid.") + "\n";
-        }
-        if (!parseInt(endDate.getTime())) {
-            ok=1;
-            _alertString += _("- End date missing or invalid.") + "\n";
-        }
-
-        if (startDate > endDate) {
-            ok=1;
-            _alertString += _("Wrong date! start date cannot be after end date.") + "\n";
-        }
-        if (endDate < (new Date)) {
-            ok=1;
-            _alertString += _("End date before today, Invalid end date!") + "\n";
-        }
-        if (ok) { // if there is a problem
-            alert(_alertString);
-        return false;
-    }
-    // if all is good
-        ff.submit();
-    }
-    //]]>
+        $(document).ready(function(){
+            $("#add_modify_contract").validate({
+                rules: {
+                    contractstartdate: "required",
+                    contractname: "required",
+                    contractenddate: {
+                        required: true,
+                        date_on_or_after: "#from"
+                    }
+                },
+                messages: {
+                    contractenddate: {
+                        date_on_or_after: _("Contract end date must be on or after contract start date")
+                    }
+                }
+            });
+        });
     </script>
 [% END %]
 [% INCLUDE 'intranet-bottom.inc' %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt
index 941854a21b..79c3dbac8c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/aqplan.tt
@@ -5,7 +5,7 @@
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Administration &rsaquo; Budgets &rsaquo; Funds &rsaquo; Planning for [% budget_period_description | html %] by [% authcat | html %]</title>
 [% INCLUDE 'doc-head-close.inc' %]
-<style type="text/css">td.locked { background-image: url('[% interface | html %]/[% theme | html %]/img/locked.png'); padding-left : 20px; background-repeat: no-repeat; background-position: 5px 5px; } a.control { font-size:85%;text-decoration:none; }</style>
+<style>td.locked { background-image: url('[% interface | html %]/[% theme | html %]/img/locked.png'); padding-left : 20px; background-repeat: no-repeat; background-position: 5px 5px; } a.control { font-size:85%;text-decoration:none; } .plan_entry { text-align: right; width:90%; }</style>
 </head>
 
 <body id="admin_aqplan" class="admin">
@@ -99,7 +99,7 @@
                     [% line.estimated_amount | html %]&nbsp;
                     <input type="hidden" style="text-align: right;"  name="[% line.cell_name | html %]" value="[% line.estimated_amount | html %]"   />
                 [% ELSE %]
-                    <input type="text" style="text-align: right;  width:90%; " size="6" name="[% line.cell_name | html %]"  value="[% line.estimated_amount | html %]"  id="budget_[% line.budget_id | html %][% line.colnum | html %]" class="plan_entry_[% line.budget_id | html %]" onchange="calcTotalRow(this);" />
+                    <input type="text" size="6" name="[% line.cell_name | html %]"  value="[% line.estimated_amount | html %]"  id="budget_[% line.budget_id | html %][% line.colnum | html %]" class="plan_entry plan_entry_[% line.budget_id | html %]" />
                 [% END %]
 
                 </td></tr>
@@ -137,7 +137,7 @@
 
         <td>
              [% UNLESS ( budget_line.budget_lock ) %]
-                    <input type="button"  onclick="autoFillRow('[% budget_line.budget_id | html %]')" value="Auto-fill row"/>
+                    <input type="button" class="auto_fill_row" data-budget-id="[% budget_line.budget_id | html %]" value="Auto-fill row"/>
             [% ELSE %]
              <div style="color:red;">not owned</div>
             [% END %]
@@ -151,7 +151,7 @@
     [% IF ( budget_period_locked ) %]
         <!-- <input STYLE="background: gray;"   type="submit" value="Save" disabled="disabled"/> -->
     [% ELSE %]
-        <fieldset class="action"><input type="button"  onclick="Check(this.form)" value="Save"/></fieldset>
+        <fieldset class="action"><input type="submit" value="Save"/></fieldset>
     [% END %]
 
 
@@ -276,41 +276,10 @@
      </div> <!-- /.row -->
 
 [% MACRO jsinclude BLOCK %]
-    [% Asset.js("lib/jquery/plugins/jquery.checkboxes.min.js") | $raw %]
     [% Asset.js("js/acq.js") | $raw %]
     <script>
-    //<![CDATA[
-        function Check(f) {
-                var ok=1;
-                var _alertString="";
-                var alertString2;
-                var arr = document.getElementsByName('est_total')
-
-                for ( var i=0, len=arr.length; i<len; ++i ){
-                        var tot = arr[i].innerHTML;
-
-                        if (tot == 'NaN') {
-                            _alertString += "\n- " + _("One or more cell values is non-numeric");
-                        }
-                }
-
-                if (_alertString.length==0) {
-
-                    var op  = document.createElement('input');
-                    op.setAttribute("type","hidden");
-                    op.setAttribute("name","op");
-                    op.setAttribute("value","save");   //ohh the pain...
-
-                    document.Aform.appendChild(op);
-                    document.Aform.submit()
-
-                } else {
-                        alertString2  = _("Form not submitted because of the following problem(s)");
-                        alertString2 += "\n------------------------------------------------------------------------------------\n";
-                        alertString2 += _alertString;
-                        alert(alertString2);
-                }
-        }
+        jQuery.validator.addClassRules({ plan_entry: { pnumber: true } }); // require that input be a number in fields with the class 'plan_entry'
+        jQuery.validator.addMethod("pnumber", $.validator.methods.number, _("Please enter a valid number.") );
         $(document).ready(function() {
             $("#selections input").on("change",function(e){
                 var num = $(this).attr("id");
@@ -328,8 +297,15 @@
                     }
                 }
             });
+            $(".plan_entry").on("change",function(){
+                calcTotalRow( this );
+            });
+            $("#Aform").validate();
+            $(".auto_fill_row").on("click",function(){
+                var budget_id = $(this).data("budget-id");
+                autoFillRow( budget_id );
+            });
         });
-    //]]>
     </script>
 [% END %]
 [% INCLUDE 'intranet-bottom.inc' %]
-- 
2.39.5