From b52dac83fd59f45e94dcf05c5b9c633c4ad05135 Mon Sep 17 00:00:00 2001
From: Thomas Berezansky <tsbere@mvlc.org>
Date: Fri, 30 Mar 2012 15:58:32 -0400
Subject: [PATCH] Stop saving patron barcode in cookies

This causes bad interactions between patron windows and normal catalog
windows. Examples:

Load a patron, load place hold catalog in patron window. Future loads of
non-patron catalogs will default to this patron.

Load Patron A from written note, and open Place Hold catalog. Start
searching for the item Patron A wants.
Get interupted by Patron B calling. Load Patron B, and open Place Hold
catalog. Search, place a hold for Patron B.
Return to Patron A, find item, go to place hold. Find Patron B's barcode
populated.

Instead, teach the staff javascript, that already knows how to load hold
preferences, how to grab the barcode from xulG. This barcode should be
specific to the patron window the catalog is embedded in, and won't 'leak'
to other catalog interfaces.

Signed-off-by: Thomas Berezansky <tsbere@mvlc.org>
Signed-off-by: Lebbeous Fogle-Weekley <lebbeous@esilibrary.com>
---
 .../perlmods/lib/OpenILS/WWW/EGCatLoader.pm   | 12 --------
 .../lib/OpenILS/WWW/EGCatLoader/Account.pm    | 30 ++++++-------------
 .../src/templates/opac/parts/place_hold.tt2   |  4 +--
 Open-ILS/web/js/ui/default/opac/staff.js      | 16 ++++++----
 .../xul/staff_client/server/patron/holds.js   |  5 +---
 5 files changed, 23 insertions(+), 44 deletions(-)

diff --git a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm
index 2e83f0f013..f41faf838f 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader.pm
@@ -217,18 +217,6 @@ sub load_simple {
     $self->ctx->{page} = $page;
     $self->ctx->{search_ou} = $self->_get_search_lib();
 
-    if (my $patron_barcode = $self->cgi->param("patron_barcode")) {
-        # Special CGI variable from staff client; propagate henceforth as cookie
-        $self->apache->headers_out->add(
-            "Set-Cookie" => $self->cgi->cookie(
-                -name => "patron_barcode",
-                -path => "/",
-                -secure => 1,
-                -value => $patron_barcode,
-                -expires => undef
-            )
-        );
-    }
     return Apache2::Const::OK;
 }
 
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 bf01bc4433..76a88d8594 100644
--- a/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
+++ b/Open-ILS/src/perlmods/lib/OpenILS/WWW/EGCatLoader/Account.pm
@@ -608,31 +608,19 @@ sub load_place_hold {
 
     $logger->info("Looking at hold_type: " . $ctx->{hold_type} . " and targets: @targets");
 
-    # if the staff client provides a patron barcode, fetch the patron
-    if (my $bc = $self->cgi->cookie("patron_barcode")) {
-        $ctx->{patron_recipient} = $U->simplereq(
-            "open-ils.actor", "open-ils.actor.user.fleshed.retrieve_by_barcode",
-            $self->editor->authtoken, $bc
-        ) or return Apache2::Const::HTTP_BAD_REQUEST;
-
-        $ctx->{default_pickup_lib} = $ctx->{patron_recipient}->home_ou;
-    } else {
-        $ctx->{staff_recipient} = $self->editor->retrieve_actor_user([
-            $e->requestor->id,
-            {
-                flesh => 1,
-                flesh_fields => {
-                    au => ['settings', 'card']
-                }
+    $ctx->{staff_recipient} = $self->editor->retrieve_actor_user([
+        $e->requestor->id,
+        {
+            flesh => 1,
+            flesh_fields => {
+                au => ['settings', 'card']
             }
-        ]) or return Apache2::Const::HTTP_INTERNAL_SERVER_ERROR;
-    }
+        }
+    ]) or return Apache2::Const::HTTP_INTERNAL_SERVER_ERROR;
     my $user_setting_map = {
         map { $_->name => OpenSRF::Utils::JSON->JSON2perl($_->value) }
             @{
-                $ctx->{patron_recipient}
-                ? $ctx->{patron_recipient}->settings
-                : $ctx->{staff_recipient}->settings
+                $ctx->{staff_recipient}->settings
             }
     };
     $ctx->{user_setting_map} = $user_setting_map;
diff --git a/Open-ILS/src/templates/opac/parts/place_hold.tt2 b/Open-ILS/src/templates/opac/parts/place_hold.tt2
index 71c9bdea42..ec979e311b 100644
--- a/Open-ILS/src/templates/opac/parts/place_hold.tt2
+++ b/Open-ILS/src/templates/opac/parts/place_hold.tt2
@@ -18,11 +18,11 @@
             <input type="radio" id="hold_usr_is_requestor_not"
                 onchange="staff_hold_usr_input_disabler(this);"
                 name="hold_usr_is_requestor" value="0"
-                [% IF ctx.patron_recipient; ' checked="checked"'; END %] />
+                />
             <label for="hold_usr_is_requestor_not">
                 [% l("Place hold for patron by barcode:") %]
             </label>
-            <input type="text" name="hold_usr" id="hold_usr_input" onchange="staff_hold_usr_barcode_changed();" value="[% ctx.patron_recipient.card.barcode | html %]" /><br />[%# XXX multi-barcode users? %]
+            <input type="text" name="hold_usr" id="hold_usr_input" onchange="staff_hold_usr_barcode_changed();" /><br />
             <input type="hidden" id="staff_barcode" value="[% ctx.staff_recipient.card.barcode | html %]"/>
             <span>
                 <input type="radio" id="hold_usr_is_requestor"
diff --git a/Open-ILS/web/js/ui/default/opac/staff.js b/Open-ILS/web/js/ui/default/opac/staff.js
index 09d4e06ef8..4d805a5f1b 100644
--- a/Open-ILS/web/js/ui/default/opac/staff.js
+++ b/Open-ILS/web/js/ui/default/opac/staff.js
@@ -22,12 +22,14 @@ function staff_hold_usr_input_disabler(input) {
 var cur_hold_barcode = undefined;
 function staff_hold_usr_barcode_changed(isload) {
     if(typeof xulG != 'undefined' && xulG.get_barcode_and_settings) {
-        var barcode = document.getElementById('staff_barcode').value;
+        var barcode = isload;
+        if(!barcode || barcode === true) barcode = document.getElementById('staff_barcode').value;
         var only_settings = true;
         if(!document.getElementById('hold_usr_is_requestor').checked) {
-            barcode = document.getElementById('hold_usr_input').value;
-            if(!isload)
+            if(!isload) {
+                barcode = document.getElementById('hold_usr_input').value;
                 only_settings = false;
+            }
             if(barcode && barcode != '' && !document.getElementById('hold_usr_is_requestor_not').checked)
                 document.getElementById('hold_usr_is_requestor_not').checked = 'checked';
         }
@@ -37,7 +39,7 @@ function staff_hold_usr_barcode_changed(isload) {
         if(load_info == false || load_info == undefined)
             return;
         cur_hold_barcode = load_info.barcode;
-        if(!only_settings) document.getElementById('hold_usr_input').value = load_info.barcode; // Safe at this point as we already set cur_hold_barcode
+        if(!only_settings || (isload && isload !== true)) document.getElementById('hold_usr_input').value = load_info.barcode; // Safe at this point as we already set cur_hold_barcode
         if(load_info.settings['opac.default_pickup_location'])
             document.getElementById('pickup_lib').value = load_info.settings['opac.default_pickup_location'];
         if(!load_info.settings['opac.default_phone']) load_info.settings['opac.default_phone'] = '';
@@ -70,7 +72,11 @@ window.onload = function() {
         runEvt('rdetail', 'MFHDDrawn');
     }
     if(location.href.match(/place_hold/)) {
-        staff_hold_usr_barcode_changed(true);
+        if(xulG.patron_barcode) {
+            staff_hold_usr_barcode_changed(xulG.patron_barcode);
+        } else {
+            staff_hold_usr_barcode_changed(true);
+        }
     }
 }
 
diff --git a/Open-ILS/xul/staff_client/server/patron/holds.js b/Open-ILS/xul/staff_client/server/patron/holds.js
index cb45955da9..478056c121 100644
--- a/Open-ILS/xul/staff_client/server/patron/holds.js
+++ b/Open-ILS/xul/staff_client/server/patron/holds.js
@@ -1426,9 +1426,6 @@ patron.holds.prototype = {
                         ['command'],
                         function(ev) {
                             try {
-                                var use_url = urls.browser;
-                                use_url += use_url.match(/\?/) ? "&" : "?";
-                                use_url += "patron_barcode=" + obj.patron_barcode;
                                 var content_params = {
                                     'show_nav_buttons' : false,
                                     'show_print_button' : true,
@@ -1466,7 +1463,7 @@ patron.holds.prototype = {
                                         'patron_barcode' : obj.patron_barcode
                                     },
                                     'url_prefix' : xulG.url_prefix,
-                                    'url' : xulG.url_prefix(use_url)
+                                    'url' : xulG.url_prefix(urls.browser)
                                 };
                                 xulG.display_window.g.patron.right_deck.set_iframe( urls.XUL_REMOTE_BROWSER + '?patron_hold=1', {}, content_params);
                             } catch(E) {
-- 
2.43.2