mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Make configured and EditEngine defaults work correctly for custom fields
Summary: Ref T10004. Fixes T5158. There was a long-standing issue with defaults not working properly, but EditEngine has made it more obvious because it's a lot easier to set defaults now. The issue is basically that the defaults are getting set as the field's real value early on, so when we go to generate the transaction "old value" later, we build a transaction that uses the //new// value as both the "new value" and "old value". Then the engine says "you didn't change anything, so I'm going to ignore this" and drops it. To fix this, return `null` as the "old value" by default, and add a call to overwrite that after we load a legitimate old value. This fix is a touch iffy, but I have some grand plans to clean up the CustomField stuff more broadly later on. Test Plan: - Set config defaults on select/typeahead fields, created and edited tasks. - Set form defaults on select/typehaead fields, created and edited tasks. - In all cases, transactions and state accurately reflected edits. - Set defaults on //hidden// fields, verified forms respected them correctly. - This does generate some fluffy transactions, but I'll deal with those in T7661. Reviewers: chad Reviewed By: chad Maniphest Tasks: T5158, T10004 Differential Revision: https://secure.phabricator.com/D14809
This commit is contained in:
parent
38e31375ea
commit
8a0dfa94d4
3 changed files with 23 additions and 0 deletions
|
@ -594,6 +594,13 @@ abstract class PhabricatorCustomField extends Phobject {
|
||||||
throw new PhabricatorCustomFieldImplementationIncompleteException($this);
|
throw new PhabricatorCustomFieldImplementationIncompleteException($this);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function didSetValueFromStorage() {
|
||||||
|
if ($this->proxy) {
|
||||||
|
return $this->proxy->didSetValueFromStorage();
|
||||||
|
}
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
/* -( ApplicationSearch )-------------------------------------------------- */
|
/* -( ApplicationSearch )-------------------------------------------------- */
|
||||||
|
|
||||||
|
|
|
@ -73,10 +73,12 @@ final class PhabricatorCustomFieldList extends Phobject {
|
||||||
$storage = idx($objects, $key);
|
$storage = idx($objects, $key);
|
||||||
if ($storage) {
|
if ($storage) {
|
||||||
$field->setValueFromStorage($storage->getFieldValue());
|
$field->setValueFromStorage($storage->getFieldValue());
|
||||||
|
$field->didSetValueFromStorage();
|
||||||
} else if ($object->getPHID()) {
|
} else if ($object->getPHID()) {
|
||||||
// NOTE: We set this only if the object exists. Otherwise, we allow the
|
// NOTE: We set this only if the object exists. Otherwise, we allow the
|
||||||
// field to retain any default value it may have.
|
// field to retain any default value it may have.
|
||||||
$field->setValueFromStorage(null);
|
$field->setValueFromStorage(null);
|
||||||
|
$field->didSetValueFromStorage();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -16,6 +16,7 @@ abstract class PhabricatorStandardCustomField
|
||||||
private $required;
|
private $required;
|
||||||
private $default;
|
private $default;
|
||||||
private $isCopyable;
|
private $isCopyable;
|
||||||
|
private $hasStorageValue;
|
||||||
|
|
||||||
abstract public function getFieldType();
|
abstract public function getFieldType();
|
||||||
|
|
||||||
|
@ -215,6 +216,19 @@ abstract class PhabricatorStandardCustomField
|
||||||
return $this->setFieldValue($value);
|
return $this->setFieldValue($value);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public function didSetValueFromStorage() {
|
||||||
|
$this->hasStorageValue = true;
|
||||||
|
return $this;
|
||||||
|
}
|
||||||
|
|
||||||
|
public function getOldValueForApplicationTransactions() {
|
||||||
|
if ($this->hasStorageValue) {
|
||||||
|
return $this->getValueForStorage();
|
||||||
|
} else {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public function shouldAppearInApplicationTransactions() {
|
public function shouldAppearInApplicationTransactions() {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue