From f91e94eb90f9bc856b1e3c7103aa6238295e2c23 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 21 Feb 2014 11:53:48 -0800 Subject: [PATCH] Implement view and edit policies in Differential CustomFields Summary: Ref T3886. Ref T418. - Adds "View Policy" and "Edit Policy" fields. - Allows CustomFields to produce arbitrary types of transactions, so these fields can produce standard view/edit policy transactions and get all the strings and validation associated with them. Test Plan: {F116001} Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T418, T3886 Differential Revision: https://secure.phabricator.com/D8287 --- src/__phutil_library_map__.php | 4 ++ .../DifferentialEditPolicyField.php | 56 +++++++++++++++++++ .../DifferentialViewPolicyField.php | 56 +++++++++++++++++++ .../editor/DifferentialTransactionEditor.php | 21 ++++++- .../storage/DifferentialRevision.php | 2 + ...habricatorApplicationTransactionEditor.php | 7 ++- .../field/PhabricatorCustomField.php | 11 ++++ .../field/PhabricatorCustomFieldList.php | 10 +++- 8 files changed, 162 insertions(+), 5 deletions(-) create mode 100644 src/applications/differential/customfield/DifferentialEditPolicyField.php create mode 100644 src/applications/differential/customfield/DifferentialViewPolicyField.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 2ff259ee0c..680f7e098d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -381,6 +381,7 @@ phutil_register_library_map(array( 'DifferentialDiffViewPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialDiffViewPolicyFieldSpecification.php', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'applications/differential/doorkeeper/DifferentialDoorkeeperRevisionFeedStoryPublisher.php', 'DifferentialDraft' => 'applications/differential/storage/DifferentialDraft.php', + 'DifferentialEditPolicyField' => 'applications/differential/customfield/DifferentialEditPolicyField.php', 'DifferentialEditPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialEditPolicyFieldSpecification.php', 'DifferentialException' => 'applications/differential/exception/DifferentialException.php', 'DifferentialExceptionMail' => 'applications/differential/mail/DifferentialExceptionMail.php', @@ -478,6 +479,7 @@ phutil_register_library_map(array( 'DifferentialUnitFieldSpecification' => 'applications/differential/field/specification/DifferentialUnitFieldSpecification.php', 'DifferentialUnitStatus' => 'applications/differential/constants/DifferentialUnitStatus.php', 'DifferentialUnitTestResult' => 'applications/differential/constants/DifferentialUnitTestResult.php', + 'DifferentialViewPolicyField' => 'applications/differential/customfield/DifferentialViewPolicyField.php', 'DifferentialViewPolicyFieldSpecification' => 'applications/differential/field/specification/DifferentialViewPolicyFieldSpecification.php', 'DiffusionBranchTableController' => 'applications/diffusion/controller/DiffusionBranchTableController.php', 'DiffusionBranchTableView' => 'applications/diffusion/view/DiffusionBranchTableView.php', @@ -2926,6 +2928,7 @@ phutil_register_library_map(array( 'DifferentialDiffViewPolicyFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialDoorkeeperRevisionFeedStoryPublisher' => 'DoorkeeperFeedStoryPublisher', 'DifferentialDraft' => 'DifferentialDAO', + 'DifferentialEditPolicyField' => 'DifferentialCoreCustomField', 'DifferentialEditPolicyFieldSpecification' => 'DifferentialFieldSpecification', 'DifferentialException' => 'Exception', 'DifferentialExceptionMail' => 'DifferentialMail', @@ -3021,6 +3024,7 @@ phutil_register_library_map(array( 'DifferentialTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'DifferentialTransactionView' => 'PhabricatorApplicationTransactionView', 'DifferentialUnitFieldSpecification' => 'DifferentialFieldSpecification', + 'DifferentialViewPolicyField' => 'DifferentialCoreCustomField', 'DifferentialViewPolicyFieldSpecification' => 'DifferentialFieldSpecification', 'DiffusionBranchTableController' => 'DiffusionController', 'DiffusionBranchTableView' => 'DiffusionView', diff --git a/src/applications/differential/customfield/DifferentialEditPolicyField.php b/src/applications/differential/customfield/DifferentialEditPolicyField.php new file mode 100644 index 0000000000..cede24c3c3 --- /dev/null +++ b/src/applications/differential/customfield/DifferentialEditPolicyField.php @@ -0,0 +1,56 @@ +getEditPolicy(); + } + + protected function writeValueToRevision( + DifferentialRevision $revision, + $value) { + $revision->setEditPolicy($value); + } + + public function readValueFromRequest(AphrontRequest $request) { + $this->setValue($request->getStr($this->getFieldKey())); + } + + public function renderEditControl(array $handles) { + $viewer = $this->getViewer(); + $revision = $this->getObject(); + + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($revision) + ->execute(); + + return id(new AphrontFormPolicyControl()) + ->setUser($viewer) + ->setCapability(PhabricatorPolicyCapability::CAN_EDIT) + ->setPolicyObject($revision) + ->setPolicies($policies) + ->setName($this->getFieldKey()) + ->setValue($this->getValue()) + ->setError($this->getFieldError()); + } + + public function getApplicationTransactionType() { + return PhabricatorTransactions::TYPE_EDIT_POLICY; + } + +} diff --git a/src/applications/differential/customfield/DifferentialViewPolicyField.php b/src/applications/differential/customfield/DifferentialViewPolicyField.php new file mode 100644 index 0000000000..2c3c8c8d4e --- /dev/null +++ b/src/applications/differential/customfield/DifferentialViewPolicyField.php @@ -0,0 +1,56 @@ +getViewPolicy(); + } + + protected function writeValueToRevision( + DifferentialRevision $revision, + $value) { + $revision->setViewPolicy($value); + } + + public function readValueFromRequest(AphrontRequest $request) { + $this->setValue($request->getStr($this->getFieldKey())); + } + + public function renderEditControl(array $handles) { + $viewer = $this->getViewer(); + $revision = $this->getObject(); + + $policies = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->setObject($revision) + ->execute(); + + return id(new AphrontFormPolicyControl()) + ->setUser($viewer) + ->setCapability(PhabricatorPolicyCapability::CAN_VIEW) + ->setPolicyObject($revision) + ->setPolicies($policies) + ->setName($this->getFieldKey()) + ->setValue($this->getValue()) + ->setError($this->getFieldError()); + } + + public function getApplicationTransactionType() { + return PhabricatorTransactions::TYPE_VIEW_POLICY; + } + +} diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php index 70743468dd..68f299c773 100644 --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -6,11 +6,12 @@ final class DifferentialTransactionEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); -/* - $types[] = PhabricatorTransactions::TYPE_EDGE; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; +/* + $types[] = PhabricatorTransactions::TYPE_EDGE; + $types[] = DifferentialTransaction::TYPE_INLINE; $types[] = DifferentialTransaction::TYPE_UPDATE; $types[] = DifferentialTransaction::TYPE_ACTION; @@ -24,6 +25,10 @@ final class DifferentialTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + return $object->getViewPolicy(); + case PhabricatorTransactions::TYPE_EDIT_POLICY: + return $object->getEditPolicy(); } return parent::getCustomTransactionOldValue($object, $xaction); @@ -34,6 +39,9 @@ final class DifferentialTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + case PhabricatorTransactions::TYPE_EDIT_POLICY: + return $xaction->getNewValue(); } return parent::getCustomTransactionNewValue($object, $xaction); @@ -44,6 +52,12 @@ final class DifferentialTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + $object->setViewPolicy($xaction->getNewValue()); + return; + case PhabricatorTransactions::TYPE_EDIT_POLICY: + $object->setEditPolicy($xaction->getNewValue()); + return; } return parent::applyCustomInternalTransaction($object, $xaction); @@ -54,6 +68,9 @@ final class DifferentialTransactionEditor PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { + case PhabricatorTransactions::TYPE_VIEW_POLICY: + case PhabricatorTransactions::TYPE_EDIT_POLICY: + return; } return parent::applyCustomExternalTransaction($object, $xaction); diff --git a/src/applications/differential/storage/DifferentialRevision.php b/src/applications/differential/storage/DifferentialRevision.php index bdfa201017..64bf43473b 100644 --- a/src/applications/differential/storage/DifferentialRevision.php +++ b/src/applications/differential/storage/DifferentialRevision.php @@ -470,6 +470,8 @@ final class DifferentialRevision extends DifferentialDAO new DifferentialSummaryField(), new DifferentialTestPlanField(), new DifferentialRepositoryField(), + new DifferentialViewPolicyField(), + new DifferentialEditPolicyField(), ); return array_fill_keys( diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php index fa30680855..6217763d07 100644 --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -738,7 +738,12 @@ abstract class PhabricatorApplicationTransactionEditor $type = $xaction->getTransactionType(); if (empty($types[$type])) { - throw new Exception("Transaction has unknown type '{$type}'."); + throw new Exception( + pht( + 'Transaction has type "%s", but that transaction type is not '. + 'supported by this editor (%s).', + $type, + get_class($this))); } } diff --git a/src/infrastructure/customfield/field/PhabricatorCustomField.php b/src/infrastructure/customfield/field/PhabricatorCustomField.php index 6b07434480..1167c49ab8 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomField.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomField.php @@ -733,6 +733,17 @@ abstract class PhabricatorCustomField { } + /** + * @task appxaction + */ + public function getApplicationTransactionType() { + if ($this->proxy) { + return $this->proxy->getApplicationTransactionType(); + } + return PhabricatorTransactions::TYPE_CUSTOMFIELD; + } + + /** * @task appxaction */ diff --git a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php index c590365f40..5b184aca5c 100644 --- a/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php +++ b/src/infrastructure/customfield/field/PhabricatorCustomFieldList.php @@ -202,13 +202,19 @@ final class PhabricatorCustomFieldList extends Phobject { $old_value = $field->getOldValueForApplicationTransactions(); $field->readValueFromRequest($request); + $transaction_type = $field->getApplicationTransactionType(); $xaction = id(clone $template) - ->setTransactionType(PhabricatorTransactions::TYPE_CUSTOMFIELD) + ->setTransactionType($transaction_type) ->setMetadataValue('customfield:key', $field->getFieldKey()) - ->setOldValue($old_value) ->setNewValue($field->getNewValueForApplicationTransactions()); + if ($transaction_type == PhabricatorTransactions::TYPE_CUSTOMFIELD) { + // For TYPE_CUSTOMFIELD transactions only, we provide the old value + // as an input. + $xaction->setOldValue($old_value); + } + $xactions[] = $xaction; }