Page MenuHomePhabricator

D17947.id43163.diff
No OneTemporary

D17947.id43163.diff

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',
@@ -9052,6 +9054,7 @@
'PhabricatorProjectMenuItemController' => 'PhabricatorProjectController',
'PhabricatorProjectMoveController' => 'PhabricatorProjectController',
'PhabricatorProjectNameContextFreeGrammar' => 'PhutilContextFreeGrammar',
+ 'PhabricatorProjectNameTransaction' => 'PhabricatorProjectTransactionType',
'PhabricatorProjectNoProjectsDatasource' => 'PhabricatorTypeaheadDatasource',
'PhabricatorProjectObjectHasProjectEdgeType' => 'PhabricatorEdgeType',
'PhabricatorProjectOrUserDatasource' => 'PhabricatorTypeaheadCompositeDatasource',
@@ -9082,9 +9085,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 @@
<?php
final class PhabricatorProjectTransaction
- extends PhabricatorApplicationTransaction {
+ extends PhabricatorModularTransaction {
- const TYPE_NAME = 'project:name';
const TYPE_SLUGS = 'project:slugs';
const TYPE_STATUS = 'project:status';
const TYPE_IMAGE = 'project:image';
@@ -33,6 +32,10 @@
return PhabricatorProjectProjectPHIDType::TYPECONST;
}
+ public function getBaseTransactionClass() {
+ return 'PhabricatorProjectTransactionType';
+ }
+
public function getRequiredHandlePHIDs() {
$old = $this->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 @@
+<?php
+
+final class PhabricatorProjectNameTransaction
+ extends PhabricatorProjectTransactionType {
+
+ const TRANSACTIONTYPE = 'project:name';
+
+ public function generateOldValue($object) {
+ return $object->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 @@
+<?php
+
+abstract class PhabricatorProjectTransactionType
+ extends PhabricatorModularTransactionType {}
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
@@ -592,6 +592,8 @@
$xtype = $this->getModularTransactionType($type);
if ($xtype) {
+ $xtype = clone $xtype;
+ $xtype->setStorage($xaction);
return $xtype->applyExternalEffects($object, $xaction->getNewValue());
}

File Metadata

Mime Type
text/plain
Expires
Fri, Mar 21, 4:32 AM (2 w, 9 h ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7714972
Default Alt Text
D17947.id43163.diff (19 KB)

Event Timeline