Attachment 79896 Details for Bug 16754 – Bug 16754: Use validation plugin in budgets, planning, and contracts

[patch] Bug 16754: Use validation plugin in budgets, planning, and contracts

Bug-16754-Use-validation-plugin-in-budgets-plannin.patch (text/plain), 22.89 KB, created by Owen Leonard on 2018-10-03 14:53:11 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Owen Leonard

Created: 2018-10-03 14:53:11 UTC

Size: 22.89 KB

patch

obsolete

>From d169efdd0e8d5a49987503965095cccdb4d4b66d 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.
>---
> .../intranet-tmpl/prog/en/includes/calendar.inc    |  33 +++++++
> .../prog/en/modules/admin/aqbudgetperiods.tt       | 108 +++++++--------------
> .../prog/en/modules/admin/aqcontract.tt            |  71 ++++++--------
> .../intranet-tmpl/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 653221e..1d06bca 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 22b6d2e..cf7d6b3 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">
>     
>     
>@@ -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">
>         
>         
>-        <input type="button" value="Save"  onclick="Check(this.form)" />
>-        
>+        <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 8e596f2..b25ba75 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 941854a..79c3dba 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 ) %]
>         
>     [% 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> 
> 
> [% 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.1.4

Actions: View | Diff | Splinter Review

Attachments on bug 16754: 52463 | 52476 | 54057 | 79896 | 79940 | 80169