Attachment 166947 Details for Bug 15536 – Bug 15536: Refactor to remove duplicated code.

[patch] Bug 15536: Refactor to remove duplicated code.

Bug-15536-Refactor-to-remove-duplicated-code.patch (text/plain), 26.35 KB, created by Andreas Jonsson on 2024-05-21 00:49:13 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Andreas Jonsson

Created: 2024-05-21 00:49:13 UTC

Size: 26.35 KB

patch

obsolete

>From befca21d24a1ede94cf3a972a50141cb15e4a02f Mon Sep 17 00:00:00 2001
>From: Andreas Jonsson <andreas.jonsson@kreablo.se>
>Date: Mon, 20 May 2024 23:16:06 +0200
>Subject: [PATCH] Bug 15536: Refactor to remove duplicated code.
>
>---
> admin/matching-rules.pl                       |  46 ++--
> .../prog/en/modules/admin/matching-rules.tt   | 222 +++++-------------
> 2 files changed, 75 insertions(+), 193 deletions(-)
>
>diff --git a/admin/matching-rules.pl b/admin/matching-rules.pl
>index 3021bdb990..6c3d3d3ffb 100755
>--- a/admin/matching-rules.pl
>+++ b/admin/matching-rules.pl
>@@ -128,43 +128,27 @@ sub add_update_matching_rule {
>     # match checks
>     my @mc_nums = sort map { /^mc_(\d+)_id/ ? int($1): () } $input->multi_param;
>     foreach my $mc_num (@mc_nums) {
>-        # source components
>         my $src_components = [];
>-        my @src_comp_nums = sort map { /^mc_${mc_num}_src_c_(\d+)_tag/ ? int($1): () } $input->multi_param;
>-        foreach my $comp_num (@src_comp_nums) {
>-            my $component = {};
>-            $component->{'tag'} = $input->param("mc_${mc_num}_src_c_${comp_num}_tag");
>-            $component->{'subfields'} = $input->param("mc_${mc_num}_src_c_${comp_num}_subfields");
>-            $component->{'offset'} = $input->param("mc_${mc_num}_src_c_${comp_num}_offset");
>-            $component->{'length'} = $input->param("mc_${mc_num}_src_c_${comp_num}_length");
>-            # norms
>-            $component->{'norms'} = [];
>-            my @norm_nums = sort map { /^mc_${mc_num}_src_c_${comp_num}_n_(\d+)_norm/ ? int($1): () } $input->multi_param;
>-            foreach my $norm_num (@norm_nums) {
>-                push @{ $component->{'norms'} }, $input->multi_param("mc_${mc_num}_src_c_${comp_num}_n_${norm_num}_norm");
>-            }
>-            push @$src_components, $component;
>-        }
>-        # target components
>         my $tgt_components = [];
>-        my @tgt_comp_nums = sort map { /^mc_${mc_num}_tgt_c_(\d+)_tag/ ? int($1): () } $input->multi_param;
>-        foreach my $comp_num (@tgt_comp_nums) {
>-            my $component = {};
>-            $component->{'tag'} = $input->param("mc_${mc_num}_tgt_c_${comp_num}_tag");
>-            $component->{'subfields'} = $input->param("mc_${mc_num}_tgt_c_${comp_num}_subfields");
>-            $component->{'offset'} = $input->param("mc_${mc_num}_tgt_c_${comp_num}_offset");
>-            $component->{'length'} = $input->param("mc_${mc_num}_tgt_c_${comp_num}_length");
>-            # norms
>-            $component->{'norms'} = [];
>-            my @norm_nums = sort map { /^mc_${mc_num}_tgt_c_${comp_num}_n_(\d+)_norm/ ? int($1): () } $input->multi_param;
>-            foreach my $norm_num (@norm_nums) {
>-                push @{ $component->{'norms'} }, $input->multi_param("mc_${mc_num}_tgt_c_${comp_num}_n_${norm_num}_norm");
>+        foreach my $w (['src', $src_components], ['tgt', $tgt_components]) {
>+            my ($which, $components) = @$w;
>+            my @comp_nums = sort map { /^mc_${mc_num}_${which}_c_(\d+)_tag/ ? int($1): () } $input->multi_param;
>+            foreach my $comp_num (@comp_nums) {
>+                my $component = {};
>+                my $p = "mc_${mc_num}_${which}_c_${comp_num}_";
>+                map { $component->{$_} = $input->param("${p}$_") } qw(tag subfields offset length);
>+                # norms
>+                $component->{'norms'} = [];
>+                my @norm_nums = sort map { /^${p}n_(\d+)_norm/ ? int($1): () } $input->multi_param;
>+                foreach my $norm_num (@norm_nums) {
>+                    push @{ $component->{'norms'} }, $input->multi_param("${p}n_${norm_num}_norm");
>+                }
>+                push @$components, $component;
>             }
>-            push @$tgt_components, $component;
>         }
>         $matcher->add_required_check($src_components, $tgt_components);
>     }
>-    
>+
>     if (defined $matcher_id and $matcher_id =~ /^\d+/) {
>         $matcher->_id($matcher_id);
>         $template->param(edited_matching_rule => $matcher->code());
>diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/matching-rules.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/matching-rules.tt
>index 7349551b57..7abd5784e2 100644
>--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/matching-rules.tt
>+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/matching-rules.tt
>@@ -54,6 +54,60 @@
>     </select>
> [% END %]
> 
>+[% BLOCK match_check_component %]
>+    [%# PARAMS mc_num, which, components %]
>+    [% IF (!components) %]
>+        [% SET components = [
>+            {
>+                comp_num => 1, norms => [
>+                    {
>+                        norm_num => 1,
>+                        norm = "none"
>+                    }
>+                ],
>+                tag => '',
>+                subfields => '',
>+                offset => '',
>+                length => ''
>+            }
>+         ] %]
>+    [% END %]
>+    [% FOREACH component IN components %]
>+        [% SET pc = "mc_${mc_num}_${which}_c_${component.comp_num}" %]
>+        <fieldset class="rows" id="[% pc | html %]">
>+            [% IF (which == 'src') %]
>+            <legend>Source (incoming) record check field</legend>
>+            [% ELSE %]
>+            <legend>Target (database) record check field</legend>
>+            [% END %]
>+            <ol>
>+                <li>
>+                    <label for="[% pc | html %]_tag">Tag: </label>
>+                    <input type="text" id="[% pc | html %]_tag" name="[% pc | html %]_tag" value="[% component.tag | html %]" size="3" maxlength="3" />
>+                </li>
>+                <li>
>+                    <label for="[$ pc | html %]_subfields">Subfields: </label>
>+                    <input type="text" id="[% pc | html %]_subfields" name="[% pc | html %]_subfields" value="[% component.subfields | html %]" size="10" maxlength="40" />
>+                </li>
>+                <li>
>+                    <label for="[% pc | html %]_offset">Offset: </label>
>+                    <input type="text" id="[% pc | html %]_offset" name="[% pc | html %]_offset" value="[% component.offset | html %]" size="5" maxlength="5" />
>+                </li>
>+                <li>
>+                    <label for="[% pc | html %]_length">Length: </label>
>+                    <input type="text" id="[% pc | html %]_length" name="[% pc | html %]_length" value="[% component.length | html %]" size="5" maxlength="5" />
>+                </li>
>+                [% FOREACH norm IN component.norms %]
>+                    <li id="[% pc | html %]_n_[% norm.norm_num | html %]">
>+                        <label for="[% pc | html %]_n_[% norm.norm_num | html %]_norm">Normalization rule: </label>
>+                        [% PROCESS norms_select selected_norm=norm.norm id="${pc}_n_${norm.norm_num}_norm" name="${pc}_n_${norm.norm_num}_norm" %]
>+                    </li>
>+                [% END %]
>+            </ol>
>+        </fieldset> 
>+    [% END #/FOREACH component %]
>+[% END %]
>+
> <style>
>     fieldset.rows fieldset.rows { border-width : 0; }
>     fieldset.rows fieldset.rows fieldset.rows { border-width : 1px; }
>@@ -287,67 +341,8 @@
>                                             <legend>Match check [% matchcheck.mc_num | html %]</legend>
>                                             <p><a href="#" class="btn btn-link" onclick="InsertMatchcheck('mc_[% matchcheck.mc_num | html %]', 'mc_template'); return false;"><i class="fa fa-plus" aria-hidden="true"></i> New match check</a> | <a href="#" class="btn btn-link" onclick="DeleteMatchcheck(this); return false;"><i class="fa fa-trash-can" aria-hidden="true"></i> Remove this match check</a></p>
>                                             <input type="hidden" id="mc_[% matchcheck.mc_num | html %]_id" name="mc_[% matchcheck.mc_num | html %]_id" value="1" />
>-                                            [% FOREACH src_component IN matchcheck.src_components %]
>-                                                <fieldset class="rows" id="mc_[% src_component.mc_num | html %]_src_c_[% src_component.comp_num | html %]">
>-                                                    <legend>Source (incoming) record check field</legend>
>-                                                    <ol>
>-                                                        <li>
>-                                                            <label for="mc_[% src_component.mc_num | html %]_src_c_[% src_component.comp_num | html %]_tag">Tag: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_tag" name="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_tag" value="[% src_component.tag | html %]" size="3" maxlength="3" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_subfields">Subfields: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_subfields" name="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_subfields" value="[% src_component.subfields | html %]" size="10" maxlength="40" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_offset">Offset: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_offset" name="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_offset" value="[% src_component.offset | html %]" size="5" maxlength="5" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_length">Length: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_length" name="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_length" value="[% src_component.length | html %]" size="5" maxlength="5" />
>-                                                        </li>
>-                                                        [% FOREACH norm IN src_component.norms %]
>-                                                            <li id="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_n_[% norm.norm_num | html %]">
>-                                                                <label for="mc_[% matchcheck.mc_num | html %]_src_c_[% src_component.comp_num | html %]_n_[% norm.norm_num | html %]_norm">Normalization rule: </label>
>-                                                                [%# Note: Following directive in one line, otherwise we have translatabiltiy problems %]
>-                                                                [% PROCESS norms_select selected_norm=norm.norm id="mc_${matchcheck.mc_num}_src_c_${src_component.comp_num}_n_${norm.norm_num}_norm" name="mc_${matchcheck.mc_num}_src_c_${src_component.comp_num}_n_${norm.norm_num}_norm" %]
>-                                                            </li>
>-                                                        [% END %]
>-                                                    </ol>
>-                                                </fieldset> 
>-                                            [% END # /FOREACH src_component %]
>-
>-                                            [% FOREACH tgt_component IN matchcheck.tgt_components %]
>-                                                <fieldset class="rows" id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]">
>-                                                    <legend>Target (database) record check field</legend>
>-                                                    <ol>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_tag">Tag: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_tag" name="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_tag" value="[% tgt_component.tag | html %]" size="3" maxlength="3" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_subfields">Subfields: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_subfields" name="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_subfields" value="[% tgt_component.subfields | html %]" size="10" maxlength="40" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_offset">Offset: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_offset" name="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_offset" value="[% tgt_component.offset | html %]" size="5" maxlength="5" />
>-                                                        </li>
>-                                                        <li>
>-                                                            <label for="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_length">Length: </label>
>-                                                            <input type="text" id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_length" name="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_length" value="[% tgt_component.length | html %]" size="5" maxlength="5" />
>-                                                        </li>
>-                                                        [% FOREACH norm IN tgt_component.norms %]
>-                                                            <li id="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_n_[% norm.norm_num | html %]">
>-                                                                <label for="mc_[% matchcheck.mc_num | html %]_tgt_c_[% tgt_component.comp_num | html %]_n_[% norm.norm_num | html %]_norm">Normalization rule: </label>
>-                                                                [%# Note: Following directive in one line, otherwise we have translatabiltiy problems %]
>-                                                                [% PROCESS norms_select selected_norm=norm.norm id="mc_${matchcheck.mc_num}_tgt_c_${tgt_component.comp_num}_n_${norm.norm_num}_norm" name="mc_${matchcheck.mc_num}_tgt_c_${tgt_component.comp_num}_n_${norm.norm_num}_norm" %]
>-                                                            </li>
>-                                                        [% END %]
>-                                                    </ol>
>-                                                </fieldset> 
>-                                            [% END # /FOREACH tgt_component %]
>+                                            [% PROCESS match_check_component mc_num=matchcheck.mc_num which="src" components=matchcheck.src_components %]
>+                                            [% PROCESS match_check_component mc_num=matchcheck.mc_num which="tgt" components=matchcheck.tgt_components %]
>                                         </fieldset> 
>                                         <br style="clear:both;" />
>                                     </div> 
>@@ -358,57 +353,8 @@
>                                         <legend>Match check 1</legend>
>                                         <p><a href="#" class="btn btn-link" onclick="InsertMatchcheck('mc_1', 'mc_template'); return false;"><i class="fa fa-plus" aria-hidden="true"></i> New match check</a> | <a href="#" class="btn btn-link" onclick="DeleteMatchcheck(this); return false;"><i class="fa fa-trash-can" aria-hidden="true"></i> Remove this match check</a></p>
>                                         <input type="hidden" id="mc_1_id" name="mc_1_id" value="1" />
>-                                        <fieldset class="rows" id="mc_1_src_c_1">
>-                                            <legend>Source (incoming) record check field</legend>
>-                                            <ol>
>-                                                <li>
>-                                                    <label for="mc_1_src_c_1_tag">Tag: </label>
>-                                                    <input type="text" id="mc_1_src_c_1_tag" name="mc_1_src_c_1_tag" size="3" maxlength="3" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_src_c_1_subfields">Subfields: </label>
>-                                                    <input type="text" id="mc_1_src_c_1_subfields" name="mc_1_src_c_1_subfields" size="10" maxlength="40" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_src_c_1_offset">Offset: </label>
>-                                                    <input type="text" id="mc_1_src_c_1_offset" name="mc_1_src_c_1_offset" size="5" maxlength="5" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_src_c_1_length">Length: </label>
>-                                                    <input type="text" id="mc_1_src_c_1_length" name="mc_1_src_c_1_length" size="5" maxlength="5" />
>-                                                </li>
>-                                                <li id="mc_1_src_c_1_n_1">
>-                                                    <label for="mc_1_src_c_1_n_1_norm">Normalization rule: </label>
>-                                                    [% PROCESS norms_select selected_norm="none" id="mc_1_src_c_1_n_1_norm" name="mc_1_src_c_1_n_1_norm" %]
>-                                                </li>
>-                                            </ol>
>-                                        </fieldset>
>-
>-                                        <fieldset class="rows" id="mc_1_tgt_c_1">
>-                                            <legend>Target (database) record check field</legend>
>-                                            <ol>
>-                                                <li>
>-                                                    <label for="mc_1_tgt_c_1_tag">Tag: </label>
>-                                                    <input type="text" id="mc_1_tgt_c_1_tag" name="mc_1_tgt_c_1_tag" size="3" maxlength="3" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_tgt_c_1_subfields">Subfields: </label>
>-                                                    <input type="text" id="mc_1_tgt_c_1_subfields" name="mc_1_tgt_c_1_subfields" size="10" maxlength="40" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_tgt_c_1_offset">Offset: </label>
>-                                                    <input type="text" id="mc_1_tgt_c_1_offset" name="mc_1_tgt_c_1_offset" size="5" maxlength="5" />
>-                                                </li>
>-                                                <li>
>-                                                    <label for="mc_1_tgt_c_1_length">Length: </label>
>-                                                    <input type="text" id="mc_1_tgt_c_1_length" name="mc_1_tgt_c_1_length" size="5" maxlength="5" />
>-                                                </li>
>-                                                <li id="mc_1_tgt_c_1_n_1">
>-                                                    <label for="mc_1_tgt_c_1_n_1_norm">Normalization rule: </label>
>-                                                    [% PROCESS norms_select selected_norm="none" id="mc_1_tgt_c_1_n_1_norm" name="mc_1_tgt_c_1_n_1_norm" %]
>-                                                </li>
>-                                            </ol>
>-                                        </fieldset> 
>+                                        [% PROCESS match_check_component mc_num=1 which="src" components=0 %]
>+                                        [% PROCESS match_check_component mc_num=1 which="tgt" components=0 %]
>                                     </fieldset> 
>                                     <br style="clear:both;" />
>                                 </div> 
>@@ -535,56 +481,8 @@
>                             <legend>Match check <span class="counter"></span></legend>
>                             <p><a href="#" class="button" onclick="DeleteMatchcheck(this); return false;"><i class="fa fa-trash-can" aria-hidden="true"></i>  Remove this match check</a></p>
>                             <input type="hidden" id="mc_num_id" name="mc_num_id" value="1" />
>-                            <fieldset class="rows" id="mc_num_src_c_1">
>-                                <legend>Source (incoming) record check field</legend>
>-                                <ol>
>-                                    <li>
>-                                        <label for="mc_num_src_c_1_tag">Tag: </label>
>-                                        <input type="text" id="mc_num_src_c_1_tag" name="mc_num_src_c_1_tag" size="3" maxlength="3" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_src_c_1_subfields">Subfields: </label>
>-                                        <input type="text" id="mc_num_src_c_1_subfields" name="mc_num_src_c_1_subfields" size="10" maxlength="40" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_src_c_1_offset">Offset: </label>
>-                                        <input type="text" id="mc_num_src_c_1_offset" name="mc_num_src_c_1_offset" size="5" maxlength="5" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_src_c_1_length">Length: </label>
>-                                        <input type="text" id="mc_num_src_c_1_length" name="mc_num_src_c_1_length" size="5" maxlength="5" />
>-                                    </li>
>-                                    <li id="mc_num_src_c_1_n_1">
>-                                        <label for="mc_num_src_c_1_n_1_norm">Normalization rule: </label>
>-                                        [% PROCESS norms_select selected_norm="none" id="mc_num_src_c_1_n_1_norm" name="mc_num_src_c_1_n_1_norm" %]
>-                                    </li>
>-                                </ol>
>-                            </fieldset> 
>-                            <fieldset class="rows" id="mc_num_tgt_c_1">
>-                                <legend>Target (database) record check field</legend>
>-                                <ol>
>-                                    <li>
>-                                        <label for="mc_num_tgt_c_1_tag">Tag: </label>
>-                                        <input type="text" id="mc_num_tgt_c_1_tag" name="mc_num_tgt_c_1_tag" size="3" maxlength="3" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_tgt_c_1_subfields">Subfields: </label>
>-                                        <input type="text" id="mc_num_tgt_c_1_subfields" name="mc_num_tgt_c_1_subfields" size="10" maxlength="40" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_tgt_c_1_offset">Offset: </label>
>-                                        <input type="text" id="mc_num_tgt_c_1_offset" name="mc_num_tgt_c_1_offset" size="5" maxlength="5" />
>-                                    </li>
>-                                    <li>
>-                                        <label for="mc_num_tgt_c_1_length">Length: </label>
>-                                        <input type="text" id="mc_num_tgt_c_1_length" name="mc_num_tgt_c_1_length" size="5" maxlength="5" />
>-                                    </li>
>-                                    <li id="mc_num_tgt_c_1_n_1">
>-                                        <label for="mc_num_tgt_c_1_n_1_norm">Normalization rule: </label>
>-                                        [% PROCESS norms_select selected_norm="none" id="mc_num_tgt_c_1_n_1_norm" name="mc_num_tgt_c_1_n_1_norm" %]
>-                                    </li>
>-                                </ol>
>-                            </fieldset> 
>+                            [% PROCESS match_check_component mc_num="num" which="src" components=0 %]
>+                            [% PROCESS match_check_component mc_num="num" which="tgt" components=0 %]
>                         </fieldset> 
>                         <br style="clear:both;" />
>                     </div> 
>-- 
>2.39.2

Actions: View | Diff | Splinter Review

Attachments on bug 15536: 166947 | 166948 | 166949 | 166950