mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-26 16:52:41 +01:00
Fix a bug with setting custom PHID list field values via Conduit and prepare for bulk edits
Summary: Ref T13025. Custom field transactions work somewhat unusually: the values sometimes need to be encoded. We currently do not apply this encoding correctly via Conduit. For example, setting some custom PHID field to `["PHID-X-Y"]` fails with a bunch of JSON errors. Add an extra hook callback so that EditTypes can apply processing to transaction values, then apply the correct CustomField processing. This only affects Conduit. In a future diff, this also allows bulk edit of custom fields to work correctly. Test Plan: Added a custom field to Maniphest with a list of projects. Used Conduit to bulk edit it (which now works, but did not before). Used the web UI to bulk edit it. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13025 Differential Revision: https://secure.phabricator.com/D18876
This commit is contained in:
parent
1e51f1d0dc
commit
a26cf20dd1
3 changed files with 32 additions and 4 deletions
|
@ -2169,6 +2169,8 @@ abstract class PhabricatorEditEngine
|
||||||
->setTransactionType(PhabricatorTransactions::TYPE_CREATE);
|
->setTransactionType(PhabricatorTransactions::TYPE_CREATE);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
$is_strict = $request->getIsStrictlyTyped();
|
||||||
|
|
||||||
foreach ($xactions as $xaction) {
|
foreach ($xactions as $xaction) {
|
||||||
$type = $types[$xaction['type']];
|
$type = $types[$xaction['type']];
|
||||||
|
|
||||||
|
@ -2179,10 +2181,10 @@ abstract class PhabricatorEditEngine
|
||||||
$parameter_type->setViewer($viewer);
|
$parameter_type->setViewer($viewer);
|
||||||
|
|
||||||
try {
|
try {
|
||||||
$xaction['value'] = $parameter_type->getValue(
|
$value = $xaction['value'];
|
||||||
$xaction,
|
$value = $parameter_type->getValue($xaction, 'value', $is_strict);
|
||||||
'value',
|
$value = $type->getTransactionValueFromConduit($value);
|
||||||
$request->getIsStrictlyTyped());
|
$xaction['value'] = $value;
|
||||||
} catch (Exception $ex) {
|
} catch (Exception $ex) {
|
||||||
throw new PhutilProxyException(
|
throw new PhutilProxyException(
|
||||||
pht(
|
pht(
|
||||||
|
@ -2498,6 +2500,10 @@ abstract class PhabricatorEditEngine
|
||||||
// but it's possible that this isn't the case.
|
// but it's possible that this isn't the case.
|
||||||
$xaction['type'] = $edit_type->getTransactionType();
|
$xaction['type'] = $edit_type->getTransactionType();
|
||||||
|
|
||||||
|
$value = $xaction['value'];
|
||||||
|
$value = $edit_type->getTransactionValueFromBulkEdit($value);
|
||||||
|
$xaction['value'] = $value;
|
||||||
|
|
||||||
$xaction_objects = $edit_type->generateTransactions(
|
$xaction_objects = $edit_type->generateTransactions(
|
||||||
clone $template,
|
clone $template,
|
||||||
$xaction);
|
$xaction);
|
||||||
|
|
|
@ -91,6 +91,10 @@ abstract class PhabricatorEditType extends Phobject {
|
||||||
return $this->editField;
|
return $this->editField;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getTransactionValueFromValue($value) {
|
||||||
|
return $value;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( Bulk )--------------------------------------------------------------- */
|
/* -( Bulk )--------------------------------------------------------------- */
|
||||||
|
|
||||||
|
@ -114,6 +118,10 @@ abstract class PhabricatorEditType extends Phobject {
|
||||||
return $this->newBulkParameterType();
|
return $this->newBulkParameterType();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getTransactionValueFromBulkEdit($value) {
|
||||||
|
return $this->getTransactionValueFromValue($value);
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( Conduit )------------------------------------------------------------ */
|
/* -( Conduit )------------------------------------------------------------ */
|
||||||
|
|
||||||
|
@ -192,4 +200,8 @@ abstract class PhabricatorEditType extends Phobject {
|
||||||
return $this->conduitDocumentation;
|
return $this->conduitDocumentation;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function getTransactionValueFromConduit($value) {
|
||||||
|
return $this->getTransactionValueFromValue($value);
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -40,4 +40,14 @@ final class PhabricatorCustomFieldEditType
|
||||||
return array($xaction);
|
return array($xaction);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
protected function getTransactionValueFromValue($value) {
|
||||||
|
$field = $this->getCustomField();
|
||||||
|
|
||||||
|
// Avoid changing the value of the field itself, since later calls would
|
||||||
|
// incorrectly reflect the new value.
|
||||||
|
$clone = clone $field;
|
||||||
|
$clone->setValueFromApplicationTransactions($value);
|
||||||
|
return $clone->getNewValueForApplicationTransactions();
|
||||||
|
}
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue