From dff52c907f46cf9884990734f5055177e46a601a Mon Sep 17 00:00:00 2001
From: Demian Katz <demian.katz@villanova.edu>
Date: Mon, 14 Mar 2016 15:58:11 -0400
Subject: [PATCH] Improved cart/bulk workflows. - Allows better follow-up for
 bulk actions. - Reduces odds of strange behavior caused by unexpected user
 actions.

---
 module/VuFind/config/module.config.php        | 10 +--
 .../src/VuFind/Controller/CartController.php  | 88 +++++++++++++------
 .../RecordDriver/SolrDefault/toolbar.phtml    |  2 +-
 .../templates/RecordTab/collectionlist.phtml  |  2 +-
 themes/bootstrap3/templates/cart/cart.phtml   |  2 +-
 .../templates/combined/results.phtml          |  2 +-
 .../bootstrap3/templates/search/results.phtml |  2 +-
 7 files changed, 72 insertions(+), 36 deletions(-)

diff --git a/module/VuFind/config/module.config.php b/module/VuFind/config/module.config.php
index 78ba0300af5..0c4d4ae08e3 100644
--- a/module/VuFind/config/module.config.php
+++ b/module/VuFind/config/module.config.php
@@ -741,11 +741,11 @@ $staticRoutes = [
     'Alphabrowse/Home', 'Author/Home', 'Author/Search',
     'Authority/Home', 'Authority/Record', 'Authority/Search',
     'Browse/Author', 'Browse/Dewey', 'Browse/Era', 'Browse/Genre', 'Browse/Home',
-    'Browse/LCC', 'Browse/Region', 'Browse/Tag', 'Browse/Topic',
-    'Cart/doExport', 'Cart/Email', 'Cart/Export', 'Cart/Home', 'Cart/MyResearchBulk',
-    'Cart/Save', 'Collections/ByTitle', 'Collections/Home',
-    'Combined/Home', 'Combined/Results', 'Combined/SearchBox', 'Confirm/Confirm',
-    'Cover/Show', 'Cover/Unavailable',
+    'Browse/LCC', 'Browse/Region', 'Browse/Tag', 'Browse/Topic', 'Cart/doExport',
+    'Cart/Email', 'Cart/Export', 'Cart/Home', 'Cart/MyResearchBulk',
+    'Cart/Processor', 'Cart/Save', 'Cart/SearchResultsBulk', 'Collections/ByTitle',
+    'Collections/Home', 'Combined/Home', 'Combined/Results', 'Combined/SearchBox',
+    'Confirm/Confirm', 'Cover/Show', 'Cover/Unavailable',
     'EDS/Advanced', 'EDS/Home', 'EDS/Search',
     'EIT/Advanced', 'EIT/Home', 'EIT/Search',
     'Error/Unavailable', 'Feedback/Email', 'Feedback/Home', 'Help/Home',
diff --git a/module/VuFind/src/VuFind/Controller/CartController.php b/module/VuFind/src/VuFind/Controller/CartController.php
index c57a7bb3181..dfadc07a704 100644
--- a/module/VuFind/src/VuFind/Controller/CartController.php
+++ b/module/VuFind/src/VuFind/Controller/CartController.php
@@ -67,39 +67,63 @@ class CartController extends AbstractBase
         return $this->getServiceLocator()->get('VuFind\Cart');
     }
 
+    /**
+     * Figure out an action from the request....
+     *
+     * @param string $default Default action if none can be determined.
+     *
+     * @return string
+     */
+    protected function getCartActionFromRequest($default = 'Home')
+    {
+        if (strlen($this->params()->fromPost('email', '')) > 0) {
+            return 'Email';
+        } else if (strlen($this->params()->fromPost('print', '')) > 0) {
+            return 'PrintCart';
+        } else if (strlen($this->params()->fromPost('saveCart', '')) > 0) {
+            return 'Save';
+        } else if (strlen($this->params()->fromPost('export', '')) > 0) {
+            return 'Export';
+        }
+        // Check if the user is in the midst of a login process; if not,
+        // use the provided default.
+        return $this->followup()->retrieveAndClear('cartAction', $default);
+    }
+
+    /**
+     * Process requests for bulk actions from search results.
+     *
+     * @return mixed
+     */
+    public function searchresultsbulkAction()
+    {
+        // We came in from a search, so let's remember that context so we can
+        // return to it later. However, if we came in from a previous instance
+        // of this action (for example, because of a login screen), we should
+        // ignore that!
+        $referer = $this->getRequest()->getServer()->get('HTTP_REFERER');
+        $bulk = $this->url()->fromRoute('cart-searchresultsbulk');
+        if (substr($referer, -strlen($bulk)) != $bulk) {
+            $this->session->url = $referer;
+        }
+
+        // Now forward to the requested action:
+        return $this->forwardTo('Cart', $this->getCartActionFromRequest());
+    }
+
     /**
      * Process requests for main cart.
      *
      * @return mixed
      */
-    public function homeAction()
+    public function processorAction()
     {
-        // We came in from the cart -- let's remember this we can redirect there
+        // We came in from the cart -- let's remember this so we can redirect there
         // when we're done:
         $this->session->url = $this->url()->fromRoute('cart-home');
 
-        // If the cart is disabled, going to cart home is not going to help us;
-        // use the referer instead.
-        if (!$this->getCart()->isActive()) {
-            $this->session->url
-                = $this->getRequest()->getServer()->get('HTTP_REFERER');
-        }
-
         // Now forward to the requested action:
-        if (strlen($this->params()->fromPost('email', '')) > 0) {
-            $action = 'Email';
-        } else if (strlen($this->params()->fromPost('print', '')) > 0) {
-            $action = 'PrintCart';
-        } else if (strlen($this->params()->fromPost('saveCart', '')) > 0) {
-            $action = 'Save';
-        } else if (strlen($this->params()->fromPost('export', '')) > 0) {
-            $action = 'Export';
-        } else {
-            // Check if the user is in the midst of a login process; if not,
-            // default to cart home.
-            $action = $this->followup()->retrieveAndClear('cartAction', 'Cart');
-        }
-        return $this->forwardTo('Cart', $action);
+        return $this->forwardTo('Cart', $this->getCartActionFromRequest());
     }
 
     /**
@@ -107,13 +131,19 @@ class CartController extends AbstractBase
      *
      * @return mixed
      */
-    public function cartAction()
+    public function homeAction()
     {
         // Bail out if cart is disabled.
         if (!$this->getCart()->isActive()) {
             return $this->redirect()->toRoute('home');
         }
 
+        // If a user is coming directly to the cart, we should clear out any
+        // existing context information to prevent weird, unexpected workflows
+        // caused by unusual user behavior.
+        $this->followup()->retrieveAndClear('cartAction');
+        $this->followup()->retrieveAndClear('cartIds');
+
         $ids = is_null($this->params()->fromPost('selectAll'))
             ? $this->params()->fromPost('ids')
             : $this->params()->fromPost('idsAll');
@@ -140,7 +170,13 @@ class CartController extends AbstractBase
                 }
             }
         }
-        return $this->createViewModel();
+        // Using the cart/cart template for the cart/home action is a legacy of
+        // an earlier controller design; we may want to rename the template for
+        // clarity, but right now we are retaining the old template name for
+        // backward compatibility.
+        $view = $this->createViewModel();
+        $view->setTemplate('cart/cart');
+        return $view;
     }
 
     /**
@@ -168,7 +204,7 @@ class CartController extends AbstractBase
             $controller = 'MyResearch';
             $action = 'Delete';
         } else if (strlen($this->params()->fromPost('add', '')) > 0) {
-            $action = 'Cart';
+            $action = 'Home';
         } else if (strlen($this->params()->fromPost('export', '')) > 0) {
             $action = 'Export';
         } else {
diff --git a/themes/bootstrap3/templates/RecordDriver/SolrDefault/toolbar.phtml b/themes/bootstrap3/templates/RecordDriver/SolrDefault/toolbar.phtml
index 76d560fe4e4..16703dc569c 100644
--- a/themes/bootstrap3/templates/RecordDriver/SolrDefault/toolbar.phtml
+++ b/themes/bootstrap3/templates/RecordDriver/SolrDefault/toolbar.phtml
@@ -41,7 +41,7 @@
       <a class="cart-add hidden<? if(!$cart->contains($cartId)): ?> correct<? endif ?>" href="#"><i class="fa fa-plus"></i> <?=$this->transEsc('Add to Book Bag') ?></a>
       <a class="cart-remove hidden<? if($cart->contains($cartId)): ?> correct<? endif ?>"href="#"><i class="fa fa-minus-circle"></i> <?=$this->transEsc('Remove from Book Bag') ?></a>
       <noscript>
-        <form method="post" name="addForm" action="<?=$this->url('cart-home')?>">
+        <form method="post" name="addForm" action="<?=$this->url('cart-processor')?>">
           <input type="hidden" name="ids[]" value="<?=$this->escapeHtmlAttr($cartId)?>" />
           <? if ($cart->contains($cartId)): ?>
             <input class="btn btn-default" type="submit" name="delete" value="<?=$this->transEsc('Remove from Book Bag')?>"/>
diff --git a/themes/bootstrap3/templates/RecordTab/collectionlist.phtml b/themes/bootstrap3/templates/RecordTab/collectionlist.phtml
index 4478f08d32d..75194259062 100644
--- a/themes/bootstrap3/templates/RecordTab/collectionlist.phtml
+++ b/themes/bootstrap3/templates/RecordTab/collectionlist.phtml
@@ -24,7 +24,7 @@
       <?=$this->render('search/controls/sort.phtml', $searchDetails)?>
     </div>
   </div>
-  <form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-home')?>">
+  <form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-searchresultsbulk')?>">
     <?=$this->context($this)->renderInContext('search/bulk-action-buttons.phtml', $searchDetails + array('idPrefix' => ''))?>
     <?=$this->render('search/list-' . $results->getParams()->getView() . '.phtml', $searchDetails)?>
     <?=$this->paginationControl($results->getPaginator(), 'Sliding', 'search/pagination.phtml', array('results' => $results))?>
diff --git a/themes/bootstrap3/templates/cart/cart.phtml b/themes/bootstrap3/templates/cart/cart.phtml
index 7078e5225a6..e709d701747 100644
--- a/themes/bootstrap3/templates/cart/cart.phtml
+++ b/themes/bootstrap3/templates/cart/cart.phtml
@@ -7,7 +7,7 @@
 ?>
 <h2><?=$this->transEsc('Book Bag') ?></h2>
 <?=$this->flashmessages()?>
-<form class="form-inline" action="<?=$this->url('cart-home')?>" method="post"  name="cartForm" data-lightbox-onsubmit="cartFormHandler">
+<form class="form-inline" action="<?=$this->url('cart-processor')?>" method="post"  name="cartForm" data-lightbox-onsubmit="cartFormHandler">
   <input type="hidden" id="dropdown_value"/>
   <? if (!$this->cart()->isEmpty()): ?>
     <div class="cart-controls clearfix">
diff --git a/themes/bootstrap3/templates/combined/results.phtml b/themes/bootstrap3/templates/combined/results.phtml
index 1ce94d7bdf5..93885ef57ab 100644
--- a/themes/bootstrap3/templates/combined/results.phtml
+++ b/themes/bootstrap3/templates/combined/results.phtml
@@ -47,7 +47,7 @@
   $this->headLink()->appendStylesheet('combined-search.css');
 ?>
 <?=$this->flashmessages()?>
-<form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-home')?>">
+<form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-searchresultsbulk')?>">
   <? $recs = $combinedResults->getRecommendations('top'); if (!empty($recs)): ?>
     <div>
       <? foreach ($recs as $current): ?>
diff --git a/themes/bootstrap3/templates/search/results.phtml b/themes/bootstrap3/templates/search/results.phtml
index c8a99f8162c..c2c4520e52e 100644
--- a/themes/bootstrap3/templates/search/results.phtml
+++ b/themes/bootstrap3/templates/search/results.phtml
@@ -99,7 +99,7 @@
         <? endif; ?>
       <? endforeach; ?>
     <? else: ?>
-      <form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-home')?>" data-lightbox data-lightbox-onsubmit="bulkFormHandler">
+      <form class="form-inline" method="post" name="bulkActionForm" action="<?=$this->url('cart-searchresultsbulk')?>" data-lightbox data-lightbox-onsubmit="bulkFormHandler">
         <?=$this->context($this)->renderInContext('search/bulk-action-buttons.phtml', array('idPrefix' => ''))?>
         <?=$this->render('search/list-' . $this->params->getView() . '.phtml')?>
         <?=$this->context($this)->renderInContext('search/bulk-action-buttons.phtml', array('idPrefix' => 'bottom_'))?>
-- 
GitLab