From a3099e27bc1ed65343860440ee1b1e4b6bc0b45c Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 7 Mar 2013 17:22:36 -0800 Subject: [PATCH] Provide Maniphest auxiliary fields access to the viewer and task Summary: Maniphest auxiliary fields currently do not have access to the viewing user or task. This is fine for very simple fields, but insufficient for more complex fields. Generally, bring these in line with DifferentialFieldSpecifications. This supercedes the additional $user/$viewer threading provided by D5247 and provides viewers to all fields, as well as access to the task object itself. Test Plan: Created, viewed and edited a task with custom fields. Created a similar subtask. Reviewers: hach-que, btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T2575 Differential Revision: https://secure.phabricator.com/D5280 --- .../ManiphestAuxiliaryFieldSpecification.php | 20 +++++++++++++++++++ .../ManiphestTaskDetailController.php | 7 +------ .../ManiphestTaskEditController.php | 13 +----------- .../extensions/ManiphestTaskExtensions.php | 18 +++++++++++++++++ 4 files changed, 40 insertions(+), 18 deletions(-) diff --git a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php index 216227194d..0eba1db06e 100644 --- a/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php +++ b/src/applications/maniphest/auxiliaryfield/ManiphestAuxiliaryFieldSpecification.php @@ -12,6 +12,26 @@ abstract class ManiphestAuxiliaryFieldSpecification { private $auxiliaryKey; private $caption; private $value; + private $user; + private $task; + + public function setTask(ManiphestTask $task) { + $this->task = $task; + return $this; + } + + public function getTask() { + return $this->task; + } + + public function setUser(PhabricatorUser $user) { + $this->user = $user; + return $this; + } + + public function getUser() { + return $this->user; + } public function setLabel($val) { $this->label = $val; diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index b4cdbdec75..dc6980c071 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -129,10 +129,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $engine->process(); $extensions = ManiphestTaskExtensions::newExtensions(); - $aux_fields = $extensions->getAuxiliaryFieldSpecifications(); - if ($aux_fields) { - $task->loadAndAttachAuxiliaryAttributes(); - } + $aux_fields = $extensions->loadFields($task, $user); $transaction_types = ManiphestTransactionType::getTransactionTypeMap(); $resolution_types = ManiphestTaskStatus::getTaskStatusMap(); @@ -467,8 +464,6 @@ final class ManiphestTaskDetailController extends ManiphestController { : phutil_tag('em', array(), pht('None'))); foreach ($aux_fields as $aux_field) { - $aux_key = $aux_field->getAuxiliaryKey(); - $aux_field->setValue($task->getAuxiliaryAttribute($aux_key)); $value = $aux_field->renderForDetailView(); if (strlen($value)) { $view->addProperty($aux_field->getLabel(), $value); diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index 3ca0812884..ed96eb7453 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -70,10 +70,9 @@ final class ManiphestTaskEditController extends ManiphestController { $e_title = true; $extensions = ManiphestTaskExtensions::newExtensions(); - $aux_fields = $extensions->getAuxiliaryFieldSpecifications(); + $aux_fields = $extensions->loadFields($task, $user); if ($request->isFormPost()) { - $changes = array(); $new_title = $request->getStr('title'); @@ -185,7 +184,6 @@ final class ManiphestTaskEditController extends ManiphestController { } if ($aux_fields) { - $task->loadAndAttachAuxiliaryAttributes(); foreach ($aux_fields as $aux_field) { $transaction = clone $template; $transaction->setTransactionType( @@ -253,15 +251,6 @@ final class ManiphestTaskEditController extends ManiphestController { ->setURI($redirect_uri); } } else { - if ($aux_fields) { - $task->loadAndAttachAuxiliaryAttributes(); - foreach ($aux_fields as $aux_field) { - $aux_key = $aux_field->getAuxiliaryKey(); - $value = $task->getAuxiliaryAttribute($aux_key); - $aux_field->setValueFromStorage($value); - } - } - if (!$task->getID()) { $task->setCCPHIDs(array( $user->getPHID(), diff --git a/src/applications/maniphest/extensions/ManiphestTaskExtensions.php b/src/applications/maniphest/extensions/ManiphestTaskExtensions.php index 5d26bb0bd0..06e8f4402b 100644 --- a/src/applications/maniphest/extensions/ManiphestTaskExtensions.php +++ b/src/applications/maniphest/extensions/ManiphestTaskExtensions.php @@ -17,4 +17,22 @@ abstract class ManiphestTaskExtensions { return PhabricatorEnv::newObjectFromConfig($key); } + public function loadFields(ManiphestTask $task, PhabricatorUser $viewer) { + $aux_fields = $this->getAuxiliaryFieldSpecifications(); + if (!$aux_fields) { + return array(); + } + + $task->loadAndAttachAuxiliaryAttributes(); + foreach ($aux_fields as $aux) { + $aux->setUser($viewer); + $aux->setTask($task); + + $key = $aux->getAuxiliaryKey(); + $aux->setValueFromStorage($task->getAuxiliaryAttribute($key)); + } + + return $aux_fields; + } + }