Allow Phriction document edits to be saved as drafts

Summary:
Depends on D19661. Ref T13077. See PHI840.

When a user edits a page normally, add a "Save as Draft" button. Much of this change is around making that button render and behave properly: it needs to be an `<input type="submit" ...>` so browsers submit it and we can figure out which button the user clicked.

Then there are a few minor rules:

  - If you're editing a page which is already a draft, we only give you "Save as Draft". This makes edits to update/revise a draft more natural.
  - Highlight "Publish" if it's a likely action that you might want to take.

Internally, there are two types of edits. Both types create a new version with the new content. However:

  - A "content" edit sets the version shown on the live page to the newly-created version.
  - A "draft" edit does not update the version shown on the live page.

Test Plan: Edited a published document, edited the draft. Published documents. Reverted documents.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13077

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

resources/celerity/map.php

@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => 'e68cf1fa',
     'conpherence.pkg.js' => '15191c65',
-    'core.pkg.css' => 'badf3f16',
+    'core.pkg.css' => '2574c199',
     'core.pkg.js' => 'b5a949ca',
     'differential.pkg.css' => '06dc617c',
     'differential.pkg.js' => 'c1cfa143',
@@ -122,7 +122,7 @@ return array(
     'rsrc/css/layout/phabricator-source-code-view.css' => '2ab25dfa',
     'rsrc/css/phui/button/phui-button-bar.css' => 'f1ff5494',
     'rsrc/css/phui/button/phui-button-simple.css' => '8e1baf68',
-    'rsrc/css/phui/button/phui-button.css' => '1863cc6e',
+    'rsrc/css/phui/button/phui-button.css' => '6ccb303c',
     'rsrc/css/phui/calendar/phui-calendar-day.css' => '572b1893',
     'rsrc/css/phui/calendar/phui-calendar-list.css' => '576be600',
     'rsrc/css/phui/calendar/phui-calendar-month.css' => '21154caf',
@@ -151,7 +151,7 @@ return array(
     'rsrc/css/phui/phui-document.css' => 'c4ac41f9',
     'rsrc/css/phui/phui-feed-story.css' => '44a9c8e9',
     'rsrc/css/phui/phui-fontkit.css' => '1320ed01',
-    'rsrc/css/phui/phui-form-view.css' => 'f808e5be',
+    'rsrc/css/phui/phui-form-view.css' => '2f43fae7',
     'rsrc/css/phui/phui-form.css' => '7aaa04e3',
     'rsrc/css/phui/phui-head-thing.css' => 'fd311e5f',
     'rsrc/css/phui/phui-header-view.css' => '1ba8b707',
@@ -800,7 +800,7 @@ return array(
     'phui-box-css' => '4bd6cdb9',
     'phui-bulk-editor-css' => '9a81e5d5',
     'phui-button-bar-css' => 'f1ff5494',
-    'phui-button-css' => '1863cc6e',
+    'phui-button-css' => '6ccb303c',
     'phui-button-simple-css' => '8e1baf68',
     'phui-calendar-css' => 'f1ddf11c',
     'phui-calendar-day-css' => '572b1893',
@@ -819,7 +819,7 @@ return array(
     'phui-font-icon-base-css' => '870a7360',
     'phui-fontkit-css' => '1320ed01',
     'phui-form-css' => '7aaa04e3',
-    'phui-form-view-css' => 'f808e5be',
+    'phui-form-view-css' => '2f43fae7',
     'phui-head-thing-view-css' => 'fd311e5f',
     'phui-header-view-css' => '1ba8b707',
     'phui-hovercard' => '1bd28176',

src/__phutil_library_map__.php

@@ -5021,7 +5021,9 @@ phutil_register_library_map(array(
     'PhrictionDocumentController' => 'applications/phriction/controller/PhrictionDocumentController.php',
     'PhrictionDocumentDatasource' => 'applications/phriction/typeahead/PhrictionDocumentDatasource.php',
     'PhrictionDocumentDeleteTransaction' => 'applications/phriction/xaction/PhrictionDocumentDeleteTransaction.php',
+    'PhrictionDocumentDraftTransaction' => 'applications/phriction/xaction/PhrictionDocumentDraftTransaction.php',
     'PhrictionDocumentEditEngine' => 'applications/phriction/editor/PhrictionDocumentEditEngine.php',
+    'PhrictionDocumentEditTransaction' => 'applications/phriction/xaction/PhrictionDocumentEditTransaction.php',
     'PhrictionDocumentFerretEngine' => 'applications/phriction/search/PhrictionDocumentFerretEngine.php',
     'PhrictionDocumentFulltextEngine' => 'applications/phriction/search/PhrictionDocumentFulltextEngine.php',
     'PhrictionDocumentHeraldAdapter' => 'applications/phriction/herald/PhrictionDocumentHeraldAdapter.php',
@@ -11138,11 +11140,13 @@ phutil_register_library_map(array(
     ),
     'PhrictionDocumentAuthorHeraldField' => 'PhrictionDocumentHeraldField',
     'PhrictionDocumentContentHeraldField' => 'PhrictionDocumentHeraldField',
-    'PhrictionDocumentContentTransaction' => 'PhrictionDocumentVersionTransaction',
+    'PhrictionDocumentContentTransaction' => 'PhrictionDocumentEditTransaction',
     'PhrictionDocumentController' => 'PhrictionController',
     'PhrictionDocumentDatasource' => 'PhabricatorTypeaheadDatasource',
     'PhrictionDocumentDeleteTransaction' => 'PhrictionDocumentVersionTransaction',
+    'PhrictionDocumentDraftTransaction' => 'PhrictionDocumentEditTransaction',
     'PhrictionDocumentEditEngine' => 'PhabricatorEditEngine',
+    'PhrictionDocumentEditTransaction' => 'PhrictionDocumentVersionTransaction',
     'PhrictionDocumentFerretEngine' => 'PhabricatorFerretEngine',
     'PhrictionDocumentFulltextEngine' => 'PhabricatorFulltextEngine',
     'PhrictionDocumentHeraldAdapter' => 'HeraldAdapter',

src/applications/phriction/controller/PhrictionDocumentController.php

@@ -450,19 +450,28 @@ final class PhrictionDocumentController
       $publish_name = pht('Publish Revert');
     }
 
+    // If you're looking at the current version; and it's an unpublished
+    // draft; and you can publish it, add a UI hint that this might be an
+    // interesting action to take.
+    $hint_publish = false;
+    if ($is_draft) {
+      if ($can_publish) {
+        if ($document->getMaxVersion() == $content->getVersion()) {
+          $hint_publish = true;
+        }
+      }
+    }
+
     $publish_uri = "/phriction/publish/{$id}/{$content_id}/";
 
-    if (PhabricatorEnv::getEnvConfig('phabricator.show-prototypes')) {
-      $publish_name = pht('Publish (Prototype!)');
-
-      $curtain->addAction(
-        id(new PhabricatorActionView())
-        ->setName($publish_name)
-        ->setIcon('fa-upload')
-        ->setDisabled(!$can_publish)
-        ->setWorkflow(true)
-        ->setHref($publish_uri));
-    }
+    $curtain->addAction(
+      id(new PhabricatorActionView())
+      ->setName($publish_name)
+      ->setIcon('fa-upload')
+      ->setSelected($hint_publish)
+      ->setDisabled(!$can_publish)
+      ->setWorkflow(true)
+      ->setHref($publish_uri));
 
     if ($document->getStatus() == PhrictionDocumentStatus::STATUS_EXISTS) {
       $curtain->addAction(

src/applications/phriction/controller/PhrictionEditController.php

@@ -95,8 +95,10 @@ final class PhrictionEditController
     $v_space = $document->getSpacePHID();
 
     $content_text = $content->getContent();
+    $is_draft_mode = ($document->getContent()->getVersion() != $max_version);
 
     if ($request->isFormPost()) {
+      $save_as_draft = ($is_draft_mode || $request->getExists('draft'));
 
       $title = $request->getStr('title');
       $content_text = $request->getStr('content');
@@ -108,13 +110,18 @@ final class PhrictionEditController
       $v_projects = $request->getArr('projects');
       $v_space = $request->getStr('spacePHID');
 
+      if ($save_as_draft) {
+        $edit_type = PhrictionDocumentDraftTransaction::TRANSACTIONTYPE;
+      } else {
+        $edit_type = PhrictionDocumentContentTransaction::TRANSACTIONTYPE;
+      }
+
       $xactions = array();
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType(PhrictionDocumentTitleTransaction::TRANSACTIONTYPE)
         ->setNewValue($title);
       $xactions[] = id(new PhrictionTransaction())
-        ->setTransactionType(
-          PhrictionDocumentContentTransaction::TRANSACTIONTYPE)
+        ->setTransactionType($edit_type)
         ->setNewValue($content_text);
       $xactions[] = id(new PhrictionTransaction())
         ->setTransactionType(PhabricatorTransactions::TYPE_VIEW_POLICY)
@@ -147,6 +154,14 @@ final class PhrictionEditController
         $editor->applyTransactions($document, $xactions);
 
         $uri = PhrictionDocument::getSlugURI($document->getSlug());
+        $uri = new PhutilURI($uri);
+
+        // If the user clicked "Save as Draft", take them to the draft, not
+        // to the current published page.
+        if ($save_as_draft) {
+          $uri = $uri->alter('v', $document->getMaxVersion());
+        }
+
         return id(new AphrontRedirectResponse())->setURI($uri);
       } catch (PhabricatorApplicationTransactionValidationException $ex) {
         $validation_exception = $ex;
@@ -176,7 +191,7 @@ final class PhrictionEditController
       if ($overwrite) {
         $submit_button = pht('Overwrite Changes');
       } else {
-        $submit_button = pht('Save Changes');
+        $submit_button = pht('Save and Publish');
       }
     } else {
       $submit_button = pht('Create Document');
@@ -196,7 +211,6 @@ final class PhrictionEditController
     $view_capability = PhabricatorPolicyCapability::CAN_VIEW;
     $edit_capability = PhabricatorPolicyCapability::CAN_EDIT;
 
-
     $form = id(new AphrontFormView())
       ->setUser($viewer)
       ->addHiddenInput('slug', $document->getSlug())
@@ -253,11 +267,26 @@ final class PhrictionEditController
           ->setLabel(pht('Edit Notes'))
           ->setValue($notes)
           ->setError(null)
-          ->setName('description'))
-      ->appendChild(
+          ->setName('description'));
+
+    if ($is_draft_mode) {
+      $form->appendControl(
         id(new AphrontFormSubmitControl())
+          ->addCancelButton($cancel_uri)
+          ->setValue(pht('Save Draft')));
+    } else {
+      $draft_button = id(new PHUIButtonView())
+        ->setTag('input')
+        ->setName('draft')
+        ->setText(pht('Save as Draft'))
+        ->setColor(PHUIButtonView::GREEN);
+
+      $form->appendControl(
+        id(new AphrontFormSubmitControl())
+          ->addButton($draft_button)
           ->addCancelButton($cancel_uri)
           ->setValue($submit_button));
+    }
 
     $form_box = id(new PHUIObjectBoxView())
       ->setHeaderText($page_title)

src/applications/phriction/editor/PhrictionTransactionEditor.php

@@ -74,6 +74,21 @@ final class PhrictionTransactionEditor
     return $this;
   }
 
+  public function setShouldPublishContent(
+    PhrictionDocument $object,
+    $publish) {
+
+    if ($publish) {
+      $content_phid = $this->getNewContent()->getPHID();
+    } else {
+      $content_phid = $this->getOldContent()->getPHID();
+    }
+
+    $object->setContentPHID($content_phid);
+
+    return $this;
+  }
+
   public function getEditorApplicationClass() {
     return 'PhabricatorPhrictionApplication';
   }

src/applications/phriction/xaction/PhrictionDocumentContentTransaction.php

@@ -1,94 +1,16 @@
 <?php
 
 final class PhrictionDocumentContentTransaction
-  extends PhrictionDocumentVersionTransaction {
+  extends PhrictionDocumentEditTransaction {
 
   const TRANSACTIONTYPE = 'content';
 
-  public function generateOldValue($object) {
-    if ($this->getEditor()->getIsNewObject()) {
-      return null;
-    }
-    return $object->getContent()->getContent();
-  }
-
-  public function generateNewValue($object, $value) {
-    return $value;
-  }
-
   public function applyInternalEffects($object, $value) {
+    parent::applyInternalEffects($object, $value);
+
     $object->setStatus(PhrictionDocumentStatus::STATUS_EXISTS);
 
-    $content = $this->getNewDocumentContent($object);
-    $content->setContent($value);
-  }
-
-  public function shouldHide() {
-    if ($this->getOldValue() === null) {
-      return true;
-    } else {
-      return false;
-    }
-  }
-
-  public function getActionStrength() {
-    return 1.3;
-  }
-
-  public function getActionName() {
-    return pht('Edited');
-  }
-
-  public function getTitle() {
-    return pht(
-      '%s edited the content of this document.',
-      $this->renderAuthor());
-  }
-
-  public function getTitleForFeed() {
-    return pht(
-      '%s edited the content of %s.',
-      $this->renderAuthor(),
-      $this->renderObject());
-  }
-
-  public function hasChangeDetailView() {
-    return true;
-  }
-
-  public function getMailDiffSectionHeader() {
-    return pht('CHANGES TO DOCUMENT CONTENT');
-  }
-
-  public function newChangeDetailView() {
-    $viewer = $this->getViewer();
-
-    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
-      ->setViewer($viewer)
-      ->setOldText($this->getOldValue())
-      ->setNewText($this->getNewValue());
-  }
-
-  public function newRemarkupChanges() {
-    $changes = array();
-
-    $changes[] = $this->newRemarkupChange()
-      ->setOldValue($this->getOldValue())
-      ->setNewValue($this->getNewValue());
-
-    return $changes;
-  }
-
-  public function validateTransactions($object, array $xactions) {
-    $errors = array();
-
-    $content = $object->getContent()->getContent();
-    if ($this->isEmptyTextTransaction($content, $xactions)) {
-      $errors[] = $this->newRequiredError(
-        pht('Documents must have content.'));
-    }
-
-    return $errors;
+    $this->getEditor()->setShouldPublishContent($object, true);
   }
 
 }

src/applications/phriction/xaction/PhrictionDocumentDraftTransaction.php

@@ -0,0 +1,14 @@
+<?php
+
+final class PhrictionDocumentDraftTransaction
+  extends PhrictionDocumentEditTransaction {
+
+  const TRANSACTIONTYPE = 'draft';
+
+  public function applyInternalEffects($object, $value) {
+    parent::applyInternalEffects($object, $value);
+
+    $this->getEditor()->setShouldPublishContent($object, false);
+  }
+
+}

src/applications/phriction/xaction/PhrictionDocumentEditTransaction.php

@@ -0,0 +1,95 @@
+<?php
+
+abstract class PhrictionDocumentEditTransaction
+  extends PhrictionDocumentVersionTransaction {
+
+  public function generateOldValue($object) {
+    if ($this->getEditor()->getIsNewObject()) {
+      return null;
+    }
+
+    // NOTE: We want to get the newest version of the content here, regardless
+    // of whether it's published or not.
+
+    $actor = $this->getActor();
+
+    return id(new PhrictionContentQuery())
+      ->setViewer($actor)
+      ->withDocumentPHIDs(array($object->getPHID()))
+      ->setOrder('newest')
+      ->setLimit(1)
+      ->executeOne()
+      ->getContent();
+  }
+
+  public function generateNewValue($object, $value) {
+    return $value;
+  }
+
+  public function applyInternalEffects($object, $value) {
+    $content = $this->getNewDocumentContent($object);
+    $content->setContent($value);
+  }
+
+  public function getActionStrength() {
+    return 1.3;
+  }
+
+  public function getActionName() {
+    return pht('Edited');
+  }
+
+  public function getTitle() {
+    return pht(
+      '%s edited the content of this document.',
+      $this->renderAuthor());
+  }
+
+  public function getTitleForFeed() {
+    return pht(
+      '%s edited the content of %s.',
+      $this->renderAuthor(),
+      $this->renderObject());
+  }
+
+  public function getMailDiffSectionHeader() {
+    return pht('CHANGES TO DOCUMENT CONTENT');
+  }
+
+  public function hasChangeDetailView() {
+    return true;
+  }
+
+  public function newChangeDetailView() {
+    $viewer = $this->getViewer();
+
+    return id(new PhabricatorApplicationTransactionTextDiffDetailView())
+      ->setViewer($viewer)
+      ->setOldText($this->getOldValue())
+      ->setNewText($this->getNewValue());
+  }
+
+  public function newRemarkupChanges() {
+    $changes = array();
+
+    $changes[] = $this->newRemarkupChange()
+      ->setOldValue($this->getOldValue())
+      ->setNewValue($this->getNewValue());
+
+    return $changes;
+  }
+
+  public function validateTransactions($object, array $xactions) {
+    $errors = array();
+
+    $content = $object->getContent()->getContent();
+    if ($this->isEmptyTextTransaction($content, $xactions)) {
+      $errors[] = $this->newRequiredError(
+        pht('Documents must have content.'));
+    }
+
+    return $errors;
+  }
+
+
+}

src/view/phui/PHUIButtonView.php

@@ -233,14 +233,15 @@ final class PHUIButtonView extends AphrontTagView {
       $classes = array();
     }
 
-    // See PHI823. If we aren't rendering a "<button>" tag, give the tag we
-    // are rendering a "button" role as a hint to screen readers.
+    // See PHI823. If we aren't rendering a "<button>" or "<input>" tag,
+    // give the tag we are rendering a "button" role as a hint to screen
+    // readers.
     $role = null;
-    if ($this->tag !== 'button') {
+    if ($this->tag !== 'button' && $this->tag !== 'input') {
       $role = 'button';
     }
 
-    return array(
+    $attrs = array(
       'class'  => $classes,
       'href'   => $this->href,
       'name'   => $this->name,
@@ -249,9 +250,19 @@ final class PHUIButtonView extends AphrontTagView {
       'meta'   => $meta,
       'role' => $role,
     );
+
+    if ($this->tag == 'input') {
+      $attrs['type'] = 'submit';
+      $attrs['value'] = $this->text;
+    }
+
+    return $attrs;
   }
 
   protected function getTagContent() {
+    if ($this->tag === 'input') {
+      return null;
+    }
 
     $icon = $this->icon;
     $text = null;

webroot/rsrc/css/phui/button/phui-button.css

@@ -4,7 +4,8 @@
 
 
 button,
-a.button {
+a.button,
+input[type="submit"] {
   font: {$basefont};
   -webkit-font-smoothing: antialiased;
   -webkit-user-select: none;
@@ -72,7 +73,8 @@ a.icon:visited {
 
 button.button-green,
 a.button-green.button,
-a.button-green.button:visited {
+a.button-green.button:visited,
+input[type="submit"].button-green {
   background-color: {$green.button.color};
   border-color: {$green.button.color};
   background-image: {$green.button.gradient};
@@ -80,16 +82,17 @@ a.button-green.button:visited {
 
 button.button-red,
 a.button-red.button,
-a.button-red.button:visited {
+a.button-red.button:visited,
+input[type="submit"].button-red {
   background-color: {$red.button.color};
   border-color: {$red.button.color};
   background-image: {$red.button.gradient};
 }
 
 button.button-grey,
-input[type="submit"].button-grey,
 a.button-grey,
-a.button-grey:visited {
+a.button-grey:visited,
+input[type="submit"].button-grey {
   background-color: {$grey.button.color};
   background-image: {$grey.button.gradient};
   border: 1px solid rgba({$alphablue}, 0.3);

webroot/rsrc/css/phui/phui-form-view.css

@@ -133,19 +133,12 @@
 }
 
 .aphront-form-control-submit button,
-.aphront-form-control-submit a.button {
+.aphront-form-control-submit a.button,
+.aphront-form-control-submit input[type="submit"] {
   float: right;
   margin: 4px 0 0 8px;
 }
 
-.phui-form-control-multi-submit input,
-.phui-form-control-multi-submit button,
-.phui-form-control-multi-submit a {
-  float: right;
-  margin: 4px 0 0 8px;
-  width: auto;
-}
-
 .aphront-form-control-textarea textarea.aphront-textarea-very-short {
   height: 44px;
 }