From 22b18ba988c72b6802421ae2e96237a716ae436e Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Fri, 25 Mar 2016 13:06:03 -0400
Subject: [PATCH] Bug 16437 - Automatic item modifications by age needs
 prettying
MIME-Version: 1.0
Content-Type: text/plain; charset=utf8
Content-Transfer-Encoding: 8bit

This patch makes layout and behavior changes to the automatic item
modifications by age interface, bringing some aspects of it closer into
conformance with established interface patterns.

- The intial view is now a standard table of information about existing
  rules, or a message dialog saying there are no rules.
- If there are no rules, a toolbar button reads "Add rules."
- If there are existing rules, the toolbar button reads "Edit rules."
  - Clicking the button leads to the rules edit interface, which now has
    a floating toolbar with "Add rule," "Save," and "Cancel" buttons.
  - Clicking the "Add rule" button displays a blank rule block.
    - If you are adding a rule to existing rules, the new block is
      appended at the bottom, and the page scrolls to the new rule.
    - As you add or remove rule blocks, the legend containing the rule
      count updates so that the numbers are sequential.
    - In each rule block, "age" and "substitutions" are now required.
      The age field is now validated to require a number.
    - The add/remove condition/substitution links now have more
      descriptive text labels.
    - The control to remove a rule is now a link in the <legend> element
      associated with each rule.
- Most JavaScript for this page is now in a separate file.
- Breadcrumbs are updated to be a little more specific.

To test, apply the patch and clear your browser cache if necessary.

- Go to Tools -> Automatic item modifications by age.
- Test adding and removing rules.
- Test removing all rules.
- Test adding and removing conditions and substitutions within rules.
- Test submitting the form without filling in required fields.

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
---
 .../intranet-tmpl/prog/css/staff-global.css   |  25 +-
 .../automatic_item_modification_by_age.tt     | 321 ++++++++----------
 .../js/automatic_item_modification_by_age.js  | 123 +++++++
 3 files changed, 268 insertions(+), 201 deletions(-)
 create mode 100644 koha-tmpl/intranet-tmpl/prog/js/automatic_item_modification_by_age.js

diff --git a/koha-tmpl/intranet-tmpl/prog/css/staff-global.css b/koha-tmpl/intranet-tmpl/prog/css/staff-global.css
index 35fa6f8d94..7a594b152c 100644
--- a/koha-tmpl/intranet-tmpl/prog/css/staff-global.css
+++ b/koha-tmpl/intranet-tmpl/prog/css/staff-global.css
@@ -2694,7 +2694,12 @@ div#cn_browser_table_wrapper > table#cn_browser_table {
 div.rules {
     display: block;
 }
-div#new_rule, div.rule {
+
+#new_rule {
+    display: none;
+}
+
+#new_rule, div.rule {
     background-color: #F4F8F9;
     border: 2px solid #B9D8D9;
     border-radius: 5px;
@@ -2702,21 +2707,13 @@ div#new_rule, div.rule {
     padding: .3em;
 }
 
-div.duration, div.blocks {
-    border: 2px solid #B9D8D9;
-    border-radius: 5px 5px 5px 5px;
-    margin: .3em;
-    padding: 0 .3em .3em .3em;
+.blocks {
+    margin-bottom: .3em;
 }
 
-div.duration h5, div.blocks h5 {
-    padding-bottom: 4px;
-    padding-left: 0.2em;
-    background-color: #E6F0F2;
-    border-radius: 1px;
-}
-div.duration span, div.blocks div {
-    display:block;
+.remove_rule {
+    padding-left: .7em;
+    font-size: 80%;
 }
 
 div[class$="_table_controls"] {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/automatic_item_modification_by_age.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/automatic_item_modification_by_age.tt
index 7e1e800175..5c1984c034 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/tools/automatic_item_modification_by_age.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/tools/automatic_item_modification_by_age.tt
@@ -1,134 +1,73 @@
 [% INCLUDE 'doc-head-open.inc' %]
 <title>Koha &rsaquo; Tools &rsaquo; Automatic item modifications by age</title>
 [% INCLUDE 'doc-head-close.inc' %]
-<script type="text/javascript">//<![CDATA[
-  function clear_inputs(node, new_node) {
-    var selects = $(node).find("select");
-    $(selects).each(function(i) {
-      var select = this;
-      $(new_node).find("select").eq(i).val($(select).val());
-    });
-    var inputs = $(node).find("input");
-    $(inputs).each(function(i) {
-      var input = this;
-      $(new_node).find("input").eq(i).val($(input).val());
-    });
-  }
-
-  function remove_block_action( link ) {
-    var blocks = $(link).parent().parent();
-    if( $(blocks).find(".block").length > 2 ) {
-      $(blocks).find("a.remove_block").show();
-    } else {
-      $(blocks).find("a.remove_block").hide();
-    }
-    $(link).parent().remove();
-  }
-
-  function remove_rule_action( link ) {
-    if( $("#rules").find("div.rule").length < 2 ) {
-        $("#rules").hide();
-        $("#norules").show();
-    }
-    $(link).parent().remove();
-  }
-
-  function clone_block(block) {
-    var new_block = $(block).clone(1);
-    clear_inputs(block, new_block);
-    $(new_block).find('a.remove_block').show();
-    var blocks = $(block).parent();
-    $(blocks).append(new_block);
-    $(blocks).find('a.remove_block').click(function(){
-      remove_block_action($(this));
-    }).show();
-  }
-
-  $(document).ready(function() {
-    $("#new_rule a.remove_rule").hide();
-    $("#new_rule a.remove_block").hide();
-    $("#rules a.remove_block").click(function(){
-      remove_block_action($(this));
-    });
-    $("#rules a.remove_rule").click(function(){
-      remove_rule_action($(this));
-    });
-
-    var unique_id = $("div.rule").length + 1;
-    $("a.add_rule").click(function(){
-      var rule = $("#new_rule");
-      var new_rule = $(rule).clone(1);
-      $(new_rule).removeAttr('id');
-      $(new_rule).attr('class', 'rule');
-      clear_inputs(rule, new_rule);
-      $(new_rule).find("select[name='condition_field']").attr('name', 'condition_field_' + unique_id);
-      $(new_rule).find("select[name='substitution_field']").attr('name', 'substitution_field_' + unique_id);
-      $(new_rule).find("input[name='condition_value']").attr('name', 'condition_value_' + unique_id);
-      $(new_rule).find("input[name='substitution_value']").attr('name', 'substitution_value_' + unique_id);
-      $(new_rule).find("input[name='age']").attr('name', 'age_' + unique_id);
-      $(new_rule).find("input[name='unique_id']").val(unique_id);
-
-      $("#rules").append(new_rule);
-
-      if( $("#rules").find("div.rule").length > 0 ) {
-          $("#rules").show();
-          $("#norules").hide();
-      }
-      if( $("#rules").find(".conditions > .condition").length > 1 ) {
-
-      }
-      if( $("#rules").find(".conditions > .condition").length > 1 ) {
-
-      }
-      $(new_rule).find('a.remove_rule').click(function(){
-        remove_rule_action( $(this) );
-      }).show();
-      $(new_rule).find('a.add_rule').remove();
-      unique_id++;
-    });
-
-    $("a.add_block").click(function(){
-      clone_block( $(this).parent() );
-    });
-
-    if( $("#rules").find("div.rule").length < 1 ) {
-        $("#rules").hide();
-        $("#norules").show();
-    }
-
-    $("#rules .rule .blocks").each(function(){
-      if ( $(this).find(".block").length == 1 ) {
-        $(this).find("a.remove_block").hide();
-      }
-    });
-
-    [% IF op == 'edit_form' %]
-      [% IF rules.size > 0 %]
-        $("#norules").hide();
-      [% ELSE %]
-        $("#rules").show();
-      [% END %]
-    [% END %]
-  });
-//]]>
-</script>
+<script type="text/javascript" src="[% interface %]/lib/jquery/plugins/jquery.fixFloat.js"></script>
+<script type="text/javascript" src="[% interface %]/[% theme %]/js/automatic_item_modification_by_age.js"></script>
+[% IF op == 'edit_form' %]
+    <script type="text/javascript">//<![CDATA[
+      $(document).ready(function() {
+            [% IF ( op == 'edit_form' ) %]
+                $('#toolbar').fixFloat();
+            [% END %]
+          [% IF rules.size > 0 %]
+            $("#norules").hide();
+          [% ELSE %]
+            $("#rules").show();
+          [% END %]
+      });
+    //]]>
+    </script>
+[% END %]
 </head>
 <body id="tools_automatic_item_modification_by_age" class="tools">
 [% INCLUDE 'header.inc' %]
 [% INCLUDE 'cat-search.inc' %]
-<div id="breadcrumbs"><a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo; <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo; <a href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl">Automatic item modifications by age</a></div>
+    <div id="breadcrumbs">
+        <a href="/cgi-bin/koha/mainpage.pl">Home</a> &rsaquo;
+        <a href="/cgi-bin/koha/tools/tools-home.pl">Tools</a> &rsaquo;
+        [% IF ( op == 'edit_form' ) %]
+            <a href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl">Automatic item modifications by age</a> &rsaquo;
+            Rules
+        [% ELSE %]
+            Automatic item modifications by age
+        [% END %]
+    </div>
 
 <div id="doc3" class="yui-t2">
   <div id="bd">
     <div id="yui-main">
       <div class="yui-b">
-        <h3>Automatic item modifications by age</h3>
-        <div id="toolbar" class="btn-toolbar">
-          <a class="btn btn-small" id="newentry" href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl?op=edit_form"><i class="fa fa-pencil"></i> Edit</a>
-        </div>
+
+        [% IF ( op == 'edit_form' ) %]
+          <form method="post" id="rules_form" action="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl">
+            <h3>Rules for automatic item modifications by age</h3>
+            <div id="toolbar" class="btn-toolbar">
+                <div class="btn-group">
+                    <button class="btn btn-small add_rule"><i class="fa fa-plus"></i> Add rule</button>
+                </div>
+                <div class="btn-group">
+                    <button type="submit" id="save_rules" class="btn btn-small"><i class="fa fa-save"></i> Save</button>
+                </div>
+                <div class="btn-group">
+                    <a class="btn btn-small" href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl"><i class="fa fa-remove"></i> Cancel</a>
+                </div>
+            </div>
+        [% ELSE %]
+            <h3>Automatic item modifications by age</h3>
+            [% IF ( rules ) %]
+                <div id="toolbar" class="btn-toolbar">
+                    <a class="btn btn-small" id="newentry" href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl?op=edit_form"><i class="fa fa-pencil"></i> Edit rules</a>
+                </div>
+            [% ELSE %]
+                <div id="toolbar" class="btn-toolbar">
+                    <a class="btn btn-small" id="newentry" href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl?op=edit_form"><i class="fa fa-plus"></i> Add rules</a>
+                </div>
+            [% END %]
+        [% END %]
+
         [% FOR message IN messages %]
           [% IF message.type == "error" %]
-            <div class="dialog error">
+            <div class="dialog alert">
           [% END %]
           [% IF message.code == "unable_to_load_configuration" %]
             An error occurs: Unable to load the configuration.
@@ -137,17 +76,17 @@
         [% END %]
 
         [% IF op == 'edit_form' %]
-          <form method="post" action="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl">
             <div id="edit_rules">
-              <h4>List of rules</h4>
                 <div id="rules">
                 [% FOR rule IN rules %]
                   [% SET id = loop.count %]
-                  <div class="rule">
+                  <fieldset class="rule">
+                    <legend>Rule <span class="rulecount">[% loop.count %]</span> <a href="#" class="remove_rule"><i class="fa fa-trash"></i> Remove this rule</a></legend>
                     <input type="hidden" name="unique_id" value="[% loop.count %]" /> <!-- FIXME on update, the unique_id should be filled -->
                     <div class="age">
-                      <h5>Age</h5>
-                      <input type="number" value="[% rule.age %]" name="age_[% id %]" /> days
+                      <h5>Age in days</h5>
+                      <input class="age" required="required" type="number" value="[% rule.age %]" name="age_[% id %]" />
+                      <span class="required">Required</span>
                     </div>
                     <div class="blocks">
                       <h5>Conditions</h5>
@@ -165,8 +104,8 @@
                           </select>
                           =
                           <input type="text" value="[% condition.value %]" name="condition_value_[% id%]" />
-                          <a class="add_block" style="cursor:pointer"><i class="fa fa-plus"></i></a>
-                          <a class="remove_block" style="cursor:pointer"><i class="fa fa-trash"></i></a>
+                          <a class="add_block" href="#"><i class="fa fa-plus"></i> Add a condition</a>
+                          <a class="remove_block" href="#"><i class="fa fa-trash"></i> Remove condition</a>
                         </div>
                       [% END %]
                     </div>
@@ -174,7 +113,7 @@
                       <h5>Substitutions</h5>
                       [% FOR substitution IN rule.substitutions %]
                         <div class="block">
-                          <select name="substitution_field_[% id %]">
+                          <select class="required" required="required" name="substitution_field_[% id %]">
                             <option value="">Choose a field name</option>
                             [% FOR field IN substitution_fields %]
                               [% IF substitution.field == field %]
@@ -185,32 +124,31 @@
                             [% END %]
                           </select>
                           =
-                          <input type="text" value="[% substitution.value %]" name="substitution_value_[% id %]" />
-                          <a class="add_block" style="cursor:pointer"><i class="fa fa-plus"></i></a>
-                          <a class="remove_block" style="cursor:pointer"><i class="fa fa-trash"></i></a>
+                          <input class="required" required="required" type="text" value="[% substitution.value %]" name="substitution_value_[% id %]" />
+                          <a class="add_block" href="#"><i class="fa fa-plus"></i> Add a substitution</a>
+                          <a class="remove_block" href="#"><i class="fa fa-trash"></i> Remove substitution</a>
+                          <span class="required">Required</span>
                         </div>
                       [% END %]
                     </div>
-                    <a class="remove_rule" style="cursor:pointer">Remove this rule</a>
-                  </div>
+                  </fieldset>
                 [% END %]
                 </div>
-                <div id="norules">
-                  There is no rule defined.
-                </div>
-              <fieldset class="action">
                 <input type="hidden" name="op" value="update" />
-                <a class="cancel" href="/cgi-bin/koha/tools/automatic_item_modification_by_age.pl">Cancel</a>
-                <input type="submit" value="Submit these rules" />
-              </fieldset>
             </div>
           </form>
-          <h4>Add a new rule</h4>
-          <div id="new_rule">
+
+        <div id="norules" class="dialog message">
+          There are no rules defined.
+        </div>
+
+          <fieldset id="new_rule">
+            <legend>Rule <span class="rulecount"></span> <a href="#" class="remove_rule"><i class="fa fa-trash"></i> Remove this rule</a></legend>
             <input type="hidden" name="unique_id" />
             <div class="age">
-              <h5>Age</h5>
-              <input type="number" value="" name="age" /> days
+              <h5>Age in days</h5>
+              <input class="age" type="number" value="" name="age" />
+              <span class="required">Required</span>
             </div>
             <div class="blocks">
               <h5>Conditions</h5>
@@ -223,66 +161,75 @@
                 </select>
                 =
                 <input type="text" value="" name="condition_value" />
-                <a class="add_block" style="cursor:pointer"><i class="fa fa-plus"></i></a>
-                <a class="remove_block" style="cursor:pointer"><i class="fa fa-trash"></i></a>
+                <a class="add_block" href="#"><i class="fa fa-plus"></i> Add a condition</a>
+                <a class="remove_block" href="#"><i class="fa fa-trash"></i> Remove condition</a>
               </div>
             </div>
             <div class="blocks">
               <h5>Substitutions</h5>
               <div class="block">
-                <select name="substitution_field">
+                <select required="required" class="required" name="substitution_field">
                   <option value="">Choose a field name</option>
                   [% FOR field IN substitution_fields %]
                     <option value="[% field %]">[% field %]</option>
                   [% END %]
                 </select>
                 =
-                <input type="text" value="" name="substitution_value" />
-                <a class="add_block" style="cursor:pointer"><i class="fa fa-plus"></i></a>
-                <a class="remove_block" style="cursor:pointer"><i class="fa fa-trash"></i></a>
+                <input class="required" required="required" type="text" value="" name="substitution_value" />
+                <a class="add_block" href="#"><i class="fa fa-plus"></i> Add a substitution</a>
+                <a class="remove_block" href="#"><i class="fa fa-trash"></i> Remove substitution</a>
+                <span class="required">Required</span>
               </div>
             </div>
-          <a class="add_rule" style="cursor:pointer">Add this rule</a>
-          <a class="remove_rule" style="cursor:pointer">Remove this rule</a>
-          </div>
-        [% ELSIF rules and op == 'show' %]
-          <div id="rules">
-            <h4>List of rules</h4>
-            [% FOR rule IN rules %]
-              <div class="rule">
-                <div class="age">
-                  <h5>Age</h5>
-                  [% IF rule.age.defined and rule.age.length > 0 %]
-                    [% rule.age %] days
-                  [% ELSE %]
-                    There is no age for this rule.
-                  [% END %]
-                </div>
-                <div class="blocks">
-                  <h5>Conditions</h5>
-                  [% FOR condition IN rule.conditions %]
-                    [% IF condition.field %]
-                      <div class="block">
-                        [% condition.field %] = [% condition.value %]
-                      </div>
-                    [% ELSE %]
-                      There is no condition for this rule.
-                    [% END %]
-                  [% END %]
-                </div>
-                <div class="blocks">
-                  <h5>Substitutions</h5>
-                  [% FOR substitution IN rule.substitutions %]
-                    <div class="block">
-                      [% substitution.field %] = [% substitution.value %]
-                    </div>
-                  [% END %]
-                </div>
-              </div>
-            [% END %]
-          </div>
+          </fieldset>
+        [% ELSIF rules %]
+            <div>
+                <h4>List of rules</h4>
+                    <table id="rulest">
+                        <thead>
+                          <tr>
+                               <th>Age</th>
+                                <th>Conditions</th>
+                                <th>Substitutions</th>
+                            </tr>
+                        </thead>
+                        <tbody>
+                            [% FOR rule IN rules %]
+                                <tr>
+                                    <td>
+                                        [% IF rule.age.defined and rule.age.length > 0 %]
+                                            [% rule.age %] days
+                                        [% ELSE %]
+                                            There is no age for this rule.
+                                        [% END %]
+                                    </td>
+                                    <td>
+                                        [% FOR condition IN rule.conditions %]
+                                            [% IF condition.field %]
+                                                <div class="block">
+                                                [% condition.field %] = [% condition.value %]
+                                                </div>
+                                            [% ELSE %]
+                                                There is no condition for this rule.
+                                            [% END %]
+                                        [% END %]
+                                    </td>
+                                    <td>
+                                        [% FOR substitution IN rule.substitutions %]
+                                            <div class="block">
+                                                [% substitution.field %] = [% substitution.value %]
+                                            </div>
+                                        [% END %]
+                                    </td>
+                                </tr>
+                            [% END %]
+                        </tbody>
+                    </table>
+            </div>
         [% ELSE %]
-          There is no rule defined. Please click on the edit button.
+            <div class="dialog message">
+                There are no rules defined.
+            </div>
         [% END %]
 
       </div>
diff --git a/koha-tmpl/intranet-tmpl/prog/js/automatic_item_modification_by_age.js b/koha-tmpl/intranet-tmpl/prog/js/automatic_item_modification_by_age.js
new file mode 100644
index 0000000000..1ebce82860
--- /dev/null
+++ b/koha-tmpl/intranet-tmpl/prog/js/automatic_item_modification_by_age.js
@@ -0,0 +1,123 @@
+function clear_inputs(node, new_node) {
+    var selects = $(node).find("select");
+    $(selects).each(function(i) {
+        var select = this;
+        $(new_node).find("select").eq(i).val($(select).val());
+    });
+    var inputs = $(node).find("input");
+    $(inputs).each(function(i) {
+        var input = this;
+        $(new_node).find("input").eq(i).val($(input).val());
+    });
+}
+
+function remove_block_action( link ) {
+    var blocks = $(link).parent().parent();
+    if( $(blocks).find(".block").length > 2 ) {
+        $(blocks).find("a.remove_block").show();
+    } else {
+        $(blocks).find("a.remove_block").hide();
+    }
+    $(link).parent().remove();
+}
+
+function remove_rule_action( link ) {
+    if( $("#rules").find(".rule").length < 2 ) {
+            $("#rules").hide();
+            $("#norules").show();
+    }
+    $(link).parent().parent().remove();
+    update_rule_count();
+}
+
+function clone_block(block) {
+    var new_block = $(block).clone(1);
+    clear_inputs(block, new_block);
+    $(new_block).find('a.remove_block').show();
+    var blocks = $(block).parent();
+    $(blocks).append(new_block);
+    $(blocks).find('a.remove_block').click(function(){
+        remove_block_action($(this));
+    }).show();
+}
+
+function update_rule_count(){
+    rules = $(".rulecount");
+    rules.each( function( i ){
+        $(this).text( i + 1 );
+    });
+}
+
+$(document).ready(function() {
+    $("#new_rule .remove_rule").hide();
+    $("#new_rule a.remove_block").hide();
+    $("#rules a.remove_block").click(function(e){
+        e.preventDefault();
+        remove_block_action($(this));
+    });
+    $("#rules .remove_rule").click(function(e){
+        e.preventDefault();
+        remove_rule_action($(this));
+    });
+
+    var unique_id = $(".rule").length + 1;
+    $(".add_rule").click(function(e){
+        e.preventDefault();
+        var rule = $("#new_rule");
+        var rules = $("#rules");
+        var new_rule = rule.clone(1);
+        new_rule.removeAttr('id');
+        new_rule.attr('class', 'rule');
+        clear_inputs(rule, new_rule);
+        new_rule.find("select[name='condition_field']").attr('name', 'condition_field_' + unique_id);
+        new_rule.find("select[name='substitution_field']").attr('name', 'substitution_field_' + unique_id);
+        new_rule.find("input[name='condition_value']").attr('name', 'condition_value_' + unique_id);
+        new_rule.find("input[name='substitution_value']").attr('name', 'substitution_value_' + unique_id);
+        new_rule.find("input[name='age']").attr('name', 'age_' + unique_id);
+        new_rule.find("input[name='unique_id']").val(unique_id);
+
+        $("#rules").append(new_rule);
+        update_rule_count();
+        var scrollToPoint = new_rule.position();
+        window.scroll(0, scrollToPoint.top - $("#toolbar").height() );
+
+        if( $("#rules").find(".rule").length > 0 ) {
+                $("#rules").show();
+                $("#norules").hide();
+        }
+        if( $("#rules").find(".conditions > .condition").length > 1 ) {
+
+        }
+        if( $("#rules").find(".conditions > .condition").length > 1 ) {
+
+        }
+        new_rule.find('.remove_rule').click(function(){
+            remove_rule_action( $(this) );
+        }).show();
+        new_rule.find('.add_rule').remove();
+        unique_id++;
+    });
+
+    $("a.add_block").click(function(e){
+        e.preventDefault();
+        clone_block( $(this).parent() );
+    });
+
+    if( $("#rules").find(".rule").length < 1 ) {
+            $("#rules").hide();
+            $("#norules").show();
+    }
+
+    $("#rules .rule .blocks").each(function(){
+        if ( $(this).find(".block").length == 1 ) {
+            $(this).find("a.remove_block").hide();
+        }
+    });
+
+    jQuery.validator.addClassRules("age", {
+        required: true,
+        digits: true
+    });
+
+    $("#rules_form").validate();
+});
-- 
2.39.5