From 5c3ee79814c8bd55f7baf94bcccda76e5e0dba7d Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Date: Mon, 6 Oct 2025 15:22:12 +0200
Subject: [PATCH] Bug 32176: Remove duplicated code to select patron from
 autocomplete

On bug 31782 we centralized the code to autocomplete when searching for patron
There were several occurrences in the code where some code were
duplicated (when selecting patrons and allowing to remove them from a
list).

This patch centralizes this code by adding another parameter to
patron_autocomplete: on-select-add-to

Test plan:
When duplicating an order, searching for orders, editing a course (and
adding instructors) and adding patrons to a patron list, you should now
see the correct patron details displayed: with preferred name, without
"null" in case of missing surname, correct spacing, etc. Like (almost)
everywhere else in Koha.

Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
---
 .../prog/css/src/staff-global.scss            |  4 ++
 .../prog/en/modules/acqui/duplicate_orders.tt | 29 ++---------
 .../prog/en/modules/acqui/histsearch.tt       | 23 +--------
 .../prog/en/modules/course_reserves/course.tt | 49 +++----------------
 .../prog/en/modules/patron_lists/list.tt      | 21 ++------
 .../intranet-tmpl/prog/js/staff-global.js     | 30 +++++++++++-
 patron_lists/list.pl                          |  2 +-
 7 files changed, 51 insertions(+), 107 deletions(-)

diff --git a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
index f8c9cefcd7..44c3229cf1 100644
--- a/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
+++ b/koha-tmpl/intranet-tmpl/prog/css/src/staff-global.scss
@@ -2863,6 +2863,10 @@ td.bundle {
     background: #FFF url( "../img/spinner-small.gif" ) right center no-repeat;
 }
 
+.patron-detail-autocomplete-selection {
+    padding: 0.3em;
+}
+
 .toptabs {
     margin: 1rem 0;
 }
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/duplicate_orders.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/duplicate_orders.tt
index c2aa545245..c660a925bb 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/duplicate_orders.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/duplicate_orders.tt
@@ -323,7 +323,6 @@
             $("input[name='ordernumbers']").val(ordernumbers.join(','));
         }
 
-        var MSG_REMOVE_PATRON = _("Remove");
         var MSG_NO_ITEM_SELECTED = _("Nothing is selected.");
         var MSG_NO_FUND_SELECTED = _("No fund selected.");
         $(document).ready(function() {
@@ -333,34 +332,16 @@
             });
 
             [% IF op == 'search' OR op == 'select' %]
-                function AddPatron( patron_name, value, container, input_name ) {
-                    div = "<div id='borrower_" + value + "'>" + patron_name + " ( <a href='#' class='removePatron'><i class='fa fa-trash-can' aria-hidden='true'></i> " + MSG_REMOVE_PATRON + " </a> ) <input type='hidden' name='" + input_name + "' value='" + value + "' /></div>";
-                    $(container).append( div );
-
-                    $(container).parent().show( 800 );
-                }
-                function RemovePatron( cardnumber, container ) {
-                    $( '#borrower_' + cardnumber ).remove();
-
-                    if ( ! $(container).html() ) {
-                        $(container).parent("fieldset").hide( 800 );
-                    }
-                }
                 patron_autocomplete($("#find_patron"), {
-                    'on-select-callback': function( event, ui ) {
-                        var field = ui.item.patron_id;
-                        AddPatron( ui.item.firstname + " " + ( ui.item.middle_name || "" ) + " " + ui.item.surname, field, $("#basket_creators"), 'created_by' );
+                    "on-select-add-to": {
+                        container: $("#basket_creators"),
+                        input_name: 'created_by'
+                    },
+                    "on-select-callback": function( event, ui ) {
                         $("#find_patron").val('').focus();
                         return false;
                     }
                 });
-                $("body").on("click",".removePatron",function(e){
-                    e.preventDefault();
-                    var divid = $(this).parent().attr("id");
-                    var cardnumber = divid.replace("borrower_","");
-                    RemovePatron(cardnumber, $("#basket_creators"));
-                });
-
             [% END %]
 
             $("#show_orders_filters, #hide_orders_filters").on('click', function(e) {
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
index 59a838856b..5f6656c9cd 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/histsearch.tt
@@ -168,7 +168,6 @@
     [% INCLUDE 'calendar.inc' %]
     [% INCLUDE 'datatables.inc' %]
     <script>
-        var MSG_REMOVE_PATRON = _("Remove");
         $(document).ready(function() {
             var table_settings = [% TablesSettings.GetTableSettings( 'acqui', 'histsearch', 'histsearcht', 'json' ) | $raw %];
             $("#histsearcht").kohaTable(
@@ -178,33 +177,13 @@
                 table_settings
             );
 
-            function AddPatron( patron_name, value, container, input_name ) {
-                div = "<div id='borrower_" + value + "'>" + patron_name + " ( <a href='#' class='removePatron'><i class='fa fa-trash-can' aria-hidden='true'></i> " + MSG_REMOVE_PATRON + " </a> ) <input type='hidden' name='" + input_name + "' value='" + value + "' /></div>";
-                $(container).append( div );
-
-                $(container).parent().show( 800 );
-            }
-            function RemovePatron( cardnumber, container ) {
-                $( '#borrower_' + cardnumber ).remove();
-
-                if ( ! $(container).html() ) {
-                    $(container).parent("fieldset").hide( 800 );
-                }
-            }
             patron_autocomplete($("#find_patron"), {
+                'on-select-add-to': {container: $("#basket_creators"), input_name: 'created_by' },
                 'on-select-callback': function( event, ui ) {
-                    var field = ui.item.patron_id;
-                    AddPatron( ui.item.firstname + " " + ( ui.item.middle_name || "" ) + " " + ui.item.surname, field, $("#basket_creators"), 'created_by' );
                     $("#find_patron").val('').focus();
                     return false;
                 }
             });
-            $("body").on("click",".removePatron",function(e){
-                e.preventDefault();
-                var divid = $(this).parent().attr("id");
-                var cardnumber = divid.replace("borrower_","");
-                RemovePatron(cardnumber, $("#basket_creators"));
-            });
 
         });
     </script>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course.tt
index 939d292f1e..ac3202e5cf 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/course_reserves/course.tt
@@ -19,9 +19,6 @@
     #course_instructors {
         float: left;
     }
-    .instructor_line {
-        padding: 0.3em;
-    }
     fieldset.rows label[for="find_instructor"] {
         text-align: left;
         margin: 0;
@@ -131,13 +128,13 @@
                         <div id="course_instructors">
                             <div id="instructors">
                                 [% FOREACH i IN instructors %]
-                                    <div class="instructor_line" id="borrower_[% i.borrowernumber | html %]">
-                                        [% INCLUDE 'patron-title.inc' patron=i no_cardnumber=1 %] (<a href="#" class="removeInstructor"><i class="fa fa-trash-can"></i> Remove </a>)
+                                    <div class="patron-detail-autocomplete-selection" id="patron-detail-[% i.borrowernumber | html %]">
+                                        [% INCLUDE 'patron-title.inc' patron=i no_cardnumber=1 %] (<a href="#" class="removePatron"><i class="fa fa-trash-can"></i> Remove </a>)
                                         <input type="hidden" name="instructors" value="[% i.borrowernumber | html %]" />
                                     </div>
                                 [% END %]
                             </div>
-                            <div id="instructor_search" class="instructor_line">
+                            <div id="instructor_search" class="patron-detail-autocomplete-selection">
                                 <label for="find_instructor">Instructor search:</label>
                                 <input autocomplete="off" id="find_instructor" type="text" style="width:150px" class="noEnterSubmit" />
                             </div>
@@ -198,48 +195,16 @@
         <script>
             $(document).ready(function () {
                 patron_autocomplete($("#find_instructor"), {
+                    "on-select-add-to": { container: $("#instructors"), input_name: "instructors" },
                     "on-select-callback": function (event, ui) {
-                        AddInstructor($patron_to_html(ui.item, { display_cardnumber: false }), ui.item.patron_id);
-                        return false;
+                        $("#find_instructor").val("").focus();
                     },
                 });
-
-                if (!$("#instructors").html()) {
-                    $("#course_instructors").hide();
-                }
-
-                $("body").on("click", ".removeInstructor", function (e) {
+                $(".removePatron").on("click", function (e) {
                     e.preventDefault();
-                    var divid = $(this).parent().attr("id");
-                    var borrowernumber = divid.replace("borrower_", "");
-                    RemoveInstructor(borrowernumber);
+                    $(this).closest('div[id^="patron-detail-"]').remove();
                 });
             });
-
-            function AddInstructor(name, borrowernumber) {
-                div =
-                    "<div class='instructor_line' id='borrower_" +
-                    borrowernumber +
-                    "'>" +
-                    (name ? name.escapeHtml() : "") +
-                    " ( <a href='#' class='removeInstructor'><i class='fa fa-trash-can'></i> " +
-                    _("Remove") +
-                    " </a> ) <input type='hidden' name='instructors' value='" +
-                    borrowernumber +
-                    "' /></div>";
-                $("#instructors").append(div);
-
-                $("#find_instructor").val("").focus();
-                $("#course_instructors").show();
-            }
-
-            function RemoveInstructor(borrowernumber) {
-                $("#borrower_" + borrowernumber).remove();
-
-                if (!$("#instructors").html()) {
-                    $("#course_instructors").hide(800);
-                }
-            }
         </script>
     [% END %]
 [% END %]
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt
index c1e85a7770..3646995cb2 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/patron_lists/list.tt
@@ -235,7 +235,6 @@
     [% Asset.js("js/tools-menu.js") | $raw %]
     [% INCLUDE 'datatables.inc' %]
     <script>
-        var MSG_REMOVE_PATRON = _("Remove");
         $(document).ready(function() {
             $('#patrons_to_add_fieldset').hide();
 
@@ -247,12 +246,6 @@
                     table_settings
                 );
 
-            function AddPatron( patron_name, value, container, input_name ) {
-                div = "<div id='borrower_" + value + "'>" + patron_name + " ( <a href='#' class='removePatron'><i class='fa fa-trash-can' aria-hidden='true'></i> " + MSG_REMOVE_PATRON + " </a> ) <input type='hidden' name='" + input_name + "' value='" + value + "' /></div>";
-                $(container).append( div );
-
-                $(container).parent().show( 800 );
-            }
             function RemovePatron( cardnumber, container ) {
                 $( '#borrower_' + cardnumber ).remove();
 
@@ -261,19 +254,15 @@
                 }
             }
             patron_autocomplete($("#find_patron"), {
-                'on-select-callback': function( event, ui ) {
-                    var field = ui.item.cardnumber;
-                    AddPatron( ( ui.item.firstname || "" ) + " " + ( ui.item.middle_name || "" ) + " " + ( ui.item.surname || "" ), field, $("#patrons_to_add"), 'patrons_to_add' );
+                "on-select-add-to": {
+                    container: $("#patrons_to_add"),
+                    input_name: 'patrons_to_add',
+                },
+                "on-select-callback": function( event, ui ) {
                     $("#find_patron").val('').focus();
                     return false;
                 }
             });
-            $("body").on("click",".removePatron",function(e){
-                e.preventDefault();
-                var divid = $(this).parent().attr("id");
-                var cardnumber = divid.replace("borrower_","");
-                RemovePatron(cardnumber, $("#patrons_to_add"));
-            });
 
             $("body").on("click", "#CheckAll", function(e){
                 e.preventDefault();
diff --git a/koha-tmpl/intranet-tmpl/prog/js/staff-global.js b/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
index 29a1f3b064..916497d063 100644
--- a/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
+++ b/koha-tmpl/intranet-tmpl/prog/js/staff-global.js
@@ -691,6 +691,7 @@ function patron_autocomplete(node, options) {
     let link_to;
     let url_params;
     let on_select_callback;
+    let on_select_add_to;
 
     if (options) {
         if (options["link-to"]) {
@@ -702,6 +703,9 @@ function patron_autocomplete(node, options) {
         if (options["on-select-callback"]) {
             on_select_callback = options["on-select-callback"];
         }
+        if (options["on-select-add-to"]) {
+            on_select_add_to = options["on-select-add-to"];
+        }
     }
     return (node
         .autocomplete({
@@ -738,9 +742,31 @@ function patron_autocomplete(node, options) {
             },
             minLength: 3,
             select: function (event, ui) {
-                if (ui.item.link) {
+                if (on_select_add_to) {
+                    let container = on_select_add_to.container;
+
+                    const patron_name =
+                        `${ui.item.firstname} ${ui.item.middle_name || ""} ${ui.item.surname}`.trim();
+                    const node = `
+                      <div id='patron-detail-${ui.item.patron_id}' class='patron-detail-autocomplete-selection'>
+                        ${patron_name}
+                        (<a href='#' class='removePatron'>
+                          <i class='fa fa-trash-can' aria-hidden='true'></i> ${__("Remove")}
+                        </a>)
+                        <input type='hidden' name='${on_select_add_to.input_name}' value='${ui.item.patron_id}' />
+                      </div>
+                    `;
+
+                    $(container).append(node).parent().show(800);
+
+                    $(container).on("click", ".removePatron", function (e) {
+                        e.preventDefault();
+                        $(this).closest('div[id^="patron-detail-"]').remove();
+                    });
+                } else if (ui.item.link) {
                     window.location.href = ui.item.link;
-                } else if (on_select_callback) {
+                }
+                if (on_select_callback) {
                     return on_select_callback(event, ui);
                 }
             },
diff --git a/patron_lists/list.pl b/patron_lists/list.pl
index 3fbbd5f1f7..d4d9c9ed1e 100755
--- a/patron_lists/list.pl
+++ b/patron_lists/list.pl
@@ -74,7 +74,7 @@ if ( $op eq 'cud-add' && $patrons_by_id ) {
 
 my @patrons_to_add = $cgi->multi_param('patrons_to_add');
 if ( $op eq 'cud-add' && @patrons_to_add ) {
-    AddPatronsToList( { list => $list, cardnumbers => \@patrons_to_add } );
+    AddPatronsToList( { list => $list, borrowernumbers => \@patrons_to_add } );
 }
 
 my @patrons_to_remove = $cgi->multi_param('patrons_to_remove');