Bug 16437 - Automatic item modifications by age needs prettying

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>

diff --git a/koha-tmpl/intranet-tmpl/prog/css/staff-global.css b/koha-tmpl/intranet-tmpl/prog/css/staff-global.css
index 35fa6f8..7a594b1 100644 (file)
--- 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 7e1e800..5c1984c 100644 (file)
--- 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 (file)
index 0000000..1ebce82
--- /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();
+});