diff --git a/resources/sql/autopatches/20140521.projectslug.1.create.sql b/resources/sql/autopatches/20140521.projectslug.1.create.sql new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140521.projectslug.1.create.sql @@ -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; diff --git a/resources/sql/autopatches/20140521.projectslug.2.mig.php b/resources/sql/autopatches/20140521.projectslug.2.mig.php new file mode 100644 --- /dev/null +++ b/resources/sql/autopatches/20140521.projectslug.2.mig.php @@ -0,0 +1,33 @@ +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"; 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 @@ -1963,6 +1963,7 @@ '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 @@ 'PhabricatorProjectQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorProjectSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorProjectSearchIndexer' => 'PhabricatorSearchDocumentIndexer', + 'PhabricatorProjectSlug' => 'PhabricatorProjectDAO', 'PhabricatorProjectStandardCustomField' => array( 0 => 'PhabricatorProjectCustomField', diff --git a/src/applications/project/controller/PhabricatorProjectCreateController.php b/src/applications/project/controller/PhabricatorProjectCreateController.php --- a/src/applications/project/controller/PhabricatorProjectCreateController.php +++ b/src/applications/project/controller/PhabricatorProjectCreateController.php @@ -15,13 +15,16 @@ $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 @@ $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 @@ 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 @@ 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 @@ $form_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Create New Project')) - ->setFormErrors($errors) + ->setValidationException($validation_exception) ->setForm($form); return $this->buildApplicationPage( diff --git a/src/applications/project/controller/PhabricatorProjectEditDetailsController.php b/src/applications/project/controller/PhabricatorProjectEditDetailsController.php --- a/src/applications/project/controller/PhabricatorProjectEditDetailsController.php +++ b/src/applications/project/controller/PhabricatorProjectEditDetailsController.php @@ -16,6 +16,7 @@ $project = id(new PhabricatorProjectQuery()) ->setViewer($viewer) ->withIDs(array($this->id)) + ->needSlugs(true) ->requireCapabilities( array( PhabricatorPolicyCapability::CAN_VIEW, @@ -37,16 +38,24 @@ $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 @@ $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 @@ $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 @@ ->setViewer($viewer) ->setObject($project) ->execute(); + $v_slugs = implode(', ', $v_slugs); $form = new AphrontFormView(); $form @@ -112,11 +128,23 @@ ->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) ->setName('can_view') 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 @@ -12,6 +12,7 @@ $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 @@ 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 @@ 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 @@ $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 @@ 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 @@ $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 @@ ($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; } diff --git a/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php b/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php --- a/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php +++ b/src/applications/project/phid/PhabricatorProjectPHIDTypeProject.php @@ -79,14 +79,17 @@ $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 @@ // should not. normalize() strips out most punctuation and leads to // excessively aggressive matches. - return phutil_utf8_strtolower($slug).'/'; + return phutil_utf8_strtolower($slug); } diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -7,6 +7,7 @@ private $phids; private $memberPHIDs; private $slugs; + private $phrictionSlugs; private $names; private $status = 'status-any'; @@ -16,6 +17,7 @@ const STATUS_ACTIVE = 'status-active'; const STATUS_ARCHIVED = 'status-archived'; + private $needSlugs; private $needMembers; private $needWatchers; private $needImages; @@ -40,11 +42,16 @@ 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 @@ return $this; } + public function needSlugs($need_slugs) { + $this->needSlugs = $need_slugs; + return $this; + } + protected function getPagingColumn() { return 'name'; } @@ -184,6 +196,18 @@ } } + 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 @@ 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 @@ 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); diff --git a/src/applications/project/storage/PhabricatorProject.php b/src/applications/project/storage/PhabricatorProject.php --- a/src/applications/project/storage/PhabricatorProject.php +++ b/src/applications/project/storage/PhabricatorProject.php @@ -24,6 +24,7 @@ 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 @@ 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 @@ return $this->assertAttached($this->watcherPHIDs); } + public function attachSlugs(array $slugs) { + $this->slugs = $slugs; + return $this; + } + + public function getSlugs() { + return $this->assertAttached($this->slugs); + } + /* -( PhabricatorSubscribableInterface )----------------------------------- */ diff --git a/src/applications/project/storage/PhabricatorProjectSlug.php b/src/applications/project/storage/PhabricatorProjectSlug.php new file mode 100644 --- /dev/null +++ b/src/applications/project/storage/PhabricatorProjectSlug.php @@ -0,0 +1,8 @@ +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 @@ return parent::getTitle(); } + private function renderSlugList($slugs) { + return implode(', ', $slugs); + } } diff --git a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php --- a/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorBaseEnglishTranslation.php @@ -835,6 +835,28 @@ ), ), + '%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',