Always automatically generate Phame slugs

Summary:
Fixes T9995. I think letting users customize slugs is not a hugely compelling as a product feature, and this fixes the issue with slugs that have "/" characters in them and makes the move to EditEngine easier since I don't have to deal with the weird JS thing.

Instead, just generate slugs automatically. No more JS, no more separate field, things automatically update if you rename a blog, and now that URIs have IDs in them the old URI will still work after a rename.

Test Plan:
  - Applied migration.
  - Created new posts.
  - Edited existing posts.
  - Visited various posts.
  - Created a post with a bunch of "/" in the title, things still worked fine.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T9995

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

resources/celerity/map.php

@@ -410,7 +410,6 @@ return array(
     'rsrc/js/application/owners/OwnersPathEditor.js' => 'aa1733d0',
     'rsrc/js/application/owners/owners-path-editor.js' => '7a68dda3',
     'rsrc/js/application/passphrase/passphrase-credential-control.js' => '3cb0b2fc',
-    'rsrc/js/application/phame/phame-post-preview.js' => 'd6bba572',
     'rsrc/js/application/pholio/behavior-pholio-mock-edit.js' => '246dc085',
     'rsrc/js/application/pholio/behavior-pholio-mock-view.js' => 'fbe497e7',
     'rsrc/js/application/phortune/behavior-stripe-payment-form.js' => '3f5d6dbf',
@@ -648,7 +647,6 @@ return array(
     'javelin-behavior-phabricator-transaction-comment-form' => 'b23b49e6',
     'javelin-behavior-phabricator-transaction-list' => '13c739ea',
     'javelin-behavior-phabricator-watch-anchor' => '9f36c42d',
-    'javelin-behavior-phame-post-preview' => 'd6bba572',
     'javelin-behavior-pholio-mock-edit' => '246dc085',
     'javelin-behavior-pholio-mock-view' => 'fbe497e7',
     'javelin-behavior-phui-dropdown-menu' => '54733475',
@@ -1856,12 +1854,6 @@ return array(
       'javelin-dom',
       'javelin-stratcom',
     ),
-    'd6bba572' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'phabricator-shaped-request',
-    ),
     'd75709e6' => array(
       'javelin-behavior',
       'javelin-workflow',

resources/sql/autopatches/20151215.phame.1.autotitle.sql

@@ -0,0 +1,5 @@
+ALTER TABLE {$NAMESPACE}_phame.phame_post
+  DROP KEY phameTitle;
+
+ALTER TABLE {$NAMESPACE}_phame.phame_post
+  CHANGE phameTitle phameTitle VARCHAR(64) COLLATE {$COLLATE_SORT};

src/applications/phame/conduit/PhameCreatePostConduitAPIMethod.php

@@ -19,7 +19,6 @@ final class PhameCreatePostConduitAPIMethod extends PhameConduitAPIMethod {
       'blogPHID'      => 'required phid',
       'title'         => 'required string',
       'body'          => 'required string',
-      'phameTitle'    => 'optional string',
       'bloggerPHID'   => 'optional phid',
       'isDraft'       => 'optional bool',
     );
@@ -89,12 +88,6 @@ final class PhameCreatePostConduitAPIMethod extends PhameConduitAPIMethod {
       $post->setVisibility(PhameConstants::VISIBILITY_PUBLISHED);
     }
     $post->setTitle($title);
-    $phame_title = $request->getValue(
-      'phameTitle',
-      id(new PhutilUTF8StringTruncator())
-      ->setMaximumBytes(64)
-      ->truncateString($title));
-    $post->setPhameTitle(PhabricatorSlug::normalize($phame_title));
     $post->setBody($body);
     $post->save();
 

src/applications/phame/conduit/PhameQueryPostsConduitAPIMethod.php

@@ -20,7 +20,6 @@ final class PhameQueryPostsConduitAPIMethod extends PhameConduitAPIMethod {
       'phids'          => 'optional list<phid>',
       'blogPHIDs'      => 'optional list<phid>',
       'bloggerPHIDs'   => 'optional list<phid>',
-      'phameTitles'    => 'optional list<string>',
       'published'      => 'optional bool',
       'publishedAfter' => 'optional date',
       'before'         => 'optional int',
@@ -58,11 +57,6 @@ final class PhameQueryPostsConduitAPIMethod extends PhameConduitAPIMethod {
       $query->withBloggerPHIDs($blogger_phids);
     }
 
-    $phame_titles = $request->getValue('phameTitles', array());
-    if ($phame_titles) {
-      $query->withPhameTitles($phame_titles);
-    }
-
     $published = $request->getValue('published', null);
     if ($published === true) {
       $query->withVisibility(PhameConstants::VISIBILITY_PUBLISHED);

src/applications/phame/controller/post/PhamePostEditController.php

@@ -60,17 +60,13 @@ final class PhamePostEditController extends PhamePostController {
     }
 
     $title = $post->getTitle();
-    $phame_title = $post->getPhameTitle();
     $body = $post->getBody();
     $visibility = $post->getVisibility();
 
     $e_title       = true;
-    $e_phame_title = true;
     $validation_exception = null;
     if ($request->isFormPost()) {
       $title = $request->getStr('title');
-      $phame_title = $request->getStr('phame_title');
-      $phame_title = PhabricatorSlug::normalize($phame_title);
       $body = $request->getStr('body');
       $v_projects = $request->getArr('projects');
       $v_cc = $request->getArr('cc');
@@ -80,9 +76,6 @@ final class PhamePostEditController extends PhamePostController {
         id(new PhamePostTransaction())
           ->setTransactionType(PhamePostTransaction::TYPE_TITLE)
           ->setNewValue($title),
-        id(new PhamePostTransaction())
-          ->setTransactionType(PhamePostTransaction::TYPE_PHAME_TITLE)
-          ->setNewValue($phame_title),
         id(new PhamePostTransaction())
           ->setTransactionType(PhamePostTransaction::TYPE_BODY)
           ->setNewValue($body),
@@ -115,8 +108,6 @@ final class PhamePostEditController extends PhamePostController {
         $validation_exception = $ex;
         $e_title = $validation_exception->getShortMessage(
           PhamePostTransaction::TYPE_TITLE);
-        $e_phame_title = $validation_exception->getShortMessage(
-          PhamePostTransaction::TYPE_PHAME_TITLE);
       }
     }
 
@@ -139,15 +130,6 @@ final class PhamePostEditController extends PhamePostController {
         ->setValue($title)
         ->setID('post-title')
         ->setError($e_title))
-      ->appendChild(
-        id(new AphrontFormTextControl())
-        ->setLabel(pht('Phame Title'))
-        ->setName('phame_title')
-        ->setValue(rtrim($phame_title, '/'))
-        ->setID('post-phame-title')
-        ->setCaption(pht('Up to 64 alphanumeric characters '.
-                     'with underscores for spaces.'))
-        ->setError($e_phame_title))
       ->appendChild(
         id(new AphrontFormSelectControl())
         ->setLabel(pht('Visibility'))
@@ -187,13 +169,6 @@ final class PhamePostEditController extends PhamePostController {
       ->setControlID('post-body')
       ->setPreviewType(PHUIRemarkupPreviewPanel::DOCUMENT);
 
-    Javelin::initBehavior(
-      'phame-post-preview',
-      array(
-        'title'       => 'post-title',
-        'phame_title' => 'post-phame-title',
-      ));
-
     $form_box = id(new PHUIObjectBoxView())
       ->setHeaderText($page_title)
       ->setValidationException($validation_exception)

src/applications/phame/editor/PhamePostEditor.php

@@ -15,7 +15,6 @@ final class PhamePostEditor
     $types = parent::getTransactionTypes();
 
     $types[] = PhamePostTransaction::TYPE_TITLE;
-    $types[] = PhamePostTransaction::TYPE_PHAME_TITLE;
     $types[] = PhamePostTransaction::TYPE_BODY;
     $types[] = PhamePostTransaction::TYPE_VISIBILITY;
     $types[] = PhabricatorTransactions::TYPE_COMMENT;
@@ -30,8 +29,6 @@ final class PhamePostEditor
     switch ($xaction->getTransactionType()) {
       case PhamePostTransaction::TYPE_TITLE:
         return $object->getTitle();
-      case PhamePostTransaction::TYPE_PHAME_TITLE:
-        return $object->getPhameTitle();
       case PhamePostTransaction::TYPE_BODY:
         return $object->getBody();
       case PhamePostTransaction::TYPE_VISIBILITY:
@@ -45,7 +42,6 @@ final class PhamePostEditor
 
     switch ($xaction->getTransactionType()) {
       case PhamePostTransaction::TYPE_TITLE:
-      case PhamePostTransaction::TYPE_PHAME_TITLE:
       case PhamePostTransaction::TYPE_BODY:
       case PhamePostTransaction::TYPE_VISIBILITY:
         return $xaction->getNewValue();
@@ -59,8 +55,6 @@ final class PhamePostEditor
     switch ($xaction->getTransactionType()) {
       case PhamePostTransaction::TYPE_TITLE:
         return $object->setTitle($xaction->getNewValue());
-      case PhamePostTransaction::TYPE_PHAME_TITLE:
-        return $object->setPhameTitle($xaction->getNewValue());
       case PhamePostTransaction::TYPE_BODY:
         return $object->setBody($xaction->getNewValue());
       case PhamePostTransaction::TYPE_VISIBILITY:
@@ -81,7 +75,6 @@ final class PhamePostEditor
 
     switch ($xaction->getTransactionType()) {
       case PhamePostTransaction::TYPE_TITLE:
-      case PhamePostTransaction::TYPE_PHAME_TITLE:
       case PhamePostTransaction::TYPE_BODY:
       case PhamePostTransaction::TYPE_VISIBILITY:
         return;
@@ -113,43 +106,6 @@ final class PhamePostEditor
           $error->setIsMissingFieldError(true);
           $errors[] = $error;
         }
-        break;
-      case PhamePostTransaction::TYPE_PHAME_TITLE:
-        if (!$xactions) {
-          continue;
-        }
-        $missing = $this->validateIsEmptyTextField(
-          $object->getPhameTitle(),
-          $xactions);
-        $phame_title = last($xactions)->getNewValue();
-
-        if ($missing || $phame_title == '/') {
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Required'),
-            pht('Phame title is required.'),
-            nonempty(last($xactions), null));
-
-          $error->setIsMissingFieldError(true);
-          $errors[] = $error;
-        }
-
-        $duplicate_post = id(new PhamePostQuery())
-          ->setViewer(PhabricatorUser::getOmnipotentUser())
-          ->withPhameTitles(array($phame_title))
-          ->executeOne();
-        if ($duplicate_post && $duplicate_post->getID() != $object->getID()) {
-          $error_text = pht(
-            'Phame title must be unique; another post already has this phame '.
-            'title.');
-          $error = new PhabricatorApplicationTransactionValidationError(
-            $type,
-            pht('Not Unique'),
-            $error_text,
-            nonempty(last($xactions), null));
-          $errors[] = $error;
-        }
-
         break;
     }
     return $errors;

src/applications/phame/query/PhamePostQuery.php

@@ -5,7 +5,6 @@ final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery {
   private $ids;
   private $blogPHIDs;
   private $bloggerPHIDs;
-  private $phameTitles;
   private $visibility;
   private $publishedAfter;
   private $phids;
@@ -30,11 +29,6 @@ final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery {
     return $this;
   }
 
-  public function withPhameTitles(array $phame_titles) {
-    $this->phameTitles = $phame_titles;
-    return $this;
-  }
-
   public function withVisibility($visibility) {
     $this->visibility = $visibility;
     return $this;
@@ -106,13 +100,6 @@ final class PhamePostQuery extends PhabricatorCursorPagedPolicyAwareQuery {
         $this->bloggerPHIDs);
     }
 
-    if ($this->phameTitles) {
-      $where[] = qsprintf(
-        $conn,
-        'p.phameTitle IN (%Ls)',
-        $this->phameTitles);
-    }
-
     if ($this->visibility !== null) {
       $where[] = qsprintf(
         $conn,

src/applications/phame/storage/PhamePost.php

@@ -98,7 +98,7 @@ final class PhamePost extends PhameDAO
       ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'title' => 'text255',
-        'phameTitle' => 'sort64',
+        'phameTitle' => 'sort64?',
         'visibility' => 'uint32',
         'mailKey' => 'bytes20',
 
@@ -118,10 +118,6 @@ final class PhamePost extends PhameDAO
           'columns' => array('phid'),
           'unique' => true,
         ),
-        'phameTitle' => array(
-          'columns' => array('bloggerPHID', 'phameTitle'),
-          'unique' => true,
-        ),
         'bloggerPosts' => array(
           'columns' => array(
             'bloggerPHID',
@@ -147,7 +143,7 @@ final class PhamePost extends PhameDAO
   }
 
   public function getSlug() {
-    return rtrim($this->getPhameTitle(), '/');
+    return PhabricatorSlug::normalizeProjectSlug($this->getTitle(), true);
   }
 
   public function toDictionary() {
@@ -158,7 +154,6 @@ final class PhamePost extends PhameDAO
       'bloggerPHID'   => $this->getBloggerPHID(),
       'viewURI'       => $this->getViewURI(),
       'title'         => $this->getTitle(),
-      'phameTitle'    => $this->getPhameTitle(),
       'body'          => $this->getBody(),
       'summary'       => PhabricatorMarkupEngine::summarize($this->getBody()),
       'datePublished' => $this->getDatePublished(),

webroot/rsrc/js/application/phame/phame-post-preview.js

@@ -1,67 +0,0 @@
-/**
- * @provides javelin-behavior-phame-post-preview
- * @requires javelin-behavior
- *           javelin-dom
- *           javelin-util
- *           phabricator-shaped-request
- */
-
-JX.behavior('phame-post-preview', function(config) {
-
-  var title       = JX.$(config.title);
-  var phame_title = JX.$(config.phame_title);
-  var sync_titles = true;
-
-  var titleCallback = function() {
-    if (!sync_titles) {
-      return;
-    }
-    var title_string  = title.value;
-    phame_title.value = normalizeSlug(title_string);
-  };
-
-  var phameTitleKeyupCallback = function () {
-    // stop sync'ing once user edits phame_title directly
-    sync_titles    = false;
-    var normalized = normalizeSlug(phame_title.value, true);
-    if (normalized == phame_title.value) {
-      return;
-    }
-    var position = phame_title.value.length;
-    if ('selectionStart' in phame_title) {
-      position = phame_title.selectionStart;
-    }
-    phame_title.value = normalized;
-    if ('setSelectionRange' in phame_title) {
-      phame_title.focus();
-      phame_title.setSelectionRange(position, position);
-    }
-  };
-
-  var phameTitleBlurCallback = function () {
-    phame_title.value = normalizeSlug(phame_title.value);
-  };
-
-  // This is a sort of implementation of PhabricatorSlug::normalize
-  var normalizeSlug = function (slug, spare_trailing_underscore) {
-    var s = slug.toLowerCase().replace(/[^a-z0-9/]+/g, '_').substr(0, 63);
-    if (spare_trailing_underscore) {
-      // do nothing
-    } else {
-      s = s.replace(/_$/g, '');
-    }
-    return s;
-  };
-
-  var getdata = function() {
-    return {
-      title       : title.value,
-      phame_title : phame_title.value
-    };
-  };
-
-  JX.DOM.listen(title,       'keyup',   null, titleCallback);
-  JX.DOM.listen(phame_title, 'keyup',   null, phameTitleKeyupCallback);
-  JX.DOM.listen(phame_title, 'blur',    null, phameTitleBlurCallback);
-
-});