TPac; hold success confirmation; redirect repairs

1. After a successful hold placement, provide feedback to the user that
the hold placement succeeded.  After placement, the user is taken to a
page (same page for failed holds) where the title(s) are listed along
with the success/failure information for each.

2. In some cases, the user may be redirected to the my account page
instead of the point of origin for holds placement.  This occurs
generally when the user is not already logged in and is asked to log in
prior to holds placement.

3. As a side effect, this change replaces one of the ornery
history.go(-1) actions with a true href/URL.

Signed-off-by: Bill Erickson <berick@esilibrary.com>
Signed-off-by: Mike Rylander <mrylander@gmail.com>

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
index 21c08ab..e97f499 100644 (file)
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
@@ -530,7 +530,7 @@ sub load_place_hold {
     $ctx->{hold_type} = $cgi->param('hold_type');
     $ctx->{default_pickup_lib} = $e->requestor->home_ou; # unless changed below
 
-    return $self->post_hold_redirect unless @targets;
+    return $self->generic_redirect unless @targets;
 
     $logger->info("Looking at hold targets: @targets");
 
@@ -715,45 +715,19 @@ sub load_place_hold {
         $bses->kill_me;
     }
 
-    # stay on the current page and display the results
-    return Apache2::Const::OK if 
-        (grep {$_->{hold_failed}} @hold_data) or $ctx->{general_hold_error};
+    # NOTE: we are leaving the staff-placed patron barcode cookie 
+    # in place.  Otherwise, it's not possible to place more than 
+    # one hold for the patron within a staff/patron session.  This 
+    # does leave the barcode to linger longer than is ideal, but 
+    # normal staff work flow will cause the cookie to be replaced 
+    # with each new patron anyway.
+    # TODO: See about getting the staff client to clear the cookie
 
-    # if successful, do some cleanup and return the 
-    # user to the requesting page.
-
-    return $self->post_hold_redirect;
-}
-
-sub post_hold_redirect {
-    my $self = shift;
-    
-    # XXX: Leave the barcode cookie in place.  Otherwise, it's not 
-    # possible to place more than one hold for the patron within 
-    # a staff/patron session.  This does leave the barcode to linger 
-    # longer than is ideal, but normal staff work flow will cause the 
-    # cookie to be replaced with each new patron anyway.
-    # TODO:  See about getting the staff client to clear the cookie
-    return $self->generic_redirect;
-
-    # We also clear the patron_barcode (from the staff client)
-    # cookie at this point (otherwise it haunts the staff user
-    # later). XXX todo make sure this is best; also see that
-    # template when staff mode calls xulG.opac_hold_placed()
-
-    return $self->generic_redirect(
-        undef,
-        $self->cgi->cookie(
-            -name => "patron_barcode",
-            -path => "/",
-            -secure => 1,
-            -value => "",
-            -expires => "-1h"
-        )
-    );
+    # return to the place_hold page so the results of the hold
+    # placement attempt can be reported to the user
+    return Apache2::Const::OK;
 }
 
-
 sub fetch_user_circs {
     my $self = shift;
     my $flesh = shift; # flesh bib data, etc.

diff --git a/Open-ILS/src/templates/opac/parts/place_hold.tt2 b/Open-ILS/src/templates/opac/parts/place_hold.tt2
index 6da63da..4580b9c 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/place_hold.tt2
+++ b/Open-ILS/src/templates/opac/parts/place_hold.tt2
@@ -6,15 +6,12 @@
     <h1>[% l('Place Hold') %]</h1>
     <form method="POST">
         <input type="hidden" name="hold_type" value="[% CGI.param('hold_type') | html %]" />
-        [%#
-            new_redirect_to = ctx.referer;
-            IF new_redirect_to.match('redirect_to');
-                new_redirect_to = 'https://' _ ctx.hostname _ ctx.opac_root _ '/home';
-            ELSE;
-                new_redirect_to = new_redirect_to | replace('^http:', 'https:');
-            END;
+        [%  
+            redirect = CGI.param('hold_source_page') || CGI.param('redirect_to') || CGI.referer;
+            # since we have to be logged in to get this far, return to a secure page
+            redirect = redirect.replace('^http:', 'https:') 
         %]
-        <input type="hidden" name="redirect_to" value="[% CGI.param('redirect_to') || CGI.referer | html %]" />
+        <input type="hidden" name="redirect_to" value="[% redirect | html %]" />
 
         [% IF ctx.is_staff %]
         <p class="staff-hold">

diff --git a/Open-ILS/src/templates/opac/parts/place_hold_result.tt2 b/Open-ILS/src/templates/opac/parts/place_hold_result.tt2
index 7ff5541..f92c38f 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/place_hold_result.tt2
+++ b/Open-ILS/src/templates/opac/parts/place_hold_result.tt2
@@ -1,6 +1,7 @@
 [%  PROCESS "opac/parts/misc_util.tt2";
     PROCESS "opac/parts/hold_error_messages.tt2";
     override_possible = 0;
+    any_failures = 0;
 %]
 
 <!-- TODO: CSS for big/strong-->
@@ -37,9 +38,9 @@
                     <div>
                         [% IF hdata.hold_success %]
 
-                        <div>[% l("Hold was successfully placed"); %]</div>
+                        <div class='success'>[% l("Hold was successfully placed"); %]</div>
 
-                        [% ELSIF hdata.hold_failed %]
+                        [% ELSIF hdata.hold_failed; any_failures = 1 %]
 
                             <div><big><strong>[% l("Hold was not successfully placed"); %]</strong></big></div>
                             [% IF hdata.hold_local_block %]
@@ -90,8 +91,11 @@
             </span>
         [% END %]
         <span>
-        <input type="reset" onclick="javascript:history.go(-1);"
-            id="holds_cancel" value="[% l('Cancel') %]" class="opac-button" />
+        [% IF any_failures OR ctx.general_hold_error %]
+        <a href="[% CGI.param('redirect_to') || CGI.referer | html %]">[% l('Cancel') %]</a>
+        [% ELSE %]
+        <a href="[% CGI.param('redirect_to') || CGI.referer | html %]">[% l('Continue') %]</a>
+        [% END %]
         </span>
     </form>
 </div>

diff --git a/Open-ILS/src/templates/opac/parts/record/issues.tt2 b/Open-ILS/src/templates/opac/parts/record/issues.tt2
index 00a3eca..4ed20af 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/record/issues.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/issues.tt2
@@ -14,7 +14,8 @@ FOREACH type IN ctx.holding_summaries.keys;
                 <td class="rdetail-issue-issue">[% blob.issuance.label | html %]</td>
                 [% IF blob.has_units %]
                 <td class="rdetail-issue-place-hold">
-                    <a href="[% ctx.opac_root %]/place_hold?hold_target=[% blob.issuance.id %]&amp;hold_type=I">[% l("Place Hold") %]</a>
+                    <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
+                        {hold_target => blob.issuance.id, hold_type => 'I', hold_source_page => mkurl()}) %]">[% l("Place Hold") %]</a>
                 </td>
                 [% END %]
             </tr>

diff --git a/Open-ILS/src/templates/opac/parts/record/summary.tt2 b/Open-ILS/src/templates/opac/parts/record/summary.tt2
index 490a860..bbfb1c1 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/record/summary.tt2
+++ b/Open-ILS/src/templates/opac/parts/record/summary.tt2
@@ -23,7 +23,8 @@
  
 <div id="rdetail_actions_div" style="float: right; margin-left: 1em;">
     <div class="rdetail_aux_utils opac-auto-010">
-        <a href="[% mkurl(ctx.opac_root _ '/place_hold', {hold_target => ctx.bre_id, hold_type => 'T'}) %]" 
+        <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
+            {hold_target => ctx.bre_id, hold_type => 'T', hold_source_page => mkurl()}) %]" 
         class="no-dec"><img src="[% ctx.media_prefix %]/images/green_check.png" alt="[% l('place hold') %]" /><span 
         style="position:relative;top:-3px;left:3px;">[% l('Place Hold') %]</span></a>
     </div>
@@ -137,11 +138,13 @@
                         copy_info.status_holdable == 't');
                     IF overall_holdable;
                         l("Place on"); %]
-                <a href="[% mkurl(ctx.opac_root _ '/place_hold', {hold_target => copy_info.id, hold_type => 'C'}) %]">[% l("copy") %]</a>
+                <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
+                    {hold_target => copy_info.id, hold_type => 'C', hold_source_page => mkurl()}) %]">[% l("copy") %]</a>
                 [%-      IF copy_info.call_number != last_cn;
                             last_cn = copy_info.call_number;
                             l(" / "); %]
-                <a href="[% mkurl(ctx.opac_root _ '/place_hold', {hold_target => copy_info.call_number, hold_type => 'V'}) %]">[% l("volume") %]</a>
+                <a href="[% mkurl(ctx.opac_root _ '/place_hold', 
+                    {hold_target => copy_info.call_number, hold_type => 'V', hold_source_page => mkurl()}) %]">[% l("volume") %]</a>
                 [%-      END;
                     ELSE;
                         l("No");

diff --git a/Open-ILS/src/templates/opac/parts/result/table.tt2 b/Open-ILS/src/templates/opac/parts/result/table.tt2
index 83e69b9..422a378 100644 (file)
--- a/Open-ILS/src/templates/opac/parts/result/table.tt2
+++ b/Open-ILS/src/templates/opac/parts/result/table.tt2
@@ -208,7 +208,8 @@
                                                 <div style="width:250px;text-align:left;">
                                                     <div style="float:right;">
                                                         <div class="results_aux_utils opac-auto-010"><a
-                                                                href="[% mkurl(ctx.opac_root _ '/place_hold', {hold_target => rec.id, hold_type => 'T'}) %]" 
+                                                                href="[% mkurl(ctx.opac_root _ '/place_hold', 
+                                                                    {hold_target => rec.id, hold_type => 'T', hold_source_page => mkurl()}) %]" 
                                                                     name="place_hold_link" class="no-dec"><img
                                                                 src="[% ctx.media_prefix %]/images/green_check.png"
                                                                 alt=""/><span style="position:relative;top:-3px;left:3px;">[% l('Place Hold') %]</span></a>