Store Phriction max version on Document, improve editing rules for editing documents with drafts

Summary:
Ref T13077. We need to know the maximum version of a document in several cases, so denormalize it onto the Document object.

Then clean up some behaviors where we edit a document with, e.g., 7 versions but version 5 is currently published. For now, we: edit starting with version 7, save as version 8, and immediately publish the new version.

Test Plan:
  - Ran migration.
  - Edited a draft page without hitting any weird version errors.
  - Checked database for sensible `maxVersion` values.

Reviewers: amckinley

Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam

Maniphest Tasks: T13077

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

resources/sql/autopatches/20180830.phriction.01.maxversion.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_phriction.phriction_document
+  ADD maxVersion INT UNSIGNED NOT NULL;

resources/sql/autopatches/20180830.phriction.02.maxes.php

@@ -0,0 +1,30 @@
+<?php
+
+// Populate the "maxVersion" column by copying the maximum "version" from the
+// content table.
+
+$document_table = new PhrictionDocument();
+$content_table = new PhrictionContent();
+
+$conn = $document_table->establishConnection('w');
+
+$iterator = new LiskRawMigrationIterator(
+  $conn,
+  $document_table->getTableName());
+foreach ($iterator as $row) {
+  $content = queryfx_one(
+    $conn,
+    'SELECT MAX(version) max FROM %T WHERE documentPHID = %s',
+    $content_table->getTableName(),
+    $row['phid']);
+  if (!$content) {
+    continue;
+  }
+
+  queryfx(
+    $conn,
+    'UPDATE %T SET maxVersion = %d WHERE id = %d',
+    $document_table->getTableName(),
+    $content['max'],
+    $row['id']);
+}

src/applications/phriction/controller/PhrictionDocumentController.php

@@ -66,12 +66,7 @@ final class PhrictionDocumentController
         ->addAction($create_button);
 
     } else {
-      $draft_content = id(new PhrictionContentQuery())
+      $max_version = (int)$document->getMaxVersion();
-        ->setViewer($viewer)
-        ->withDocumentPHIDs(array($document->getPHID()))
-        ->setLimit(1)
-        ->executeOne();
-      $max_version = (int)$draft_content->getVersion();
 
       $version = $request->getInt('v');
       if ($version) {

src/applications/phriction/controller/PhrictionEditController.php

@@ -7,7 +7,7 @@ final class PhrictionEditController
     $viewer = $request->getViewer();
     $id = $request->getURIData('id');
 
-    $current_version = null;
+    $max_version = null;
     if ($id) {
       $is_new = false;
       $document = id(new PhrictionDocumentQuery())
@@ -24,7 +24,7 @@ final class PhrictionEditController
         return new Aphront404Response();
       }
 
-      $current_version = $document->getContent()->getVersion();
+      $max_version = $document->getMaxVersion();
 
       $revert = $request->getInt('revert');
       if ($revert) {
@@ -37,9 +37,12 @@ final class PhrictionEditController
           return new Aphront404Response();
         }
       } else {
-        $content = $document->getContent();
+        $content = id(new PhrictionContentQuery())
+          ->setViewer($viewer)
+          ->withDocumentPHIDs(array($document->getPHID()))
+          ->setLimit(1)
+          ->executeOne();
       }
-
     } else {
       $slug = $request->getStr('slug');
       $slug = PhabricatorSlug::normalize($slug);
@@ -54,8 +57,13 @@ final class PhrictionEditController
         ->executeOne();
 
       if ($document) {
-        $content = $document->getContent();
+        $content = id(new PhrictionContentQuery())
-        $current_version = $content->getVersion();
+          ->setViewer($viewer)
+          ->withDocumentPHIDs(array($document->getPHID()))
+          ->setLimit(1)
+          ->executeOne();
+
+        $max_version = $document->getMaxVersion();
         $is_new = false;
       } else {
         $document = PhrictionDocument::initializeNewDocument($viewer, $slug);
@@ -128,7 +136,7 @@ final class PhrictionEditController
       $title = $request->getStr('title');
       $content_text = $request->getStr('content');
       $notes = $request->getStr('description');
-      $current_version = $request->getInt('contentVersion');
+      $max_version = $request->getInt('contentVersion');
       $v_view = $request->getStr('viewPolicy');
       $v_edit = $request->getStr('editPolicy');
       $v_cc = $request->getArr('cc');
@@ -168,7 +176,7 @@ final class PhrictionEditController
         ->setContinueOnNoEffect(true)
         ->setDescription($notes)
         ->setProcessContentVersionError(!$request->getBool('overwrite'))
-        ->setContentVersion($current_version);
+        ->setContentVersion($max_version);
 
       try {
         $editor->applyTransactions($document, $xactions);
@@ -232,7 +240,7 @@ final class PhrictionEditController
       ->setUser($viewer)
       ->addHiddenInput('slug', $document->getSlug())
       ->addHiddenInput('nodraft', $request->getBool('nodraft'))
-      ->addHiddenInput('contentVersion', $current_version)
+      ->addHiddenInput('contentVersion', $max_version)
       ->addHiddenInput('overwrite', $overwrite)
       ->appendChild(
         id(new AphrontFormTextControl())

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -468,7 +468,7 @@ final class PhrictionTransactionEditor
 
     $error = null;
     if ($this->getContentVersion() &&
-       ($object->getContent()->getVersion() != $this->getContentVersion())) {
+       ($object->getMaxVersion() != $this->getContentVersion())) {
       $error = new PhabricatorApplicationTransactionValidationError(
         $type,
         pht('Edit Conflict'),
@@ -519,6 +519,7 @@ final class PhrictionTransactionEditor
       $document->setContentPHID($content_phid);
       $document->attachContent($content);
       $document->setEditedEpoch(PhabricatorTime::getNow());
+      $document->setMaxVersion($content->getVersion());
 
       $this->newContent = $content;
     }
@@ -539,7 +540,7 @@ final class PhrictionTransactionEditor
       $content->setDescription($this->getDescription());
     }
 
-    $content->setVersion($this->getOldContent()->getVersion() + 1);
+    $content->setVersion($document->getMaxVersion() + 1);
 
     return $content;
   }

src/applications/phriction/storage/PhrictionDocument.php

@@ -23,6 +23,7 @@ final class PhrictionDocument extends PhrictionDAO
   protected $editPolicy;
   protected $spacePHID;
   protected $editedEpoch;
+  protected $maxVersion;
 
   private $contentObject = self::ATTACHABLE;
   private $ancestors = array();
@@ -36,6 +37,7 @@ final class PhrictionDocument extends PhrictionDAO
         'depth' => 'uint32',
         'status' => 'text32',
         'editedEpoch' => 'epoch',
+        'maxVersion' => 'uint32',
       ),
       self::CONFIG_KEY_SCHEMA => array(
         'slug' => array(
@@ -89,6 +91,7 @@ final class PhrictionDocument extends PhrictionDAO
     }
 
     $document->setEditedEpoch(PhabricatorTime::getNow());
+    $document->setMaxVersion(0);
 
     return $document;
   }