From dc9023f02f17068ca0c54bb613a0f4f52db41017 Mon Sep 17 00:00:00 2001
From: Owen Leonard <oleonard@myacpl.org>
Date: Mon, 29 Jun 2020 14:31:24 +0000
Subject: [PATCH] Bug 25892: Clarify the visual hierarchy of holds by branch
 and itemtype

This patch makes minor changes to the markup of the holds list to make
the page hierarchy clearer when holds are grouped by branch and
itemtype.

Test plan:

 - Enable the  AllowHoldItemTypeSelection system preference.
 - Locate a record in the catalog and place multiple holds:
   - Holds at different branches
   - Holds with different itemtypes selected
 - View the holds page with various values of the HoldsSplitQueue
   preference enabled:
   - nothing
   - pickup library
   - hold itemtype
   - pickup library & itemtype

   In each case verify that the interface works well.

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
---
 .../prog/en/modules/reserve/request.tt             | 176 ++++++++++-----------
 1 file changed, 86 insertions(+), 90 deletions(-)

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 44c46d2395..1795eed1c0 100644
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reserve/request.tt
@@ -794,122 +794,118 @@
                             <input type = "hidden" name="biblionumbers" value="[% biblionumbers | html %]"/>
                         [% END %]
 
-                        <fieldset class="rows">
-                            <legend>Existing holds</legend>
+                        <h2>Existing holds</h2>
+
+                        [% FOREACH biblioloo IN biblioloop %]
+                            [% IF ( biblioloo.reserveloop ) %]
+                                [% IF ( multi_hold ) %]
+                                    <h3>
+                                        <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblioloo.biblionumber | uri %]">
+                                            [% biblioloo.title | html %]
+                                        </a>
+                                    </h3>
+                                [% END %]
 
-                            [% FOREACH biblioloo IN biblioloop %]
-                                [% IF ( biblioloo.reserveloop ) %]
-                                    [% IF ( multi_hold ) %]
-                                        <h3>
-                                            <a href="/cgi-bin/koha/catalogue/detail.pl?biblionumber=[% biblioloo.biblionumber | uri %]">
-                                                [% biblioloo.title | html %]
-                                            </a>
-                                        </h3>
+                                [% IF Koha.Preference('HoldsSplitQueue') == 'branch' %]
+                                    [% SET branchcodes = [] %]
+
+                                    [% FOREACH h IN biblioloo.reserveloop %]
+                                        [% branchcodes.push( h.branchcode ) %]
                                     [% END %]
+                                    [% branchcodes = branchcodes.unique %]
 
-                                    [% IF Koha.Preference('HoldsSplitQueue') == 'branch' %]
-                                        [% SET branchcodes = [] %]
+                                    [% FOREACH b IN branchcodes.sort %]
+                                        [% SET holds_by_branch = [] %]
+                                        [% FOREACH h IN biblioloo.reserveloop %]
+                                            [% IF h.branchcode == b %]
+                                                [% holds_by_branch.push( h ) %]
+                                            [% END %]
+                                        [% END %]
+                                        <fieldset>
+                                            <legend>[% Branches.GetName( b ) | html %]</legend>
+                                            [% INCLUDE holds_table.inc holds=holds_by_branch %]
+                                        </fieldset>
+                                    [% END # /FOREACh b %]
+                                [% ELSIF Koha.Preference('HoldsSplitQueue') == 'itemtype' %]
+                                    [% SET itemtypes = [] %]
+
+                                    [% FOREACH h IN biblioloo.reserveloop %]
+                                        [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
+                                        [% itemtypes.push( hold_itemtype ) %]
+                                    [% END %]
+                                    [% itemtypes = itemtypes.unique %]
 
+                                    [% FOREACH i IN itemtypes.sort %]
+                                        [% SET holds_by_itemtype = [] %]
                                         [% FOREACH h IN biblioloo.reserveloop %]
-                                            [% branchcodes.push( h.branchcode ) %]
+                                            [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
+                                            [% IF hold_itemtype == i %]
+                                                [% holds_by_itemtype.push( h ) %]
+                                            [% END %]
                                         [% END %]
-                                        [% branchcodes = branchcodes.unique %]
 
-                                        [% FOREACH b IN branchcodes.sort %]
+                                        <fieldset>
+                                            [% IF i %]
+                                                <legend>[% ItemTypes.GetDescription( i ) | html %]</legend>
+                                            [% ELSE %]
+                                                <legend>Any item type</legend>
+                                            [% END %]
+                                            [% INCLUDE holds_table.inc holds=holds_by_itemtype %]
+                                        </fieldset>
+                                    [% END # /FOREACH i %]
+                                [% ELSIF Koha.Preference('HoldsSplitQueue') == 'branch_itemtype' %]
+                                    [% SET branchcodes = [] %]
+
+                                    [% FOREACH h IN biblioloo.reserveloop %]
+                                        [% branchcodes.push( h.branchcode ) %]
+                                    [% END %]
+                                    [% branchcodes = branchcodes.unique %]
+
+                                    [% FOREACH b IN branchcodes.sort %]
+                                        <fieldset>
+                                            <legend>[% Branches.GetName( b ) | html %]</legend>
                                             [% SET holds_by_branch = [] %]
                                             [% FOREACH h IN biblioloo.reserveloop %]
                                                 [% IF h.branchcode == b %]
                                                     [% holds_by_branch.push( h ) %]
                                                 [% END %]
                                             [% END %]
-                                            <fieldset>
-                                                <legend>[% Branches.GetName( b ) | html %]</legend>
-                                                [% INCLUDE holds_table.inc holds=holds_by_branch %]
-                                            </fieldset>
-                                        [% END # /FOREACh b %]
-                                    [% ELSIF Koha.Preference('HoldsSplitQueue') == 'itemtype' %]
-                                        [% SET itemtypes = [] %]
-
-                                        [% FOREACH h IN biblioloo.reserveloop %]
-                                            [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
-                                            [% itemtypes.push( hold_itemtype ) %]
-                                        [% END %]
-                                        [% itemtypes = itemtypes.unique %]
 
-                                        [% FOREACH i IN itemtypes.sort %]
-                                            [% SET holds_by_itemtype = [] %]
-                                            [% FOREACH h IN biblioloo.reserveloop %]
+                                            [% SET itemtypes = [] %]
+                                            [% FOREACH h IN holds_by_branch %]
                                                 [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
-                                                [% IF hold_itemtype == i %]
-                                                    [% holds_by_itemtype.push( h ) %]
-                                                [% END %]
+                                                [% itemtypes.push( hold_itemtype ) %]
                                             [% END %]
+                                            [% itemtypes = itemtypes.unique %]
 
-                                            <fieldset>
+                                            [% FOREACH i IN itemtypes.sort %]
                                                 [% IF i %]
-                                                    <legend>[% ItemTypes.GetDescription( i ) | html %]</legend>
+                                                    <h3>[% ItemTypes.GetDescription( i ) | html %]</h3>
                                                 [% ELSE %]
-                                                    <legend>Any item type</legend>
+                                                    <h3>Any item type</h3>
                                                 [% END %]
-                                                [% INCLUDE holds_table.inc holds=holds_by_itemtype %]
-                                            </fieldset>
-                                        [% END # /FOREACH i %]
-                                    [% ELSIF Koha.Preference('HoldsSplitQueue') == 'branch_itemtype' %]
-                                        [% SET branchcodes = [] %]
 
-                                        [% FOREACH h IN biblioloo.reserveloop %]
-                                            [% branchcodes.push( h.branchcode ) %]
-                                        [% END %]
-                                        [% branchcodes = branchcodes.unique %]
-
-                                        [% FOREACH b IN branchcodes.sort %]
-                                            <fieldset class="contrast">
-                                                <legend>[% Branches.GetName( b ) | html %]</legend>
-                                                [% SET holds_by_branch = [] %]
-                                                [% FOREACH h IN biblioloo.reserveloop %]
-                                                    [% IF h.branchcode == b %]
-                                                        [% holds_by_branch.push( h ) %]
-                                                    [% END %]
-                                                [% END %]
-
-                                                [% SET itemtypes = [] %]
+                                                [% SET holds_by_itemtype = [] %]
                                                 [% FOREACH h IN holds_by_branch %]
                                                     [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
-                                                    [% itemtypes.push( hold_itemtype ) %]
-                                                [% END %]
-                                                [% itemtypes = itemtypes.unique %]
-
-                                                [% FOREACH i IN itemtypes.sort %]
-                                                    <fieldset class="standard">
-                                                        [% IF i %]
-                                                            <legend>[% ItemTypes.GetDescription( i ) | html %]</legend>
-                                                        [% ELSE %]
-                                                            <legend>Any item type</legend>
-                                                        [% END %]
-
-                                                        [% SET holds_by_itemtype = [] %]
-                                                        [% FOREACH h IN holds_by_branch %]
-                                                            [% SET hold_itemtype = h.object.item.effective_itemtype || h.itemtype %]
-                                                            [% IF hold_itemtype == i %]
-                                                                [% holds_by_itemtype.push( h ) %]
-                                                            [% END %]
-                                                        [% END %]
-                                                        [% INCLUDE holds_table.inc holds=holds_by_itemtype %]
-                                                    </fieldset>
+                                                    [% IF hold_itemtype == i %]
+                                                        [% holds_by_itemtype.push( h ) %]
+                                                    [% END %]
                                                 [% END %]
-                                            </fieldset>
-                                        [% END # /FOREACH b %]
-                                    [% ELSE %]
-                                        [% INCLUDE holds_table.inc holds=biblioloo.reserveloop %]
-                                    [% END # /IF HoldsSplitQueue %]
+                                                [% INCLUDE holds_table.inc holds=holds_by_itemtype %]
+                                            [% END %]
+                                        </fieldset>
+                                    [% END # /FOREACH b %]
+                                [% ELSE %]
+                                    [% INCLUDE holds_table.inc holds=biblioloo.reserveloop %]
+                                [% END # /IF HoldsSplitQueue %]
 
-                                [% END # /IF biblioloo.reserveloop %]
-                            [% END # FOREACH biblioloo %]
+                            [% END # /IF biblioloo.reserveloop %]
+                        [% END # FOREACH biblioloo %]
 
-                            <fieldset class="action">
-                                <input type="submit" name="submit" value="Update hold(s)" />
-                            </fieldset>
-                        </fieldset> <!-- /.rows -->
+                        <fieldset class="action">
+                            <input type="submit" name="submit" value="Update hold(s)" />
+                        </fieldset>
                     </form> <!-- /name=TTime -->
                 [% END # IF reserveloop %]
             [% END # UNLESS patron %]
-- 
2.11.0