From 7eab75410a196272212d6a555a2dd35d1e8cd120 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 1 Mar 2017 17:01:21 -0800 Subject: [PATCH] When editing a subtyped object, use edit forms of the same subtype Summary: Ref T12314. When we pick an "Edit" form for a subtyped object, only consider forms with the same subtype. For example, editing an "Animal" uses the forms with subtype "animal" which are marked as edit forms. This also makes "Create Subtask" carry the parent task's type. Test Plan: - Edited an Animal, got an animal edit form. - Edited a normal task, got a normal task form. - Edited a paste, got the normal workflow. Reviewers: chad Reviewed By: chad Maniphest Tasks: T12314 Differential Revision: https://secure.phabricator.com/D17445 --- .../ManiphestTaskDetailController.php | 2 +- .../PhabricatorProjectBoardViewController.php | 2 +- .../editengine/PhabricatorEditEngine.php | 27 ++++++++++++------- ...habricatorEditEngineConfigurationQuery.php | 24 +++++++++++++++++ 4 files changed, 44 insertions(+), 11 deletions(-) diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 369986705c..60d3a81b1b 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -267,7 +267,7 @@ final class ManiphestTaskDetailController extends ManiphestController { ->setDisabled(!$can_edit) ->setWorkflow(!$can_edit)); - $edit_config = $edit_engine->loadDefaultEditConfiguration(); + $edit_config = $edit_engine->loadDefaultEditConfiguration($task); $can_create = (bool)$edit_config; $can_reassign = $edit_engine->hasEditAccessToTransaction( diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 7e682fb258..f504d43426 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -910,7 +910,7 @@ final class PhabricatorProjectBoardViewController // for each column or board? $edit_config = id(new ManiphestEditEngine()) ->setViewer($viewer) - ->loadDefaultEditConfiguration(); + ->loadDefaultEditConfiguration(new ManiphestTask()); if ($edit_config) { $form_key = $edit_config->getIdentifier(); $create_uri = "/maniphest/task/edit/form/{$form_key}/"; diff --git a/src/applications/transactions/editengine/PhabricatorEditEngine.php b/src/applications/transactions/editengine/PhabricatorEditEngine.php index 82af2143c7..75f906f9bf 100644 --- a/src/applications/transactions/editengine/PhabricatorEditEngine.php +++ b/src/applications/transactions/editengine/PhabricatorEditEngine.php @@ -408,11 +408,20 @@ abstract class PhabricatorEditEngine 'getCreateSortKey'); } - public function loadDefaultEditConfiguration() { + public function loadDefaultEditConfiguration($object) { $query = $this->newConfigurationQuery() ->withIsEdit(true) ->withIsDisabled(false); + // If this object supports subtyping, we edit it with a form of the same + // subtype: so "bug" tasks get edited with "bug" forms. + if ($object instanceof PhabricatorEditEngineSubtypeInterface) { + $query->withSubtypes( + array( + $object->getEditEngineSubtype(), + )); + } + return $this->loadEditEngineConfigurationWithQuery( $query, 'getEditSortKey'); @@ -891,7 +900,7 @@ abstract class PhabricatorEditEngine } } else { if ($id) { - $config = $this->loadDefaultEditConfiguration(); + $config = $this->loadDefaultEditConfiguration($object); if (!$config) { return $this->buildNoEditResponse($object); } @@ -1370,16 +1379,16 @@ abstract class PhabricatorEditEngine final public function hasEditAccessToTransaction($xaction_type) { $viewer = $this->getViewer(); - $config = $this->loadDefaultEditConfiguration(); - if (!$config) { - return false; - } - $object = $this->getTargetObject(); if (!$object) { $object = $this->newEditableObject(); } + $config = $this->loadDefaultEditConfiguration($object); + if (!$config) { + return false; + } + $fields = $this->buildEditFields($object); $field = null; @@ -1535,7 +1544,7 @@ abstract class PhabricatorEditEngine } final public function buildEditEngineCommentView($object) { - $config = $this->loadDefaultEditConfiguration(); + $config = $this->loadDefaultEditConfiguration($object); if (!$config) { // TODO: This just nukes the entire comment form if you don't have access @@ -1755,7 +1764,7 @@ abstract class PhabricatorEditEngine return new Aphront400Response(); } - $config = $this->loadDefaultEditConfiguration(); + $config = $this->loadDefaultEditConfiguration($object); if (!$config) { return new Aphront404Response(); } diff --git a/src/applications/transactions/query/PhabricatorEditEngineConfigurationQuery.php b/src/applications/transactions/query/PhabricatorEditEngineConfigurationQuery.php index b57e4b7675..d2cbc3e697 100644 --- a/src/applications/transactions/query/PhabricatorEditEngineConfigurationQuery.php +++ b/src/applications/transactions/query/PhabricatorEditEngineConfigurationQuery.php @@ -12,6 +12,7 @@ final class PhabricatorEditEngineConfigurationQuery private $isEdit; private $disabled; private $ignoreDatabaseConfigurations; + private $subtypes; public function withIDs(array $ids) { $this->ids = $ids; @@ -58,6 +59,11 @@ final class PhabricatorEditEngineConfigurationQuery return $this; } + public function withSubtypes(array $subtypes) { + $this->subtypes = $subtypes; + return $this; + } + public function newResultObject() { return new PhabricatorEditEngineConfiguration(); } @@ -183,6 +189,17 @@ final class PhabricatorEditEngineConfigurationQuery } } + if ($this->subtypes !== null) { + $subtypes = array_fuse($this->subtypes); + foreach ($page as $key => $config) { + if (isset($subtypes[$config->getSubtype()])) { + continue; + } + + unset($page[$key]); + } + } + return $page; } @@ -250,6 +267,13 @@ final class PhabricatorEditEngineConfigurationQuery $this->identifiers); } + if ($this->subtypes !== null) { + $where[] = qsprintf( + $conn, + 'subtype IN (%Ls)', + $this->subtypes); + } + return $where; }