Use EditEngine stacked comments in Diffusion

Summary: Ref T10978. Ref T8739. Fixes T10446. Converts Diffusion to modern comment/preview code, like Differential. Test Plan: {F2342933} Reviewers: chad Reviewed By: chad Maniphest Tasks: T10978, T10446, T8739 Differential Revision: https://secure.phabricator.com/D17183
2017-01-11 14:01:59 -08:00
parent 82c891f586
commit b5722a9963
15 changed files with 144 additions and 404 deletions
--- a/src/applications/audit/application/PhabricatorAuditApplication.php
+++ b/src/applications/audit/application/PhabricatorAuditApplication.php
@@ -35,8 +35,6 @@ final class PhabricatorAuditApplication extends PhabricatorApplication {
    return array(
      '/audit/' => array(
        '(?:query/(?P<queryKey>[^/]+)/)?' => 'PhabricatorAuditListController',
-        'addcomment/' => 'PhabricatorAuditAddCommentController',
-        'preview/(?P<id>[1-9]\d*)/' => 'PhabricatorAuditPreviewController',
      ),
    );
  }
--- a/src/applications/audit/controller/PhabricatorAuditAddCommentController.php
+++ b/src/applications/audit/controller/PhabricatorAuditAddCommentController.php
@@ -1,90 +0,0 @@
-<?php
-
-final class PhabricatorAuditAddCommentController
-  extends PhabricatorAuditController {
-
-  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getViewer();
-
-    if (!$request->isFormPost()) {
-      return new Aphront403Response();
-    }
-
-    $commit_phid = $request->getStr('commit');
-    $commit = id(new DiffusionCommitQuery())
-      ->setViewer($viewer)
-      ->withPHIDs(array($commit_phid))
-      ->needAuditRequests(true)
-      ->executeOne();
-    if (!$commit) {
-      return new Aphront404Response();
-    }
-
-    $xactions = array();
-
-    // make sure we only add auditors or ccs if the action matches
-    $action = $request->getStr('action');
-    switch ($action) {
-      case PhabricatorAuditActionConstants::ADD_AUDITORS:
-        $auditors = $request->getArr('auditors');
-        $xactions[] = id(new PhabricatorAuditTransaction())
-          ->setTransactionType(PhabricatorAuditActionConstants::ADD_AUDITORS)
-          ->setNewValue(array_fuse($auditors));
-        break;
-      case PhabricatorAuditActionConstants::ADD_CCS:
-        $xactions[] = id(new PhabricatorAuditTransaction())
-          ->setTransactionType(PhabricatorTransactions::TYPE_SUBSCRIBERS)
-          ->setNewValue(
-            array(
-              '+' => $request->getArr('ccs'),
-            ));
-        break;
-      case PhabricatorAuditActionConstants::COMMENT:
-        // We'll deal with this below.
-        break;
-      default:
-        $xactions[] = id(new PhabricatorAuditTransaction())
-          ->setTransactionType(PhabricatorAuditActionConstants::ACTION)
-          ->setNewValue($action);
-        break;
-    }
-
-    $content = $request->getStr('content');
-    if (strlen($content)) {
-      $xactions[] = id(new PhabricatorAuditTransaction())
-        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-        ->attachComment(
-          id(new PhabricatorAuditTransactionComment())
-            ->setCommitPHID($commit->getPHID())
-            ->setContent($content));
-    }
-
-    $inlines = PhabricatorAuditInlineComment::loadDraftComments(
-      $viewer,
-      $commit->getPHID());
-    foreach ($inlines as $inline) {
-      $xactions[] = id(new PhabricatorAuditTransaction())
-        ->setTransactionType(PhabricatorAuditActionConstants::INLINE)
-        ->attachComment($inline->getTransactionComment());
-    }
-
-    id(new PhabricatorAuditEditor())
-      ->setActor($viewer)
-      ->setContentSourceFromRequest($request)
-      ->setContinueOnMissingFields(true)
-      ->applyTransactions($commit, $xactions);
-
-    $draft = id(new PhabricatorDraft())->loadOneWhere(
-      'authorPHID = %s AND draftKey = %s',
-      $viewer->getPHID(),
-      'diffusion-audit-'.$commit->getID());
-    if ($draft) {
-      $draft->delete();
-    }
-
-    $uri = $commit->getURI();
-
-    return id(new AphrontRedirectResponse())->setURI($uri);
-  }
-
-}
--- a/src/applications/audit/controller/PhabricatorAuditPreviewController.php
+++ b/src/applications/audit/controller/PhabricatorAuditPreviewController.php
@@ -1,82 +0,0 @@
-<?php
-
-final class PhabricatorAuditPreviewController
-  extends PhabricatorAuditController {
-
-  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getViewer();
-    $id = $request->getURIData('id');
-
-    $commit = id(new PhabricatorRepositoryCommit())->load($id);
-    if (!$commit) {
-      return new Aphront404Response();
-    }
-
-    $xactions = array();
-
-    $action = $request->getStr('action');
-    if ($action != PhabricatorAuditActionConstants::COMMENT) {
-      $action_xaction = id(new PhabricatorAuditTransaction())
-        ->setAuthorPHID($viewer->getPHID())
-        ->setObjectPHID($commit->getPHID())
-        ->setTransactionType(PhabricatorAuditActionConstants::ACTION)
-        ->setNewValue($action);
-
-      $auditors = $request->getStrList('auditors');
-      if ($action == PhabricatorAuditActionConstants::ADD_AUDITORS &&
-        $auditors) {
-        $action_xaction->setTransactionType($action);
-        $action_xaction->setNewValue(array_fuse($auditors));
-      }
-
-      $ccs = $request->getStrList('ccs');
-      if ($action == PhabricatorAuditActionConstants::ADD_CCS && $ccs) {
-        $action_xaction->setTransactionType(
-          PhabricatorTransactions::TYPE_SUBSCRIBERS);
-
-        // NOTE: This doesn't get processed before use, so just provide fake
-        // values.
-        $action_xaction->setOldValue(array());
-        $action_xaction->setNewValue($ccs);
-      }
-
-      $xactions[] = $action_xaction;
-    }
-
-    $content = $request->getStr('content');
-    if (strlen($content)) {
-      $xactions[] = id(new PhabricatorAuditTransaction())
-        ->setAuthorPHID($viewer->getPHID())
-        ->setObjectPHID($commit->getPHID())
-        ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT)
-        ->attachComment(
-          id(new PhabricatorAuditTransactionComment())
-            ->setContent($content));
-    }
-
-    $phids = array();
-    foreach ($xactions as $xaction) {
-      $phids[] = $xaction->getRequiredHandlePHIDs();
-    }
-    $phids = array_mergev($phids);
-    $handles = $this->loadViewerHandles($phids);
-    foreach ($xactions as $xaction) {
-      $xaction->setHandles($handles);
-    }
-
-    $view = id(new PhabricatorAuditTransactionView())
-      ->setIsPreview(true)
-      ->setUser($viewer)
-      ->setObjectPHID($commit->getPHID())
-      ->setTransactions($xactions);
-
-    id(new PhabricatorDraft())
-      ->setAuthorPHID($viewer->getPHID())
-      ->setDraftKey('diffusion-audit-'.$id)
-      ->setDraft($content)
-      ->replaceOrDelete();
-
-    return id(new AphrontAjaxResponse())->setContent(hsprintf('%s', $view));
-  }
-
-}
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -10,7 +10,7 @@ final class PhabricatorAuditEditor
  private $rawPatch;
  private $auditorPHIDs = array();

-  private $didExpandInlineState;
+  private $didExpandInlineState = false;

  public function addAuditReason($phid, $reason) {
    if (!isset($this->auditReasonMap[$phid])) {
@@ -67,6 +67,21 @@ final class PhabricatorAuditEditor
    return $types;
  }

+  protected function expandTransactions(
+    PhabricatorLiskDAO $object,
+    array $xactions) {
+
+    foreach ($xactions as $xaction) {
+      switch ($xaction->getTransactionType()) {
+        case PhabricatorTransactions::TYPE_INLINESTATE:
+          $this->didExpandInlineState = true;
+          break;
+      }
+    }
+
+    return parent::expandTransactions($object, $xactions);
+  }
+
  protected function transactionHasEffect(
    PhabricatorLiskDAO $object,
    PhabricatorApplicationTransaction $xaction) {
--- a/src/applications/audit/storage/PhabricatorAuditInlineComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditInlineComment.php
@@ -68,7 +68,8 @@ final class PhabricatorAuditInlineComment

  public static function loadDraftComments(
    PhabricatorUser $viewer,
-    $commit_phid) {
+    $commit_phid,
+    $raw = false) {

    $inlines = id(new DiffusionDiffInlineCommentQuery())
      ->setViewer($viewer)
@@ -80,6 +81,10 @@ final class PhabricatorAuditInlineComment
      ->needReplyToComments(true)
      ->execute();

+    if ($raw) {
+      return $inlines;
+    }
+
    return self::buildProxies($inlines);
  }

--- a/src/applications/audit/view/PhabricatorAuditTransactionView.php
+++ b/src/applications/audit/view/PhabricatorAuditTransactionView.php
@@ -3,7 +3,7 @@
 final class PhabricatorAuditTransactionView
  extends PhabricatorApplicationTransactionView {

-  private $pathMap;
+  private $pathMap = array();

  public function setPathMap(array $path_map) {
    $this->pathMap = $path_map;
@@ -55,12 +55,17 @@ final class PhabricatorAuditTransactionView
    $type_inline = PhabricatorAuditActionConstants::INLINE;

    $group = $xaction->getTransactionGroup();
+
    if ($xaction->getTransactionType() == $type_inline) {
      array_unshift($group, $xaction);
    } else {
      $out[] = parent::renderTransactionContent($xaction);
    }

+    if ($this->getIsPreview()) {
+      return $out;
+    }
+
    if (!$group) {
      return $out;
    }
--- a/src/applications/differential/controller/DifferentialRevisionViewController.php
+++ b/src/applications/differential/controller/DifferentialRevisionViewController.php
@@ -463,12 +463,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
    }

    Javelin::initBehavior('differential-user-select');
-
-    Javelin::initBehavior(
-      'differential-keyboard-navigation',
-      array(
-        'haunt' => null,
-      ));
+    Javelin::initBehavior('differential-keyboard-navigation');

    $view = id(new PHUITwoColumnView())
      ->setHeader($header)
--- a/src/applications/diffusion/controller/DiffusionCommitController.php
+++ b/src/applications/diffusion/controller/DiffusionCommitController.php
@@ -369,7 +369,9 @@ final class DiffusionCommitController extends DiffusionController {

    }

-    $add_comment = $this->renderAddCommentPanel($commit, $audit_requests);
+    $add_comment = $this->renderAddCommentPanel(
+      $commit,
+      $timeline);

    $filetree_on = $viewer->compareUserSetting(
      PhabricatorShowFiletreeSetting::SETTINGKEY,
@@ -717,150 +719,24 @@ final class DiffusionCommitController extends DiffusionController {

  private function renderAddCommentPanel(
    PhabricatorRepositoryCommit $commit,
-    array $audit_requests) {
-    assert_instances_of($audit_requests, 'PhabricatorRepositoryAuditRequest');
+    $timeline) {

    $request = $this->getRequest();
    $viewer = $request->getUser();

-    if (!$viewer->isLoggedIn()) {
-      return id(new PhabricatorApplicationTransactionCommentView())
-        ->setUser($viewer)
-        ->setRequestURI($request->getRequestURI());
-    }
-
-    $is_serious = PhabricatorEnv::getEnvConfig('phabricator.serious-business');
-
-    $pane_id = celerity_generate_unique_node_id();
-    Javelin::initBehavior(
-      'differential-keyboard-navigation',
-      array(
-        'haunt' => $pane_id,
-      ));
-
-    $draft = id(new PhabricatorDraft())->loadOneWhere(
-      'authorPHID = %s AND draftKey = %s',
-      $viewer->getPHID(),
-      'diffusion-audit-'.$commit->getID());
-    if ($draft) {
-      $draft = $draft->getDraft();
-    } else {
-      $draft = null;
-    }
-
-    $actions = $this->getAuditActions($commit, $audit_requests);
-
-    $mailable_source = new PhabricatorMetaMTAMailableDatasource();
-    $auditor_source = new DiffusionAuditorDatasource();
-
-    $form = id(new AphrontFormView())
-      ->setUser($viewer)
-      ->setAction('/audit/addcomment/')
-      ->addHiddenInput('commit', $commit->getPHID())
-      ->appendChild(
-        id(new AphrontFormSelectControl())
-          ->setLabel(pht('Action'))
-          ->setName('action')
-          ->setID('audit-action')
-          ->setOptions($actions))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setLabel(pht('Add Auditors'))
-          ->setName('auditors')
-          ->setControlID('add-auditors')
-          ->setControlStyle('display: none')
-          ->setID('add-auditors-tokenizer')
-          ->setDisableBehavior(true)
-          ->setDatasource($auditor_source))
-      ->appendControl(
-        id(new AphrontFormTokenizerControl())
-          ->setLabel(pht('Add CCs'))
-          ->setName('ccs')
-          ->setControlID('add-ccs')
-          ->setControlStyle('display: none')
-          ->setID('add-ccs-tokenizer')
-          ->setDisableBehavior(true)
-          ->setDatasource($mailable_source))
-      ->appendChild(
-        id(new PhabricatorRemarkupControl())
-          ->setLabel(pht('Comments'))
-          ->setName('content')
-          ->setValue($draft)
-          ->setID('audit-content')
-          ->setUser($viewer))
-      ->appendChild(
-        id(new AphrontFormSubmitControl())
-          ->setValue(pht('Submit')));
-
-    $header = new PHUIHeaderView();
-    $header->setHeader(
-      $is_serious ? pht('Audit Commit') : pht('Creative Accounting'));
-
-    Javelin::initBehavior(
-      'differential-add-reviewers-and-ccs',
-      array(
-        'dynamic' => array(
-          'add-auditors-tokenizer' => array(
-            'actions' => array('add_auditors' => 1),
-            'src' => $auditor_source->getDatasourceURI(),
-            'row' => 'add-auditors',
-            'placeholder' => $auditor_source->getPlaceholderText(),
-          ),
-          'add-ccs-tokenizer' => array(
-            'actions' => array('add_ccs' => 1),
-            'src' => $mailable_source->getDatasourceURI(),
-            'row' => 'add-ccs',
-            'placeholder' => $mailable_source->getPlaceholderText(),
-          ),
-        ),
-        'select' => 'audit-action',
-      ));
-
-    Javelin::initBehavior('differential-feedback-preview', array(
-      'uri'       => '/audit/preview/'.$commit->getID().'/',
-      'preview'   => 'audit-preview',
-      'content'   => 'audit-content',
-      'action'    => 'audit-action',
-      'previewTokenizers' => array(
-        'auditors' => 'add-auditors-tokenizer',
-        'ccs'      => 'add-ccs-tokenizer',
-      ),
-      'inline'     => 'inline-comment-preview',
-      'inlineuri'  => '/diffusion/inline/preview/'.$commit->getPHID().'/',
-    ));
-
-    $loading = phutil_tag_div(
-      'aphront-panel-preview-loading-text',
-      pht('Loading preview...'));
-
-    $preview_panel = phutil_tag_div(
-      'aphront-panel-preview aphront-panel-flush',
-      array(
-        phutil_tag('div', array('id' => 'audit-preview'), $loading),
-        phutil_tag('div', array('id' => 'inline-comment-preview')),
-      ));
+    Javelin::initBehavior('differential-keyboard-navigation');

    // TODO: This is pretty awkward, unify the CSS between Diffusion and
    // Differential better.
    require_celerity_resource('differential-core-view-css');

-    $anchor = id(new PhabricatorAnchorView())
-      ->setAnchorName('comment')
-      ->setNavigationMarker(true)
-      ->render();
+    $comment_view = id(new DiffusionCommitEditEngine())
+      ->setViewer($viewer)
+      ->buildEditEngineCommentView($commit);

-    $comment_box = id(new PHUIObjectBoxView())
-      ->setHeader($header)
-      ->appendChild($form);
+    $comment_view->setTransactionTimeline($timeline);

-    return phutil_tag(
-      'div',
-      array(
-        'id' => $pane_id,
-      ),
-      phutil_tag_div(
-        'differential-add-comment-panel',
-        array($anchor, $comment_box, $preview_panel)));
+    return $comment_view;
  }

  /**
--- a/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
+++ b/src/applications/diffusion/editor/DiffusionCommitEditEngine.php
@@ -48,6 +48,21 @@ final class DiffusionCommitEditEngine
      ->needAuditRequests(true);
  }

+  protected function getEditorURI() {
+    return $this->getApplication()->getApplicationURI('commit/edit/');
+  }
+
+  protected function newCommentActionGroups() {
+    return array(
+      id(new PhabricatorEditEngineCommentActionGroup())
+        ->setKey(self::ACTIONGROUP_AUDIT)
+        ->setLabel(pht('Audit Actions')),
+      id(new PhabricatorEditEngineCommentActionGroup())
+        ->setKey(self::ACTIONGROUP_COMMIT)
+        ->setLabel(pht('Commit Actions')),
+    );
+  }
+
  protected function getObjectCreateTitleText($object) {
    return pht('Create Commit');
  }
@@ -143,4 +158,76 @@ final class DiffusionCommitEditEngine
    return $fields;
  }

+  protected function newAutomaticCommentTransactions($object) {
+    $viewer = $this->getViewer();
+    $xactions = array();
+
+    $inlines = PhabricatorAuditInlineComment::loadDraftComments(
+      $viewer,
+      $object->getPHID(),
+      $raw = true);
+    $inlines = msort($inlines, 'getID');
+
+    foreach ($inlines as $inline) {
+      $xactions[] = $object->getApplicationTransactionTemplate()
+        ->setTransactionType(PhabricatorAuditActionConstants::INLINE)
+        ->attachComment($inline);
+    }
+
+    $viewer_phid = $viewer->getPHID();
+    $viewer_is_author = ($object->getAuthorPHID() == $viewer_phid);
+    if ($viewer_is_author) {
+      $state_map = PhabricatorTransactions::getInlineStateMap();
+
+      $inlines = id(new DiffusionDiffInlineCommentQuery())
+        ->setViewer($viewer)
+        ->withCommitPHIDs(array($object->getPHID()))
+        ->withFixedStates(array_keys($state_map))
+        ->execute();
+      if ($inlines) {
+        $old_value = mpull($inlines, 'getFixedState', 'getPHID');
+        $new_value = array();
+        foreach ($old_value as $key => $state) {
+          $new_value[$key] = $state_map[$state];
+        }
+
+        $xactions[] = $object->getApplicationTransactionTemplate()
+          ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
+          ->setIgnoreOnNoEffect(true)
+          ->setOldValue($old_value)
+          ->setNewValue($new_value);
+      }
+    }
+
+    return $xactions;
+  }
+
+  protected function newCommentPreviewContent($object, array $xactions) {
+    $viewer = $this->getViewer();
+    $type_inline = PhabricatorAuditActionConstants::INLINE;
+
+    $inlines = array();
+    foreach ($xactions as $xaction) {
+      if ($xaction->getTransactionType() === $type_inline) {
+        $inlines[] = $xaction->getComment();
+      }
+    }
+
+    $content = array();
+
+    if ($inlines) {
+      $inline_preview = id(new PHUIDiffInlineCommentPreviewListView())
+        ->setViewer($viewer)
+        ->setInlineComments($inlines);
+
+      $content[] = phutil_tag(
+        'div',
+        array(
+          'id' => 'inline-comment-preview',
+        ),
+        $inline_preview);
+    }
+
+    return $content;
+  }
 }