Currently, subclasses of PhabricatorCustomField that return PhabricatorTransactions::TYPE_EDGE from getApplicationTransactionType can't apply custom validation logic using validateApplicationTransactions. This diff adds support for doing so.
Details
Details
- Reviewers
epriestley - Group Reviewers
Blessed Reviewers
Plonked the following classes in src/extensions and verified that attempting to modify a project resulted in bogus validation errors.
src/extensions/PhabricatorExampleEdgeType.php
<?php final class PhabricatorExampleEdgeType extends PhabricatorEdgeType { const EDGECONST = 9999; }
src/extensions/PhabricatorProjectExampleCustomField.php
<?php final class PhabricatorProjectExampleCustomField extends PhabricatorProjectCustomField { private $value = []; protected function getDatasource(): PhabricatorTypeaheadDatasource { return new PhabricatorProjectDatasource(); } protected function getApplicationTransactionEdgeType(): PhabricatorEdgeType { return new PhabricatorExampleEdgeType(); } public function getValue(): array { return $this->value; } public function setValue(array $value): void { $this->value = $value; } /* -( PhabricatorCustomField )--------------------------------------------- */ public function getFieldKey(): string { return 'example'; } public function getFieldName(): string { return pht('Example'); } public function readValueFromObject(PhabricatorCustomFieldInterface $object): void { $edges = PhabricatorEdgeQuery::loadDestinationPHIDs( $this->getObject()->getPHID(), $this->getApplicationTransactionEdgeConstant()); $this->setValue($edges); } public function getValueForStorage(): array { return ['=' => array_fuse($this->getValue())]; } public function setValueFromStorage($value): void { $this->setValue(array_values($value['='])); } public function shouldAppearInApplicationTransactions(): bool { return true; } public function getApplicationTransactionType(): string { return PhabricatorTransactions::TYPE_EDGE; } public function getApplicationTransactionMetadata(): array { return [ 'edge:type' => $this->getApplicationTransactionEdgeConstant(), ]; } public function validateApplicationTransactions( PhabricatorApplicationTransactionEditor $editor, $type, array $xactions): array { $errors = parent::validateApplicationTransactions( $editor, $type, $xactions); foreach ($xactions as $xaction) { $errors[] = new PhabricatorApplicationTransactionValidationError( $type, pht('Invalid value'), pht('This value is not allowed.'), $xaction); } return $errors; } public function shouldAppearInEditView(): bool { return true; } public function readValueFromRequest(AphrontRequest $request): void { $this->setValue($request->getArr($this->getFieldKey())); } public function getRequiredHandlePHIDsForEdit(): array { return $this->getValue(); } public function renderEditControl(array $handles): AphrontFormControl { return (new AphrontFormTokenizerControl()) ->setViewer($this->getViewer()) ->setLabel($this->getFieldName()) ->setName($this->getFieldKey()) ->setValue($this->getValue()) ->setDatasource($this->getDatasource()); } public function shouldAppearInPropertyView(): bool { return true; } public function renderPropertyViewValue(array $handles): ?PhutilSafeHTML { return $this->renderHandleList($handles); } public function getRequiredHandlePHIDsForPropertyView(): array { return $this->getValue(); } }
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- master
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 22729 Build 31160: Run Core Tests Build 31159: arc lint + arc unit
Event Timeline
Comment Actions
I'm not sure if the upstream would be interested in this change, but I figured there's no harm in trying.
Comment Actions
I like this goal achieved by this change -- it's silly that edge validation isn't really extensible anywhere -- but I think this also digs us deeper into the mess of T13248.
But T13248 is likely to be somewhat disruptive to custom fields anyway, so the harm here is probably quite small, and I'm not sure when it will move forward.