diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2577,6 +2577,7 @@ 'PhabricatorSpacesNamespaceSearchEngine' => 'applications/spaces/query/PhabricatorSpacesNamespaceSearchEngine.php', 'PhabricatorSpacesNamespaceTransaction' => 'applications/spaces/storage/PhabricatorSpacesNamespaceTransaction.php', 'PhabricatorSpacesNamespaceTransactionQuery' => 'applications/spaces/query/PhabricatorSpacesNamespaceTransactionQuery.php', + 'PhabricatorSpacesTestCase' => 'applications/spaces/__tests__/PhabricatorSpacesTestCase.php', 'PhabricatorSpacesViewController' => 'applications/spaces/controller/PhabricatorSpacesViewController.php', 'PhabricatorStandardCustomField' => 'infrastructure/customfield/standard/PhabricatorStandardCustomField.php', 'PhabricatorStandardCustomFieldBool' => 'infrastructure/customfield/standard/PhabricatorStandardCustomFieldBool.php', @@ -6057,6 +6058,7 @@ 'PhabricatorSpacesDAO', 'PhabricatorPolicyInterface', 'PhabricatorApplicationTransactionInterface', + 'PhabricatorDestructibleInterface', ), 'PhabricatorSpacesNamespaceEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorSpacesNamespacePHIDType' => 'PhabricatorPHIDType', @@ -6064,6 +6066,7 @@ 'PhabricatorSpacesNamespaceSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorSpacesNamespaceTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorSpacesNamespaceTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorSpacesTestCase' => 'PhabricatorTestCase', 'PhabricatorSpacesViewController' => 'PhabricatorSpacesController', 'PhabricatorStandardCustomField' => 'PhabricatorCustomField', 'PhabricatorStandardCustomFieldBool' => 'PhabricatorStandardCustomField', diff --git a/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php new file mode 100644 --- /dev/null +++ b/src/applications/spaces/__tests__/PhabricatorSpacesTestCase.php @@ -0,0 +1,135 @@ + true, + ); + } + + public function testSpacesAnnihilation() { + $this->destroyAllSpaces(); + + // Test that our helper methods work correctly. + + $actor = $this->generateNewTestUser(); + $this->newSpace($actor, pht('Test Space'), true); + $this->assertEqual(1, count($this->loadAllSpaces())); + $this->destroyAllSpaces(); + $this->assertEqual(0, count($this->loadAllSpaces())); + } + + public function testSpacesSeveralSpaces() { + $this->destroyAllSpaces(); + + // Try creating a few spaces, one of which is a default space. This should + // work fine. + + $actor = $this->generateNewTestUser(); + $this->newSpace($actor, pht('Default Space'), true); + $this->newSpace($actor, pht('Alternate Space'), false); + $this->assertEqual(2, count($this->loadAllSpaces())); + } + + public function testSpacesRequireNames() { + $this->destroyAllSpaces(); + + // Spaces must have nonempty names. + + $actor = $this->generateNewTestUser(); + + $caught = null; + try { + $options = array( + 'continueOnNoEffect' => true, + ); + $this->newSpace($actor, '', true, $options); + } catch (PhabricatorApplicationTransactionValidationException $ex) { + $caught = $ex; + } + + $this->assertTrue(($caught instanceof Exception)); + } + + public function testSpacesUniqueDefaultSpace() { + $this->destroyAllSpaces(); + + // It shouldn't be possible to create two default spaces. + + $actor = $this->generateNewTestUser(); + $this->newSpace($actor, pht('Default Space'), true); + + $caught = null; + try { + $this->newSpace($actor, pht('Default Space #2'), true); + } catch (AphrontDuplicateKeyQueryException $ex) { + $caught = $ex; + } + + $this->assertTrue(($caught instanceof Exception)); + } + + private function loadAllSpaces() { + return id(new PhabricatorSpacesNamespaceQuery()) + ->setViewer(PhabricatorUser::getOmnipotentUser()) + ->execute(); + } + + private function destroyAllSpaces() { + $spaces = $this->loadAllSpaces(); + foreach ($spaces as $space) { + $engine = new PhabricatorDestructionEngine(); + $engine->destroyObject($space); + } + } + + private function newSpace( + PhabricatorUser $actor, + $name, + $is_default, + array $options = array()) { + + $space = PhabricatorSpacesNamespace::initializeNewNamespace($actor); + + $type_name = PhabricatorSpacesNamespaceTransaction::TYPE_NAME; + $type_default = PhabricatorSpacesNamespaceTransaction::TYPE_DEFAULT; + $type_view = PhabricatorTransactions::TYPE_VIEW_POLICY; + $type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY; + + $xactions = array(); + + $xactions[] = id(new PhabricatorSpacesNamespaceTransaction()) + ->setTransactionType($type_name) + ->setNewValue($name); + + $xactions[] = id(new PhabricatorSpacesNamespaceTransaction()) + ->setTransactionType($type_view) + ->setNewValue($actor->getPHID()); + + $xactions[] = id(new PhabricatorSpacesNamespaceTransaction()) + ->setTransactionType($type_edit) + ->setNewValue($actor->getPHID()); + + if ($is_default) { + $xactions[] = id(new PhabricatorSpacesNamespaceTransaction()) + ->setTransactionType($type_default) + ->setNewValue($is_default); + } + + $content_source = PhabricatorContentSource::newConsoleSource(); + + $editor = id(new PhabricatorSpacesNamespaceEditor()) + ->setActor($actor) + ->setContentSource($content_source); + + if (isset($options['continueOnNoEffect'])) { + $editor->setContinueOnNoEffect(true); + } + + $editor->applyTransactions($space, $xactions); + + return $space; + } + +} diff --git a/src/applications/spaces/storage/PhabricatorSpacesNamespace.php b/src/applications/spaces/storage/PhabricatorSpacesNamespace.php --- a/src/applications/spaces/storage/PhabricatorSpacesNamespace.php +++ b/src/applications/spaces/storage/PhabricatorSpacesNamespace.php @@ -4,7 +4,8 @@ extends PhabricatorSpacesDAO implements PhabricatorPolicyInterface, - PhabricatorApplicationTransactionInterface { + PhabricatorApplicationTransactionInterface, + PhabricatorDestructibleInterface { protected $namespaceName; protected $viewPolicy; @@ -103,4 +104,13 @@ return $timeline; } + +/* -( PhabricatorDestructibleInterface )----------------------------------- */ + + + public function destroyObjectPermanently( + PhabricatorDestructionEngine $engine) { + $this->delete(); + } + } diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php --- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php +++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php @@ -764,7 +764,12 @@ $xactions = $this->didApplyInternalEffects($object, $xactions); - $object->save(); + try { + $object->save(); + } catch (AphrontDuplicateKeyQueryException $ex) { + $object->killTransaction(); + throw $ex; + } foreach ($xactions as $xaction) { $xaction->setObjectPHID($object->getPHID());