From b35f578ae9159a094cb35b8fca514118dd339ac7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 30 Nov 2015 06:41:43 -0800 Subject: [PATCH] Modernize Transaction value controller, fixing logged-out policy issue Summary: Fixes T9869. This specific transaction endpoint was missing `shouldAllowPublic()`. Also modernize things a little. Test Plan: Viewed a policy change by clicking the policy name from the transaction record on a public object while logged out. Reviewers: chad Reviewed By: chad Maniphest Tasks: T9869 Differential Revision: https://secure.phabricator.com/D14606 --- ...rApplicationTransactionValueController.php | 28 +++++++++---------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php index 37008db6d3..3a73a982e0 100644 --- a/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php +++ b/src/applications/transactions/controller/PhabricatorApplicationTransactionValueController.php @@ -3,21 +3,20 @@ final class PhabricatorApplicationTransactionValueController extends PhabricatorApplicationTransactionController { - private $value; - private $phid; - - public function willProcessRequest(array $data) { - $this->phid = $data['phid']; - $this->value = $data['value']; + public function shouldAllowPublic() { + return true; } - public function processRequest() { + public function handleRequest(AphrontRequest $request) { $request = $this->getRequest(); $viewer = $request->getUser(); + $phid = $request->getURIData('phid'); + $type = $request->getURIData('value'); + $xaction = id(new PhabricatorObjectQuery()) ->setViewer($viewer) - ->withPHIDs(array($this->phid)) + ->withPHIDs(array($phid)) ->executeOne(); if (!$xaction) { return new Aphront404Response(); @@ -42,11 +41,12 @@ final class PhabricatorApplicationTransactionValueController break; } - if ($this->value == 'old') { + if ($type == 'old') { $value = $xaction->getOldValue(); } else { $value = $xaction->getNewValue(); } + $policy = id(new PhabricatorPolicyQuery()) ->setViewer($viewer) ->withPHIDs(array($value)) @@ -54,6 +54,7 @@ final class PhabricatorApplicationTransactionValueController if (!$policy) { return new Aphront404Response(); } + if ($policy->getType() != PhabricatorPolicyType::TYPE_CUSTOM) { return new Aphront404Response(); } @@ -66,15 +67,12 @@ final class PhabricatorApplicationTransactionValueController $this->requireResource('policy-transaction-detail-css'); $cancel_uri = $this->guessCancelURI($viewer, $xaction); - $dialog = id(new AphrontDialogView()) - ->setUser($viewer) + + return $this->newDialog() ->setTitle($policy->getFullName()) ->setWidth(AphrontDialogView::WIDTH_FORM) - ->appendChild( - $this->renderPolicyDetails($policy, $rule_objects)) + ->appendChild($this->renderPolicyDetails($policy, $rule_objects)) ->addCancelButton($cancel_uri, pht('Close')); - - return id(new AphrontDialogResponse())->setDialog($dialog); } private function extractPHIDs(