From d77a3f8760b4c73cfb98af526b597c155a9892c6 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 4 May 2017 21:48:13 +0000 Subject: [PATCH] Update Spaces for modular transactions Summary: Updates the Spaces application for modular transactions, seemed easy to bang out. Test Plan: Create a space, edit a space, archive a space. Verify default space works as intended. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D17829 --- src/__phutil_library_map__.php | 12 +- .../__tests__/PhabricatorSpacesTestCase.php | 6 +- .../PhabricatorSpacesArchiveController.php | 3 +- .../PhabricatorSpacesEditController.php | 9 +- .../PhabricatorSpacesViewController.php | 2 +- .../PhabricatorSpacesNamespaceEditor.php | 143 +----------------- .../PhabricatorSpacesNamespaceTransaction.php | 81 +--------- ...catorSpacesNamespaceArchiveTransaction.php | 60 ++++++++ ...catorSpacesNamespaceDefaultTransaction.php | 44 ++++++ ...rSpacesNamespaceDescriptionTransaction.php | 57 +++++++ ...bricatorSpacesNamespaceNameTransaction.php | 62 ++++++++ ...bricatorSpacesNamespaceTransactionType.php | 4 + 12 files changed, 258 insertions(+), 225 deletions(-) create mode 100644 src/applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php create mode 100644 src/applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php create mode 100644 src/applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php create mode 100644 src/applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php create mode 100644 src/applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index bed1a60426..14305c04da 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -3937,13 +3937,18 @@ phutil_register_library_map(array( 'PhabricatorSpacesInterface' => 'applications/spaces/interface/PhabricatorSpacesInterface.php', 'PhabricatorSpacesListController' => 'applications/spaces/controller/PhabricatorSpacesListController.php', 'PhabricatorSpacesNamespace' => 'applications/spaces/storage/PhabricatorSpacesNamespace.php', + 'PhabricatorSpacesNamespaceArchiveTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php', 'PhabricatorSpacesNamespaceDatasource' => 'applications/spaces/typeahead/PhabricatorSpacesNamespaceDatasource.php', + 'PhabricatorSpacesNamespaceDefaultTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php', + 'PhabricatorSpacesNamespaceDescriptionTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php', 'PhabricatorSpacesNamespaceEditor' => 'applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php', + 'PhabricatorSpacesNamespaceNameTransaction' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php', 'PhabricatorSpacesNamespacePHIDType' => 'applications/spaces/phid/PhabricatorSpacesNamespacePHIDType.php', 'PhabricatorSpacesNamespaceQuery' => 'applications/spaces/query/PhabricatorSpacesNamespaceQuery.php', 'PhabricatorSpacesNamespaceSearchEngine' => 'applications/spaces/query/PhabricatorSpacesNamespaceSearchEngine.php', 'PhabricatorSpacesNamespaceTransaction' => 'applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php', 'PhabricatorSpacesNamespaceTransactionQuery' => 'applications/spaces/query/PhabricatorSpacesNamespaceTransactionQuery.php', + 'PhabricatorSpacesNamespaceTransactionType' => 'applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php', 'PhabricatorSpacesNoAccessController' => 'applications/spaces/controller/PhabricatorSpacesNoAccessController.php', 'PhabricatorSpacesRemarkupRule' => 'applications/spaces/remarkup/PhabricatorSpacesRemarkupRule.php', 'PhabricatorSpacesSchemaSpec' => 'applications/spaces/storage/PhabricatorSpacesSchemaSpec.php', @@ -9373,13 +9378,18 @@ phutil_register_library_map(array( 'PhabricatorApplicationTransactionInterface', 'PhabricatorDestructibleInterface', ), + 'PhabricatorSpacesNamespaceArchiveTransaction' => 'PhabricatorSpacesNamespaceTransactionType', 'PhabricatorSpacesNamespaceDatasource' => 'PhabricatorTypeaheadDatasource', + 'PhabricatorSpacesNamespaceDefaultTransaction' => 'PhabricatorSpacesNamespaceTransactionType', + 'PhabricatorSpacesNamespaceDescriptionTransaction' => 'PhabricatorSpacesNamespaceTransactionType', 'PhabricatorSpacesNamespaceEditor' => 'PhabricatorApplicationTransactionEditor', + 'PhabricatorSpacesNamespaceNameTransaction' => 'PhabricatorSpacesNamespaceTransactionType', 'PhabricatorSpacesNamespacePHIDType' => 'PhabricatorPHIDType', 'PhabricatorSpacesNamespaceQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorSpacesNamespaceSearchEngine' => 'PhabricatorApplicationSearchEngine', - 'PhabricatorSpacesNamespaceTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorSpacesNamespaceTransaction' => 'PhabricatorModularTransaction', 'PhabricatorSpacesNamespaceTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorSpacesNamespaceTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorSpacesNoAccessController' => 'PhabricatorSpacesController', 'PhabricatorSpacesRemarkupRule' => 'PhabricatorObjectRemarkupRule', 'PhabricatorSpacesSchemaSpec' => 'PhabricatorConfigSchemaSpec', diff --git a/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php index 4215ec96fe..b2a6b0b87f 100644 --- a/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php +++ b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php @@ -190,8 +190,10 @@ final class PhabricatorSpacesTestCase extends PhabricatorTestCase { $space = PhabricatorSpacesNamespace::initializeNewNamespace($actor); - $type_name = PhabricatorSpacesNamespaceTransaction::TYPE_NAME; - $type_default = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT; + $type_name = + PhabricatorSpacesNamespaceNameTransaction::TRANSACTIONTYPE; + $type_default = + PhabricatorSpacesNamespaceDefaultTransaction::TRANSACTIONTYPE; $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; diff --git a/src/applications/spaces/controller/PhabricatorSpacesArchiveController.php b/src/applications/spaces/controller/PhabricatorSpacesArchiveController.php index 82cf19e9e8..b0ab8cd45b 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesArchiveController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesArchiveController.php @@ -23,7 +23,8 @@ final class PhabricatorSpacesArchiveController $cancel_uri = '/'.$space->getMonogram(); if ($request->isFormPost()) { - $type_archive = PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE; + $type_archive = + PhabricatorSpacesNamespaceArchiveTransaction::TRANSACTIONTYPE; $xactions = array(); $xactions[] = id(new PhabricatorSpacesNamespaceTransaction()) diff --git a/src/applications/spaces/controller/PhabricatorSpacesEditController.php b/src/applications/spaces/controller/PhabricatorSpacesEditController.php index 3f7dae9e82..faca39d634 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesEditController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesEditController.php @@ -67,9 +67,12 @@ final class PhabricatorSpacesEditController $v_view = $request->getStr('viewPolicy'); $v_edit = $request->getStr('editPolicy'); - $type_name = PhabricatorSpacesNamespaceTransaction::TYPE_NAME; - $type_desc = PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION; - $type_default = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT; + $type_name = + PhabricatorSpacesNamespaceNameTransaction::TRANSACTIONTYPE; + $type_desc = + PhabricatorSpacesNamespaceDescriptionTransaction::TRANSACTIONTYPE; + $type_default = + PhabricatorSpacesNamespaceDefaultTransaction::TRANSACTIONTYPE; $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; diff --git a/src/applications/spaces/controller/PhabricatorSpacesViewController.php b/src/applications/spaces/controller/PhabricatorSpacesViewController.php index 495a0c8dee..5fa1c01143 100644 --- a/src/applications/spaces/controller/PhabricatorSpacesViewController.php +++ b/src/applications/spaces/controller/PhabricatorSpacesViewController.php @@ -39,7 +39,7 @@ final class PhabricatorSpacesViewController ->setHeaderIcon('fa-th-large'); if ($space->getIsArchived()) { - $header->setStatus('fa-ban', 'red', pht('Archived')); + $header->setStatus('fa-ban', 'indigo', pht('Archived')); } else { $header->setStatus('fa-check', 'bluegrey', pht('Active')); } diff --git a/src/applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php b/src/applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php index caa45f28c2..e4dc9c5b69 100644 --- a/src/applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php +++ b/src/applications/spaces/editor/PhabricatorSpacesNamespaceEditor.php @@ -14,153 +14,18 @@ final class PhabricatorSpacesNamespaceEditor public function getTransactionTypes() { $types = parent::getTransactionTypes(); - $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_NAME; - $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION; - $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT; - $types[] = PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE; - $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY; $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; return $types; } - protected function getCustomTransactionOldValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorSpacesNamespaceTransaction::TYPE_NAME: - $name = $object->getNamespaceName(); - if (!strlen($name)) { - return null; - } - return $name; - case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION: - if ($this->getIsNewObject()) { - return null; - } - return $object->getDescription(); - case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE: - return $object->getIsArchived(); - case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT: - return $object->getIsDefaultNamespace() ? 1 : null; - case PhabricatorTransactions::TYPE_VIEW_POLICY: - return $object->getViewPolicy(); - case PhabricatorTransactions::TYPE_EDIT_POLICY: - return $object->getEditPolicy(); - } - - return parent::getCustomTransactionOldValue($object, $xaction); + public function getCreateObjectTitle($author, $object) { + return pht('%s created this space.', $author); } - protected function getCustomTransactionNewValue( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorSpacesNamespaceTransaction::TYPE_NAME: - case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION: - case PhabricatorTransactions::TYPE_VIEW_POLICY: - case PhabricatorTransactions::TYPE_EDIT_POLICY: - return $xaction->getNewValue(); - case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE: - return $xaction->getNewValue() ? 1 : 0; - case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT: - return $xaction->getNewValue() ? 1 : null; - } - - return parent::getCustomTransactionNewValue($object, $xaction); - } - - protected function applyCustomInternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - $new = $xaction->getNewValue(); - - switch ($xaction->getTransactionType()) { - case PhabricatorSpacesNamespaceTransaction::TYPE_NAME: - $object->setNamespaceName($new); - return; - case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION: - $object->setDescription($new); - return; - case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT: - $object->setIsDefaultNamespace($new ? 1 : null); - return; - case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE: - $object->setIsArchived($new ? 1 : 0); - return; - case PhabricatorTransactions::TYPE_VIEW_POLICY: - $object->setViewPolicy($new); - return; - case PhabricatorTransactions::TYPE_EDIT_POLICY: - $object->setEditPolicy($new); - return; - } - - return parent::applyCustomInternalTransaction($object, $xaction); - } - - protected function applyCustomExternalTransaction( - PhabricatorLiskDAO $object, - PhabricatorApplicationTransaction $xaction) { - - switch ($xaction->getTransactionType()) { - case PhabricatorSpacesNamespaceTransaction::TYPE_NAME: - case PhabricatorSpacesNamespaceTransaction::TYPE_DESCRIPTION: - case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT: - case PhabricatorSpacesNamespaceTransaction::TYPE_ARCHIVE: - case PhabricatorTransactions::TYPE_VIEW_POLICY: - case PhabricatorTransactions::TYPE_EDIT_POLICY: - return; - } - - return parent::applyCustomExternalTransaction($object, $xaction); - } - - protected function validateTransaction( - PhabricatorLiskDAO $object, - $type, - array $xactions) { - - $errors = parent::validateTransaction($object, $type, $xactions); - - switch ($type) { - case PhabricatorSpacesNamespaceTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getNamespaceName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Spaces must have a name.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - break; - case PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT: - if (!$this->getIsNewObject()) { - foreach ($xactions as $xaction) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Only the first space created can be the default space, and '. - 'it must remain the default space evermore.'), - $xaction); - } - } - break; - - } - - return $errors; + public function getCreateObjectTitleForFeed($author, $object) { + return pht('%s created space %s.', $author, $object); } } diff --git a/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php b/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php index 4c438537f1..0f50a870f6 100644 --- a/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php +++ b/src/applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php @@ -1,12 +1,7 @@ getOldValue(); - - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - return ($old === null); - } - - return parent::shouldHide(); - } - - public function hasChangeDetails() { - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - return true; - } - - return parent::hasChangeDetails(); - } - - public function getRemarkupBlocks() { - $blocks = parent::getRemarkupBlocks(); - - switch ($this->getTransactionType()) { - case self::TYPE_DESCRIPTION: - $blocks[] = $this->getNewValue(); - break; - } - - return $blocks; - } - - public function getTitle() { - $old = $this->getOldValue(); - $new = $this->getNewValue(); - - $author_phid = $this->getAuthorPHID(); - - switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if ($old === null) { - return pht( - '%s created this space.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s renamed this space from "%s" to "%s".', - $this->renderHandleLink($author_phid), - $old, - $new); - } - case self::TYPE_DESCRIPTION: - return pht( - '%s updated the description for this space.', - $this->renderHandleLink($author_phid)); - case self::TYPE_DEFAULT: - return pht( - '%s made this the default space.', - $this->renderHandleLink($author_phid)); - case self::TYPE_ARCHIVE: - if ($new) { - return pht( - '%s archived this space.', - $this->renderHandleLink($author_phid)); - } else { - return pht( - '%s activated this space.', - $this->renderHandleLink($author_phid)); - } - } - - return parent::getTitle(); + public function getBaseTransactionClass() { + return 'PhabricatorSpacesNamespaceTransactionType'; } } diff --git a/src/applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php new file mode 100644 index 0000000000..a1dedbd85f --- /dev/null +++ b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceArchiveTransaction.php @@ -0,0 +1,60 @@ +getIsArchived(); + } + + public function applyInternalEffects($object, $value) { + $object->setIsArchived((int)$value); + } + + public function getTitle() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s archived this space.', + $this->renderAuthor()); + } else { + return pht( + '%s activated this space.', + $this->renderAuthor()); + } + } + + public function getTitleForFeed() { + $new = $this->getNewValue(); + if ($new) { + return pht( + '%s archived space %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + return pht( + '%s activated space %s.', + $this->renderAuthor(), + $this->renderObject()); + } + } + + public function getIcon() { + $new = $this->getNewValue(); + if ($new) { + return 'fa-ban'; + } else { + return 'fa-check'; + } + } + + public function getColor() { + $new = $this->getNewValue(); + if ($new) { + return 'indigo'; + } + } + +} diff --git a/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php new file mode 100644 index 0000000000..bc09b06d91 --- /dev/null +++ b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDefaultTransaction.php @@ -0,0 +1,44 @@ +getIsDefaultNamespace(); + } + + public function applyInternalEffects($object, $value) { + $object->setIsDefaultNamespace($value); + } + + public function getTitle() { + return pht( + '%s made this the default space.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s made space %s the default space.', + $this->renderAuthor(), + $this->renderObject()); + + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + if (!$this->isNewObject()) { + foreach ($xactions as $xaction) { + $errors[] = $this->newInvalidError( + pht('Only the first space created can be the default space, and '. + 'it must remain the default space evermore.')); + } + } + + return $errors; + } + +} diff --git a/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php new file mode 100644 index 0000000000..22b0faa5d1 --- /dev/null +++ b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceDescriptionTransaction.php @@ -0,0 +1,57 @@ +getDescription(); + } + + public function applyInternalEffects($object, $value) { + $object->setDescription($value); + } + + public function getTitle() { + return pht( + '%s updated the space description.', + $this->renderAuthor()); + } + + public function getTitleForFeed() { + return pht( + '%s updated the space description for %s.', + $this->renderAuthor(), + $this->renderObject()); + } + + public function hasChangeDetailView() { + return true; + } + + public function getMailDiffSectionHeader() { + return pht('CHANGES TO SPACE DESCRIPTION'); + } + + public function newChangeDetailView() { + $viewer = $this->getViewer(); + + return id(new PhabricatorApplicationTransactionTextDiffDetailView()) + ->setViewer($viewer) + ->setOldText($this->getOldValue()) + ->setNewText($this->getNewValue()); + } + + public function newRemarkupChanges() { + $changes = array(); + + $changes[] = $this->newRemarkupChange() + ->setOldValue($this->getOldValue()) + ->setNewValue($this->getNewValue()); + + return $changes; + } + + +} diff --git a/src/applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php new file mode 100644 index 0000000000..d7fcbc2c7a --- /dev/null +++ b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceNameTransaction.php @@ -0,0 +1,62 @@ +getNamespaceName(); + } + + public function applyInternalEffects($object, $value) { + $object->setNamespaceName($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + if (!strlen($old)) { + return pht( + '%s created this space.', + $this->renderAuthor()); + } else { + return pht( + '%s renamed this space from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + } + + public function getTitleForFeed() { + return pht( + '%s renamed space %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->getNamespaceName(), $xactions)) { + $errors[] = $this->newRequiredError( + pht('Spaces must have a name.')); + } + + $max_length = $object->getColumnMaximumByteLength('namespaceName'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newInvalidError( + pht('The name can be no longer than %s characters.', + new PhutilNumber($max_length))); + } + } + + return $errors; + } + +} diff --git a/src/applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php new file mode 100644 index 0000000000..a6f206a16c --- /dev/null +++ b/src/applications/spaces/xaction/PhabricatorSpacesNamespaceTransactionType.php @@ -0,0 +1,4 @@ +