Bug 36880: Record overlay rules are not validated

This patch fixes validation of rules, confirmation on deletion of rules and removes some unused source code.

1) Validation of record overlay rules on edit and add action
2) Validation when editing an existing rule
3) Adds confirm when deleting multiple rules

Test plan:
1)
 a) open http://localhost:8081/cgi-bin/koha/admin/marc-overlay-rules.pl
 b) just click + Add rule
 c) a new rule with an empty tag is saved

2)
 a) edit an existing rule
 b) empty input value for tag
 c) click Save and check that the rule has now an empty value for tag

3)
 a) delete a rule by checking the checkbox and clicking Delete selected
 b) delete a rule by clicking the Delete button under Actions
 c) notice that b) asks for confirmation

apply patch
1) redo steps and check that form does not get submitted and the input is marked as required

2) redo steps and check that clicking on Save will not submit the form and mark input as required

3) redo steps and check that a standard confirm popup appears

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>

admin/marc-overlay-rules.pl

@@ -66,23 +66,12 @@ my $get_rules = sub {
 };
 my $rules = $get_rules->();
 
-if ($op eq 'remove' || $op eq 'cud-remove') {
+if ($op eq 'cud-remove') {
     my @remove_ids = $input->multi_param('batchremove');
     push @remove_ids, scalar $input->param('id') if $input->param('id');
-    if ($op eq 'remove') {
+    Koha::MarcOverlayRules->search({ id => { in => \@remove_ids } })->delete();
-        $template->{VARS}->{removeConfirm} = 1;
+    # Update $rules after deletion
-        my %remove_ids = map { $_ => undef } @remove_ids;
+    $rules = $get_rules->();
-        for my $rule (@{$rules}) {
-            $rule->{'removemarked'} = 1 if exists $remove_ids{$rule->{id}};
-        }
-    }
-    elsif ($op eq 'cud-remove') {
-        my @remove_ids = $input->multi_param('batchremove');
-        push @remove_ids, scalar $input->param('id') if $input->param('id');
-        Koha::MarcOverlayRules->search({ id => { in => \@remove_ids } })->delete();
-        # Update $rules after deletion
-        $rules = $get_rules->();
-    }
 }
 elsif ($op eq 'edit') {
     $template->param( edit => 1 );

koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt

@@ -66,21 +66,11 @@
                     The [%  pref_MARCOverlayRules_link | $raw | $KohaSpan %] preference is not set, don't forget to enable it for rules to take effect.
                 </div>
                 [% END %]
-                [% IF removeConfirm %]
-                <div class="dialog alert">
-                    <h3>Remove rule?</h3>
-                    <p>Are you sure you want to remove the selected rule(s)?</p>
-
-                    <form action="/cgi-bin/koha/admin/marc-overlay-rules.pl" method="GET">
-                        <button type="submit" class="deny"><i class="fa fa-fw fa-times"></i> No, do not remove</button>
-                    </form>
-                    <button type="button" class="approve" id="doremove"><i class="fa fa-fw fa-check"></i> Yes, remove</button>
-                </div>
-                [% END %]
 
                 <div class="page-section">
                 <form action="/cgi-bin/koha/admin/marc-overlay-rules.pl" method="POST" id="marc-overlay-rules-form">
                     [% INCLUDE 'csrf-token.inc' %]
+                    <input type="hidden" name="op">
                     <table id="marc-overlay-rules">
                         <thead><tr>
                             <th>Rule</th>
@@ -229,7 +219,7 @@
                                     </td>
                                     <td class="actions">
                                         <button class="btn btn-default btn-xs" title="Save" id="doedit" value="[% rule.id | html %]"><i class="fa fa-check"></i> Save</button>
-                                        <button type="submit" class="btn btn-default btn-xs" title="Cancel" ><i class="fa fa-times"></i> Cancel</button>
+                                        <a href="/cgi-bin/koha/admin/marc-overlay-rules.pl" class="btn btn-default btn-xs" title="Cancel"><i class="fa fa-times"></i> Cancel</a>
                                     </td>
                                     <td></td>
                                 [% ELSE %]
@@ -270,15 +260,11 @@
                                     <td class="rule-operation-action" data-operation="remove" data-value="[% rule.remove | html %]">[% IF rule.remove %]Remove[% ELSE %]Skip[% END %]</td>
                                     <td class="rule-operation-action" data-operation="delete" data-value="[% rule.delete | html %]">[% IF rule.delete %]Delete[% ELSE %]Skip[% END %]</td>
                                     <td class="actions">
-                                        <a href="?op=remove&id=[% rule.id | uri %]" title="Delete" class="btn btn-default btn-xs"><i class="fa fa-trash-can"></i> Delete</a>
+                                        <a title="Delete" class="btn btn-default btn-xs btn_remove"><i class="fa fa-trash-can"></i> Delete</a>
                                         <a href="?op=edit&id=[% rule.id | uri %]" title="Edit" class="btn btn-default btn-xs"><i class="fa-solid fa-pencil" aria-hidden="true"></i> Edit</a>
                                     </td>
                                     <td>
-                                        [% IF rule.removemarked %]
+                                        <input type="checkbox" name="batchremove" value="[% rule.id | html %]"/>
-                                            <input type="checkbox" name="batchremove" value="[% rule.id | html %]" checked="checked"/>
-                                        [% ELSE %]
-                                            <input type="checkbox" name="batchremove" value="[% rule.id | html %]"/>
-                                        [% END %]
                                     </td>
                                 [% END %]
                                 </tr>
@@ -288,11 +274,6 @@
                 </form>
                 </div> <!-- /.page-section -->
 
-                <form action="/cgi-bin/koha/admin/marc-overlay-rules.pl" method="post">
-                    [% INCLUDE 'csrf-token.inc' %]
-                    <input type="hidden" name="op" value="cud-redo-matching" />
-                </form>
-
             </div><!-- /.col-sm-10.col-sm-push-2 -->
 
             <div class="col-sm-2 col-sm-pull-10">
@@ -311,10 +292,7 @@
     <script>
     $(document).ready(function(){
         function doSubmit(op, id) {
-            $('<input type="hidden"/>')
+            $('#marc-overlay-rules-form > input[name="op"]').val(op);
-            .attr('name', 'op')
-            .attr('value', op)
-            .appendTo('#marc-overlay-rules-form');
 
             if(id) {
                 $('<input type="hidden"/>')
@@ -324,15 +302,16 @@
             }
 
             var valid = true;
-            if (op == 'add' || op == 'edit') {
+            if (op == 'cud-add' || op == 'cud-edit') {
                 var validate = [
-                    $('#marc-overlay-rules-form input[name="filter"]'),
+                    $('#marc-overlay-rules-form select[name="filter"]'),
                     $('#marc-overlay-rules-form input[name="tag"]')
                 ];
                 for(var i = 0; i < validate.length; i++) {
                     if (validate[i].length) {
                         if(validate[i].val().length == 0) {
                             validate[i].addClass('required');
+                            validate[i].focus();
                             valid = false;
                         } else {
                             validate[i].removeClass('required');
@@ -342,25 +321,37 @@
             }
 
             if (valid) {
-                $('#marc-overlay-rules-form').submit();
+                return $('#marc-overlay-rules-form').submit();
             }
 
             return valid;
         }
 
-        $('#doremove').on('click', function(){
-            doSubmit('cud-remove');
-        });
         $('#doedit').on('click', function(){
             doSubmit('cud-edit', $("#doedit").attr('value'));
+            return false;
         });
         $('#add').on('click', function(){
             doSubmit('cud-add');
             return false;
         });
-        $('#btn_batchremove').on('click', function(){
+        $('.btn_remove').on('click', function(el){
-            doSubmit('cud-remove');
+            // mark delete checkbox in row
+            $(el.target).closest('tr').find('[name="batchremove"]').prop( "checked", true );
+            $('#btn_batchremove').removeAttr('disabled');
+            confirm_remove();
+            return false;
         });
+        $('#btn_batchremove').on('click', function(){
+            confirm_remove();
+            return false;
+        });
+
+        function confirm_remove(){
+            if (confirm(_("Are you sure you want to remove the selected rule(s)?"))){
+                doSubmit('cud-remove');
+            }
+        }
 
         /* Disable batch remove unless one or more checkboxes are checked */
         $('input[name="batchremove"]').change(function() {