Make PhabricatorActionListView logged-out user savvy

Summary:
Fixes T2691. Now, all PhabricatorActionListViews in the codebase setObjectHref to $request->getRequestURI. This value is passed over to PhabricatorActionItems right before they are rendered. If a PhabricatorActionItem is a workflow and there is no user OR the user is logged out, we used this objectURI to construct a log in URI.

Potentially added some undesirable behavior to aggressively setUser (and later setObjectURI) from within the List on Actions... This should be okay-ish unless there was a vision of actions having different user objects associated with them. I think this is a safe assumption.

Test Plan: played around with a mock all logged out (Ref T2652) and it worked!

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Korvin

Maniphest Tasks: T2691

Differential Revision: https://secure.phabricator.com/D6416

src/applications/daemon/controller/PhabricatorWorkerTaskDetailController.php

@@ -73,13 +73,14 @@ final class PhabricatorWorkerTaskDetailController
   }
 
   private function buildActionListView(PhabricatorWorkerTask $task) {
-    $user = $this->getRequest()->getUser();
+    $request = $this->getRequest();
-
+    $user = $request->getUser();
-    $view = new PhabricatorActionListView();
-    $view->setUser($user);
-
     $id = $task->getID();
 
+    $view = id(new PhabricatorActionListView())
+      ->setUser($user)
+      ->setObjectURI($request->getRequestURI());
+
     if ($task->isArchived()) {
       $result_success = PhabricatorWorkerArchiveTask::RESULT_SUCCESS;
       $can_retry = ($task->getResult() != $result_success);

src/applications/differential/events/DifferentialPeopleMenuEventListener.php

@@ -25,7 +25,6 @@ final class DifferentialPeopleMenuEventListener extends PhutilEventListener {
     $actions = $event->getValue('actions');
 
     $actions[] = id(new PhabricatorActionView())
-      ->setUser($event->getUser())
       ->setRenderAsForm(true)
       ->setIcon('differential-dark')
       ->setIconSheet(PHUIIconView::SPRITE_APPS)

src/applications/differential/view/DifferentialRevisionDetailView.php

@@ -45,7 +45,8 @@ final class DifferentialRevisionDetailView extends AphrontView {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
-      ->setObject($revision);
+      ->setObject($revision)
+      ->setObjectURI($this->getRequest()->getRequestURI());
     foreach ($this->getActions() as $action) {
       $obj = id(new PhabricatorActionView())
         ->setIcon(idx($action, 'icon', 'edit'))

src/applications/diffusion/controller/DiffusionBrowseFileController.php

@@ -378,6 +378,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
     return id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->addAction($blame_button)
       ->addAction($highlight_button)
       ->addAction($lint_button)
@@ -821,6 +822,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($this->getRequest()->getUser())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->addAction($this->createEditAction());
 
     return array($actions, $properties);
@@ -837,6 +839,7 @@ final class DiffusionBrowseFileController extends DiffusionController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($this->getRequest()->getUser())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->addAction($this->createEditAction())
       ->addAction(
         id(new PhabricatorActionView())

src/applications/diffusion/controller/DiffusionCommitController.php

@@ -860,7 +860,8 @@ final class DiffusionCommitController extends DiffusionController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
-      ->setObject($commit);
+      ->setObject($commit)
+      ->setObjectURI($request->getRequestURI());
 
     // TODO -- integrate permissions into whether or not this action is shown
     $uri = '/diffusion/'.$repository->getCallSign().'/commit/'.

src/applications/diffusion/controller/DiffusionRepositoryEditController.php

@@ -70,6 +70,7 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     $user = $this->getRequest()->getUser();
 
     $view = id(new PhabricatorActionListView())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($user);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(
@@ -122,6 +123,7 @@ final class DiffusionRepositoryEditController extends DiffusionController {
     $user = $this->getRequest()->getUser();
 
     $view = id(new PhabricatorActionListView())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($user);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/drydock/controller/DrydockLeaseViewController.php

@@ -63,6 +63,7 @@ final class DrydockLeaseViewController extends DrydockController {
   private function buildActionListView(DrydockLease $lease) {
     $view = id(new PhabricatorActionListView())
       ->setUser($this->getRequest()->getUser())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($lease);
 
     $id = $lease->getID();

src/applications/drydock/controller/DrydockResourceViewController.php

@@ -76,6 +76,7 @@ final class DrydockResourceViewController extends DrydockController {
   private function buildActionListView(DrydockResource $resource) {
     $view = id(new PhabricatorActionListView())
       ->setUser($this->getRequest()->getUser())
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($resource);
 
     $can_close = ($resource->getStatus() == DrydockResourceStatus::STATUS_OPEN);

src/applications/files/controller/PhabricatorFileInfoController.php

@@ -65,6 +65,7 @@ final class PhabricatorFileInfoController extends PhabricatorFileController {
 
     $view = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($file);
 
     if ($file->isViewableInBrowser()) {

src/applications/legalpad/controller/LegalpadDocumentViewController.php

@@ -123,6 +123,7 @@ final class LegalpadDocumentViewController extends LegalpadController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($document);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/macro/controller/PhabricatorMacroViewController.php

@@ -106,10 +106,12 @@ final class PhabricatorMacroViewController
   }
 
   private function buildActionView(PhabricatorFileImageMacro $macro) {
-    $view = new PhabricatorActionListView();
+    $request = $this->getRequest();
-    $view->setUser($this->getRequest()->getUser());
+    $view = id(new PhabricatorActionListView())
-    $view->setObject($macro);
+      ->setUser($request->getUser())
-    $view->addAction(
+      ->setObject($macro)
+      ->setObjectURI($request->getRequestURI())
+      ->addAction(
         id(new PhabricatorActionView())
         ->setName(pht('Edit Macro'))
         ->setHref($this->getApplicationURI('/edit/'.$macro->getID().'/'))

src/applications/maniphest/controller/ManiphestTaskDetailController.php

@@ -381,11 +381,11 @@ final class ManiphestTaskDetailController extends ManiphestController {
     $id = $task->getID();
     $phid = $task->getPHID();
 
-    $view = new PhabricatorActionListView();
+    $view = id(new PhabricatorActionListView())
-    $view->setUser($viewer);
+      ->setUser($viewer)
-    $view->setObject($task);
+      ->setObject($task)
-
+      ->setObjectURI($this->getRequest()->getRequestURI())
-    $view->addAction(
+      ->addAction(
       id(new PhabricatorActionView())
         ->setName(pht('Edit Task'))
         ->setIcon('edit')

src/applications/meta/controller/PhabricatorApplicationDetailViewController.php

@@ -79,7 +79,8 @@ final class PhabricatorApplicationDetailViewController
     PhabricatorUser $user, PhabricatorApplication $selected) {
 
     $view = id(new PhabricatorActionListView())
-          ->setUser($user);
+      ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI());
 
     if ($selected->canUninstall()) {
       if ($selected->isInstalled()) {

src/applications/paste/controller/PhabricatorPasteViewController.php

@@ -94,6 +94,7 @@ final class PhabricatorPasteViewController extends PhabricatorPasteController {
     return id(new PhabricatorActionListView())
       ->setUser($user)
       ->setObject($paste)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->addAction(
         id(new PhabricatorActionView())
           ->setName(pht('Fork This Paste'))

src/applications/people/controller/PhabricatorPeopleProfileController.php

@@ -38,6 +38,7 @@ final class PhabricatorPeopleProfileController
 
     $actions = id(new PhabricatorActionListView())
       ->setObject($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($viewer);
 
     $can_edit = ($user->getPHID() == $viewer->getPHID());

src/applications/phame/controller/blog/PhameBlogViewController.php

@@ -144,6 +144,7 @@ final class PhameBlogViewController extends PhameController {
 
     $actions = id(new PhabricatorActionListView())
       ->setObject($blog)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($user);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/phame/controller/post/PhamePostViewController.php

@@ -87,6 +87,7 @@ final class PhamePostViewController extends PhameController {
 
     $actions = id(new PhabricatorActionListView())
       ->setObject($post)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setUser($user);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/phlux/controller/PhluxViewController.php

@@ -35,6 +35,7 @@ final class PhluxViewController extends PhluxController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($request->getRequestURI())
       ->setObject($var);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/pholio/controller/PholioMockViewController.php

@@ -109,6 +109,7 @@ final class PholioMockViewController extends PholioController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($mock);
 
     $can_edit = PhabricatorPolicyFilter::hasCapability(

src/applications/phortune/controller/PhortuneAccountViewController.php

@@ -34,6 +34,7 @@ final class PhortuneAccountViewController extends PhortuneController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($request->getRequestURI())
       ->addAction(
         id(new PhabricatorActionView())
           ->setName(pht('Edit Account'))
@@ -88,6 +89,7 @@ final class PhortuneAccountViewController extends PhortuneController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($request->getRequestURI())
       ->addAction(
         id(new PhabricatorActionView())
           ->setName(pht('Add Payment Method'))

src/applications/phortune/controller/PhortuneProductViewController.php

@@ -34,6 +34,7 @@ final class PhortuneProductViewController extends PhortuneController {
 
     $actions = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($request->getRequestURI())
       ->addAction(
         id(new PhabricatorActionView())
           ->setName(pht('Edit Product'))

src/applications/phrequent/event/PhrequentUIEventListener.php

@@ -38,7 +38,6 @@ final class PhrequentUIEventListener
       $object->getPHID());
     if (!$tracking) {
       $track_action = id(new PhabricatorActionView())
-        ->setUser($user)
         ->setName(pht('Start Tracking Time'))
         ->setIcon('history')
         ->setWorkflow(true)
@@ -46,7 +45,6 @@ final class PhrequentUIEventListener
         ->setHref('/phrequent/track/start/'.$object->getPHID().'/');
     } else {
       $track_action = id(new PhabricatorActionView())
-        ->setUser($user)
         ->setName(pht('Stop Tracking Time'))
         ->setIcon('history')
         ->setWorkflow(true)

src/applications/phriction/controller/PhrictionDocumentController.php

@@ -273,6 +273,7 @@ final class PhrictionDocumentController
 
     $action_view = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI())
       ->setObject($document);
 
     if (!$document->getID()) {

src/applications/ponder/controller/PonderQuestionViewController.php

@@ -100,13 +100,11 @@ final class PonderQuestionViewController extends PonderController {
   }
 
   private function buildActionListView(PonderQuestion $question) {
-    $viewer = $this->getRequest()->getUser();
+    $request = $this->getRequest();
-    $view = new PhabricatorActionListView();
+    return id(new PhabricatorActionListView())
-
+      ->setUser($request->getUser())
-    $view->setUser($viewer);
+      ->setObject($question)
-    $view->setObject($question);
+      ->setObjectURI($request->getRequestURI());
-
-    return $view;
   }
 
   private function buildPropertyListView(

src/applications/project/controller/PhabricatorProjectProfileController.php

@@ -94,6 +94,7 @@ final class PhabricatorProjectProfileController
 
     $action_list = id(new PhabricatorActionListView())
       ->setUser($user)
+      ->setObjectURI($request->getRequestURI())
       ->addAction($action);
 
     $nav_view->appendChild($header);

src/applications/subscriptions/events/PhabricatorSubscriptionsUIEventListener.php

@@ -36,7 +36,6 @@ final class PhabricatorSubscriptionsUIEventListener
     if ($object->isAutomaticallySubscribed($user->getPHID())) {
       $sub_action = id(new PhabricatorActionView())
         ->setWorkflow(true)
-        ->setUser($user)
         ->setDisabled(true)
         ->setRenderAsForm(true)
         ->setHref('/subscriptions/add/'.$object->getPHID().'/')
@@ -59,7 +58,6 @@ final class PhabricatorSubscriptionsUIEventListener
 
       if ($subscribed) {
         $sub_action = id(new PhabricatorActionView())
-          ->setUser($user)
           ->setWorkflow(true)
           ->setRenderAsForm(true)
           ->setHref('/subscriptions/delete/'.$object->getPHID().'/')
@@ -67,7 +65,6 @@ final class PhabricatorSubscriptionsUIEventListener
           ->setIcon('disable');
       } else {
         $sub_action = id(new PhabricatorActionView())
-          ->setUser($user)
           ->setWorkflow(true)
           ->setRenderAsForm(true)
           ->setHref('/subscriptions/add/'.$object->getPHID().'/')

src/applications/tokens/event/PhabricatorTokenUIEventListener.php

@@ -41,14 +41,12 @@ final class PhabricatorTokenUIEventListener
 
     if (!$current) {
       $token_action = id(new PhabricatorActionView())
-        ->setUser($user)
         ->setWorkflow(true)
         ->setHref('/token/give/'.$object->getPHID().'/')
         ->setName(pht('Award Token'))
         ->setIcon('like');
     } else {
       $token_action = id(new PhabricatorActionView())
-        ->setUser($user)
         ->setWorkflow(true)
         ->setHref('/token/give/'.$object->getPHID().'/')
         ->setName(pht('Rescind Token'))

src/applications/uiexample/examples/PhabricatorActionListExample.php

@@ -41,8 +41,9 @@ final class PhabricatorActionListExample extends PhabricatorUIExample {
       return id(new AphrontDialogResponse())->setDialog($dialog);
     }
 
-    $view = new PhabricatorActionListView();
+    $view = id(new PhabricatorActionListView())
-    $view->setUser($user);
+      ->setUser($user)
+      ->setObjectURI($this->getRequest()->getRequestURI());
 
     $view->addAction(
       id(new PhabricatorActionView())

src/view/layout/PhabricatorActionListView.php

@@ -4,6 +4,7 @@ final class PhabricatorActionListView extends AphrontView {
 
   private $actions = array();
   private $object;
+  private $objectURI;
   private $id = null;
 
   public function setObject(PhabricatorLiskDAO $object) {
@@ -11,6 +12,11 @@ final class PhabricatorActionListView extends AphrontView {
     return $this;
   }
 
+  public function setObjectURI($uri) {
+    $this->objectURI = $uri;
+    return $this;
+  }
+
   public function addAction(PhabricatorActionView $view) {
     $this->actions[] = $view;
     return $this;
@@ -41,6 +47,11 @@ final class PhabricatorActionListView extends AphrontView {
       return null;
     }
 
+    foreach ($actions as $action) {
+      $action->setObjectURI($this->objectURI);
+      $action->setUser($this->user);
+    }
+
     require_celerity_resource('phabricator-action-list-view-css');
 
     return phutil_tag(

src/view/layout/PhabricatorActionView.php

@@ -10,6 +10,15 @@ final class PhabricatorActionView extends AphrontView {
   private $workflow;
   private $renderAsForm;
   private $download;
+  private $objectURI;
+
+  public function setObjectURI($object_uri) {
+    $this->objectURI = $object_uri;
+    return $this;
+  }
+  public function getObjectURI() {
+    return $this->objectURI;
+  }
 
   public function setDownload($download) {
     $this->download = $download;
@@ -25,6 +34,22 @@ final class PhabricatorActionView extends AphrontView {
     return $this;
   }
 
+  /**
+   * If the user is not logged in and the action is relatively complicated,
+   * give them a generic login link that will re-direct to the page they're
+   * viewing.
+   */
+  public function getHref() {
+    if ($this->workflow || $this->renderAsForm) {
+      if (!$this->user || !$this->user->isLoggedIn()) {
+        return id(new PhutilURI('/auth/start/'))
+          ->setQueryParam('next', (string)$this->getObjectURI());
+      }
+    }
+
+    return $this->href;
+  }
+
   public function setIcon($icon) {
     $this->icon = $icon;
     return $this;
@@ -97,7 +122,7 @@ final class PhabricatorActionView extends AphrontView {
         $item = phabricator_form(
           $this->user,
           array(
-            'action'    => $this->href,
+            'action'    => $this->getHref(),
             'method'    => 'POST',
             'sigil'     => implode(' ', $sigils),
           ),
@@ -106,7 +131,7 @@ final class PhabricatorActionView extends AphrontView {
         $item = javelin_tag(
           'a',
           array(
-            'href'  => $this->href,
+            'href'  => $this->getHref(),
             'class' => 'phabricator-action-view-item',
             'sigil' => $this->workflow ? 'workflow' : null,
           ),