diff --git a/resources/sql/patches/20131020.pxactionmig.php b/resources/sql/patches/20131020.pxactionmig.php --- a/resources/sql/patches/20131020.pxactionmig.php +++ b/resources/sql/patches/20131020.pxactionmig.php @@ -32,7 +32,7 @@ $project_phid = $project_row['phid']; $type_map = array( - 'name' => PhabricatorProjectTransaction::TYPE_NAME, + 'name' => PhabricatorProjectNameTransaction::TRANSACTIONTYPE, 'members' => PhabricatorProjectTransaction::TYPE_MEMBERS, 'status' => PhabricatorProjectTransaction::TYPE_STATUS, 'canview' => PhabricatorTransactions::TYPE_VIEW_POLICY, 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 @@ -3644,6 +3644,7 @@ 'PhabricatorProjectMenuItemController' => 'applications/project/controller/PhabricatorProjectMenuItemController.php', 'PhabricatorProjectMoveController' => 'applications/project/controller/PhabricatorProjectMoveController.php', 'PhabricatorProjectNameContextFreeGrammar' => 'applications/project/lipsum/PhabricatorProjectNameContextFreeGrammar.php', + 'PhabricatorProjectNameTransaction' => 'applications/project/xaction/PhabricatorProjectNameTransaction.php', 'PhabricatorProjectNoProjectsDatasource' => 'applications/project/typeahead/PhabricatorProjectNoProjectsDatasource.php', 'PhabricatorProjectObjectHasProjectEdgeType' => 'applications/project/edge/PhabricatorProjectObjectHasProjectEdgeType.php', 'PhabricatorProjectOrUserDatasource' => 'applications/project/typeahead/PhabricatorProjectOrUserDatasource.php', @@ -3674,6 +3675,7 @@ 'PhabricatorProjectTransaction' => 'applications/project/storage/PhabricatorProjectTransaction.php', 'PhabricatorProjectTransactionEditor' => 'applications/project/editor/PhabricatorProjectTransactionEditor.php', 'PhabricatorProjectTransactionQuery' => 'applications/project/query/PhabricatorProjectTransactionQuery.php', + 'PhabricatorProjectTransactionType' => 'applications/project/xaction/PhabricatorProjectTransactionType.php', 'PhabricatorProjectUIEventListener' => 'applications/project/events/PhabricatorProjectUIEventListener.php', 'PhabricatorProjectUpdateController' => 'applications/project/controller/PhabricatorProjectUpdateController.php', 'PhabricatorProjectUserFunctionDatasource' => 'applications/project/typeahead/PhabricatorProjectUserFunctionDatasource.php', @@ -9053,6 +9055,7 @@ 'PhabricatorProjectMenuItemController' => 'PhabricatorProjectController', 'PhabricatorProjectMoveController' => 'PhabricatorProjectController', 'PhabricatorProjectNameContextFreeGrammar' => 'PhutilContextFreeGrammar', + 'PhabricatorProjectNameTransaction' => 'PhabricatorProjectTransactionType', 'PhabricatorProjectNoProjectsDatasource' => 'PhabricatorTypeaheadDatasource', 'PhabricatorProjectObjectHasProjectEdgeType' => 'PhabricatorEdgeType', 'PhabricatorProjectOrUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource', @@ -9083,9 +9086,10 @@ 'PhabricatorProjectSubprojectsController' => 'PhabricatorProjectController', 'PhabricatorProjectSubprojectsProfileMenuItem' => 'PhabricatorProfileMenuItem', 'PhabricatorProjectTestDataGenerator' => 'PhabricatorTestDataGenerator', - 'PhabricatorProjectTransaction' => 'PhabricatorApplicationTransaction', + 'PhabricatorProjectTransaction' => 'PhabricatorModularTransaction', 'PhabricatorProjectTransactionEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorProjectTransactionQuery' => 'PhabricatorApplicationTransactionQuery', + 'PhabricatorProjectTransactionType' => 'PhabricatorModularTransactionType', 'PhabricatorProjectUIEventListener' => 'PhabricatorEventListener', 'PhabricatorProjectUpdateController' => 'PhabricatorProjectController', 'PhabricatorProjectUserFunctionDatasource' => 'PhabricatorTypeaheadCompositeDatasource', diff --git a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php --- a/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php +++ b/src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php @@ -356,7 +356,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setNewValue($name); $this->applyTransactions($project, $user, $xactions); @@ -382,7 +382,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setNewValue($name2); $xactions[] = id(new PhabricatorProjectTransaction()) @@ -503,7 +503,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setNewValue($name); $xactions[] = id(new PhabricatorProjectTransaction()) @@ -601,7 +601,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setNewValue($name); $xactions[] = id(new PhabricatorProjectTransaction()) @@ -1290,7 +1290,8 @@ $new_name = $proj->getName().' '.mt_rand(); $xaction = new PhabricatorProjectTransaction(); - $xaction->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME); + $xaction->setTransactionType( + PhabricatorProjectNameTransaction::TRANSACTIONTYPE); $xaction->setNewValue($new_name); $this->applyTransactions($proj, $user, array($xaction)); @@ -1440,7 +1441,7 @@ $xactions = array(); $xactions[] = id(new PhabricatorProjectTransaction()) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setNewValue($name); if ($parent) { diff --git a/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php b/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php --- a/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php +++ b/src/applications/project/conduit/ProjectCreateConduitAPIMethod.php @@ -42,7 +42,7 @@ $user); $project = PhabricatorProject::initializeNewProject($user); - $type_name = PhabricatorProjectTransaction::TYPE_NAME; + $type_name = PhabricatorProjectNameTransaction::TRANSACTIONTYPE; $members = $request->getValue('members'); $xactions = array(); diff --git a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php --- a/src/applications/project/editor/PhabricatorProjectTransactionEditor.php +++ b/src/applications/project/editor/PhabricatorProjectTransactionEditor.php @@ -10,7 +10,7 @@ return $this; } - private function getIsMilestone() { + public function getIsMilestone() { return $this->isMilestone; } @@ -30,7 +30,6 @@ $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY; $types[] = PhabricatorTransactions::TYPE_JOIN_POLICY; - $types[] = PhabricatorProjectTransaction::TYPE_NAME; $types[] = PhabricatorProjectTransaction::TYPE_SLUGS; $types[] = PhabricatorProjectTransaction::TYPE_STATUS; $types[] = PhabricatorProjectTransaction::TYPE_IMAGE; @@ -52,8 +51,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_NAME: - return $object->getName(); case PhabricatorProjectTransaction::TYPE_SLUGS: $slugs = $object->getSlugs(); $slugs = mpull($slugs, 'getSlug', 'getSlug'); @@ -90,7 +87,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_NAME: case PhabricatorProjectTransaction::TYPE_STATUS: case PhabricatorProjectTransaction::TYPE_IMAGE: case PhabricatorProjectTransaction::TYPE_ICON: @@ -121,13 +117,6 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_NAME: - $name = $xaction->getNewValue(); - $object->setName($name); - if (!$this->getIsMilestone()) { - $object->setPrimarySlug(PhabricatorSlug::normalizeProjectSlug($name)); - } - return; case PhabricatorProjectTransaction::TYPE_SLUGS: return; case PhabricatorProjectTransaction::TYPE_STATUS: @@ -178,16 +167,6 @@ $new = $xaction->getNewValue(); switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_NAME: - // First, add the old name as a secondary slug; this is helpful - // for renames and generally a good thing to do. - if (!$this->getIsMilestone()) { - if ($old !== null) { - $this->addSlug($object, $old, false); - } - $this->addSlug($object, $new, false); - } - return; case PhabricatorProjectTransaction::TYPE_SLUGS: $old = $xaction->getOldValue(); $new = $xaction->getNewValue(); @@ -299,59 +278,6 @@ $errors = parent::validateTransaction($object, $type, $xactions); switch ($type) { - case PhabricatorProjectTransaction::TYPE_NAME: - $missing = $this->validateIsEmptyTextField( - $object->getName(), - $xactions); - - if ($missing) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Required'), - pht('Project name is required.'), - nonempty(last($xactions), null)); - - $error->setIsMissingFieldError(true); - $errors[] = $error; - } - - if (!$xactions) { - break; - } - - if ($this->getIsMilestone()) { - break; - } - - $name = last($xactions)->getNewValue(); - - if (!PhabricatorSlug::isValidProjectSlug($name)) { - $errors[] = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Invalid'), - pht( - 'Project names must contain at least one letter or number.'), - last($xactions)); - break; - } - - $slug = PhabricatorSlug::normalizeProjectSlug($name); - - $slug_used_already = id(new PhabricatorProjectSlug()) - ->loadOneWhere('slug = %s', $slug); - if ($slug_used_already && - $slug_used_already->getProjectPHID() != $object->getPHID()) { - $error = new PhabricatorApplicationTransactionValidationError( - $type, - pht('Duplicate'), - pht( - 'Project name generates the same hashtag ("%s") as another '. - 'existing project. Choose a unique name.', - '#'.$slug), - nonempty(last($xactions), null)); - $errors[] = $error; - } - break; case PhabricatorProjectTransaction::TYPE_SLUGS: if (!$xactions) { break; @@ -497,7 +423,7 @@ PhabricatorApplicationTransaction $xaction) { switch ($xaction->getTransactionType()) { - case PhabricatorProjectTransaction::TYPE_NAME: + case PhabricatorProjectNameTransaction::TRANSACTIONTYPE: case PhabricatorProjectTransaction::TYPE_STATUS: case PhabricatorProjectTransaction::TYPE_IMAGE: case PhabricatorProjectTransaction::TYPE_ICON: @@ -717,7 +643,7 @@ return parent::applyFinalEffects($object, $xactions); } - private function addSlug(PhabricatorProject $project, $slug, $force) { + public function addSlug(PhabricatorProject $project, $slug, $force) { $slug = PhabricatorSlug::normalizeProjectSlug($slug); $table = new PhabricatorProjectSlug(); $project_phid = $project->getPHID(); diff --git a/src/applications/project/engine/PhabricatorProjectEditEngine.php b/src/applications/project/engine/PhabricatorProjectEditEngine.php --- a/src/applications/project/engine/PhabricatorProjectEditEngine.php +++ b/src/applications/project/engine/PhabricatorProjectEditEngine.php @@ -235,7 +235,7 @@ id(new PhabricatorTextEditField()) ->setKey('name') ->setLabel(pht('Name')) - ->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME) + ->setTransactionType(PhabricatorProjectNameTransaction::TRANSACTIONTYPE) ->setIsRequired(true) ->setDescription(pht('Project name.')) ->setConduitDescription(pht('Rename the project')) diff --git a/src/applications/project/lipsum/PhabricatorProjectTestDataGenerator.php b/src/applications/project/lipsum/PhabricatorProjectTestDataGenerator.php --- a/src/applications/project/lipsum/PhabricatorProjectTestDataGenerator.php +++ b/src/applications/project/lipsum/PhabricatorProjectTestDataGenerator.php @@ -16,7 +16,7 @@ $xactions = array(); $xactions[] = $this->newTransaction( - PhabricatorProjectTransaction::TYPE_NAME, + PhabricatorProjectNameTransaction::TRANSACTIONTYPE, $this->newProjectTitle()); $xactions[] = $this->newTransaction( diff --git a/src/applications/project/storage/PhabricatorProjectTransaction.php b/src/applications/project/storage/PhabricatorProjectTransaction.php --- a/src/applications/project/storage/PhabricatorProjectTransaction.php +++ b/src/applications/project/storage/PhabricatorProjectTransaction.php @@ -1,9 +1,8 @@ getOldValue(); $new = $this->getNewValue(); @@ -149,20 +152,6 @@ '%s created this project.', $this->renderHandleLink($author_phid)); - case self::TYPE_NAME: - if ($old === null) { - return pht( - '%s created this project.', - $author_handle); - } else { - return pht( - '%s renamed this project from "%s" to "%s".', - $author_handle, - $old, - $new); - } - break; - case self::TYPE_STATUS: if ($old == 0) { return pht( @@ -329,20 +318,6 @@ $new = $this->getNewValue(); switch ($this->getTransactionType()) { - case self::TYPE_NAME: - if ($old === null) { - return pht( - '%s created %s.', - $author_handle, - $object_handle); - } else { - return pht( - '%s renamed %s from "%s" to "%s".', - $author_handle, - $object_handle, - $old, - $new); - } case self::TYPE_STATUS: if ($old == 0) { return pht( diff --git a/src/applications/project/xaction/PhabricatorProjectNameTransaction.php b/src/applications/project/xaction/PhabricatorProjectNameTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/PhabricatorProjectNameTransaction.php @@ -0,0 +1,112 @@ +getName(); + } + + public function applyInternalEffects($object, $value) { + $object->setName($value); + if (!$this->getEditor()->getIsMilestone()) { + $object->setPrimarySlug(PhabricatorSlug::normalizeProjectSlug($value)); + } + } + + public function applyExternalEffects($object, $value) { + $old = $this->getOldValue(); + + // First, add the old name as a secondary slug; this is helpful + // for renames and generally a good thing to do. + if (!$this->getEditor()->getIsMilestone()) { + if ($old !== null) { + $this->getEditor()->addSlug($object, $old, false); + } + $this->getEditor()->addSlug($object, $value, false); + } + return; + } + + public function getTitle() { + $old = $this->getOldValue(); + if ($old === null) { + return pht( + '%s created this project.', + $this->renderAuthor()); + } else { + return pht( + '%s renamed this project from %s to %s.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } + } + + public function getTitleForFeed() { + $old = $this->getOldValue(); + if ($old === null) { + return pht( + '%s created %s.', + $this->renderAuthor(), + $this->renderObject()); + } else { + 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('Projects must have a name.')); + } + + $max_length = $object->getColumnMaximumByteLength('name'); + foreach ($xactions as $xaction) { + $new_value = $xaction->getNewValue(); + $new_length = strlen($new_value); + if ($new_length > $max_length) { + $errors[] = $this->newInvalidError( + pht( + 'Project names must not be longer than %s character(s).', + new PhutilNumber($max_length))); + } + } + + if ($this->getEditor()->getIsMilestone() || !$xactions) { + return $errors; + } + + $name = last($xactions)->getNewValue(); + + if (!PhabricatorSlug::isValidProjectSlug($name)) { + $errors[] = $this->newInvalidError( + pht('Project names must contain at least one letter or number.')); + } + + $slug = PhabricatorSlug::normalizeProjectSlug($name); + + $slug_used_already = id(new PhabricatorProjectSlug()) + ->loadOneWhere('slug = %s', $slug); + if ($slug_used_already && + $slug_used_already->getProjectPHID() != $object->getPHID()) { + + $errors[] = $this->newInvalidError( + pht( + 'Project name generates the same hashtag ("%s") as another '. + 'existing project. Choose a unique name.', + '#'.$slug)); + } + + return $errors; + } + +} diff --git a/src/applications/project/xaction/PhabricatorProjectTransactionType.php b/src/applications/project/xaction/PhabricatorProjectTransactionType.php new file mode 100644 --- /dev/null +++ b/src/applications/project/xaction/PhabricatorProjectTransactionType.php @@ -0,0 +1,4 @@ +getModularTransactionType($type); if ($xtype) { + $xtype = clone $xtype; + $xtype->setStorage($xaction); return $xtype->applyExternalEffects($object, $xaction->getNewValue()); }