1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 14:52:41 +01:00

Add basic, rough support for changing field behavior based on object subtype

Summary:
Ref T13248. This will probably need quite a bit of refinement, but we can reasonably allow subtype definitions to adjust custom field behavior.

Some places where we use fields are global, and always need to show all the fields. For example, on `/maniphest/`, where you can search across all tasks, you need to be able to search across all fields that are present on any task.

Likewise, if you "export" a bunch of tasks into a spreadsheet, we need to have columns for every field.

However, when you're clearly in the scope of a particular task (like viewing or editing `T123`), there's no reason we can't hide fields based on the task subtype.

To start with, allow subtypes to override "disabled" and "name" for custom fields.

Test Plan:
  - Defined several custom fields and several subtypes.
  - Disabled/renamed some fields for some subtypes.
  - Viewed/edited tasks of different subtypes, got desired field behavior.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13248

Differential Revision: https://secure.phabricator.com/D20161
This commit is contained in:
epriestley 2019-02-13 09:13:46 -08:00
parent 4b10bc2b64
commit 0b2d25778d
5 changed files with 175 additions and 2 deletions

View file

@ -342,6 +342,7 @@ dictionary with these keys:
- `icon` //Optional string.// Icon for the subtype. - `icon` //Optional string.// Icon for the subtype.
- `children` //Optional map.// Configure options shown to the user when - `children` //Optional map.// Configure options shown to the user when
they "Create Subtask". See below. they "Create Subtask". See below.
- `fields` //Optional map.// Configure field behaviors. See below.
Each subtype must have a unique key, and you must define a subtype with Each subtype must have a unique key, and you must define a subtype with
the key "%s", which is used as a default subtype. the key "%s", which is used as a default subtype.
@ -397,6 +398,28 @@ be used when presenting options to the user.
If only one option would be presented, the user will be taken directly to the If only one option would be presented, the user will be taken directly to the
appropriate form instead of being prompted to choose a form. appropriate form instead of being prompted to choose a form.
The `fields` key can configure the behavior of custom fields on specific
task subtypes. For example:
```
{
...
"fields": {
"custom.some-field": {
"disabled": true
}
}
...
}
```
Each field supports these options:
- `disabled` //Optional bool.// Allows you to disable fields on certain
subtypes.
- `name` //Optional string.// Custom name of this field for the subtype.
EOTEXT EOTEXT
, ,
$subtype_default_key)); $subtype_default_key));

View file

@ -165,14 +165,29 @@ abstract class PhabricatorEditEngine
$extensions = array(); $extensions = array();
} }
// See T13248. Create a template object to provide to extensions. We
// adjust the template to have the intended subtype, so that extensions
// may change behavior based on the form subtype.
$template_object = clone $object;
if ($this->getIsCreate()) {
if ($this->supportsSubtypes()) {
$config = $this->getEditEngineConfiguration();
$subtype = $config->getSubtype();
$template_object->setSubtype($subtype);
}
}
foreach ($extensions as $extension) { foreach ($extensions as $extension) {
$extension->setViewer($viewer); $extension->setViewer($viewer);
if (!$extension->supportsObject($this, $object)) { if (!$extension->supportsObject($this, $template_object)) {
continue; continue;
} }
$extension_fields = $extension->buildCustomEditFields($this, $object); $extension_fields = $extension->buildCustomEditFields(
$this,
$template_object);
// TODO: Validate this in more detail with a more tailored error. // TODO: Validate this in more detail with a more tailored error.
assert_instances_of($extension_fields, 'PhabricatorEditField'); assert_instances_of($extension_fields, 'PhabricatorEditField');

View file

@ -13,6 +13,7 @@ final class PhabricatorEditEngineSubtype
private $color; private $color;
private $childSubtypes = array(); private $childSubtypes = array();
private $childIdentifiers = array(); private $childIdentifiers = array();
private $fieldConfiguration = array();
public function setKey($key) { public function setKey($key) {
$this->key = $key; $this->key = $key;
@ -94,6 +95,17 @@ final class PhabricatorEditEngineSubtype
return $view; return $view;
} }
public function setSubtypeFieldConfiguration(
$subtype_key,
array $configuration) {
$this->fieldConfiguration[$subtype_key] = $configuration;
return $this;
}
public function getSubtypeFieldConfiguration($subtype_key) {
return idx($this->fieldConfiguration, $subtype_key);
}
public static function validateSubtypeKey($subtype) { public static function validateSubtypeKey($subtype) {
if (strlen($subtype) > 64) { if (strlen($subtype) > 64) {
throw new Exception( throw new Exception(
@ -139,6 +151,7 @@ final class PhabricatorEditEngineSubtype
'color' => 'optional string', 'color' => 'optional string',
'icon' => 'optional string', 'icon' => 'optional string',
'children' => 'optional map<string, wild>', 'children' => 'optional map<string, wild>',
'fields' => 'optional map<string, wild>',
)); ));
$key = $value['key']; $key = $value['key'];
@ -183,6 +196,18 @@ final class PhabricatorEditEngineSubtype
'or the other, but not both.')); 'or the other, but not both.'));
} }
} }
$fields = idx($value, 'fields');
if ($fields) {
foreach ($fields as $field_key => $configuration) {
PhutilTypeSpec::checkMap(
$configuration,
array(
'disabled' => 'optional bool',
'name' => 'optional string',
));
}
}
} }
if (!isset($map[self::SUBTYPE_DEFAULT])) { if (!isset($map[self::SUBTYPE_DEFAULT])) {
@ -233,6 +258,15 @@ final class PhabricatorEditEngineSubtype
$subtype->setChildFormIdentifiers($child_forms); $subtype->setChildFormIdentifiers($child_forms);
} }
$field_configurations = idx($entry, 'fields');
if ($field_configurations) {
foreach ($field_configurations as $field_key => $field_configuration) {
$subtype->setSubtypeFieldConfiguration(
$field_key,
$field_configuration);
}
}
$map[$key] = $subtype; $map[$key] = $subtype;
} }

View file

@ -74,9 +74,22 @@ abstract class PhabricatorCustomField extends Phobject {
$spec, $spec,
$object); $object);
$fields = self::adjustCustomFieldsForObjectSubtype(
$object,
$role,
$fields);
foreach ($fields as $key => $field) { foreach ($fields as $key => $field) {
// NOTE: We perform this filtering in "buildFieldList()", but may need
// to filter again after subtype adjustment.
if (!$field->isFieldEnabled()) {
unset($fields[$key]);
continue;
}
if (!$field->shouldEnableForRole($role)) { if (!$field->shouldEnableForRole($role)) {
unset($fields[$key]); unset($fields[$key]);
continue;
} }
} }
@ -1622,4 +1635,78 @@ abstract class PhabricatorCustomField extends Phobject {
return null; return null;
} }
private static function adjustCustomFieldsForObjectSubtype(
PhabricatorCustomFieldInterface $object,
$role,
array $fields) {
assert_instances_of($fields, __CLASS__);
// We only apply subtype adjustment for some roles. For example, when
// writing Herald rules or building a Search interface, we always want to
// show all the fields in their default state, so we do not apply any
// adjustments.
$subtype_roles = array(
self::ROLE_EDITENGINE,
self::ROLE_VIEW,
);
$subtype_roles = array_fuse($subtype_roles);
if (!isset($subtype_roles[$role])) {
return $fields;
}
// If the object doesn't support subtypes, we can't possibly make
// any adjustments based on subtype.
if (!($object instanceof PhabricatorEditEngineSubtypeInterface)) {
return $fields;
}
$subtype_map = $object->newEditEngineSubtypeMap();
$subtype_key = $object->getEditEngineSubtype();
$subtype_object = $subtype_map->getSubtype($subtype_key);
$map = array();
foreach ($fields as $field) {
$modern_key = $field->getModernFieldKey();
if (!strlen($modern_key)) {
continue;
}
$map[$modern_key] = $field;
}
foreach ($map as $field_key => $field) {
// For now, only support overriding standard custom fields. In the
// future there's no technical or product reason we couldn't let you
// override (some properites of) other fields like "Title", but they
// don't usually support appropriate "setX()" methods today.
if (!($field instanceof PhabricatorStandardCustomField)) {
// For fields that are proxies on top of StandardCustomField, which
// is how most application custom fields work today, we can reconfigure
// the proxied field instead.
$field = $field->getProxy();
if (!$field || !($field instanceof PhabricatorStandardCustomField)) {
continue;
}
}
$subtype_config = $subtype_object->getSubtypeFieldConfiguration(
$field_key);
if (!$subtype_config) {
continue;
}
if (isset($subtype_config['disabled'])) {
$field->setIsEnabled(!$subtype_config['disabled']);
}
if (isset($subtype_config['name'])) {
$field->setFieldName($subtype_config['name']);
}
}
return $fields;
}
} }

View file

@ -18,6 +18,7 @@ abstract class PhabricatorStandardCustomField
private $isCopyable; private $isCopyable;
private $hasStorageValue; private $hasStorageValue;
private $isBuiltin; private $isBuiltin;
private $isEnabled = true;
abstract public function getFieldType(); abstract public function getFieldType();
@ -175,6 +176,19 @@ abstract class PhabricatorStandardCustomField
return $this->rawKey; return $this->rawKey;
} }
public function setIsEnabled($is_enabled) {
$this->isEnabled = $is_enabled;
return $this;
}
public function getIsEnabled() {
return $this->isEnabled;
}
public function isFieldEnabled() {
return $this->getIsEnabled();
}
/* -( PhabricatorCustomField )--------------------------------------------- */ /* -( PhabricatorCustomField )--------------------------------------------- */