From 5ada1211cdeb978a9c2e3689241382fc5fefc730 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 10 Apr 2018 04:43:51 -0700 Subject: [PATCH] Modularize Almanac Namespace transactions Summary: Depends on D19318. Ref T13120. Ref T12414. Move transactions for Almanac Namespaces ("name" is the only meaningful one) to ModularTransactions. Test Plan: - Created a new namespace. - Edited a namespace. - Tried to choose no name, an invalid name, a duplicate name, and a name in a namespace I can't edit; got appropriate errors. - Grepped for `AlmanacNamespaceTransaction::TYPE_NAME`. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13120, T12414 Differential Revision: https://secure.phabricator.com/D19320 --- src/__phutil_library_map__.php | 6 +- .../editor/AlmanacNamespaceEditEngine.php | 2 +- .../almanac/editor/AlmanacNamespaceEditor.php | 145 +----------------- .../storage/AlmanacNamespaceTransaction.php | 27 +--- .../AlmanacNamespaceNameTransaction.php | 91 +++++++++++ .../AlmanacNamespaceTransactionType.php | 4 + 6 files changed, 112 insertions(+), 163 deletions(-) create mode 100644 src/applications/almanac/xaction/AlmanacNamespaceNameTransaction.php create mode 100644 src/applications/almanac/xaction/AlmanacNamespaceTransactionType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 5d68c3b0c6..57e58ac74e 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -75,11 +75,13 @@ phutil_register_library_map(array( 'AlmanacNamespaceEditor' => 'applications/almanac/editor/AlmanacNamespaceEditor.php', 'AlmanacNamespaceListController' => 'applications/almanac/controller/AlmanacNamespaceListController.php', 'AlmanacNamespaceNameNgrams' => 'applications/almanac/storage/AlmanacNamespaceNameNgrams.php', + 'AlmanacNamespaceNameTransaction' => 'applications/almanac/xaction/AlmanacNamespaceNameTransaction.php', 'AlmanacNamespacePHIDType' => 'applications/almanac/phid/AlmanacNamespacePHIDType.php', 'AlmanacNamespaceQuery' => 'applications/almanac/query/AlmanacNamespaceQuery.php', 'AlmanacNamespaceSearchEngine' => 'applications/almanac/query/AlmanacNamespaceSearchEngine.php', 'AlmanacNamespaceTransaction' => 'applications/almanac/storage/AlmanacNamespaceTransaction.php', 'AlmanacNamespaceTransactionQuery' => 'applications/almanac/query/AlmanacNamespaceTransactionQuery.php', + 'AlmanacNamespaceTransactionType' => 'applications/almanac/xaction/AlmanacNamespaceTransactionType.php', 'AlmanacNamespaceViewController' => 'applications/almanac/controller/AlmanacNamespaceViewController.php', 'AlmanacNetwork' => 'applications/almanac/storage/AlmanacNetwork.php', 'AlmanacNetworkController' => 'applications/almanac/controller/AlmanacNetworkController.php', @@ -5263,11 +5265,13 @@ phutil_register_library_map(array( 'AlmanacNamespaceEditor' => 'PhabricatorApplicationTransactionEditor', 'AlmanacNamespaceListController' => 'AlmanacNamespaceController', 'AlmanacNamespaceNameNgrams' => 'PhabricatorSearchNgrams', + 'AlmanacNamespaceNameTransaction' => 'AlmanacNamespaceTransactionType', 'AlmanacNamespacePHIDType' => 'PhabricatorPHIDType', 'AlmanacNamespaceQuery' => 'AlmanacQuery', 'AlmanacNamespaceSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'AlmanacNamespaceTransaction' => 'PhabricatorApplicationTransaction', + 'AlmanacNamespaceTransaction' => 'PhabricatorModularTransaction', 'AlmanacNamespaceTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'AlmanacNamespaceTransactionType' => 'AlmanacTransactionType', 'AlmanacNamespaceViewController' => 'AlmanacNamespaceController', 'AlmanacNetwork' => array( 'AlmanacDAO', diff --git a/src/applications/almanac/editor/AlmanacNamespaceEditEngine.php b/src/applications/almanac/editor/AlmanacNamespaceEditEngine.php index 3b8c4ba2a6..cf6ad36c00 100644 --- a/src/applications/almanac/editor/AlmanacNamespaceEditEngine.php +++ b/src/applications/almanac/editor/AlmanacNamespaceEditEngine.php @@ -81,7 +81,7 @@ final class AlmanacNamespaceEditEngine ->setKey('name') ->setLabel(pht('Name')) ->setDescription(pht('Name of the namespace.')) - ->setTransactionType(AlmanacNamespaceTransaction::TYPE_NAME) + ->setTransactionType(AlmanacNamespaceNameTransaction::TRANSACTIONTYPE) ->setIsRequired(true) ->setValue($object->getName()), ); diff --git a/src/applications/almanac/editor/AlmanacNamespaceEditor.php b/src/applications/almanac/editor/AlmanacNamespaceEditor.php index 5243d3c935..93d65e62b1 100644 --- a/src/applications/almanac/editor/AlmanacNamespaceEditor.php +++ b/src/applications/almanac/editor/AlmanacNamespaceEditor.php @@ -11,6 +11,14 @@ final class AlmanacNamespaceEditor return pht('Almanac Namespace'); } + public function getCreateObjectTitle($author, $object) { + return pht('%s created this namespace.', $author); + } + + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created %s.', $author, $object); + } + protected function supportsSearch() { return true; } @@ -18,149 +26,12 @@ final class AlmanacNamespaceEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = AlmanacNamespaceTransaction::TYPE_NAME; $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - switch ($xaction->getTransactionType()) { - case AlmanacNamespaceTransaction::TYPE_NAME: - return $object->getName(); - } - - return parent::getCustomTransactionOldValue($object, $xaction); - } - - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case AlmanacNamespaceTransaction::TYPE_NAME: - return $xaction->getNewValue(); - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case AlmanacNamespaceTransaction::TYPE_NAME: - $object->setName($xaction->getNewValue()); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case AlmanacNamespaceTransaction::TYPE_NAME: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case AlmanacNamespaceTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Namespace name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } else { - foreach ($xactions as $xaction) { - $name = $xaction->getNewValue(); - - $message = null; - try { - AlmanacNames::validateName($name); - } catch (Exception $ex) { - $message = $ex->getMessage(); - } - - if ($message !== null) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - $message, - $xaction); - $errors[] = $error; - continue; - } - - $other = id(new AlmanacNamespaceQuery()) - ->setViewer(PhabricatorUser::getOmnipotentUser()) - ->withNames(array($name)) - ->executeOne(); - if ($other && ($other->getID() != $object->getID())) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Not Unique'), - pht( - 'The namespace name "%s" is already in use by another '. - 'namespace. Each namespace must have a unique name.', - $name), - $xaction); - $errors[] = $error; - continue; - } - - if ($name === $object->getName()) { - continue; - } - - $namespace = AlmanacNamespace::loadRestrictedNamespace( - $this->getActor(), - $name); - if ($namespace) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Restricted'), - pht( - 'You do not have permission to create Almanac namespaces '. - 'within the "%s" namespace.', - $namespace->getName()), - $xaction); - $errors[] = $error; - continue; - } - } - } - - break; - } - - return $errors; - } - protected function didCatchDuplicateKeyException( PhabricatorLiskDAO $object, array $xactions, diff --git a/src/applications/almanac/storage/AlmanacNamespaceTransaction.php b/src/applications/almanac/storage/AlmanacNamespaceTransaction.php index 9461fa5f13..9a6f10e1de 100644 --- a/src/applications/almanac/storage/AlmanacNamespaceTransaction.php +++ b/src/applications/almanac/storage/AlmanacNamespaceTransaction.php @@ -1,9 +1,7 @@ getAuthorPHID(); - - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - switch ($this->getTransactionType()) { - case PhabricatorTransactions::TYPE_CREATE: - return pht( - '%s created this namespace.', - $this->renderHandleLink($author_phid)); - break; - case self::TYPE_NAME: - return pht( - '%s renamed this namespace from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } - - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'AlmanacNamespaceTransactionType'; } } diff --git a/src/applications/almanac/xaction/AlmanacNamespaceNameTransaction.php b/src/applications/almanac/xaction/AlmanacNamespaceNameTransaction.php new file mode 100644 index 0000000000..dfb187db89 --- /dev/null +++ b/src/applications/almanac/xaction/AlmanacNamespaceNameTransaction.php @@ -0,0 +1,91 @@ +getName(); + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + } + + public function getTitle() { + return pht( + '%s renamed this namespace from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function getTitleForFeed() { + return pht( + '%s renamed %s from %s to %s.', + $this->renderAuthor(), + $this->renderObject(), + $this->renderOldValue(), + $this->renderNewValue()); + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if ($this->isEmptyTextTransaction($object->getName(), $xactions)) { + $errors[] = $this->newRequiredError( + pht('Namespace name is required.')); + } + + foreach ($xactions as $xaction) { + $name = $xaction->getNewValue(); + + $message = null; + try { + AlmanacNames::validateName($name); + } catch (Exception $ex) { + $message = $ex->getMessage(); + } + + if ($message !== null) { + $errors[] = $this->newInvalidError($message, $xaction); + continue; + } + + if ($name === $object->getName()) { + continue; + } + + $other = id(new AlmanacNamespaceQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->withNames(array($name)) + ->executeOne(); + if ($other && ($other->getID() != $object->getID())) { + $errors[] = $this->newInvalidError( + pht( + 'The namespace name "%s" is already in use by another '. + 'namespace. Each namespace must have a unique name.', + $name), + $xaction); + continue; + } + + $namespace = AlmanacNamespace::loadRestrictedNamespace( + $this->getActor(), + $name); + if ($namespace) { + $errors[] = $this->newInvalidError( + pht( + 'You do not have permission to create Almanac namespaces '. + 'within the "%s" namespace.', + $namespace->getName()), + $xaction); + continue; + } + } + + return $errors; + } + +} diff --git a/src/applications/almanac/xaction/AlmanacNamespaceTransactionType.php b/src/applications/almanac/xaction/AlmanacNamespaceTransactionType.php new file mode 100644 index 0000000000..fee3d6d1eb --- /dev/null +++ b/src/applications/almanac/xaction/AlmanacNamespaceTransactionType.php @@ -0,0 +1,4 @@ +