From 27b2037d72b8a1c5025a885d571b5329232ffa58 Mon Sep 17 00:00:00 2001
From: Dan Scott <dan@coffeecode.net>
Date: Thu, 29 Dec 2011 00:11:25 -0500
Subject: [PATCH] TPAC: Propagate CGI params via mkurl()

Fix many instances where params such as "loc" are dropped during a
normal workflow, resulting in unexpected changes to the user's chosen
search scope. mkurl() preserves the existing CGI params by default
and passes them on to the created URL; for now we err on the side of
caution (preserving all params) rather than filtering any out. Manual
testing suggests this does not introduce any problems, and resolves
many.

Signed-off-by: Dan Scott <dan@coffeecode.net>
Signed-off-by: Bill Erickson <berick@esilibrary.com>
---
 Open-ILS/src/templates/opac/advanced.tt2           | 12 ++++++++---
 .../src/templates/opac/myopac/circ_history.tt2     | 12 ++++++-----
 Open-ILS/src/templates/opac/myopac/circs.tt2       | 10 ++++++---
 .../src/templates/opac/myopac/hold_history.tt2     | 12 ++++++-----
 Open-ILS/src/templates/opac/myopac/holds.tt2       | 14 +++++++------
 Open-ILS/src/templates/opac/myopac/lists.tt2       |  2 +-
 Open-ILS/src/templates/opac/myopac/main.tt2        |  9 +++++---
 Open-ILS/src/templates/opac/parts/myopac/base.tt2  |  2 +-
 .../src/templates/opac/parts/myopac/main_base.tt2  | 10 ++++-----
 Open-ILS/src/templates/opac/parts/topnav.tt2       | 24 ++++++++++++++--------
 10 files changed, 67 insertions(+), 40 deletions(-)

diff --git a/Open-ILS/src/templates/opac/advanced.tt2 b/Open-ILS/src/templates/opac/advanced.tt2
index 010e9238f2..17cb74f334 100644
--- a/Open-ILS/src/templates/opac/advanced.tt2
+++ b/Open-ILS/src/templates/opac/advanced.tt2
@@ -14,9 +14,15 @@
         </div>
         <div id="adv_search_parent">
             <div id="adv_search_tabs">
-                <a href="?pane=advanced[% IF loc %];loc=[% loc %][% END %]" [% IF pane == 'advanced' %]class="on" [% END %]id="adv_search">[% l('Advanced Search') %]</a>
-                <a href="?pane=numeric[% IF loc %];loc=[% loc %][% END %]" [% IF pane == 'numeric' %]class="on" [% END %]id="num_search">[% l('Numeric Search') %]</a>
-                <a href="?pane=expert[% IF loc %];loc=[% loc %][% END %]" [% IF pane == 'expert' %]class="on" [% END %]id="expert_search">[% l('Expert Search') %]</a>
+                <a href="[% mkurl('', {pane => 'advanced'}) %]"
+                    [% IF pane == 'advanced' %]class="on" [% END %]
+                    id="adv_search">[% l('Advanced Search') %]</a>
+                <a href="[% mkurl('', {pane => 'numeric'}) %]"
+                    [% IF pane == 'numeric' %]class="on" [% END %]
+                    id="num_search">[% l('Numeric Search') %]</a>
+                <a href="[% mkurl('', {pane => 'expert'}) %]"
+                    [% IF pane == 'expert' %]class="on" [% END %]
+                    id="expert_search">[% l('Expert Search') %]</a>
             </div>
         </div>
     </div>
diff --git a/Open-ILS/src/templates/opac/myopac/circ_history.tt2 b/Open-ILS/src/templates/opac/myopac/circ_history.tt2
index f983ad5d42..0728245676 100644
--- a/Open-ILS/src/templates/opac/myopac/circ_history.tt2
+++ b/Open-ILS/src/templates/opac/myopac/circ_history.tt2
@@ -10,7 +10,7 @@
     
     <div id="acct_checked_tabs">
         <div class="align">
-            <a href='circs'>[% l("Current Items Checked Out") %]</a>
+            <a href='[% mkurl('circs') %]'>[% l("Current Items Checked Out") %]</a>
         </div>
         <div class="align selected">
             <a href="#">[% l("Check Out History") %]</a>
@@ -20,10 +20,10 @@
     <div class="header_middle">
         <span class="float-left">[% l('Previously Checked Out Items') %]</span>
         <span class='float-left' style='padding-left: 10px;'>
-            <a href='circ_history?limit=[% limit %]&amp;offset=[% offset - limit %]'
+            <a href='[% mkurl('circ_history', {limit => limit, offset => (offset - limit)}) %]'
                 [% IF offset == 0 %] class='invisible' [% END %]><span class="nav_arrow_fix">&#9668;</span>[% l('Previous') %]</a>
             [%# TODO: get total to prevent paging off then end of the list.. %]
-            <a href='circ_history?limit=[% limit %]&amp;offset=[% offset + limit %]'
+            <a href='[% mkurl('circ_history', {limit => limit, offset => (offset + limit)}) %]'
                [% IF ctx.circs.size < limit %] class='invisible' [% END %] >[% l('Next') %]<span class="nav_arrow_fix">&#9658;</span></a>
         </span>
         <span class="float-right">
@@ -69,10 +69,12 @@
                     PROCESS get_marc_attrs args=attrs; %]
                     <tr>
                         <td style="padding-left:5px;padding-bottom:10px;">
-                            <a href="[% ctx.opac_root %]/record/[% circ.circ.target_copy.call_number.record.id %]" 
+                            <a href="[% mkurl(ctx.opac_root _ '/record/' _ circ.circ.target_copy.call_number.record.id) %]" 
                                 name="[% l('Catalog record') %]">[% attrs.title | html %]</a>
                             [% IF attrs.author %] /
-                            <a href="[% ctx.opac_root %]/results?qtype=author&amp;query=[% attrs.author | replace('[,\.:;]', '') | url %]">[% attrs.author | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/results',
+                                {qtype => 'author', query => attrs.author.replace('[,\.:;]', '')}
+                            )%]">[% attrs.author | html %]</a>
                             [% END %]
                         </td>
                         <td style="padding-left:5px;">
diff --git a/Open-ILS/src/templates/opac/myopac/circs.tt2 b/Open-ILS/src/templates/opac/myopac/circs.tt2
index 6b94fb614e..bf3ab0e8cd 100644
--- a/Open-ILS/src/templates/opac/myopac/circs.tt2
+++ b/Open-ILS/src/templates/opac/myopac/circs.tt2
@@ -9,7 +9,7 @@
             <a href="#">[% l("Current Items Checked Out") %]</a>
         </div>
         <div class="align">
-            <a href="circ_history">[% l("Check Out History") %]</a>
+            <a href="[% mkurl('circ_history') %]">[% l("Check Out History") %]</a>
         </div>
     </div>
 
@@ -105,9 +105,13 @@
                         <td width="40%"
                             style="padding-left:5px;padding-bottom:10px;"
                             name="author">
-                            <a href="[% ctx.opac_root %]/record/[% circ.circ.target_copy.call_number.record.id %]" name="[% l('Catalog record') %]">[% attrs.title | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/record/' _ 
+                                circ.circ.target_copy.call_number.record.id) %]"
+                                name="[% l('Catalog record') %]">[% attrs.title | html %]</a>
                             [% IF attrs.author %] /
-                            <a href="[% ctx.opac_root %]/results?qtype=author&amp;query=[% attrs.author | replace('[,\.:;]', '') | url %]">[% attrs.author | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/results',
+                                {qtype => 'author', query => attrs.author.replace('[,\.:;]', '')}
+                            ) %]">[% attrs.author | html %]</a>
                             [% END %]
                         </td>
                         <td width="8%" name="renewals" align="center">
diff --git a/Open-ILS/src/templates/opac/myopac/hold_history.tt2 b/Open-ILS/src/templates/opac/myopac/hold_history.tt2
index af464f503f..bc3cc6cfc0 100644
--- a/Open-ILS/src/templates/opac/myopac/hold_history.tt2
+++ b/Open-ILS/src/templates/opac/myopac/hold_history.tt2
@@ -11,7 +11,7 @@
 
     <div id="acct_holds_tabs">
         <div class="align">
-            <a href='holds'>[% l("Items on Hold") %]</a>
+            <a href='[% mkurl('holds') %]'>[% l("Items on Hold") %]</a>
         </div>
         <div class="align selected">
             <a href="#">[% l("Holds History") %]</a>
@@ -21,10 +21,10 @@
     <div class="header_middle">
         <span style="float:left;">[% l("Previously Held Items") %]</span>
         <span class='float-left' style='padding-left: 10px;'>
-            <a href='hold_history?limit=[% limit %]&amp;offset=[% offset - limit %]'
+            <a href='[% mkurl(hold_history, {limit => limit,offset => (offset - limit)}) %]'
                 [% IF offset == 0 %] class='invisible' [% END %]><span class="nav_arrow_fix">&#9668;</span>[% l('Previous') %]</a>
             [%# TODO: get total to prevent paging off then end of the list.. %]
-            <a href='hold_history?limit=[% limit %]&amp;offset=[% offset + limit %]'
+            <a href='[% mkurl(hold_history, {limit => limit, offset => (offset + limit)}) %]'
                [% IF ctx.holds.size < limit %] class='invisible' [% END %] >[% l('Next') %]<span class="nav_arrow_fix">&#9658;</span></a>
         </span>
         <span style="float:right;">
@@ -62,12 +62,14 @@
 
                     <td width="138">
                         <div style="margin-top:10px;margin-bottom:10px;">
-                            <a href="[% ctx.opac_root %]/record/[% hold.hold.bre_id %]">[% attrs.title | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/record/' _ hold.hold.bre_id) %]">[% attrs.title | html %]</a>
                         </div>
                     </td>
                     <td width="123">
                         <div style="margin-top:10px;margin-bottom:10px;">
-                            <a href="[% ctx.opac_root %]/results?qtype=author&amp;query=[% attrs.author | replace('[,\.:;]', '') | url %]">[% attrs.author | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/results',
+                                {qtype => author, query => attrs.author.replace('[,\.:;]', '')}
+                            ) %]">[% attrs.author | html %]</a>
                         </div>
                     </td>
                     <td width="64">
diff --git a/Open-ILS/src/templates/opac/myopac/holds.tt2 b/Open-ILS/src/templates/opac/myopac/holds.tt2
index bb0e228f29..5a5be5b92d 100644
--- a/Open-ILS/src/templates/opac/myopac/holds.tt2
+++ b/Open-ILS/src/templates/opac/myopac/holds.tt2
@@ -10,7 +10,7 @@
             <a href='#'>[% l("Items on Hold") %]</a>
         </div>
         <div class="align">
-            <a href='hold_history'>[% l("Holds History") %]</a>
+            <a href='[% mkurl('hold_history') %]'>[% l("Holds History") %]</a>
         </div>
     </div>
 
@@ -62,11 +62,11 @@
                 <td align="right">
                     [% l("Show") %] &nbsp; &nbsp;
                     [% IF CGI.param("available") %]
-                    <a href="holds">[% l('all') %]</a> |
+                    <a href="[% mkurl('holds') %]">[% l('all') %]</a> |
                     <strong>[% l("only available") %]</strong>
                     [% ELSE %]
                     <strong>[% l("all") %]</strong> |
-                    <a href="holds?available=1">[% l("only available") %]</a>
+                    <a href="[% mkurl('holds', {available => 1}) %]">[% l("only available") %]</a>
                     [% END %] &nbsp; &nbsp;
                     [% l("holds") %]
                     <select class="hide_me" id="holds_sort">
@@ -139,12 +139,14 @@
                                     title = l('[_1] ([_2])', title, hold.hold.part.label);
                                 END;
                             %]
-                            <a href="[% ctx.opac_root %]/record/[% hold.hold.bre_id %]">[% title | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/record/' _ hold.hold.bre_id) %]">[% title | html %]</a>
                         </div>
                     </td>
                     <td width="123">
                         <div style="margin-top:10px;margin-bottom:10px;">
-                            <a href="[% ctx.opac_root %]/results?qtype=author&amp;query=[% attrs.author | replace('[,\.:;]', '') | url %]">[% attrs.author | html %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/results',
+                                {qtype => 'author', query => attrs.author.replace('[,\.:;]', '')}
+                            ) %]">[% attrs.author | html %]</a>
                         </div>
                     </td>
                     <td width="64">
@@ -190,7 +192,7 @@
                         </div>
                     </td>
                     <td class="opac-auto-161">
-                        <a href="[% ctx.opac_root %]/myopac/holds/edit?id=[% ahr.id %]">Edit</a>
+                        <a href="[% mkurl(ctx.opac_root _ '/myopac/holds/edit', {id => ahr.id}) %]">Edit</a>
                     </td>
                 </tr>
                 [% END %]
diff --git a/Open-ILS/src/templates/opac/myopac/lists.tt2 b/Open-ILS/src/templates/opac/myopac/lists.tt2
index 45404e84a3..2a9b774df3 100644
--- a/Open-ILS/src/templates/opac/myopac/lists.tt2
+++ b/Open-ILS/src/templates/opac/myopac/lists.tt2
@@ -5,7 +5,7 @@
 <div id='myopac_bookbag_div' style="padding:5px;">
 
     <!-- new list creation -->
-    <form action="[% ctx.opac_root %]/myopac/list/update" method="POST" id="create_form">
+    <form action="[% mkurl(ctx.opac_root _ '/myopac/list/update') %]" method="POST" id="create_form">
         <h1>[% l('Create new list') %]</h1><a name="createnewlist"></a>
         <table cellpadding="0" border="0" id="list_create_table">
             <tr>
diff --git a/Open-ILS/src/templates/opac/myopac/main.tt2 b/Open-ILS/src/templates/opac/myopac/main.tt2
index 2398b3c4a8..793653ba5a 100644
--- a/Open-ILS/src/templates/opac/myopac/main.tt2
+++ b/Open-ILS/src/templates/opac/myopac/main.tt2
@@ -47,13 +47,16 @@
                     <td>
                         [% recid = f.xact.circulation.target_copy.call_number.record.id || f.xact.reservation.target_resource_type.record.id;
                         IF recid; %]
-                        <a href="[% ctx.opac_root %]/record/[% recid %]">[% attrs.title | html %]</a>
+                        <a href="[% mkurl(ctx.opac_root _ '/record/' _ recid,
+                            {loc => ctx.search_ou}) %]">[% attrs.title | html %]</a>
                         [% ELSE %]
                         [% attrs.title | html %]
                         [% END %]
                     </td>
-                    <td>
-                        <a href="[% ctx.opac_root %]/results?qtype=author&amp;query=[% attrs.author | replace('[,\.:;]', '') | url %]">[% attrs.author | html %]</a>
+                    <td>[%- author_query = attrs.author.replace('[,\.:;]', ''); %]
+                        <a href="[% mkurl(ctx.opac_root _ '/results',
+                            {qtype => 'author', query => author_query,
+                            loc => ctx.search_ou}) %]">[% attrs.author | html %]</a>
                     </td>
                     <td name='myopac_circ_trans_start'>
                         [% ts = f.xact.circulation.xact_start || f.xact.reservation.start_time || 0;
diff --git a/Open-ILS/src/templates/opac/parts/myopac/base.tt2 b/Open-ILS/src/templates/opac/parts/myopac/base.tt2
index a65cf2a026..9c11f132e5 100644
--- a/Open-ILS/src/templates/opac/parts/myopac/base.tt2
+++ b/Open-ILS/src/templates/opac/parts/myopac/base.tt2
@@ -23,7 +23,7 @@
                     ELSE;
                         cls_which = "acct-tab-off";
                     END -%]
-                <a href="[% ctx.opac_root _ '/myopac/' _ page.url %]"
+                <a href="[% mkurl(ctx.opac_root _ '/myopac/' _ page.url) %]"
                     class="[% cls_which %]">[% page.name; %]</a>
                 [% END %]
             </div>
diff --git a/Open-ILS/src/templates/opac/parts/myopac/main_base.tt2 b/Open-ILS/src/templates/opac/parts/myopac/main_base.tt2
index c603ada653..9607a9f550 100644
--- a/Open-ILS/src/templates/opac/parts/myopac/main_base.tt2
+++ b/Open-ILS/src/templates/opac/parts/myopac/main_base.tt2
@@ -56,7 +56,7 @@
                     <tr>
                         <td>[% l("Items Currently Checked out ([_1])", ctx.user_stats.checkouts.total_out) %]</td>
                         <td align="right">
-                            <a href="[% ctx.opac_root %]/myopac/circs">[% l("View All") %]</a>
+                            <a href="[% mkurl(ctx.opac_root + '/myopac/circs') %]">[% l("View All") %]</a>
                         </td>
                     </tr>
                 </table>
@@ -66,7 +66,7 @@
                     <tr>
                         <td>[% l('Items Currently on Hold ([_1])', ctx.user_stats.holds.total) %]</td>
                         <td align="right">
-                            <a href="[% ctx.opac_root %]/myopac/holds">[% l('View All') %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/myopac/holds') %]">[% l('View All') %]</a>
                         </td>
                     </tr>
                 </table>
@@ -76,7 +76,7 @@
                     <tr>
                         <td>[% l('Items ready for pickup ([_1])', ctx.user_stats.holds.ready) %]</td>
                         <td align="right">
-                            <a href="[% ctx.opac_root %]/myopac/holds?available=1">[% l('View All') %]</a>
+                            <a href="[% mkurl(ctx.opac_root _ '/myopac/holds', {available => 1}) %]">[% l('View All') %]</a>
                         </td>
                     </tr>
                 </table>
@@ -88,9 +88,9 @@
         <div id='acct_fines_tabs'>
             [% IF myopac_main_page == 'main' %]
             <a href='#' class="acct-tab-on">[% l("Fines") %]</a>
-            <a href='main_payments' class="acct-tab-off">[% l("Payments") %]</a>
+            <a href='[% mkurl(ctx.opac_root _ '/myopac/main_payments') %]' class="acct-tab-off">[% l("Payments") %]</a>
             [% ELSE %]
-            <a href='main' class="acct-tab-off">[% l("Fines") %]</a>
+            <a href='[% mkurl(ctx.opac_root _ '/myopac/main') %]' class="acct-tab-off">[% l("Fines") %]</a>
             <a href='#' class="acct-tab-on">[% l("Payments") %]</a>
             [% END %]
         </div>
diff --git a/Open-ILS/src/templates/opac/parts/topnav.tt2 b/Open-ILS/src/templates/opac/parts/topnav.tt2
index db1f2a4127..8971723829 100644
--- a/Open-ILS/src/templates/opac/parts/topnav.tt2
+++ b/Open-ILS/src/templates/opac/parts/topnav.tt2
@@ -7,7 +7,7 @@
     <div class="float-right">
         [% IF !ctx.user %]
         <div id="your-acct-login">
-            <a href="[% ctx.opac_root %]/myopac/main" 
+            <a href="[% mkurl(ctx.opac_root _ '/myopac/main') %]" 
                 class="opac-button" id="home_myopac_link">
                 [% l('Your Account Log in') %]
             </a>
@@ -19,27 +19,35 @@
                     [%  l('[_1] [_2]', ctx.user.first_given_name, ctx.user.family_name) | html %]
                 </span>
                 <span class="dash_divider">|</span>
-                <a href="[% ctx.opac_root %]/myopac/main" 
+                <a href="[% mkurl(ctx.opac_root _ '/myopac/main') %]" 
                     class="opac-button">[% l('My Account') %]</a>
 
-                <a href="[% ctx.opac_root %]/logout" class="opac-button"
-                    id="logout_link">[% l('Logout') %]</a>
+                <a href="[% mkurl(ctx.opac_root _ '/logout', {}, 1) %]"
+                    class="opac-button" id="logout_link">[% l('Logout') %]</a>
             </div>
             <div id="dashboard">
                 <span class="dash-align">
-                    <a class="dash-link" href="[% ctx.opac_root %]/myopac/circs"><span id="dash_checked">[% ctx.user_stats.checkouts.total_out %]</span> [% l("Checked Out") %]</a>
+                    <a class="dash-link" href="[% mkurl(ctx.opac_root _ '/myopac/circs')
+                        %]"><span id="dash_checked">[% ctx.user_stats.checkouts.total_out
+                        %]</span> [% l("Checked Out") %]</a>
                 </span>
                 <span class="dash_divider">|</span>
                 <span class="dash-align">
-                    <a class="dash-link" href="[% ctx.opac_root %]/myopac/holds"><span id="dash_holds">[% ctx.user_stats.holds.total %]</span> [% l("On Hold") %]</a>
+                    <a class="dash-link" href="[% mkurl(ctx.opac_root _ '/myopac/holds')
+                        %]"><span id="dash_holds">[% ctx.user_stats.holds.total
+                        %]</span> [% l("On Hold") %]</a>
                 </span>
                 <span class="dash_divider">|</span>
                 <span class="dash-align">
-                    <a class="dash-link" href="[% ctx.opac_root %]/myopac/holds?available=1"><span id="dash_pickup">[% ctx.user_stats.holds.ready %]</span> [% l("Ready for Pickup") %]</a>
+                    <a class="dash-link" href="[% mkurl(ctx.opac_root _ '/myopac/holds',
+                        {available => 1}) %]"><span id="dash_pickup">[%
+                        ctx.user_stats.holds.ready %]</span> [% l("Ready for Pickup") %]</a>
                 </span>
                 <span class="dash_divider">|</span>
                 <span class="dash-align">
-                    <a class="dash-link" href="[% ctx.opac_root %]/myopac/main"><span id="dash_fines">[% money(ctx.user_stats.fines.balance_owed) %]</span> [% l("Fines") %]</a>
+                    <a class="dash-link" href="[% mkurl(ctx.opac_root _ '/myopac/main')
+                        %]"><span id="dash_fines">[% money(ctx.user_stats.fines.balance_owed)
+                        %]</span> [% l("Fines") %]</a>
                 </span>
             </div>
         </div>
-- 
2.11.0