Attachment 167207 Details for Bug 36880 – Bug 36880: Record overlay rules are not validated

[patch] Bug 36880: Record overlay rules are not validated

Bug-36880-Record-overlay-rules-are-not-validated.patch (text/plain), 9.74 KB, created by Jan Kissig on 2024-05-28 08:28:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jan Kissig

Created: 2024-05-28 08:28:13 UTC

Size: 9.74 KB

patch

obsolete

>From 97079c9d9d3de319e053c90c8d4169c33d804696 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
>---
> 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 be2c8ac5aa..fc4c849e95 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 58a70e242b..b36b90cdc2 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> 
> 
>-                <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>
> 
>             <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.34.1

Actions: View | Diff | Splinter Review

Attachments on bug 36880: 166808 | 167182 | 167190 | 167207 | 167234 | 168088