revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00
Code Issues Releases Wiki Activity

Projects - add "Additional Hashtags" to projects

Browse source 
Summary:
Fixes T4021. Chooses to keep a "primary" slug based off the name - including all that lovely logic - and allow the user to specify "additional" slugs. Expose these as "hashtags" to the user.

Sets us up for a fun diff where we can delete all the Project => Phriction automagicalness. In terms of this diff, see the TODOs i added.

Test Plan:
added a primary slug as an additional slug - got an error. added a slug in use on another project - got an error. added multiple good slugs and they worked. removed slugs and it worked. made some remark using multiple new slugs and they all linked to the correct project

ran epriestley's case

 - Create project "A".
 - Give it additional slug "B".
 - Try to create project "B".

and i got a nice error about hashtag collision

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4021

Differential Revision: https://secure.phabricator.com/D9250
This commit is contained in:
Bob Trahan 2014-05-22 11:19:03 -07:00
parent 6302414883
commit 922e5c0849
12 changed files with 324 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

9
resources/sql/autopatches/20140521.projectslug.1.create.sql Normal file
View file

@ -0,0 +1,9 @@
CREATE TABLE {$NAMESPACE}_project.project_slug (
id INT UNSIGNED NOT NULL PRIMARY KEY AUTO_INCREMENT,
projectPHID VARCHAR(64) NOT NULL COLLATE utf8_bin,
slug VARCHAR(128) NOT NULL COLLATE utf8_bin,
dateCreated INT UNSIGNED NOT NULL,
dateModified INT UNSIGNED NOT NULL,
UNIQUE KEY `key_slug` (slug),
KEY `key_projectPHID` (projectPHID)
) ENGINE=InnoDB, COLLATE utf8_general_ci;

33
resources/sql/autopatches/20140521.projectslug.2.mig.php Normal file
View file

@ -0,0 +1,33 @@
<?php
$project_table = new PhabricatorProject();
$table_name = $project_table->getTableName();
$conn_w = $project_table->establishConnection('w');
$slug_table_name = id(new PhabricatorProjectSlug())->getTableName();
$time = time();
echo "Migrating project phriction slugs...\n";
foreach (new LiskMigrationIterator($project_table) as $project) {
$id = $project->getID();
echo "Migrating project {$id}...\n";
$phriction_slug = rtrim($project->getPhrictionSlug(), '/');
$slug = id(new PhabricatorProjectSlug())
->loadOneWhere('slug = %s', $phriction_slug);
if ($slug) {
echo "Already migrated {$id}... Continuing.\n";
continue;
}
queryfx(
$conn_w,
'INSERT INTO %T (projectPHID, slug, dateCreated, dateModified) '.
'VALUES (%s, %s, %d, %d)',
$slug_table_name,
$project->getPHID(),
$phriction_slug,
$time,
$time);
echo "Migrated {$id}.\n";
}
echo "Done.\n";

2
src/__phutil_library_map__.php
View file

@ -1963,6 +1963,7 @@ phutil_register_library_map(array(
'PhabricatorProjectQuery' => 'applications/project/query/PhabricatorProjectQuery.php',
'PhabricatorProjectSearchEngine' => 'applications/project/query/PhabricatorProjectSearchEngine.php',
'PhabricatorProjectSearchIndexer' => 'applications/project/search/PhabricatorProjectSearchIndexer.php',
'PhabricatorProjectSlug' => 'applications/project/storage/PhabricatorProjectSlug.php',
'PhabricatorProjectStandardCustomField' => 'applications/project/customfield/PhabricatorProjectStandardCustomField.php',
'PhabricatorProjectStatus' => 'applications/project/constants/PhabricatorProjectStatus.php',
'PhabricatorProjectTestDataGenerator' => 'applications/project/lipsum/PhabricatorProjectTestDataGenerator.php',
@ -4787,6 +4788,7 @@ phutil_register_library_map(array(
'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorProjectSearchEngine' => 'PhabricatorApplicationSearchEngine',
'PhabricatorProjectSearchIndexer' => 'PhabricatorSearchDocumentIndexer',
'PhabricatorProjectSlug' => 'PhabricatorProjectDAO',
'PhabricatorProjectStandardCustomField' =>
array(
0 => 'PhabricatorProjectCustomField',

39
src/applications/project/controller/PhabricatorProjectCreateController.php
View file

@ -15,13 +15,16 @@ final class PhabricatorProjectCreateController
$project = PhabricatorProject::initializeNewProject($user);
$e_name = true;
$errors = array();
$type_name = PhabricatorProjectTransaction::TYPE_NAME;
$v_name = $project->getName();
$validation_exception = null;
if ($request->isFormPost()) {
$xactions = array();
$v_name = $request->getStr('name');
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorProjectTransaction::TYPE_NAME)
->setNewValue($request->getStr('name'));
->setTransactionType($type_name)
->setNewValue($v_name);
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_EDGE)
@ -34,14 +37,9 @@ final class PhabricatorProjectCreateController
$editor = id(new PhabricatorProjectTransactionEditor())
->setActor($user)
->setContinueOnNoEffect(true)
->setContentSourceFromRequest($request)
->applyTransactions($project, $xactions);
// TODO: Deal with name collision exceptions more gracefully.
if (!$errors) {
$project->save();
->setContentSourceFromRequest($request);
try {
$editor->applyTransactions($project, $xactions);
if ($request->isAjax()) {
return id(new AphrontAjaxResponse())
->setContent(array(
@ -52,15 +50,12 @@ final class PhabricatorProjectCreateController
return id(new AphrontRedirectResponse())
->setURI('/project/view/'.$project->getID().'/');
}
} catch (PhabricatorApplicationTransactionValidationException $ex) {
$validation_exception = $ex;
$e_name = $ex->getShortMessage($type_name);
}
}
$error_view = null;
if ($errors) {
$error_view = new AphrontErrorView();
$error_view->setErrors($errors);
}
if ($request->isAjax()) {
$form = new PHUIFormLayoutView();
} else {
@ -73,15 +68,19 @@ final class PhabricatorProjectCreateController
id(new AphrontFormTextControl())
->setLabel(pht('Name'))
->setName('name')
->setValue($project->getName())
->setValue($v_name)
->setError($e_name));
if ($request->isAjax()) {
$errors = array();
if ($validation_exception) {
$errors = mpull($ex->getErrors(), 'getMessage');
}
$dialog = id(new AphrontDialogView())
->setUser($user)
->setWidth(AphrontDialogView::WIDTH_FORM)
->setTitle(pht('Create a New Project'))
->appendChild($error_view)
->setErrors($errors)
->appendChild($form)
->addSubmitButton(pht('Create Project'))
->addCancelButton('/project/');
@ -101,7 +100,7 @@ final class PhabricatorProjectCreateController
$form_box = id(new PHUIObjectBoxView())
->setHeaderText(pht('Create New Project'))
->setFormErrors($errors)
->setValidationException($validation_exception)
->setForm($form);
return $this->buildApplicationPage(

32
src/applications/project/controller/PhabricatorProjectEditDetailsController.php
View file

@ -16,6 +16,7 @@ final class PhabricatorProjectEditDetailsController
$project = id(new PhabricatorProjectQuery())
->setViewer($viewer)
->withIDs(array($this->id))
->needSlugs(true)
->requireCapabilities(
array(
PhabricatorPolicyCapability::CAN_VIEW,
@ -37,16 +38,24 @@ final class PhabricatorProjectEditDetailsController
$edit_uri = $this->getApplicationURI('edit/'.$project->getID().'/');
$e_name = true;
$e_slugs = false;
$e_edit = null;
$v_name = $project->getName();
$project_slugs = $project->getSlugs();
$project_slugs = mpull($project_slugs, 'getSlug', 'getSlug');
$v_primary_slug = $project->getPrimarySlug();
unset($project_slugs[$v_primary_slug]);
$v_slugs = $project_slugs;
$validation_exception = null;
if ($request->isFormPost()) {
$e_name = null;
$e_slugs = null;
$v_name = $request->getStr('name');
$v_slugs = $request->getStrList('slugs');
$v_view = $request->getStr('can_view');
$v_edit = $request->getStr('can_edit');
$v_join = $request->getStr('can_join');
@ -56,11 +65,16 @@ final class PhabricatorProjectEditDetailsController
$request);
$type_name = PhabricatorProjectTransaction::TYPE_NAME;
$type_slugs = PhabricatorProjectTransaction::TYPE_SLUGS;
$type_edit = PhabricatorTransactions::TYPE_EDIT_POLICY;
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType($type_name)
->setNewValue($request->getStr('name'));
->setNewValue($v_name);
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType($type_slugs)
->setNewValue($v_slugs);
$xactions[] = id(new PhabricatorProjectTransaction())
->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
@ -87,6 +101,7 @@ final class PhabricatorProjectEditDetailsController
$validation_exception = $ex;
$e_name = $ex->getShortMessage($type_name);
$e_slugs = $ex->getShortMessage($type_slugs);
$e_edit = $ex->getShortMessage($type_edit);
$project->setViewPolicy($v_view);
@ -102,6 +117,7 @@ final class PhabricatorProjectEditDetailsController
->setViewer($viewer)
->setObject($project)
->execute();
$v_slugs = implode(', ', $v_slugs);
$form = new AphrontFormView();
$form
@ -112,10 +128,22 @@ final class PhabricatorProjectEditDetailsController
->setName('name')
->setValue($v_name)
->setError($e_name));
$field_list->appendFieldsToForm($form);
$form
->appendChild(
id(new AphrontFormStaticControl())
->setLabel(pht('Primary Hashtag'))
->setCaption(pht('The primary hashtag is derived from the name.'))
->setValue($v_primary_slug))
->appendChild(
id(new AphrontFormTextControl())
->setLabel(pht('Additional Hashtags'))
->setCaption(pht(
'Specify a comma-separated list of additional hashtags.'))
->setName('slugs')
->setValue($v_slugs)
->setError($e_slugs))
->appendChild(
id(new AphrontFormPolicyControl())
->setUser($viewer)

105
src/applications/project/editor/PhabricatorProjectTransactionEditor.php
View file

@ -12,6 +12,7 @@ final class PhabricatorProjectTransactionEditor
$types[] = PhabricatorTransactions::TYPE_JOIN_POLICY;
$types[] = PhabricatorProjectTransaction::TYPE_NAME;
$types[] = PhabricatorProjectTransaction::TYPE_SLUGS;
$types[] = PhabricatorProjectTransaction::TYPE_STATUS;
$types[] = PhabricatorProjectTransaction::TYPE_IMAGE;
@ -25,6 +26,11 @@ final class PhabricatorProjectTransactionEditor
switch ($xaction->getTransactionType()) {
case PhabricatorProjectTransaction::TYPE_NAME:
return $object->getName();
case PhabricatorProjectTransaction::TYPE_SLUGS:
$slugs = $object->getSlugs();
$slugs = mpull($slugs, 'getSlug', 'getSlug');
unset($slugs[$object->getPrimarySlug()]);
return $slugs;
case PhabricatorProjectTransaction::TYPE_STATUS:
return $object->getStatus();
case PhabricatorProjectTransaction::TYPE_IMAGE:
@ -40,6 +46,7 @@ final class PhabricatorProjectTransactionEditor
switch ($xaction->getTransactionType()) {
case PhabricatorProjectTransaction::TYPE_NAME:
case PhabricatorProjectTransaction::TYPE_SLUGS:
case PhabricatorProjectTransaction::TYPE_STATUS:
case PhabricatorProjectTransaction::TYPE_IMAGE:
return $xaction->getNewValue();
@ -57,6 +64,8 @@ final class PhabricatorProjectTransactionEditor
$object->setName($xaction->getNewValue());
$object->setPhrictionSlug($xaction->getNewValue());
return;
case PhabricatorProjectTransaction::TYPE_SLUGS:
return;
case PhabricatorProjectTransaction::TYPE_STATUS:
$object->setStatus($xaction->getNewValue());
return;
@ -85,6 +94,24 @@ final class PhabricatorProjectTransactionEditor
switch ($xaction->getTransactionType()) {
case PhabricatorProjectTransaction::TYPE_NAME:
$new_slug = id(new PhabricatorProjectSlug())
->setSlug($object->getPrimarySlug())
->setProjectPHID($object->getPHID())
->save();
if ($xaction->getOldValue() !== null) {
$clone_object = clone $object;
$clone_object->setPhrictionSlug($xaction->getOldValue());
$old_slug = $clone_object->getPrimarySlug();
$old_slug = id(new PhabricatorProjectSlug())
->loadOneWhere('slug = %s', $old_slug);
if ($old_slug) {
$old_slug->delete();
}
}
// TODO -- delete all of the below once we sever automagical project
// to phriction stuff
if ($xaction->getOldValue() === null) {
// Project was just created, we don't need to move anything.
return;
@ -118,6 +145,29 @@ final class PhabricatorProjectTransactionEditor
$from_editor->moveAway($target_document->getID());
}
return;
case PhabricatorProjectTransaction::TYPE_SLUGS:
$old = $xaction->getOldValue();
$new = $xaction->getNewValue();
$add = array_diff($new, $old);
$rem = array_diff($old, $new);
if ($add) {
$add_slug_template = id(new PhabricatorProjectSlug())
->setProjectPHID($object->getPHID());
foreach ($add as $add_slug_str) {
$add_slug = id(clone $add_slug_template)
->setSlug($add_slug_str)
->save();
}
}
if ($rem) {
$rem_slugs = id(new PhabricatorProjectSlug())
->loadAllWhere('slug IN (%Ls)', $rem);
foreach ($rem_slugs as $rem_slug) {
$rem_slug->delete();
}
}
return;
case PhabricatorTransactions::TYPE_VIEW_POLICY:
case PhabricatorTransactions::TYPE_EDIT_POLICY:
case PhabricatorTransactions::TYPE_JOIN_POLICY:
@ -219,11 +269,64 @@ final class PhabricatorProjectTransactionEditor
($name_used_already->getPHID() != $object->getPHID())) {
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht(''),
pht('Duplicate'),
pht('Project name is already used.'),
nonempty(last($xactions), null));
$errors[] = $error;
}
$slug_builder = clone $object;
$slug_builder->setPhrictionSlug($name);
$slug = $slug_builder->getPrimarySlug();
$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 can not be used due to hashtag collision.'),
nonempty(last($xactions), null));
$errors[] = $error;
}
break;
case PhabricatorProjectTransaction::TYPE_SLUGS:
if (!$xactions) {
break;
}
$slug_xaction = last($xactions);
$new = $slug_xaction->getNewValue();
$slugs_used_already = id(new PhabricatorProjectSlug())
->loadAllWhere('slug IN (%Ls)', $new);
$slugs_used_already = mgroup($slugs_used_already, 'getProjectPHID');
foreach ($slugs_used_already as $project_phid => $used_slugs) {
$used_slug_strs = mpull($used_slugs, 'getSlug');
if ($project_phid == $object->getPHID()) {
if (in_array($object->getPrimarySlug(), $used_slug_strs)) {
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Invalid'),
pht(
'Project hashtag %s is already the primary hashtag.',
$object->getPrimarySlug()),
$slug_xaction);
$errors[] = $error;
}
continue;
}
$error = new PhabricatorApplicationTransactionValidationError(
$type,
pht('Invalid'),
pht(
'%d project hashtag(s) are already used: %s',
count($used_slug_strs),
implode(', ', $used_slug_strs)),
$slug_xaction);
$errors[] = $error;
}
break;
}

13
src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php
View file

@ -79,14 +79,17 @@ final class PhabricatorProjectPHIDTypeProject extends PhabricatorPHIDType {
$projects = id(new PhabricatorProjectQuery())
->setViewer($query->getViewer())
->withPhrictionSlugs(array_keys($map))
->withSlugs(array_keys($map))
->needSlugs(true)
->execute();
$result = array();
foreach ($projects as $project) {
$slugs = array($project->getPhrictionSlug());
foreach ($slugs as $slug) {
foreach ($map[$slug] as $original) {
$slugs = $project->getSlugs();
$slug_strs = mpull($slugs, 'getSlug');
foreach ($slug_strs as $slug) {
$slug_map = idx($map, $slug, array());
foreach ($slug_map as $original) {
$result[$original] = $project;
}
}
@ -102,7 +105,7 @@ final class PhabricatorProjectPHIDTypeProject extends PhabricatorPHIDType {
// should not. normalize() strips out most punctuation and leads to
// excessively aggressive matches.
return phutil_utf8_strtolower($slug).'/';
return phutil_utf8_strtolower($slug);
}

42
src/applications/project/query/PhabricatorProjectQuery.php
View file

@ -7,6 +7,7 @@ final class PhabricatorProjectQuery
private $phids;
private $memberPHIDs;
private $slugs;
private $phrictionSlugs;
private $names;
private $status = 'status-any';
@ -16,6 +17,7 @@ final class PhabricatorProjectQuery
const STATUS_ACTIVE = 'status-active';
const STATUS_ARCHIVED = 'status-archived';
private $needSlugs;
private $needMembers;
private $needWatchers;
private $needImages;
@ -40,11 +42,16 @@ final class PhabricatorProjectQuery
return $this;
}
public function withPhrictionSlugs(array $slugs) {
public function withSlugs(array $slugs) {
$this->slugs = $slugs;
return $this;
}
public function withPhrictionSlugs(array $slugs) {
$this->phrictionSlugs = $slugs;
return $this;
}
public function withNames(array $names) {
$this->names = $names;
return $this;
@ -65,6 +72,11 @@ final class PhabricatorProjectQuery
return $this;
}
public function needSlugs($need_slugs) {
$this->needSlugs = $need_slugs;
return $this;
}
protected function getPagingColumn() {
return 'name';
}
@ -184,6 +196,18 @@ final class PhabricatorProjectQuery
}
}
if ($this->needSlugs) {
$slugs = id(new PhabricatorProjectSlug())
->loadAllWhere(
'projectPHID IN (%Ls)',
mpull($projects, 'getPHID'));
$slugs = mgroup($slugs, 'getProjectPHID');
foreach ($projects as $project) {
$project_slugs = idx($slugs, $project->getPHID(), array());
$project->attachSlugs($project_slugs);
}
}
return $projects;
}
@ -238,10 +262,17 @@ final class PhabricatorProjectQuery
if ($this->slugs) {
$where[] = qsprintf(
$conn_r,
'phrictionSlug IN (%Ls)',
'slug.slug IN (%Ls)',
$this->slugs);
}
if ($this->phrictionSlugs) {
$where[] = qsprintf(
$conn_r,
'phrictionSlug IN (%Ls)',
$this->phrictionSlugs);
}
if ($this->names) {
$where[] = qsprintf(
$conn_r,
@ -282,6 +313,13 @@ final class PhabricatorProjectQuery
PhabricatorEdgeConfig::TYPE_PROJ_MEMBER);
}
if ($this->slugs) {
$joins[] = qsprintf(
$conn_r,
'JOIN %T slug on slug.projectPHID = p.phid',
id(new PhabricatorProjectSlug())->getTableName());
}
$joins[] = $this->buildApplicationSearchJoinClause($conn_r);
return implode(' ', $joins);

18
src/applications/project/storage/PhabricatorProject.php
View file

@ -24,6 +24,7 @@ final class PhabricatorProject extends PhabricatorProjectDAO
private $sparseMembers = self::ATTACHABLE;
private $customFields = self::ATTACHABLE;
private $profileImageFile = self::ATTACHABLE;
private $slugs = self::ATTACHABLE;
public static function initializeNewProject(PhabricatorUser $actor) {
return id(new PhabricatorProject())
@ -143,6 +144,14 @@ final class PhabricatorProject extends PhabricatorProjectDAO
return 'projects/'.$slug;
}
// TODO - once we sever project => phriction automagicalness,
// migrate getPhrictionSlug to have no trailing slash and be called
// getPrimarySlug
public function getPrimarySlug() {
$slug = $this->getPhrictionSlug();
return rtrim($slug, '/');
}
public function isArchived() {
return ($this->getStatus() == PhabricatorProjectStatus::STATUS_ARCHIVED);
}
@ -185,6 +194,15 @@ final class PhabricatorProject extends PhabricatorProjectDAO
return $this->assertAttached($this->watcherPHIDs);
}
public function attachSlugs(array $slugs) {
$this->slugs = $slugs;
return $this;
}
public function getSlugs() {
return $this->assertAttached($this->slugs);
}
/* -( PhabricatorSubscribableInterface )----------------------------------- */

8
src/applications/project/storage/PhabricatorProjectSlug.php Normal file
View file

@ -0,0 +1,8 @@
<?php
final class PhabricatorProjectSlug extends PhabricatorProjectDAO {
protected $slug;
protected $projectPHID;
}

31
src/applications/project/storage/PhabricatorProjectTransaction.php
View file

@ -4,6 +4,7 @@ final class PhabricatorProjectTransaction
extends PhabricatorApplicationTransaction {
const TYPE_NAME = 'project:name';
const TYPE_SLUGS = 'project:slugs';
const TYPE_STATUS = 'project:status';
const TYPE_IMAGE = 'project:image';
@ -84,6 +85,33 @@ final class PhabricatorProjectTransaction
$this->renderHandleLink($old),
$this->renderHandleLink($new));
}
case PhabricatorProjectTransaction::TYPE_SLUGS:
$add = array_diff($new, $old);
$rem = array_diff($old, $new);
if ($add && $rem) {
return pht(
'%s changed project hashtag(s), added %d: %s; removed %d: %s',
$author_handle,
count($add),
$this->renderSlugList($add),
count($rem),
$this->renderSlugList($rem));
} else if ($add) {
return pht(
'%s added %d project hashtag(s): %s',
$author_handle,
count($add),
$this->renderSlugList($add));
} else if ($rem) {
return pht(
'%s removed %d project hashtag(s): %s',
$author_handle,
count($rem),
$this->renderSlugList($rem));
}
case PhabricatorProjectTransaction::TYPE_MEMBERS:
$add = array_diff($new, $old);
$rem = array_diff($old, $new);
@ -126,5 +154,8 @@ final class PhabricatorProjectTransaction
return parent::getTitle();
}
private function renderSlugList($slugs) {
return implode(', ', $slugs);
}
}

22
src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php
View file

@ -835,6 +835,28 @@ abstract class PhabricatorBaseEnglishTranslation
),
),
'%d project hashtag(s) are already used: %s' => array(
'Project hashtag %2$s is already used.',
'%d project hashtags are already used: %2$s',
),
'%s changed project hashtag(s), added %d: %s; removed %d: %s' =>
'%s changed project hashtags, added %3$s; removed %5$s',
'%s added %d project hashtag(s): %s' => array(
array(
'%s added a hashtag: %3$s',
'%s added hashtags: %3$s',
),
),
'%s removed %d project hashtag(s): %s' => array(
array(
'%s removed a hashtag: %3$s',
'%s removed hashtags: %3$s',
),
),
'%d User(s) Need Approval' => array(
'%d User Needs Approval',
'%d Users Need Approval',