Allow ApplicationTransaction comments to be edited and deleted

Summary:
Allows you to edit or delete comments in appplications which support ApplicationTransactions.

UI/UX stuff:

  - The dialogs are rough but I want to do a dialog design pass more generally, @chad has some mocks.
  - When you add new mentions via edit, they don't currently count as mentions. I'm not sure what I want to do about this.
  - When you edit or delete a comment, we do not publish any notifications about it. I think this is reasonable.
  - I didn't separate "delete" out versus "edit"; I assume it will be reasonably intuitive that deleting all the text deletes effectively deletes the comment. I also want to discourage deletion somewhat (we still show the transaction, just show that the comment has been deleted).

Test Plan:
Transaction view, note "Edit" and "Edited" links:

{F26914}

Edit view, has some design issues but I want to do a pass on dialogs in general:

{F26915}

History view:

{F26913}

Reviewers: vrana, btrahan, chad

Reviewed By: vrana

CC: aran

Maniphest Tasks: T1082

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

src/__phutil_library_map__.php

@@ -599,8 +599,11 @@ phutil_register_library_map(array(
     'PhabricatorApplicationSubscriptions' => 'applications/subscriptions/application/PhabricatorApplicationSubscriptions.php',
     'PhabricatorApplicationTransaction' => 'applications/transactions/storage/PhabricatorApplicationTransaction.php',
     'PhabricatorApplicationTransactionComment' => 'applications/transactions/storage/PhabricatorApplicationTransactionComment.php',
+    'PhabricatorApplicationTransactionCommentEditController' => 'applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php',
     'PhabricatorApplicationTransactionCommentEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php',
+    'PhabricatorApplicationTransactionCommentHistoryController' => 'applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php',
     'PhabricatorApplicationTransactionCommentQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php',
+    'PhabricatorApplicationTransactionController' => 'applications/transactions/controller/PhabricatorApplicationTransactionController.php',
     'PhabricatorApplicationTransactionEditor' => 'applications/transactions/editor/PhabricatorApplicationTransactionEditor.php',
     'PhabricatorApplicationTransactionFeedStory' => 'applications/transactions/feed/PhabricatorApplicationTransactionFeedStory.php',
     'PhabricatorApplicationTransactionQuery' => 'applications/transactions/query/PhabricatorApplicationTransactionQuery.php',
@@ -1873,8 +1876,11 @@ phutil_register_library_map(array(
       1 => 'PhabricatorMarkupInterface',
       2 => 'PhabricatorPolicyInterface',
     ),
+    'PhabricatorApplicationTransactionCommentEditController' => 'PhabricatorApplicationTransactionController',
     'PhabricatorApplicationTransactionCommentEditor' => 'PhabricatorEditor',
+    'PhabricatorApplicationTransactionCommentHistoryController' => 'PhabricatorApplicationTransactionController',
     'PhabricatorApplicationTransactionCommentQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
+    'PhabricatorApplicationTransactionController' => 'PhabricatorController',
     'PhabricatorApplicationTransactionEditor' => 'PhabricatorEditor',
     'PhabricatorApplicationTransactionFeedStory' => 'PhabricatorFeedStory',
     'PhabricatorApplicationTransactionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',

src/applications/transactions/application/PhabricatorApplicationTransactions.php

@@ -8,6 +8,12 @@ final class PhabricatorApplicationTransactions extends PhabricatorApplication {
 
   public function getRoutes() {
     return array(
+      '/transactions/' => array(
+        'edit/(?<phid>[^/]+)/'
+          => 'PhabricatorApplicationTransactionCommentEditController',
+        'history/(?<phid>[^/]+)/'
+          => 'PhabricatorApplicationTransactionCommentHistoryController',
+      ),
     );
   }
 

src/applications/transactions/controller/PhabricatorApplicationTransactionCommentEditController.php

@@ -0,0 +1,82 @@
+<?php
+
+final class PhabricatorApplicationTransactionCommentEditController
+  extends PhabricatorApplicationTransactionController {
+
+  private $phid;
+  private $anchor;
+
+  public function willProcessRequest(array $data) {
+    $this->phid = $data['phid'];
+    $this->anchor = idx($data, 'anchor');
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $xactions = id(new PhabricatorObjectHandleData(array($this->phid)))
+      ->setViewer($user)
+      ->loadObjects();
+    $xaction = idx($xactions, $this->phid);
+
+    if (!$xaction) {
+      // TODO: This may also mean you don't have permission to edit the object,
+      // but we can't make that distinction via PhabricatorObjectHandleData
+      // at the moment.
+      return new Aphront404Response();
+    }
+
+    if (!$xaction->getComment()) {
+      // You can't currently edit a transaction which doesn't have a comment.
+      // Some day you may be able to edit the visibility.
+      return new Aphront404Response();
+    }
+
+    $obj_phid = $xaction->getObjectPHID();
+    $obj_handle = PhabricatorObjectHandleData::loadOneHandle($obj_phid, $user);
+    if (!$obj_handle) {
+      // Require the corresponding object exist and be visible to the user.
+      return new Aphront404Response();
+    }
+
+    if ($request->isDialogFormPost()) {
+      $text = $request->getStr('text');
+
+      $comment = $xaction->getApplicationTransactionCommentObject();
+      $comment->setContent($text);
+      if (!strlen($text)) {
+        $comment->setIsDeleted(true);
+      }
+
+      $editor = id(new PhabricatorApplicationTransactionCommentEditor())
+        ->setActor($user)
+        ->setContentSource(
+          $content_source = PhabricatorContentSource::newForSource(
+            PhabricatorContentSource::SOURCE_WEB,
+            array(
+              'ip' => $request->getRemoteAddr(),
+            )))
+        ->applyEdit($xaction, $comment);
+
+      return id(new AphrontReloadResponse())->setURI($obj_handle->getURI());
+    }
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($user)
+      ->setTitle(pht('Edit Comment'));
+
+    $dialog
+      ->appendChild(
+        id(new PhabricatorRemarkupControl())
+          ->setName('text')
+          ->setValue($xaction->getComment()->getContent()));
+
+    $dialog
+      ->addSubmitButton(pht('Edit Comment'))
+      ->addCancelButton($obj_handle->getURI());
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+
+}

src/applications/transactions/controller/PhabricatorApplicationTransactionCommentHistoryController.php

@@ -0,0 +1,81 @@
+<?php
+
+final class PhabricatorApplicationTransactionCommentHistoryController
+  extends PhabricatorApplicationTransactionController {
+
+  private $phid;
+
+  public function willProcessRequest(array $data) {
+    $this->phid = $data['phid'];
+  }
+
+  public function processRequest() {
+    $request = $this->getRequest();
+    $user = $request->getUser();
+
+    $xactions = id(new PhabricatorObjectHandleData(array($this->phid)))
+      ->setViewer($user)
+      ->loadObjects();
+    $xaction = idx($xactions, $this->phid);
+
+    if (!$xaction) {
+      // TODO: This may also mean you don't have permission to edit the object,
+      // but we can't make that distinction via PhabricatorObjectHandleData
+      // at the moment.
+      return new Aphront404Response();
+    }
+
+    if (!$xaction->getComment()) {
+      // You can't view history of a transaction with no comments.
+      return new Aphront404Response();
+    }
+
+    $obj_phid = $xaction->getObjectPHID();
+    $obj_handle = PhabricatorObjectHandleData::loadOneHandle($obj_phid, $user);
+    if (!$obj_handle) {
+      // Require the corresponding object exist and be visible to the user.
+      return new Aphront404Response();
+    }
+
+    $comments = id(new PhabricatorApplicationTransactionCommentQuery())
+      ->setViewer($user)
+      ->setTemplate($xaction->getApplicationTransactionCommentObject())
+      ->withTransactionPHIDs(array($xaction->getPHID()))
+      ->execute();
+
+    if (!$comments) {
+      return new Aphront404Response();
+    }
+
+    $comments = msort($comments, 'getCommentVersion');
+
+    $xactions = array();
+    foreach ($comments as $comment) {
+      $xactions[] = id(clone $xaction)
+        ->makeEphemeral()
+        ->setCommentVersion($comment->getCommentVersion())
+        ->setContentSource($comment->getContentSource())
+        ->setDateCreated($comment->getDateCreated())
+        ->attachComment($comment);
+    }
+
+    $view = id(new PhabricatorApplicationTransactionView())
+      ->setViewer($user)
+      ->setTransactions($xactions)
+      ->setShowEditActions(false);
+
+
+    $dialog = id(new AphrontDialogView())
+      ->setUser($user)
+      ->setWidth(AphrontDialogView::WIDTH_FULL)
+      ->setTitle(pht('Comment History'));
+
+    $dialog->appendChild($view);
+
+    $dialog
+      ->addCancelButton($obj_handle->getURI());
+
+    return id(new AphrontDialogResponse())->setDialog($dialog);
+  }
+
+}

src/applications/transactions/controller/PhabricatorApplicationTransactionController.php

@@ -0,0 +1,6 @@
+<?php
+
+abstract class PhabricatorApplicationTransactionController
+  extends PhabricatorController {
+
+}

src/applications/transactions/query/PhabricatorApplicationTransactionCommentQuery.php

@@ -19,7 +19,7 @@ final class PhabricatorApplicationTransactionCommentQuery
     return $this;
   }
 
-  public function withTranactionPHIDs(array $transaction_phids) {
+  public function withTransactionPHIDs(array $transaction_phids) {
     $this->transactionPHIDs = $transaction_phids;
     return $this;
   }

src/applications/transactions/view/PhabricatorApplicationTransactionView.php

@@ -9,6 +9,16 @@ class PhabricatorApplicationTransactionView extends AphrontView {
   private $transactions;
   private $engine;
   private $anchorOffset = 0;
+  private $showEditActions = true;
+
+  public function setShowEditActions($show_edit_actions) {
+    $this->showEditActions = $show_edit_actions;
+    return $this;
+  }
+
+  public function getShowEditActions() {
+    return $this->showEditActions;
+  }
 
   public function setAnchorOffset($anchor_offset) {
     $this->anchorOffset = $anchor_offset;
@@ -49,6 +59,7 @@ class PhabricatorApplicationTransactionView extends AphrontView {
     }
 
     $view = new PhabricatorTimelineView();
+    $viewer = $this->viewer;
 
     $anchor = $this->anchorOffset;
     foreach ($this->transactions as $xaction) {
@@ -58,7 +69,8 @@ class PhabricatorApplicationTransactionView extends AphrontView {
 
       $anchor++;
       $event = id(new PhabricatorTimelineEventView())
-        ->setViewer($this->viewer)
+        ->setViewer($viewer)
+        ->setTransactionPHID($xaction->getPHID())
         ->setUserHandle($xaction->getHandle($xaction->getAuthorPHID()))
         ->setIcon($xaction->getIcon())
         ->setColor($xaction->getColor())
@@ -67,9 +79,33 @@ class PhabricatorApplicationTransactionView extends AphrontView {
         ->setContentSource($xaction->getContentSource())
         ->setAnchor($anchor);
 
+      $has_deleted_comment = $xaction->getComment() &&
+        $xaction->getComment()->getIsDeleted();
+
+      if ($this->getShowEditActions()) {
+        if ($xaction->getCommentVersion() > 1) {
+          $event->setIsEdited(true);
+        }
+
+        $can_edit = PhabricatorPolicyCapability::CAN_EDIT;
+
+        if ($xaction->hasComment() || $has_deleted_comment) {
+          $has_edit_capability = PhabricatorPolicyFilter::hasCapability(
+            $viewer,
+            $xaction,
+            $can_edit);
+          if ($has_edit_capability) {
+            $event->setIsEditable(true);
+          }
+        }
+      }
+
       if ($xaction->hasComment()) {
         $event->appendChild(
           $this->engine->getOutput($xaction->getComment(), $field));
+      } else if ($has_deleted_comment) {
+        $event->appendChild(
+          '<em>'.pht('This comment has been deleted.').'</em>');
       }
 
       $view->addEvent($event);

src/view/AphrontDialogView.php

@@ -16,6 +16,7 @@ final class AphrontDialogView extends AphrontView {
   private $width      = 'default';
   const WIDTH_DEFAULT = 'default';
   const WIDTH_FORM    = 'form';
+  const WIDTH_FULL    = 'full';
 
   public function setUser(PhabricatorUser $user) {
     $this->user = $user;
@@ -115,6 +116,7 @@ final class AphrontDialogView extends AphrontView {
 
     switch ($this->width) {
       case self::WIDTH_FORM:
+      case self::WIDTH_FULL:
         $more .= ' aphront-dialog-view-width-'.$this->width;
         break;
       case self::WIDTH_DEFAULT:

src/view/layout/PhabricatorTimelineEventView.php

@@ -11,6 +11,37 @@ final class PhabricatorTimelineEventView extends AphrontView {
   private $dateCreated;
   private $viewer;
   private $anchor;
+  private $isEditable;
+  private $isEdited;
+  private $transactionPHID;
+
+  public function setTransactionPHID($transaction_phid) {
+    $this->transactionPHID = $transaction_phid;
+    return $this;
+  }
+
+  public function getTransactionPHID() {
+    return $this->transactionPHID;
+  }
+
+  public function setIsEdited($is_edited) {
+    $this->isEdited = $is_edited;
+    return $this;
+  }
+
+  public function getIsEdited() {
+    return $this->isEdited;
+  }
+
+
+  public function setIsEditable($is_editable) {
+    $this->isEditable = $is_editable;
+    return $this;
+  }
+
+  public function getIsEditable() {
+    return $this->isEditable;
+  }
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -78,6 +109,27 @@ final class PhabricatorTimelineEventView extends AphrontView {
     }
 
     $extra = array();
+    $xaction_phid = $this->getTransactionPHID();
+
+    if ($this->getIsEdited()) {
+      $extra[] = javelin_render_tag(
+        'a',
+        array(
+          'href'  => '/transactions/history/'.$xaction_phid.'/',
+          'sigil' => 'workflow',
+        ),
+        pht('Edited'));
+    }
+
+    if ($this->getIsEditable()) {
+      $extra[] = javelin_render_tag(
+        'a',
+        array(
+          'href'  => '/transactions/edit/'.$xaction_phid.'/',
+          'sigil' => 'workflow',
+        ),
+        pht('Edit'));
+    }
 
     $source = $this->getContentSource();
     if ($source) {

src/view/page/PhabricatorStandardPageView.php

@@ -137,7 +137,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
         'header'    => AphrontRequest::getCSRFHeaderName(),
         'current'   => $current_token,
       ));
-    Javelin::initBehavior('device', array('id' => 'base-page'));
+    Javelin::initBehavior('device');
 
     if ($console) {
       require_celerity_resource('aphront-dark-console-css');
@@ -284,29 +284,12 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
         '</div>';
     }
 
-    $agent = idx($_SERVER, 'HTTP_USER_AGENT');
-
-    // Try to guess the device resolution based on UA strings to avoid a flash
-    // of incorrectly-styled content.
-    $device_guess = 'device-desktop';
-    if (preg_match('@iPhone|iPod|(Android.*Chrome/[.0-9]* Mobile)@', $agent)) {
-      $device_guess = 'device-phone device';
-    } else if (preg_match('@iPad|(Android.*Chrome/)@', $agent)) {
-      $device_guess = 'device-tablet device';
-    }
-
-    $classes = array(
-      'phabricator-standard-page',
-      $device_guess,
-    );
-    $classes = implode(' ', $classes);
-
     return
       phutil_render_tag(
         'div',
         array(
           'id' => 'base-page',
-          'class' => $classes,
+          'class' => 'phabricator-standard-page',
         ),
         $header_chrome.
         '<div class="phabricator-standard-page-body">'.
@@ -374,6 +357,19 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       $classes[] = 'phabricator-chromeless-page';
     }
 
+    $agent = idx($_SERVER, 'HTTP_USER_AGENT');
+
+    // Try to guess the device resolution based on UA strings to avoid a flash
+    // of incorrectly-styled content.
+    $device_guess = 'device-desktop';
+    if (preg_match('@iPhone|iPod|(Android.*Chrome/[.0-9]* Mobile)@', $agent)) {
+      $device_guess = 'device-phone device';
+    } else if (preg_match('@iPad|(Android.*Chrome/)@', $agent)) {
+      $device_guess = 'device-tablet device';
+    }
+
+    $classes[] = $device_guess;
+
     return implode(' ', $classes);
   }
 

webroot/rsrc/css/aphront/dialog-view.css

@@ -21,6 +21,10 @@
   width:        600px;
 }
 
+.aphront-dialog-view-width-full {
+  width: 90%;
+}
+
 .aphront-dialog-body {
   background: #ffffff;
   padding: 16px 12px;
@@ -44,6 +48,7 @@
 .jx-client-dialog {
   position:   absolute;
   z-index:    14;
+  width:      100%;
 }
 
 .jx-mask {

webroot/rsrc/css/aphront/lightbox-attachment.css

@@ -5,7 +5,6 @@
 
 .lightbox-attached {
   overflow: hidden;
-
 }
 
 .lightbox-attachment {

webroot/rsrc/js/application/core/behavior-device.js

@@ -16,7 +16,7 @@ JX.install('Device', {
   }
 });
 
-JX.behavior('device', function(config) {
+JX.behavior('device', function() {
 
   function onresize() {
     var v = JX.Vector.getViewport();
@@ -35,7 +35,7 @@ JX.behavior('device', function(config) {
 
     JX.Device._device = device;
 
-    var e = JX.$(config.id);
+    var e = document.body;
     JX.DOM.alterClass(e, 'device-phone', (device == 'phone'));
     JX.DOM.alterClass(e, 'device-tablet', (device == 'tablet'));
     JX.DOM.alterClass(e, 'device-desktop', (device == 'desktop'));