From 4c04f6f8e0068a7cce256c3cd8ffcf5acf682fe0 Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Mon, 25 Apr 2016 09:12:05 -0400
Subject: [PATCH] Bug 16336 [Revised] UX of holds patron search with long lists
 of results

Bug 15793 made a change to an interface which is also found in the place
hold template. This patch creates a new include file to be used by both
circulation.tt and request.tt so that these pages do not diverge.

In the process, this patch removes some markup and JavaScript which was
made obsolete by Bug 15793 and should have been removed.

This patch also revises the sorting of the results table so that the
patron name is sortable (Bug 16334) and the default sort is on card
number (matching 3.20.x and 3.22.x).

To test:

In Circulation:
- Perform a search by name for a patron which will
  return multiple search results.
- The table of results which displays should look correct and work
  correctly, including DataTables sorting.
- Clicking any table row should forward you to the checkout page for
  that patron.

In the catalog:
- Locate and place a hold on a title.
- When prompted to select a patron to place the hold for, perform a
  search by name which will return multiple results.
- Confirm that the table of patron results looks correct and works
  correctly.
- Clicking any table row should forward you to the place hold page for
  that patron and the title you selected.

Revision: Although the table row was clickable, you couldn't
middle-click it to open the link in a new tab. The patron name is now a
real link you can middle-click or right-click. The row is still
clickable as well.

Signed-off-by: Aleisha <aleishaamohia@hotmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
---
 .../includes/circ-patron-search-results.inc   | 40 ++++++++++++++
 .../prog/en/modules/circ/circulation.tt       | 55 +------------------
 .../prog/en/modules/reserve/request.tt        | 46 +---------------
 3 files changed, 45 insertions(+), 96 deletions(-)
 create mode 100644 koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc

diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc
new file mode 100644
index 0000000000..c60395bd55
--- /dev/null
+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/circ-patron-search-results.inc
@@ -0,0 +1,40 @@
+<script type="text/javascript">
+    $(document).ready(function() {
+        $(".clickable").click(function() {
+            window.document.location = $(this).data('url');
+        });
+        var table = $("#table_borrowers").dataTable($.extend(true, {}, dataTablesDefaults, {
+            "aaSorting": [ 1, "asc" ],
+            "sDom": "t",
+            "iDisplayLength": -1
+        }));
+    });
+</script>
+<legend>Patron selection</legend>
+<table id="table_borrowers" class="table_borrowers">
+    <thead>
+        <tr>
+            <th>Name</th>
+            <th>Card number</th>
+            <th>Category</th>
+            <th>Library</th>
+            <th>Address</th>
+        </tr>
+    </thead>
+    <tbody>
+        [% FOREACH borrower IN borrowers %]
+            [% IF ( destination == "circ" ) %]
+                <tr class="clickable" data-url="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% borrower.borrowernumber %]">
+                    <td><a href="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% borrower.borrowernumber %]">[% borrower.surname %], [% borrower.firstname %]</a></td>
+            [% ELSE %]
+                <tr class="clickable" data-url="/cgi-bin/koha/reserve/request.pl?borrowernumber=[% borrower.borrowernumber %]&amp;biblionumber=[% biblionumber %]">
+                    <td><a href="/cgi-bin/koha/reserve/request.pl?borrowernumber=[% borrower.borrowernumber %]&amp;biblionumber=[% biblionumber %]">[% borrower.surname %], [% borrower.firstname %]</a></td>
+            [% END %]
+                <td>[% borrower.cardnumber %]</td>
+                <td>[% borrower.categorycode %]</td>
+                <td>[% borrower.branchcode %]</td>
+                <td>[% borrower.address %]</td>
+            </tr>
+        [% END %]
+    </tbody>
+</table>
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
index cd3fc35567..b6537620e3 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ -91,10 +91,6 @@ $(document).ready(function() {
         }
     });
 
-    $(".clickable").click(function() {
-        window.document.location = $(this).data('url');
-    });
-
     [% IF !( CircAutoPrintQuickSlip == 'clear' ) %]
         // listen submit to trigger qslip on empty checkout
         $('#mainform').bind('submit',function() {
@@ -107,22 +103,6 @@ $(document).ready(function() {
     $("#onsite_checkout").click(function(){
         toggle_onsite_checkout();
     });
-
-    var table = $("#table_borrowers").dataTable($.extend(true, {}, dataTablesDefaults, {
-        "aoColumnDefs": [
-            { "aTargets": [ 0 ], "bSortable": false, "bSearchable": false },
-        ],
-        "aaSorting": [ 2, "asc" ],
-        "sDom": "t",
-        "iDisplayLength": -1
-    }));
-    $(table).find('tbody tr').filter(':has(:radio:checked)').end().click(function(event) {
-        $('#table_borrowers tbody tr').removeClass('selected');
-        $(this).addClass('selected');
-        if (event.target.type !== 'radio') {
-            $(':radio', this).prop('checked', 'true')
-        }
-    });
 });
 
 //]]>
@@ -594,40 +574,9 @@ No patron matched <span class="ex">[% message %]</span>
 [% IF ( borrowers ) %]
 [% INCLUDE 'patron-toolbar.inc' %]
 
-<form method="post" action="/cgi-bin/koha/circ/circulation.pl" id="mainform" name="mainform" autocomplete="off">
-    <input type="hidden" name="restoreduedatespec" />
-<fieldset id="circ_circulation_selectborrower" class="brief">
-    <legend>Patron selection</legend>
-
-[% IF (forceallow) %]<input type="hidden" name="forceallow" value="1">[% END %]
-    <input type="hidden" name="branch" value="[% branch %]" />
-    <input type="hidden" name="duedatespec" value="[% duedatespec %]" />
-    <input type="hidden" name="stickyduedate" value="[% stickyduedate %]" />
-
-    <table id="table_borrowers" class="table_borrowers">
-        <thead>
-            <tr>
-                <th>Name</th>
-                <th>Cardnumber</th>
-                <th>Category</th>
-                <th>Library</th>
-                <th>Address</th>
-            </tr>
-        </thead>
-        <tbody>
-            [% FOREACH borrower IN borrowers %]
-            <tr class="clickable" data-url="/cgi-bin/koha/circ/circulation.pl?borrowernumber=[% borrower.borrowernumber %]">
-                <td>[% borrower.surname %], [% borrower.firstname %]</td>
-                <td>[% borrower.cardnumber %]</td>
-                <td>[% borrower.categorycode %]</td>
-                <td>[% borrower.branchcode %]</td>
-                <td>[% borrower.address %]</td>
-            </tr>
-            [% END %]
-        </tbody>
-    </table>
+<fieldset id="circ_circulation_selectborrower">
+    [% INCLUDE 'circ-patron-search-results.inc' destination = "circ" %]
 </fieldset>
-</form>
 [% ELSE %]
 
 <!-- BARCODE ENTRY -->
diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
index f6e5276aea..c2365b30cb 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -29,21 +29,6 @@ $(document).ready(function() {
         $(".suspend_until_datepicker, .datepickerfrom, .datepickerto").datepicker("option", "minDate", 1);
     [% END %]
 
-    var table = $("#table_borrowers").dataTable($.extend(true, {}, dataTablesDefaults, {
-        "aoColumnDefs": [
-            { "aTargets": [ 0 ], "bSortable": false, "bSearchable": false },
-        ],
-        "aaSorting": [[ 2, "asc" ]],
-        "sDom": "t",
-        "iDisplayLength": -1
-    }));
-    $(table).find('tbody tr').filter(':has(:radio:checked)').end().click(function(event) {
-        $('#table_borrowers tbody tr').removeClass('selected');
-        $(this).addClass('selected');
-        if (event.target.type !== 'radio') {
-            $(':radio', this).prop('checked', 'true')
-        }
-    });
     var my_table = $("#requestspecific").dataTable($.extend(true, {}, dataTablesDefaults, {
         'bPaginate': false,
         "sDom": '<"top pager"ilf>t',
@@ -254,8 +239,9 @@ function checkMultiHold() {
       <div class="dialog alert"><h3>Patron not found</h3><p>No patron with this name, please, try another</p> </div>
     [% END %]
     <form  id="holds_patronsearch" action="request.pl?biblionumber=[% biblionumber %]" method="post">
+        <fieldset id="circ_holds_selectborrower" class="brief">
+
         [% UNLESS borrowers %]
-            <fieldset class="brief">
                 <label for="patron">Patron: </label>
                 <div class="hint">Enter patron card number or partial name:</div>
                 <input type="text" size="40" id="patron" class="focus" name="findborrower" />
@@ -263,33 +249,7 @@ function checkMultiHold() {
                 <input type="hidden" name="biblionumber" value="[% biblionumber %]" />
             </fieldset>
         [% ELSE %]
-          <fieldset>
-            <table id="table_borrowers">
-                <thead>
-                    <tr>
-                        <th></th>
-                        <th>Name</th>
-                        <th>Cardnumber</th>
-                        <th>Category</th>
-                        <th>Library</th>
-                        <th>Address</th>
-                    </tr>
-                </thead>
-                <tbody>
-                    [% FOREACH borrower IN borrowers %]
-                    <tr>
-                        <td><input type="radio" name="borrowernumber" value="[% borrower.borrowernumber %]" /></td>
-                        <td>[% borrower.surname %], [% borrower.firstname %]</td>
-                        <td>[% borrower.cardnumber %]</td>
-                        <td>[% borrower.categorycode %]</td>
-                        <td>[% borrower.branchcode %]</td>
-                        <td>[% borrower.address %]</td>
-                    </tr>
-                    [% END %]
-                </tbody>
-            </table>
-            <input type="hidden" name="biblionumber" value="[% biblionumber %]" />
-            <fieldset class="action"><input type="submit" value="Select" /></fieldset>
+            [% INCLUDE 'circ-patron-search-results.inc' destination = "holds" %]
           </fieldset>
 
         [% END %]
-- 
2.39.5