Fix project hashtag bugs: duplicate tags, uppercase tags

Summary:
Ref T8509. This fixes three issues:

  - Adding a slug like `UPPERCASE` would not give you a normalized slug. (Now: normalizes as `uppercase`.)
  - Adding a slug like `UPPERCASE` would allow you to give two different projects the different tags `UPPERCASE` and `uppercase` (and `UpPeRcAsE`, etc). (Now: second tag is rejected as a duplicate.)
  - Adding multiple identical or similar slugs would produce a duplicate key exception. (Now: ignores the duplicates.)

Test Plan:
  - Added test coverage.
  - Made tests pass.
  - Hit these cases in the UI.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T8509

Differential Revision: https://secure.phabricator.com/D14870

src/applications/project/__tests__/PhabricatorProjectCoreTestCase.php

@@ -249,6 +249,90 @@ final class PhabricatorProjectCoreTestCase extends PhabricatorTestCase {
       $milestone->getMemberPHIDs());
   }
 
+  public function testDuplicateSlugs() {
+    // Creating a project with multiple duplicate slugs should succeed.
+
+    $user = $this->createUser();
+    $user->save();
+
+    $project = $this->createProject($user);
+
+    $input = 'duplicate';
+
+    $xactions = array();
+
+    $xactions[] = id(new PhabricatorProjectTransaction())
+      ->setTransactionType(PhabricatorProjectTransaction::TYPE_SLUGS)
+      ->setNewValue(array($input, $input));
+
+    $this->applyTransactions($project, $user, $xactions);
+
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($project->getPHID()))
+      ->needSlugs(true)
+      ->executeOne();
+
+    $slugs = $project->getSlugs();
+    $slugs = mpull($slugs, 'getSlug');
+
+    $this->assertTrue(in_array($input, $slugs));
+  }
+
+  public function testNormalizeSlugs() {
+    // When a user creates a project with slug "XxX360n0sc0perXxX", normalize
+    // it before writing it.
+
+    $user = $this->createUser();
+    $user->save();
+
+    $project = $this->createProject($user);
+
+    $input = 'NoRmAlIzE';
+    $expect = 'normalize';
+
+    $xactions = array();
+
+    $xactions[] = id(new PhabricatorProjectTransaction())
+      ->setTransactionType(PhabricatorProjectTransaction::TYPE_SLUGS)
+      ->setNewValue(array($input));
+
+    $this->applyTransactions($project, $user, $xactions);
+
+    $project = id(new PhabricatorProjectQuery())
+      ->setViewer($user)
+      ->withPHIDs(array($project->getPHID()))
+      ->needSlugs(true)
+      ->executeOne();
+
+    $slugs = $project->getSlugs();
+    $slugs = mpull($slugs, 'getSlug');
+
+    $this->assertTrue(in_array($expect, $slugs));
+
+
+    // If another user tries to add the same slug in denormalized form, it
+    // should be caught and fail, even though the database version of the slug
+    // is normalized.
+
+    $project2 = $this->createProject($user);
+
+    $xactions = array();
+
+    $xactions[] = id(new PhabricatorProjectTransaction())
+      ->setTransactionType(PhabricatorProjectTransaction::TYPE_SLUGS)
+      ->setNewValue(array($input));
+
+    $caught = null;
+    try {
+      $this->applyTransactions($project2, $user, $xactions);
+    } catch (PhabricatorApplicationTransactionValidationException $ex) {
+      $caught = $ex;
+    }
+
+    $this->assertTrue((bool)$caught);
+  }
+
   public function testParentProject() {
     $user = $this->createUser();
     $user->save();

src/applications/project/editor/PhabricatorProjectTransactionEditor.php

@@ -68,7 +68,6 @@ final class PhabricatorProjectTransactionEditor
 
     switch ($xaction->getTransactionType()) {
       case PhabricatorProjectTransaction::TYPE_NAME:
-      case PhabricatorProjectTransaction::TYPE_SLUGS:
       case PhabricatorProjectTransaction::TYPE_STATUS:
       case PhabricatorProjectTransaction::TYPE_IMAGE:
       case PhabricatorProjectTransaction::TYPE_ICON:
@@ -76,6 +75,8 @@ final class PhabricatorProjectTransactionEditor
       case PhabricatorProjectTransaction::TYPE_LOCKED:
       case PhabricatorProjectTransaction::TYPE_PARENT:
         return $xaction->getNewValue();
+      case PhabricatorProjectTransaction::TYPE_SLUGS:
+        return $this->normalizeSlugs($xaction->getNewValue());
       case PhabricatorProjectTransaction::TYPE_MILESTONE:
         $current = queryfx_one(
           $object->establishConnection('w'),
@@ -313,7 +314,9 @@ final class PhabricatorProjectTransactionEditor
         }
 
         $slug_xaction = last($xactions);
+
         $new = $slug_xaction->getNewValue();
+        $new = $this->normalizeSlugs($new);
 
         if ($new) {
           $slugs_used_already = id(new PhabricatorProjectSlug())
@@ -332,7 +335,7 @@ final class PhabricatorProjectTransactionEditor
                 $type,
                 pht('Invalid'),
                 pht(
-                  'Project hashtag %s is already the primary hashtag.',
+                  'Project hashtag "%s" is already the primary hashtag.',
                   $object->getPrimarySlug()),
                 $slug_xaction);
               $errors[] = $error;
@@ -344,8 +347,8 @@ final class PhabricatorProjectTransactionEditor
             $type,
             pht('Invalid'),
             pht(
-              '%d project hashtag(s) are already used: %s.',
+              '%s project hashtag(s) are already used by other projects: %s.',
-              count($used_slug_strs),
+              phutil_count($used_slug_strs),
               implode(', ', $used_slug_strs)),
             $slug_xaction);
           $errors[] = $error;
@@ -640,4 +643,15 @@ final class PhabricatorProjectTransactionEditor
     return parent::applyFinalEffects($object, $xactions);
   }
 
+  private function normalizeSlugs(array $slugs) {
+    foreach ($slugs as $key => $slug) {
+      $slugs[$key] = PhabricatorSlug::normalizeProjectSlug($slug);
+    }
+
+    $slugs = array_unique($slugs);
+    $slugs = array_values($slugs);
+
+    return $slugs;
+  }
+
 }

src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php

@@ -766,9 +766,9 @@ final class PhabricatorUSEnglishTranslation
         ),
       ),
 
-      '%d project hashtag(s) are already used: %s.' => array(
+      '%s project hashtag(s) are already used by other projects: %s.' => array(
-          'Project hashtag %2$s is already used.',
+        'Project hashtag "%2$s" is already used by another project.',
-          '%d project hashtags are already used: %2$s.',
+        'Some project hashtags are already used by other projects: %2$s.',
       ),
 
       '%s changed project hashtag(s), added %d: %s; removed %d: %s.' =>