From f9006082973ccc6a260e330ab48cec22262f1e15 Mon Sep 17 00:00:00 2001
From: Jan Kissig <jkissig@th-wildau.de>
Date: Thu, 16 May 2024 10:11:53 +0200
Subject: [PATCH] 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>
---
 admin/marc-overlay-rules.pl                   | 19 ++----
 .../en/modules/admin/marc-overlay-rules.tt    | 59 ++++++++-----------
 2 files changed, 29 insertions(+), 49 deletions(-)

diff --git a/admin/marc-overlay-rules.pl b/admin/marc-overlay-rules.pl
index be2c8ac5aaa..fc4c849e95b 100755
--- a/admin/marc-overlay-rules.pl
+++ b/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') {
-        $template->{VARS}->{removeConfirm} = 1;
-        my %remove_ids = map { $_ => undef } @remove_ids;
-        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->();
-    }
+    Koha::MarcOverlayRules->search({ id => { in => \@remove_ids } })->delete();
+    # Update $rules after deletion
+    $rules = $get_rules->();
 }
 elsif ($op eq 'edit') {
     $template->param( edit => 1 );
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt
index 58a70e242bb..b36b90cdc2c 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/marc-overlay-rules.tt
+++ b/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 %]" checked="checked"/>
-                                        [% ELSE %]
-                                            <input type="checkbox" name="batchremove" value="[% rule.id | html %]"/>
-                                        [% END %]
+                                        <input type="checkbox" name="batchremove" value="[% rule.id | html %]"/>
                                     </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"/>')
-            .attr('name', 'op')
-            .attr('value', op)
-            .appendTo('#marc-overlay-rules-form');
+            $('#marc-overlay-rules-form > input[name="op"]').val(op);
 
             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,26 +321,38 @@
             }
 
             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_remove').on('click', function(el){
+            // 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(){
-            doSubmit('cud-remove');
+            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() {
             if($('input[name="batchremove"]:checked').length > 0) {
-- 
2.39.2