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 @@ -2777,6 +2777,7 @@ 'PhabricatorStandardCustomFieldText' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldText.php', 'PhabricatorStandardCustomFieldUsers' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldUsers.php', 'PhabricatorStandardPageView' => 'view/page/PhabricatorStandardPageView.php', + 'PhabricatorStandardSelectCustomFieldDatasource' => 'infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php', 'PhabricatorStatusController' => 'applications/system/controller/PhabricatorStatusController.php', 'PhabricatorStatusUIExample' => 'applications/uiexample/examples/PhabricatorStatusUIExample.php', 'PhabricatorStorageFixtureScopeGuard' => 'infrastructure/testing/fixture/PhabricatorStorageFixtureScopeGuard.php', @@ -6678,6 +6679,7 @@ 'PhabricatorStandardCustomFieldText' => 'PhabricatorStandardCustomField', 'PhabricatorStandardCustomFieldUsers' => 'PhabricatorStandardCustomFieldPHIDs', 'PhabricatorStandardPageView' => 'PhabricatorBarePageView', + 'PhabricatorStandardSelectCustomFieldDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorStatusController' => 'PhabricatorController', 'PhabricatorStatusUIExample' => 'PhabricatorUIExample', 'PhabricatorStorageFixtureScopeGuard' => 'Phobject', 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 @@ -882,23 +882,12 @@ HeraldCondition $condition, array $handles) { - $impl = $this->getFieldImplementation($condition->getFieldName()); - if ($impl) { - return $impl->getEditorValue( - $viewer, - $condition->getValue()); - } + $field = $this->requireFieldImplementation($condition->getFieldName()); - $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; + return $field->getEditorValue( + $viewer, + $condition->getFieldCondition(), + $condition->getValue()); } public function renderRuleAsText( @@ -1020,28 +1009,12 @@ PhabricatorHandleList $handles, PhabricatorUser $viewer) { - $impl = $this->getFieldImplementation($condition->getFieldName()); - if ($impl) { - return $impl->renderConditionValue( - $viewer, - $condition->getFieldCondition(), - $condition->getValue()); - } - - $value = $condition->getValue(); - if (!is_array($value)) { - $value = array($value); - } - - foreach ($value as $index => $val) { - $handle = $handles->getHandleIfExists($val); - if ($handle) { - $value[$index] = $handle->renderLink(); - } - } + $field = $this->requireFieldImplementation($condition->getFieldName()); - $value = phutil_implode_html(', ', $value); - return $value; + return $field->renderConditionValue( + $viewer, + $condition->getFieldCondition(), + $condition->getValue()); } private function renderActionTargetAsText( diff --git a/src/applications/herald/controller/HeraldTranscriptController.php b/src/applications/herald/controller/HeraldTranscriptController.php --- a/src/applications/herald/controller/HeraldTranscriptController.php +++ b/src/applications/herald/controller/HeraldTranscriptController.php @@ -116,6 +116,9 @@ } protected function renderConditionTestValue($condition, $handles) { + // TODO: This is all a hacky mess and should be driven through FieldValue + // eventually. + switch ($condition->getFieldName()) { case HeraldAnotherRuleField::FIELDCONST: $value = array($condition->getTestValue()); @@ -128,14 +131,12 @@ if (!is_scalar($value) && $value !== null) { foreach ($value as $key => $phid) { $handle = idx($handles, $phid); - if ($handle) { + if ($handle && $handle->isComplete()) { $value[$key] = $handle->getName(); } else { - // This shouldn't ever really happen as we are supposed to have - // grabbed handles for everything, but be super liberal in what - // we accept here since we expect all sorts of weird issues as we - // version the system. - $value[$key] = pht('Unknown Object #%s', $phid); + // This happens for things like task priorities, statuses, and + // custom fields. + $value[$key] = $phid; } } sort($value); 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 @@ -24,6 +24,10 @@ throw new PhutilMethodNotImplementedException(); } + protected function getDatasourceValueMap() { + return null; + } + public function getHeraldFieldConditions() { $standard_type = $this->getHeraldFieldStandardType(); switch ($standard_type) { @@ -103,9 +107,16 @@ case HeraldAdapter::CONDITION_NOT_EXISTS: return new HeraldEmptyFieldValue(); default: - return id(new HeraldTokenizerFieldValue()) + $tokenizer = id(new HeraldTokenizerFieldValue()) ->setKey($this->getHeraldFieldName()) ->setDatasource($this->getDatasource()); + + $value_map = $this->getDatasourceValueMap(); + if ($value_map !== null) { + $tokenizer->setValueMap($value_map); + } + + return $tokenizer; } break; @@ -130,50 +141,18 @@ $value) { $value_type = $this->getHeraldFieldValueType($condition); - if ($value_type instanceof HeraldFieldValue) { - $value_type->setViewer($viewer); - return $value_type->renderFieldValue($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); + $value_type->setViewer($viewer); + return $value_type->renderFieldValue($value); } public function getEditorValue( PhabricatorUser $viewer, + $condition, $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; + $value_type = $this->getHeraldFieldValueType($condition); + $value_type->setViewer($viewer); + return $value_type->renderEditorValue($value); } final public function setAdapter(HeraldAdapter $adapter) { diff --git a/src/applications/herald/value/HeraldEmptyFieldValue.php b/src/applications/herald/value/HeraldEmptyFieldValue.php --- a/src/applications/herald/value/HeraldEmptyFieldValue.php +++ b/src/applications/herald/value/HeraldEmptyFieldValue.php @@ -15,4 +15,8 @@ return null; } + public function renderEditorValue($value) { + return null; + } + } diff --git a/src/applications/herald/value/HeraldFieldValue.php b/src/applications/herald/value/HeraldFieldValue.php --- a/src/applications/herald/value/HeraldFieldValue.php +++ b/src/applications/herald/value/HeraldFieldValue.php @@ -12,6 +12,7 @@ abstract public function getFieldValueKey(); abstract public function getControlType(); abstract public function renderFieldValue($value); + abstract public function renderEditorValue($value); public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; diff --git a/src/applications/herald/value/HeraldSelectFieldValue.php b/src/applications/herald/value/HeraldSelectFieldValue.php --- a/src/applications/herald/value/HeraldSelectFieldValue.php +++ b/src/applications/herald/value/HeraldSelectFieldValue.php @@ -61,4 +61,8 @@ return idx($options, $value, $value); } + public function renderEditorValue($value) { + return $value; + } + } diff --git a/src/applications/herald/value/HeraldTextFieldValue.php b/src/applications/herald/value/HeraldTextFieldValue.php --- a/src/applications/herald/value/HeraldTextFieldValue.php +++ b/src/applications/herald/value/HeraldTextFieldValue.php @@ -11,9 +11,12 @@ return self::CONTROL_TEXT; } - public function renderFieldValue($value) { return $value; } + public function renderEditorValue($value) { + return $value; + } + } diff --git a/src/applications/herald/value/HeraldTokenizerFieldValue.php b/src/applications/herald/value/HeraldTokenizerFieldValue.php --- a/src/applications/herald/value/HeraldTokenizerFieldValue.php +++ b/src/applications/herald/value/HeraldTokenizerFieldValue.php @@ -5,6 +5,7 @@ private $key; private $datasource; + private $valueMap; public function setKey($key) { $this->key = $key; @@ -24,6 +25,15 @@ return $this->datasource; } + public function setValueMap(array $value_map) { + $this->valueMap = $value_map; + return $this; + } + + public function getValueMap() { + return $this->valueMap; + } + public function getFieldValueKey() { if ($this->getKey() === null) { throw new PhutilInvalidStateException('setKey'); @@ -54,7 +64,40 @@ public function renderFieldValue($value) { $viewer = $this->getViewer(); + $value = (array)$value; + + if ($this->valueMap !== null) { + foreach ($value as $k => $v) { + $value[$k] = idx($this->valueMap, $v, $v); + } + return implode(', ', $value); + } + return $viewer->renderHandleList((array)$value)->setAsInline(true); } + public function renderEditorValue($value) { + $viewer = $this->getViewer(); + $value = (array)$value; + + // TODO: This should eventually render properly through the datasource + // to get icons and colors. + + if ($this->valueMap !== null) { + $map = array(); + foreach ($value as $v) { + $map[$v] = idx($this->valueMap, $v, $v); + } + return $map; + } + + $handles = $viewer->loadHandles($value); + + $map = array(); + foreach ($value as $v) { + $map[$v] = $handles[$v]->getName(); + } + return $map; + } + } diff --git a/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php --- a/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php +++ b/src/applications/maniphest/herald/ManiphestTaskPriorityHeraldField.php @@ -21,36 +21,8 @@ return new ManiphestTaskPriorityDatasource(); } - public function renderConditionValue( - PhabricatorUser $viewer, - $condition, - $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; + protected function getDatasourceValueMap() { + return ManiphestTaskPriority::getTaskPriorityMap(); } } diff --git a/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php --- a/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php +++ b/src/applications/maniphest/herald/ManiphestTaskStatusHeraldField.php @@ -21,36 +21,8 @@ return new ManiphestTaskStatusDatasource(); } - public function renderConditionValue( - PhabricatorUser $viewer, - $condition, - $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; + protected function getDatasourceValueMap() { + return ManiphestTaskStatus::getTaskStatusMap(); } } diff --git a/src/infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php b/src/infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php new file mode 100644 --- /dev/null +++ b/src/infrastructure/customfield/datasource/PhabricatorStandardSelectCustomFieldDatasource.php @@ -0,0 +1,93 @@ +getViewer(); + + $class = $this->getParameter('object'); + if (!class_exists($class)) { + throw new Exception( + pht( + 'Custom field class "%s" does not exist.', + $class)); + } + + $reflection = new ReflectionClass($class); + $interface = 'PhabricatorCustomFieldInterface'; + if (!$reflection->implementsInterface($interface)) { + throw new Exception( + pht( + 'Custom field class "%s" does not implement interface "%s".', + $class, + $interface)); + } + + $role = $this->getParameter('role'); + if (!strlen($role)) { + throw new Exception(pht('No custom field role specified.')); + } + + $object = newv($class, array()); + $field_list = PhabricatorCustomField::getObjectFields($object, $role); + + $field_key = $this->getParameter('key'); + if (!strlen($field_key)) { + throw new Exception(pht('No custom field key specified.')); + } + + $field = null; + foreach ($field_list->getFields() as $candidate_field) { + if ($candidate_field->getFieldKey() == $field_key) { + $field = $candidate_field; + break; + } + } + + if ($field === null) { + throw new Exception( + pht( + 'No field with field key "%s" exists for objects of class "%s" with '. + 'custom field role "%s".', + $field_key, + $class, + $role)); + } + + if (!($field instanceof PhabricatorStandardCustomFieldSelect)) { + $field = $field->getProxy(); + if (!($field instanceof PhabricatorStandardCustomFieldSelect)) { + throw new Exception( + pht( + 'Field "%s" is not a standard select field, nor a proxy of a '. + 'standard select field.', + $field_key)); + } + } + + $options = $field->getOptions(); + + $results = array(); + foreach ($options as $key => $option) { + $results[] = id(new PhabricatorTypeaheadResult()) + ->setName($option) + ->setPHID($key); + } + + return $this->filterResultsAgainstTokens($results); + } + +} diff --git a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php --- a/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php +++ b/src/infrastructure/customfield/standard/PhabricatorStandardCustomFieldSelect.php @@ -59,7 +59,7 @@ $form->appendChild($control); } - private function getOptions() { + public function getOptions() { return $this->getFieldConfigValue('options', array()); } @@ -79,7 +79,6 @@ return idx($this->getOptions(), $this->getFieldValue()); } - public function getApplicationTransactionTitle( PhabricatorApplicationTransaction $xaction) { $author_phid = $xaction->getAuthorPHID(); @@ -110,4 +109,31 @@ } } + public function shouldAppearInHerald() { + return true; + } + + public function getHeraldFieldConditions() { + return array( + HeraldAdapter::CONDITION_IS_ANY, + HeraldAdapter::CONDITION_IS_NOT_ANY, + ); + } + + public function getHeraldFieldValueType($condition) { + $parameters = array( + 'object' => get_class($this->getObject()), + 'role' => PhabricatorCustomField::ROLE_HERALD, + 'key' => $this->getFieldKey(), + ); + + $datasource = id(new PhabricatorStandardSelectCustomFieldDatasource()) + ->setParameters($parameters); + + return id(new HeraldTokenizerFieldValue()) + ->setKey('custom.'.$this->getFieldKey()) + ->setDatasource($datasource) + ->setValueMap($this->getOptions()); + } + }