From 83c0fda280592c76d2febde0ad3b3d2ac2a36bc6 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 6 Jul 2015 13:17:14 -0700 Subject: [PATCH] Modularize all remaining Maniphest Herald fields Summary: Ref T8726. The only notable bit here is that the "body" / "title" fields (which are currently shared across a bunch of types) are getting split into application variants. Among other things, this will let us label the field "Commit message" for commits, for example. Test Plan: - Created a rule using all four fields. - Applied patch, saw rule break ("unknown field"). - Ran storage upgrade, saw rule fix itself in the migration. - Edited tasks, triggered rule, viewed transcripts. Reviewers: btrahan Reviewed By: btrahan Subscribers: eadler, joshuaspence, epriestley Maniphest Tasks: T8726 Differential Revision: https://secure.phabricator.com/D13501 --- .../sql/autopatches/20150630.herald.2.sql | 30 +++++++++++++++++++ src/__phutil_library_map__.php | 16 +++++++--- .../herald/adapter/HeraldAdapter.php | 3 -- src/applications/herald/field/HeraldField.php | 18 ++++++++++- .../herald/HeraldManiphestTaskAdapter.php | 26 ---------------- .../ManiphestTaskAssigneeHeraldField.php | 24 +++++++++++++++ .../herald/ManiphestTaskAuthorHeraldField.php | 24 +++++++++++++++ .../ManiphestTaskDescriptionHeraldField.php | 24 +++++++++++++++ ...Field.php => ManiphestTaskHeraldField.php} | 2 +- .../ManiphestTaskPriorityHeraldField.php | 4 +-- .../herald/ManiphestTaskStatusHeraldField.php | 4 +-- .../herald/ManiphestTaskTitleHeraldField.php | 24 +++++++++++++++ 12 files changed, 160 insertions(+), 39 deletions(-) create mode 100644 resources/sql/autopatches/20150630.herald.2.sql create mode 100644 src/applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php create mode 100644 src/applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php create mode 100644 src/applications/maniphest/herald/ManiphestTaskDescriptionHeraldField.php rename src/applications/maniphest/herald/{ManiphestHeraldField.php => ManiphestTaskHeraldField.php} (63%) create mode 100644 src/applications/maniphest/herald/ManiphestTaskTitleHeraldField.php diff --git a/resources/sql/autopatches/20150630.herald.2.sql b/resources/sql/autopatches/20150630.herald.2.sql new file mode 100644 index 0000000000..3c5d6ee628 --- /dev/null +++ b/resources/sql/autopatches/20150630.herald.2.sql @@ -0,0 +1,30 @@ +# This converts old conditions which use common fields like "body" to new +# conditions which use modular rules like "Maniphest Task Description". + +UPDATE {$NAMESPACE}_herald.herald_condition c + JOIN {$NAMESPACE}_herald.herald_rule r + ON c.ruleID = r.id + SET c.fieldName = 'maniphest.task.title' + WHERE r.contentType = 'HeraldManiphestTaskAdapter' + AND c.fieldName = 'title'; + +UPDATE {$NAMESPACE}_herald.herald_condition c + JOIN {$NAMESPACE}_herald.herald_rule r + ON c.ruleID = r.id + SET c.fieldName = 'maniphest.task.description' + WHERE r.contentType = 'HeraldManiphestTaskAdapter' + AND c.fieldName = 'body'; + +UPDATE {$NAMESPACE}_herald.herald_condition c + JOIN {$NAMESPACE}_herald.herald_rule r + ON c.ruleID = r.id + SET c.fieldName = 'maniphest.task.author' + WHERE r.contentType = 'HeraldManiphestTaskAdapter' + AND c.fieldName = 'author'; + +UPDATE {$NAMESPACE}_herald.herald_condition c + JOIN {$NAMESPACE}_herald.herald_rule r + ON c.ruleID = r.id + SET c.fieldName = 'maniphest.task.assignee' + WHERE r.contentType = 'HeraldManiphestTaskAdapter' + AND c.fieldName = 'assignee'; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index f4473063a9..d79efb2086 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1072,7 +1072,6 @@ phutil_register_library_map(array( 'ManiphestExcelFormatTestCase' => 'applications/maniphest/export/__tests__/ManiphestExcelFormatTestCase.php', 'ManiphestExportController' => 'applications/maniphest/controller/ManiphestExportController.php', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestGetTaskTransactionsConduitAPIMethod.php', - 'ManiphestHeraldField' => 'applications/maniphest/herald/ManiphestHeraldField.php', 'ManiphestHovercardEventListener' => 'applications/maniphest/event/ManiphestHovercardEventListener.php', 'ManiphestInfoConduitAPIMethod' => 'applications/maniphest/conduit/ManiphestInfoConduitAPIMethod.php', 'ManiphestNameIndex' => 'applications/maniphest/storage/ManiphestNameIndex.php', @@ -1089,16 +1088,20 @@ phutil_register_library_map(array( 'ManiphestStatusEmailCommand' => 'applications/maniphest/command/ManiphestStatusEmailCommand.php', 'ManiphestSubpriorityController' => 'applications/maniphest/controller/ManiphestSubpriorityController.php', 'ManiphestTask' => 'applications/maniphest/storage/ManiphestTask.php', + 'ManiphestTaskAssigneeHeraldField' => 'applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php', + 'ManiphestTaskAuthorHeraldField' => 'applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php', 'ManiphestTaskAuthorPolicyRule' => 'applications/maniphest/policyrule/ManiphestTaskAuthorPolicyRule.php', 'ManiphestTaskClosedStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskClosedStatusDatasource.php', 'ManiphestTaskDependedOnByTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependedOnByTaskEdgeType.php', 'ManiphestTaskDependsOnTaskEdgeType' => 'applications/maniphest/edge/ManiphestTaskDependsOnTaskEdgeType.php', + 'ManiphestTaskDescriptionHeraldField' => 'applications/maniphest/herald/ManiphestTaskDescriptionHeraldField.php', 'ManiphestTaskDetailController' => 'applications/maniphest/controller/ManiphestTaskDetailController.php', 'ManiphestTaskEditBulkJobType' => 'applications/maniphest/bulk/ManiphestTaskEditBulkJobType.php', 'ManiphestTaskEditController' => 'applications/maniphest/controller/ManiphestTaskEditController.php', 'ManiphestTaskHasCommitEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasCommitEdgeType.php', 'ManiphestTaskHasMockEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasMockEdgeType.php', 'ManiphestTaskHasRevisionEdgeType' => 'applications/maniphest/edge/ManiphestTaskHasRevisionEdgeType.php', + 'ManiphestTaskHeraldField' => 'applications/maniphest/herald/ManiphestTaskHeraldField.php', 'ManiphestTaskListController' => 'applications/maniphest/controller/ManiphestTaskListController.php', 'ManiphestTaskListView' => 'applications/maniphest/view/ManiphestTaskListView.php', 'ManiphestTaskMailReceiver' => 'applications/maniphest/mail/ManiphestTaskMailReceiver.php', @@ -1116,6 +1119,7 @@ phutil_register_library_map(array( 'ManiphestTaskStatusHeraldField' => 'applications/maniphest/herald/ManiphestTaskStatusHeraldField.php', 'ManiphestTaskStatusTestCase' => 'applications/maniphest/constants/__tests__/ManiphestTaskStatusTestCase.php', 'ManiphestTaskTestCase' => 'applications/maniphest/__tests__/ManiphestTaskTestCase.php', + 'ManiphestTaskTitleHeraldField' => 'applications/maniphest/herald/ManiphestTaskTitleHeraldField.php', 'ManiphestTransaction' => 'applications/maniphest/storage/ManiphestTransaction.php', 'ManiphestTransactionComment' => 'applications/maniphest/storage/ManiphestTransactionComment.php', 'ManiphestTransactionEditor' => 'applications/maniphest/editor/ManiphestTransactionEditor.php', @@ -4605,7 +4609,6 @@ phutil_register_library_map(array( 'ManiphestExcelFormatTestCase' => 'PhabricatorTestCase', 'ManiphestExportController' => 'ManiphestController', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod', - 'ManiphestHeraldField' => 'HeraldField', 'ManiphestHovercardEventListener' => 'PhabricatorEventListener', 'ManiphestInfoConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestNameIndex' => 'ManiphestDAO', @@ -4636,16 +4639,20 @@ phutil_register_library_map(array( 'PhabricatorProjectInterface', 'PhabricatorSpacesInterface', ), + 'ManiphestTaskAssigneeHeraldField' => 'ManiphestTaskHeraldField', + 'ManiphestTaskAuthorHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskAuthorPolicyRule' => 'PhabricatorPolicyRule', 'ManiphestTaskClosedStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskDependedOnByTaskEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskDependsOnTaskEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskDescriptionHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskDetailController' => 'ManiphestController', 'ManiphestTaskEditBulkJobType' => 'PhabricatorWorkerBulkJobType', 'ManiphestTaskEditController' => 'ManiphestController', 'ManiphestTaskHasCommitEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasMockEdgeType' => 'PhabricatorEdgeType', 'ManiphestTaskHasRevisionEdgeType' => 'PhabricatorEdgeType', + 'ManiphestTaskHeraldField' => 'HeraldField', 'ManiphestTaskListController' => 'ManiphestController', 'ManiphestTaskListView' => 'ManiphestView', 'ManiphestTaskMailReceiver' => 'PhabricatorObjectMailReceiver', @@ -4653,16 +4660,17 @@ phutil_register_library_map(array( 'ManiphestTaskPHIDType' => 'PhabricatorPHIDType', 'ManiphestTaskPriority' => 'ManiphestConstants', 'ManiphestTaskPriorityDatasource' => 'PhabricatorTypeaheadDatasource', - 'ManiphestTaskPriorityHeraldField' => 'ManiphestHeraldField', + 'ManiphestTaskPriorityHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'ManiphestTaskResultListView' => 'ManiphestView', 'ManiphestTaskSearchEngine' => 'PhabricatorApplicationSearchEngine', 'ManiphestTaskStatus' => 'ManiphestConstants', 'ManiphestTaskStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskStatusFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', - 'ManiphestTaskStatusHeraldField' => 'ManiphestHeraldField', + 'ManiphestTaskStatusHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTaskStatusTestCase' => 'PhabricatorTestCase', 'ManiphestTaskTestCase' => 'PhabricatorTestCase', + 'ManiphestTaskTitleHeraldField' => 'ManiphestTaskHeraldField', 'ManiphestTransaction' => 'PhabricatorApplicationTransaction', 'ManiphestTransactionComment' => 'PhabricatorApplicationTransactionComment', 'ManiphestTransactionEditor' => 'PhabricatorApplicationTransactionEditor', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 033d91db12..c3555a8f7a 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -5,7 +5,6 @@ abstract class HeraldAdapter extends Phobject { const FIELD_TITLE = 'title'; const FIELD_BODY = 'body'; const FIELD_AUTHOR = 'author'; - const FIELD_ASSIGNEE = 'assignee'; const FIELD_REVIEWER = 'reviewer'; const FIELD_REVIEWERS = 'reviewers'; const FIELD_COMMITTER = 'committer'; @@ -355,7 +354,6 @@ abstract class HeraldAdapter extends Phobject { self::FIELD_TITLE => pht('Title'), self::FIELD_BODY => pht('Body'), self::FIELD_AUTHOR => pht('Author'), - self::FIELD_ASSIGNEE => pht('Assignee'), self::FIELD_COMMITTER => pht('Committer'), self::FIELD_REVIEWER => pht('Reviewer'), self::FIELD_REVIEWERS => pht('Reviewers'), @@ -444,7 +442,6 @@ abstract class HeraldAdapter extends Phobject { self::CONDITION_IS_NOT_ANY, ); case self::FIELD_REPOSITORY: - case self::FIELD_ASSIGNEE: case self::FIELD_AUTHOR: case self::FIELD_COMMITTER: return array( diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php index ec5f5822be..9b9c4ec49e 100644 --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -6,7 +6,9 @@ abstract class HeraldField extends Phobject { const STANDARD_LIST = 'standard.list'; const STANDARD_BOOL = 'standard.bool'; + const STANDARD_TEXT = 'standard.text'; const STANDARD_PHID = 'standard.phid'; + const STANDARD_PHID_NULLABLE = 'standard.phid.nullable'; abstract public function getHeraldFieldName(); abstract public function getHeraldFieldValue($object); @@ -26,12 +28,26 @@ abstract class HeraldField extends Phobject { HeraldAdapter::CONDITION_IS_TRUE, HeraldAdapter::CONDITION_IS_FALSE, ); + case self::STANDARD_TEXT: + return array( + HeraldAdapter::CONDITION_CONTAINS, + HeraldAdapter::CONDITION_NOT_CONTAINS, + HeraldAdapter::CONDITION_IS, + HeraldAdapter::CONDITION_IS_NOT, + HeraldAdapter::CONDITION_REGEXP, + ); case self::STANDARD_PHID: return array( HeraldAdapter::CONDITION_IS_ANY, HeraldAdapter::CONDITION_IS_NOT_ANY, ); - + case self::STANDARD_PHID_NULLABLE: + return array( + HeraldAdapter::CONDITION_IS_ANY, + HeraldAdapter::CONDITION_IS_NOT_ANY, + HeraldAdapter::CONDITION_EXISTS, + HeraldAdapter::CONDITION_NOT_EXISTS, + ); } throw new Exception(pht('Unknown standard condition set.')); diff --git a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php index 00930e3bbb..adbd724868 100644 --- a/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -67,17 +67,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { return pht('Maniphest Tasks'); } - public function getFields() { - return array_merge( - array( - self::FIELD_TITLE, - self::FIELD_BODY, - self::FIELD_AUTHOR, - self::FIELD_ASSIGNEE, - ), - parent::getFields()); - } - public function getActions($rule_type) { switch ($rule_type) { case HeraldRuleTypeConfig::RULE_TYPE_GLOBAL: @@ -112,21 +101,6 @@ final class HeraldManiphestTaskAdapter extends HeraldAdapter { return 'T'.$this->getTask()->getID(); } - public function getHeraldField($field) { - switch ($field) { - case self::FIELD_TITLE: - return $this->getTask()->getTitle(); - case self::FIELD_BODY: - return $this->getTask()->getDescription(); - case self::FIELD_AUTHOR: - return $this->getTask()->getAuthorPHID(); - case self::FIELD_ASSIGNEE: - return $this->getTask()->getOwnerPHID(); - } - - return parent::getHeraldField($field); - } - public function applyHeraldEffects(array $effects) { assert_instances_of($effects, 'HeraldEffect'); diff --git a/src/applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php new file mode 100644 index 0000000000..da900e0d24 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskAssigneeHeraldField.php @@ -0,0 +1,24 @@ +getOwnerPHID(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_PHID_NULLABLE; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_USER; + } + +} diff --git a/src/applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php new file mode 100644 index 0000000000..641b247bfb --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskAuthorHeraldField.php @@ -0,0 +1,24 @@ +getAuthorPHID(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_PHID; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_USER; + } + +} diff --git a/src/applications/maniphest/herald/ManiphestTaskDescriptionHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskDescriptionHeraldField.php new file mode 100644 index 0000000000..c1fdc0c2d1 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskDescriptionHeraldField.php @@ -0,0 +1,24 @@ +getDescription(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_TEXT; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_TEXT; + } + +} diff --git a/src/applications/maniphest/herald/ManiphestHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskHeraldField.php similarity index 63% rename from src/applications/maniphest/herald/ManiphestHeraldField.php rename to src/applications/maniphest/herald/ManiphestTaskHeraldField.php index c20e871e1a..b758671698 100644 --- a/src/applications/maniphest/herald/ManiphestHeraldField.php +++ b/src/applications/maniphest/herald/ManiphestTaskHeraldField.php @@ -1,6 +1,6 @@ getTitle(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_TEXT; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_TEXT; + } + +}