1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-23 22:10:55 +01:00

Improve rendering of "default value changed" custom form transactions to at least have all the information

Summary:
Ref T13319. Currently, transactions about changes to a default form value use a raw internal key for the affected field and don't show the actual value change.

An ideal implementation will likely require us to specialize a great deal of rendering, but we can do much better than we currently do without too much work:

  - Try to pull the actual `EditField` object for the key so we can `getLabel()` it and get a human-readable label (like `Visible To` instead of `policy.view`).
  - Add a "(Show Changes)" action that dumps the raw values as more-or-less JSON, so you can at least figure out what happened if you're sophisticated enough.

Test Plan:
Before:

{F6516640}

After:

{F6516642}

The quality of "Show Details" varies a lot. For some fields, like "Description", it's pretty good:

{F6516645}

For others, like "Assigned To", it's better than nothing but pretty technical:

{F6516647}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13319

Differential Revision: https://secure.phabricator.com/D20594
This commit is contained in:
epriestley 2019-06-19 13:17:00 -07:00
parent caaa1394ef
commit 37e26f1b45

View file

@ -58,10 +58,23 @@ final class PhabricatorEditEngineConfigurationTransaction
$this->renderHandleLink($author_phid));
case self::TYPE_DEFAULT:
$key = $this->getMetadataValue('field.key');
$object = $this->getObject();
$engine = $object->getEngine();
$fields = $engine->getFieldsForConfig($object);
$field = idx($fields, $key);
if (!$field) {
return pht(
'%s changed the default value for field "%s".',
$this->renderHandleLink($author_phid),
$key);
}
return pht(
'%s changed the default value for field "%s".',
$this->renderHandleLink($author_phid),
$key);
$field->getLabel());
case self::TYPE_LOCKS:
return pht(
'%s changed locked and hidden fields.',
@ -164,4 +177,49 @@ final class PhabricatorEditEngineConfigurationTransaction
return $changes;
}
public function hasChangeDetails() {
switch ($this->getTransactionType()) {
case self::TYPE_DEFAULT:
return true;
}
return parent::hasChangeDetails();
}
public function renderChangeDetails(PhabricatorUser $viewer) {
switch ($this->getTransactionType()) {
case self::TYPE_DEFAULT:
$old_value = $this->getOldValue();
$new_value = $this->getNewValue();
$old_value = $this->renderDefaultValueAsFallbackText($old_value);
$new_value = $this->renderDefaultValueAsFallbackText($new_value);
return $this->renderTextCorpusChangeDetails(
$viewer,
$old_value,
$new_value);
}
return parent::renderChangeDetails($viewer);
}
private function renderDefaultValueAsFallbackText($default_value) {
// See T13319. When rendering an "alice changed the default value for
// field X." story on custom forms, we may fall back to a generic
// rendering. Try to present the value change in a comprehensible way
// even if it isn't especially human readable (for example, it may
// contain PHIDs or other internal identifiers).
if (is_scalar($default_value) || is_null($default_value)) {
return $default_value;
}
if (phutil_is_natural_list($default_value)) {
return id(new PhutilJSON())->encodeAsList($default_value);
}
return id(new PhutilJSON())->encodeAsObject($default_value);
}
}