diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -950,7 +950,7 @@ 'HeraldInvalidActionException' => 'applications/herald/engine/exception/HeraldInvalidActionException.php', 'HeraldInvalidConditionException' => 'applications/herald/engine/exception/HeraldInvalidConditionException.php', 'HeraldManageGlobalRulesCapability' => 'applications/herald/capability/HeraldManageGlobalRulesCapability.php', - 'HeraldManiphestTaskAdapter' => 'applications/herald/adapter/HeraldManiphestTaskAdapter.php', + 'HeraldManiphestTaskAdapter' => 'applications/maniphest/herald/HeraldManiphestTaskAdapter.php', 'HeraldNewController' => 'applications/herald/controller/HeraldNewController.php', 'HeraldNewObjectField' => 'applications/herald/field/HeraldNewObjectField.php', 'HeraldObjectTranscript' => 'applications/herald/storage/transcript/HeraldObjectTranscript.php', @@ -1072,6 +1072,7 @@ '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', @@ -1105,12 +1106,14 @@ 'ManiphestTaskPHIDType' => 'applications/maniphest/phid/ManiphestTaskPHIDType.php', 'ManiphestTaskPriority' => 'applications/maniphest/constants/ManiphestTaskPriority.php', 'ManiphestTaskPriorityDatasource' => 'applications/maniphest/typeahead/ManiphestTaskPriorityDatasource.php', + 'ManiphestTaskPriorityHeraldField' => 'applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php', 'ManiphestTaskQuery' => 'applications/maniphest/query/ManiphestTaskQuery.php', 'ManiphestTaskResultListView' => 'applications/maniphest/view/ManiphestTaskResultListView.php', 'ManiphestTaskSearchEngine' => 'applications/maniphest/query/ManiphestTaskSearchEngine.php', 'ManiphestTaskStatus' => 'applications/maniphest/constants/ManiphestTaskStatus.php', 'ManiphestTaskStatusDatasource' => 'applications/maniphest/typeahead/ManiphestTaskStatusDatasource.php', 'ManiphestTaskStatusFunctionDatasource' => 'applications/maniphest/typeahead/ManiphestTaskStatusFunctionDatasource.php', + 'ManiphestTaskStatusHeraldField' => 'applications/maniphest/herald/ManiphestTaskStatusHeraldField.php', 'ManiphestTaskStatusTestCase' => 'applications/maniphest/constants/__tests__/ManiphestTaskStatusTestCase.php', 'ManiphestTaskTestCase' => 'applications/maniphest/__tests__/ManiphestTaskTestCase.php', 'ManiphestTransaction' => 'applications/maniphest/storage/ManiphestTransaction.php', @@ -4602,6 +4605,7 @@ 'ManiphestExcelFormatTestCase' => 'PhabricatorTestCase', 'ManiphestExportController' => 'ManiphestController', 'ManiphestGetTaskTransactionsConduitAPIMethod' => 'ManiphestConduitAPIMethod', + 'ManiphestHeraldField' => 'HeraldField', 'ManiphestHovercardEventListener' => 'PhabricatorEventListener', 'ManiphestInfoConduitAPIMethod' => 'ManiphestConduitAPIMethod', 'ManiphestNameIndex' => 'ManiphestDAO', @@ -4649,12 +4653,14 @@ 'ManiphestTaskPHIDType' => 'PhabricatorPHIDType', 'ManiphestTaskPriority' => 'ManiphestConstants', 'ManiphestTaskPriorityDatasource' => 'PhabricatorTypeaheadDatasource', + 'ManiphestTaskPriorityHeraldField' => 'ManiphestHeraldField', 'ManiphestTaskQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'ManiphestTaskResultListView' => 'ManiphestView', 'ManiphestTaskSearchEngine' => 'PhabricatorApplicationSearchEngine', 'ManiphestTaskStatus' => 'ManiphestConstants', 'ManiphestTaskStatusDatasource' => 'PhabricatorTypeaheadDatasource', 'ManiphestTaskStatusFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', + 'ManiphestTaskStatusHeraldField' => 'ManiphestHeraldField', 'ManiphestTaskStatusTestCase' => 'PhabricatorTestCase', 'ManiphestTaskTestCase' => 'PhabricatorTestCase', 'ManiphestTransaction' => 'PhabricatorApplicationTransaction', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -29,8 +29,6 @@ const FIELD_BRANCHES = 'branches'; const FIELD_AUTHOR_RAW = 'author-raw'; const FIELD_COMMITTER_RAW = 'committer-raw'; - const FIELD_TASK_PRIORITY = 'taskpriority'; - const FIELD_TASK_STATUS = 'taskstatus'; const FIELD_PUSHER_IS_COMMITTER = 'pusher-is-committer'; const FIELD_PATH = 'path'; @@ -383,8 +381,6 @@ self::FIELD_BRANCHES => pht('Commit\'s branches'), self::FIELD_AUTHOR_RAW => pht('Raw author name'), self::FIELD_COMMITTER_RAW => pht('Raw committer name'), - self::FIELD_TASK_PRIORITY => pht('Task priority'), - self::FIELD_TASK_STATUS => pht('Task status'), self::FIELD_PUSHER_IS_COMMITTER => pht('Pusher same as committer'), self::FIELD_PATH => pht('Path'), ); @@ -443,8 +439,6 @@ ); case self::FIELD_REVIEWER: case self::FIELD_PUSHER: - case self::FIELD_TASK_PRIORITY: - case self::FIELD_TASK_STATUS: return array( self::CONDITION_IS_ANY, self::CONDITION_IS_NOT_ANY, @@ -906,10 +900,6 @@ switch ($field) { case self::FIELD_REPOSITORY: return self::VALUE_REPOSITORY; - case self::FIELD_TASK_PRIORITY: - return self::VALUE_TASK_PRIORITY; - case self::FIELD_TASK_STATUS: - return self::VALUE_TASK_STATUS; default: return self::VALUE_USER; } @@ -1070,9 +1060,34 @@ return $map; } + public function getEditorValueForCondition( + PhabricatorUser $viewer, + HeraldCondition $condition, + array $handles) { + + $impl = $this->getFieldImplementation($condition->getFieldName()); + if ($impl) { + return $impl->getEditorValue( + $viewer, + $condition->getValue()); + } + + $value = $condition->getValue(); + if (is_array($value)) { + $value_map = array(); + foreach ($value as $k => $phid) { + $value_map[$phid] = $handles[$phid]->getName(); + } + $value = $value_map; + } + + return $value; + } + public function renderRuleAsText( HeraldRule $rule, - PhabricatorHandleList $handles) { + PhabricatorHandleList $handles, + PhabricatorUser $viewer) { require_celerity_resource('herald-css'); @@ -1102,7 +1117,7 @@ ), array( $icon, - $this->renderConditionAsText($condition, $handles), + $this->renderConditionAsText($condition, $handles, $viewer), )); } @@ -1147,7 +1162,8 @@ private function renderConditionAsText( HeraldCondition $condition, - PhabricatorHandleList $handles) { + PhabricatorHandleList $handles, + PhabricatorUser $viewer) { $field_type = $condition->getFieldName(); @@ -1158,7 +1174,7 @@ $condition_type = $condition->getFieldCondition(); $condition_name = idx($this->getConditionNameMap(), $condition_type); - $value = $this->renderConditionValueAsText($condition, $handles); + $value = $this->renderConditionValueAsText($condition, $handles, $viewer); return hsprintf(' %s %s %s', $field_name, $condition_name, $value); } @@ -1184,31 +1200,22 @@ private function renderConditionValueAsText( HeraldCondition $condition, - PhabricatorHandleList $handles) { + PhabricatorHandleList $handles, + PhabricatorUser $viewer) { + + $impl = $this->getFieldImplementation($condition->getFieldName()); + if ($impl) { + return $impl->renderConditionValue( + $viewer, + $condition->getValue()); + } $value = $condition->getValue(); if (!is_array($value)) { $value = array($value); } + switch ($condition->getFieldName()) { - case self::FIELD_TASK_PRIORITY: - $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); - foreach ($value as $index => $val) { - $name = idx($priority_map, $val); - if ($name) { - $value[$index] = $name; - } - } - break; - case self::FIELD_TASK_STATUS: - $status_map = ManiphestTaskStatus::getTaskStatusMap(); - foreach ($value as $index => $val) { - $name = idx($status_map, $val); - if ($name) { - $value[$index] = $name; - } - } - break; case HeraldPreCommitRefAdapter::FIELD_REF_CHANGE: $change_map = PhabricatorRepositoryPushLog::getHeraldChangeFlagConditionOptions(); diff --git a/src/applications/herald/controller/HeraldRuleController.php b/src/applications/herald/controller/HeraldRuleController.php --- a/src/applications/herald/controller/HeraldRuleController.php +++ b/src/applications/herald/controller/HeraldRuleController.php @@ -354,34 +354,11 @@ if ($rule->getConditions()) { $serial_conditions = array(); foreach ($rule->getConditions() as $condition) { - $value = $condition->getValue(); - switch ($condition->getFieldName()) { - case HeraldAdapter::FIELD_TASK_PRIORITY: - $value_map = array(); - $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); - foreach ($value as $priority) { - $value_map[$priority] = idx($priority_map, $priority); - } - $value = $value_map; - break; - case HeraldAdapter::FIELD_TASK_STATUS: - $value_map = array(); - $status_map = ManiphestTaskStatus::getTaskStatusMap(); - foreach ($value as $status) { - $value_map[$status] = idx($status_map, $status); - } - $value = $value_map; - break; - default: - if (is_array($value)) { - $value_map = array(); - foreach ($value as $k => $fbid) { - $value_map[$fbid] = $handles[$fbid]->getName(); - } - $value = $value_map; - } - break; - } + $value = $adapter->getEditorValueForCondition( + $this->getViewer(), + $condition, + $handles); + $serial_conditions[] = array( $condition->getFieldName(), $condition->getFieldCondition(), diff --git a/src/applications/herald/controller/HeraldRuleViewController.php b/src/applications/herald/controller/HeraldRuleViewController.php --- a/src/applications/herald/controller/HeraldRuleViewController.php +++ b/src/applications/herald/controller/HeraldRuleViewController.php @@ -152,7 +152,8 @@ PHUIPropertyListView::ICON_SUMMARY); $handles = $viewer->loadHandles(HeraldAdapter::getHandlePHIDs($rule)); - $view->addTextContent($adapter->renderRuleAsText($rule, $handles)); + $rule_text = $adapter->renderRuleAsText($rule, $handles, $viewer); + $view->addTextContent($rule_text); } return $view; diff --git a/src/applications/herald/field/HeraldField.php b/src/applications/herald/field/HeraldField.php --- a/src/applications/herald/field/HeraldField.php +++ b/src/applications/herald/field/HeraldField.php @@ -49,6 +49,51 @@ return array($this->getFieldConstant() => $this); } + public function renderConditionValue( + PhabricatorUser $viewer, + $value) { + + // TODO: While this is less of a mess than it used to be, it would still + // be nice to push this down into individual fields better eventually and + // stop guessing which values are PHIDs and which aren't. + + if (!is_array($value)) { + return $value; + } + + $type_unknown = PhabricatorPHIDConstants::PHID_TYPE_UNKNOWN; + + foreach ($value as $key => $val) { + if (is_string($val)) { + if (phid_get_type($val) !== $type_unknown) { + $value[$key] = $viewer->renderHandle($val); + } + } + } + + return phutil_implode_html(', ', $value); + } + + public function getEditorValue( + PhabricatorUser $viewer, + $value) { + + // TODO: This should be better structured and pushed down into individual + // fields. As it is used to manually build tokenizer tokens, it can + // probably be removed entirely. + + if (is_array($value)) { + $handles = $viewer->loadHandles($value); + $value_map = array(); + foreach ($value as $k => $phid) { + $value_map[$phid] = $handles[$phid]->getName(); + } + $value = $value_map; + } + + return $value; + } + final public function setAdapter(HeraldAdapter $adapter) { $this->adapter = $adapter; return $this; diff --git a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php rename from src/applications/herald/adapter/HeraldManiphestTaskAdapter.php rename to src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php --- a/src/applications/herald/adapter/HeraldManiphestTaskAdapter.php +++ b/src/applications/maniphest/herald/HeraldManiphestTaskAdapter.php @@ -74,8 +74,6 @@ self::FIELD_BODY, self::FIELD_AUTHOR, self::FIELD_ASSIGNEE, - self::FIELD_TASK_PRIORITY, - self::FIELD_TASK_STATUS, ), parent::getFields()); } @@ -124,10 +122,6 @@ return $this->getTask()->getAuthorPHID(); case self::FIELD_ASSIGNEE: return $this->getTask()->getOwnerPHID(); - case self::FIELD_TASK_PRIORITY: - return $this->getTask()->getPriority(); - case self::FIELD_TASK_STATUS: - return $this->getTask()->getStatus(); } return parent::getHeraldField($field); diff --git a/src/applications/maniphest/herald/ManiphestHeraldField.php b/src/applications/maniphest/herald/ManiphestHeraldField.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestHeraldField.php @@ -0,0 +1,9 @@ +getPriority(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_PHID; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_TASK_PRIORITY; + } + + public function renderConditionValue( + PhabricatorUser $viewer, + $value) { + + $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); + + $value = (array)$value; + foreach ($value as $index => $val) { + $name = idx($priority_map, $val); + if ($name !== null) { + $value[$index] = $name; + } + } + + return implode(', ', $value); + } + + public function getEditorValue( + PhabricatorUser $viewer, + $value) { + + $priority_map = ManiphestTaskPriority::getTaskPriorityMap(); + + $value_map = array(); + foreach ($value as $priority) { + $value_map[$priority] = idx($priority_map, $priority, $priority); + } + + return $value_map; + } + +} diff --git a/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php new file mode 100644 --- /dev/null +++ b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php @@ -0,0 +1,55 @@ +getStatus(); + } + + protected function getHeraldFieldStandardConditions() { + return self::STANDARD_PHID; + } + + public function getHeraldFieldValueType($condition) { + return HeraldAdapter::VALUE_TASK_STATUS; + } + + public function renderConditionValue( + PhabricatorUser $viewer, + $value) { + + $status_map = ManiphestTaskStatus::getTaskStatusMap(); + + $value = (array)$value; + foreach ($value as $index => $val) { + $name = idx($status_map, $val); + if ($name !== null) { + $value[$index] = $name; + } + } + + return implode(', ', $value); + } + + public function getEditorValue( + PhabricatorUser $viewer, + $value) { + + $status_map = ManiphestTaskStatus::getTaskStatusMap(); + + $value_map = array(); + foreach ($value as $status) { + $value_map[$status] = idx($status_map, $status, $status); + } + + return $value_map; + } + +}