From eaf59dc28b588829e325c0c3c0accbceda92264d Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Wed, 8 Oct 2014 15:31:05 -0400
Subject: [PATCH] Moved more logic into followup helper. - Better
 encapsulation, lighter controllers.

---
 .../src/VuFind/Controller/AbstractBase.php    | 11 +----
 .../src/VuFind/Controller/AbstractRecord.php  |  7 +--
 .../src/VuFind/Controller/Plugin/Followup.php | 47 +++++++++++++++++--
 .../VuFind/Controller/SearchController.php    | 10 ++--
 4 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/module/VuFind/src/VuFind/Controller/AbstractBase.php b/module/VuFind/src/VuFind/Controller/AbstractBase.php
index a38391178dc..3a5ea10e422 100644
--- a/module/VuFind/src/VuFind/Controller/AbstractBase.php
+++ b/module/VuFind/src/VuFind/Controller/AbstractBase.php
@@ -632,12 +632,8 @@ class AbstractBase extends AbstractActionController
      */
     protected function getFollowupUrl()
     {
-        $followup = $this->followup()->retrieve();
         // followups aren't used in lightboxes.
-        if (isset($followup->url) && !$this->inLightbox()) {
-            return $followup->url;
-        }
-        return '';
+        return ($this->inLightbox()) ? '' : $this->followup()->retrieve('url', '');
     }
 
     /**
@@ -647,9 +643,6 @@ class AbstractBase extends AbstractActionController
      */
     protected function clearFollowupUrl()
     {
-        $followup = $this->followup()->retrieve();
-        if (isset($followup->url)) {
-            unset($followup->url);
-        }
+        $this->followup()->clear('url');
     }
 }
diff --git a/module/VuFind/src/VuFind/Controller/AbstractRecord.php b/module/VuFind/src/VuFind/Controller/AbstractRecord.php
index 4f57e2e8c4f..2b7c4de11a8 100644
--- a/module/VuFind/src/VuFind/Controller/AbstractRecord.php
+++ b/module/VuFind/src/VuFind/Controller/AbstractRecord.php
@@ -119,12 +119,7 @@ class AbstractRecord extends AbstractBase
         // Save comment:
         $comment = $this->params()->fromPost('comment');
         if (empty($comment)) {
-            // No comment?  Try to restore from session:
-            $session = $this->followup()->retrieve();
-            if (isset($session->comment)) {
-                $comment = $session->comment;
-                unset($session->comment);
-            }
+            $comment = $this->followup()->retrieveAndClear('comment');
         }
 
         // At this point, we should have a comment to save; if we do not,
diff --git a/module/VuFind/src/VuFind/Controller/Plugin/Followup.php b/module/VuFind/src/VuFind/Controller/Plugin/Followup.php
index d3fbc652bf2..16e0c2c5099 100644
--- a/module/VuFind/src/VuFind/Controller/Plugin/Followup.php
+++ b/module/VuFind/src/VuFind/Controller/Plugin/Followup.php
@@ -55,14 +55,55 @@ class Followup extends AbstractPlugin
         $this->session = new Container('Followup');
     }
 
+    /**
+     * Clear an element of the stored followup information.
+     *
+     * @param string $key     Element to clear.
+     *
+     * @return bool           True if cleared, false if never set.
+     */
+    public function clear($key)
+    {
+        if (isset($this->session->$key)) {
+            unset($this->session->$key);
+            return true;
+        }
+        return false;
+    }
+
     /**
      * Retrieve the stored followup information.
      *
-     * @return \Zend\Session\Container
+     * @param string $key     Element to retrieve and clear (null for entire
+     * \Zend\Session\Container object)
+     * @param mixed  $default Default value to return if no stored value found
+     * (ignored when $key is null)
+     *
+     * @return mixed
+     */
+    public function retrieve($key = null, $default = null)
+    {
+        if (null === $key) {
+            return $this->session;
+        }
+        return isset($this->session->$key)
+            ? $this->session->$key : $default;
+
+    }
+
+    /**
+     * Retrieve and then clear a particular followup element.
+     *
+     * @param string $key     Element to retrieve and clear.
+     * @param mixed  $default Default value to return if no stored value found
+     *
+     * @return mixed
      */
-    public function retrieve()
+    public function retrieveAndClear($key, $default = null)
     {
-        return $this->session;
+        $value = $this->retrieve($key, $default);
+        $this->clear($key);
+        return $value;
     }
 
     /**
diff --git a/module/VuFind/src/VuFind/Controller/SearchController.php b/module/VuFind/src/VuFind/Controller/SearchController.php
index 820d27062ca..c8dbb6734c4 100644
--- a/module/VuFind/src/VuFind/Controller/SearchController.php
+++ b/module/VuFind/src/VuFind/Controller/SearchController.php
@@ -95,11 +95,11 @@ class SearchController extends AbstractSearch
             return $this->forceLogin(null, array('emailurl' => $view->url));
         }
 
-        // Check if we have a URL in login followup data:
-        $followup = $this->followup()->retrieve();
-        if (isset($followup->emailurl)) {
-            $view->url = $followup->emailurl;
-            unset($followup->emailurl);
+        // Check if we have a URL in login followup data -- this should override
+        // any existing referer to avoid emailing a login-related URL!
+        $followupUrl = $this->followup()->retrieveAndClear('emailurl');
+        if (!empty($followupUrl)) {
+            $view->url = $followupUrl;
         }
 
         // Fail if we can't figure out a URL to share:
-- 
GitLab