Allow inline comments to be individually hidden

Summary:
Ref T7447. Implements per-viewer comment hiding. Once a comment is obsolete or uninteresting, you can hide it completely.

This is sticky per-user.

My hope is that this will strike a better balance between concerns than some of the other approaches (conservative porting, summarization, hide-all).

Specifically, this adds a new action here:

{F435621}

Clicking it completely collapses the comment into a small icon on the previous line, and saves the comment state as hidden for you:

{F435626}

You can click the icon to reveal all hidden comments below the line.

Test Plan:
  - Hid comments.
  - Showed comments.
  - Created, edited, deleted and submitted comments.
  - Used Diffusion comments (hiding is not implemented there yet, but I'd plan to bring it there eventually if it works out in Differential).

Reviewers: btrahan, chad

Reviewed By: btrahan

Subscribers: jparise, yelirekim, epriestley

Maniphest Tasks: T7447

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

Author: epriestley

resources/celerity/map.php

@@ -10,8 +10,8 @@
     'core.pkg.css' => '439658b5',
     'core.pkg.js' => '328799d0',
     'darkconsole.pkg.js' => 'e7393ebb',
-    'differential.pkg.css' => 'bb338e4b',
-    'differential.pkg.js' => '63a77807',
+    'differential.pkg.css' => '30602b8c',
+    'differential.pkg.js' => '8c98ce21',
     'diffusion.pkg.css' => '591664fa',
     'diffusion.pkg.js' => '0115b37c',
     'maniphest.pkg.css' => '68d4dd3d',
@@ -60,7 +60,7 @@
     'rsrc/css/application/differential/add-comment.css' => 'c47f8c40',
     'rsrc/css/application/differential/changeset-view.css' => 'e19cfd6e',
     'rsrc/css/application/differential/core.css' => '7ac3cabc',
-    'rsrc/css/application/differential/phui-inline-comment.css' => '2174771a',
+    'rsrc/css/application/differential/phui-inline-comment.css' => 'aa16f165',
     'rsrc/css/application/differential/results-table.css' => '181aa9d9',
     'rsrc/css/application/differential/revision-comment.css' => '14b8565a',
     'rsrc/css/application/differential/revision-history.css' => '0e8eb855',
@@ -353,7 +353,7 @@
     'rsrc/js/application/differential/behavior-comment-preview.js' => 'b064af76',
     'rsrc/js/application/differential/behavior-diff-radios.js' => 'e1ff79b1',
     'rsrc/js/application/differential/behavior-dropdown-menus.js' => '2035b9cb',
-    'rsrc/js/application/differential/behavior-edit-inline-comments.js' => 'e723c323',
+    'rsrc/js/application/differential/behavior-edit-inline-comments.js' => '037b59eb',
     'rsrc/js/application/differential/behavior-keyboard-nav.js' => '2c426492',
     'rsrc/js/application/differential/behavior-populate.js' => '8694b1df',
     'rsrc/js/application/differential/behavior-show-field-details.js' => 'bba9eedf',
@@ -443,7 +443,7 @@
     'rsrc/js/core/behavior-device.js' => 'a205cf28',
     'rsrc/js/core/behavior-drag-and-drop-textarea.js' => '6d49590e',
     'rsrc/js/core/behavior-error-log.js' => '6882e80a',
-    'rsrc/js/core/behavior-fancy-datepicker.js' => '2d4029a8',
+    'rsrc/js/core/behavior-fancy-datepicker.js' => '5c0f680f',
     'rsrc/js/core/behavior-file-tree.js' => '88236f00',
     'rsrc/js/core/behavior-form.js' => '5c54cbf3',
     'rsrc/js/core/behavior-gesture.js' => '3ab51e2c',
@@ -560,7 +560,7 @@
     'javelin-behavior-differential-comment-jump' => '4fdb476d',
     'javelin-behavior-differential-diff-radios' => 'e1ff79b1',
     'javelin-behavior-differential-dropdown-menus' => '2035b9cb',
-    'javelin-behavior-differential-edit-inline-comments' => 'e723c323',
+    'javelin-behavior-differential-edit-inline-comments' => '037b59eb',
     'javelin-behavior-differential-feedback-preview' => 'b064af76',
     'javelin-behavior-differential-keyboard-navigation' => '2c426492',
     'javelin-behavior-differential-populate' => '8694b1df',
@@ -576,7 +576,7 @@
     'javelin-behavior-durable-column' => '16c695bf',
     'javelin-behavior-error-log' => '6882e80a',
     'javelin-behavior-event-all-day' => '38dcf3c8',
-    'javelin-behavior-fancy-datepicker' => '2d4029a8',
+    'javelin-behavior-fancy-datepicker' => '5c0f680f',
     'javelin-behavior-global-drag-and-drop' => 'c8e57404',
     'javelin-behavior-herald-rule-editor' => '7ebaeed3',
     'javelin-behavior-high-security-warning' => 'a464fe03',
@@ -782,7 +782,7 @@
     'phui-image-mask-css' => '5a8b09c8',
     'phui-info-panel-css' => '27ea50a1',
     'phui-info-view-css' => 'c6f0aef8',
-    'phui-inline-comment-view-css' => '2174771a',
+    'phui-inline-comment-view-css' => 'aa16f165',
     'phui-list-view-css' => '2e25ebfb',
     'phui-object-box-css' => '7d160002',
     'phui-object-item-list-view-css' => 'f3a22696',
@@ -830,6 +830,14 @@
     '029a133d' => array(
       'aphront-dialog-view-css',
     ),
+    '037b59eb' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-vector',
+      'differential-inline-comment-editor',
+    ),
     '048330fa' => array(
       'javelin-behavior',
       'javelin-typeahead-ondemand-source',
@@ -1042,13 +1050,6 @@
       'javelin-install',
       'javelin-event',
     ),
-    '2d4029a8' => array(
-      'javelin-behavior',
-      'javelin-util',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-vector',
-    ),
     '331b1611' => array(
       'javelin-install',
     ),
@@ -1241,6 +1242,13 @@
       'javelin-uri',
       'javelin-routable',
     ),
+    '5c0f680f' => array(
+      'javelin-behavior',
+      'javelin-util',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-vector',
+    ),
     '5c54cbf3' => array(
       'javelin-behavior',
       'javelin-stratcom',
@@ -1925,14 +1933,6 @@
     'e6e25838' => array(
       'javelin-install',
     ),
-    'e723c323' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-vector',
-      'differential-inline-comment-editor',
-    ),
     'e9581f08' => array(
       'javelin-behavior',
       'javelin-stratcom',

resources/sql/autopatches/20150525.diff.hidden.1.sql

@@ -0,0 +1,7 @@
+CREATE TABLE {$NAMESPACE}_differential.differential_hiddencomment (
+  id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY,
+  userPHID VARBINARY(64) NOT NULL,
+  commentID INT UNSIGNED NOT NULL,
+  UNIQUE KEY `key_user` (userPHID, commentID),
+  KEY `key_comment` (commentID)
+) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT};

src/__phutil_library_map__.php

@@ -374,6 +374,7 @@
     'DifferentialGetWorkingCopy' => 'applications/differential/DifferentialGetWorkingCopy.php',
     'DifferentialGitHubLandingStrategy' => 'applications/differential/landing/DifferentialGitHubLandingStrategy.php',
     'DifferentialGitSVNIDField' => 'applications/differential/customfield/DifferentialGitSVNIDField.php',
+    'DifferentialHiddenComment' => 'applications/differential/storage/DifferentialHiddenComment.php',
     'DifferentialHostField' => 'applications/differential/customfield/DifferentialHostField.php',
     'DifferentialHostedGitLandingStrategy' => 'applications/differential/landing/DifferentialHostedGitLandingStrategy.php',
     'DifferentialHostedMercurialLandingStrategy' => 'applications/differential/landing/DifferentialHostedMercurialLandingStrategy.php',
@@ -1178,6 +1179,7 @@
     'PHUIDiffInlineCommentUndoView' => 'infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php',
     'PHUIDiffInlineCommentView' => 'infrastructure/diff/view/PHUIDiffInlineCommentView.php',
     'PHUIDiffOneUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffOneUpInlineCommentRowScaffold.php',
+    'PHUIDiffRevealIconView' => 'infrastructure/diff/view/PHUIDiffRevealIconView.php',
     'PHUIDiffTwoUpInlineCommentRowScaffold' => 'infrastructure/diff/view/PHUIDiffTwoUpInlineCommentRowScaffold.php',
     'PHUIDocumentExample' => 'applications/uiexample/examples/PHUIDocumentExample.php',
     'PHUIDocumentView' => 'view/phui/PHUIDocumentView.php',
@@ -3616,6 +3618,7 @@
     'DifferentialGetRevisionConduitAPIMethod' => 'DifferentialConduitAPIMethod',
     'DifferentialGitHubLandingStrategy' => 'DifferentialLandingStrategy',
     'DifferentialGitSVNIDField' => 'DifferentialCustomField',
+    'DifferentialHiddenComment' => 'DifferentialDAO',
     'DifferentialHostField' => 'DifferentialCustomField',
     'DifferentialHostedGitLandingStrategy' => 'DifferentialLandingStrategy',
     'DifferentialHostedMercurialLandingStrategy' => 'DifferentialLandingStrategy',
@@ -4506,6 +4509,7 @@
     'PHUIDiffInlineCommentUndoView' => 'PHUIDiffInlineCommentView',
     'PHUIDiffInlineCommentView' => 'AphrontView',
     'PHUIDiffOneUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
+    'PHUIDiffRevealIconView' => 'AphrontView',
     'PHUIDiffTwoUpInlineCommentRowScaffold' => 'PHUIDiffInlineCommentRowScaffold',
     'PHUIDocumentExample' => 'PhabricatorUIExample',
     'PHUIDocumentView' => 'AphrontTagView',

src/applications/audit/storage/PhabricatorAuditInlineComment.php

@@ -23,6 +23,14 @@ public function getTransactionComment() {
     return $this->proxy;
   }
 
+  public function supportsHiding() {
+    return false;
+  }
+
+  public function isHidden() {
+    return false;
+  }
+
   public function getTransactionCommentForSave() {
     $content_source = PhabricatorContentSource::newForSource(
       PhabricatorContentSource::SOURCE_LEGACY,

src/applications/differential/controller/DifferentialChangesetViewController.php

@@ -191,6 +191,7 @@ public function handleRequest(AphrontRequest $request) {
     if ($revision) {
       $query = id(new DifferentialInlineCommentQuery())
         ->setViewer($viewer)
+        ->needHidden(true)
         ->withRevisionPHIDs(array($revision->getPHID()));
       $inlines = $query->execute();
       $inlines = $query->adjustInlinesForChangesets(

src/applications/differential/controller/DifferentialInlineCommentEditController.php

@@ -42,6 +42,7 @@ protected function loadComment($id) {
       ->setViewer($this->getViewer())
       ->withIDs(array($id))
       ->withDeletedDrafts(true)
+      ->needHidden(true)
       ->executeOne();
   }
 
@@ -50,6 +51,7 @@ protected function loadCommentByPHID($phid) {
       ->setViewer($this->getViewer())
       ->withPHIDs(array($phid))
       ->withDeletedDrafts(true)
+      ->needHidden(true)
       ->executeOne();
   }
 
@@ -152,4 +154,38 @@ protected function loadObjectOwnerPHID(
     return $this->loadRevision()->getAuthorPHID();
   }
 
+  protected function hideComments(array $ids) {
+    $viewer = $this->getViewer();
+    $table = new DifferentialHiddenComment();
+    $conn_w = $table->establishConnection('w');
+
+    $sql = array();
+    foreach ($ids as $id) {
+      $sql[] = qsprintf(
+        $conn_w,
+        '(%s, %d)',
+        $viewer->getPHID(),
+        $id);
+    }
+
+    queryfx(
+      $conn_w,
+      'INSERT IGNORE INTO %T (userPHID, commentID) VALUES %Q',
+      $table->getTableName(),
+      implode(', ', $sql));
+  }
+
+  protected function showComments(array $ids) {
+    $viewer = $this->getViewer();
+    $table = new DifferentialHiddenComment();
+    $conn_w = $table->establishConnection('w');
+
+    queryfx(
+      $conn_w,
+      'DELETE FROM %T WHERE userPHID = %s AND commentID IN (%Ld)',
+      $table->getTableName(),
+      $viewer->getPHID(),
+      $ids);
+  }
+
 }

src/applications/differential/controller/DifferentialInlineCommentPreviewController.php

@@ -19,6 +19,7 @@ protected function loadInlineComments() {
       ->withDrafts(true)
       ->withAuthorPHIDs(array($viewer->getPHID()))
       ->withRevisionPHIDs(array($revision->getPHID()))
+      ->needHidden(true)
       ->execute();
   }
 

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -175,6 +175,7 @@ public function processRequest() {
 
       $query = id(new DifferentialInlineCommentQuery())
         ->setViewer($user)
+        ->needHidden(true)
         ->withRevisionPHIDs(array($revision->getPHID()));
       $inlines = $query->execute();
       $inlines = $query->adjustInlinesForChangesets(

src/applications/differential/query/DifferentialInlineCommentQuery.php

@@ -15,6 +15,7 @@ final class DifferentialInlineCommentQuery
   private $authorPHIDs;
   private $revisionPHIDs;
   private $deletedDrafts;
+  private $needHidden;
 
   public function setViewer(PhabricatorUser $viewer) {
     $this->viewer = $viewer;
@@ -55,6 +56,11 @@ public function withDeletedDrafts($deleted_drafts) {
     return $this;
   }
 
+  public function needHidden($need) {
+    $this->needHidden = $need;
+    return $this;
+  }
+
   public function execute() {
     $table = new DifferentialTransactionComment();
     $conn_r = $table->establishConnection('r');
@@ -68,6 +74,26 @@ public function execute() {
 
     $comments = $table->loadAllFromArray($data);
 
+    if ($this->needHidden) {
+      $viewer_phid = $this->getViewer()->getPHID();
+      if ($viewer_phid && $comments) {
+        $hidden = queryfx_all(
+          $conn_r,
+          'SELECT commentID FROM %T WHERE userPHID = %s
+            AND commentID IN (%Ls)',
+          id(new DifferentialHiddenComment())->getTableName(),
+          $viewer_phid,
+          mpull($comments, 'getID'));
+        $hidden = array_fuse(ipull($hidden, 'commentID'));
+      } else {
+        $hidden = array();
+      }
+
+      foreach ($comments as $inline) {
+        $inline->attachIsHidden(isset($hidden[$inline->getID()]));
+      }
+    }
+
     foreach ($comments as $key => $value) {
       $comments[$key] = DifferentialInlineComment::newFromModernComment(
         $value);

src/applications/differential/render/DifferentialChangesetOneUpRenderer.php

@@ -41,8 +41,10 @@ protected function renderPrimitives(array $primitives, $rows) {
 
     $column_width = 4;
 
+    $hidden = new PHUIDiffRevealIconView();
+
     $out = array();
-    foreach ($primitives as $p) {
+    foreach ($primitives as $k => $p) {
       $type = $p['type'];
       switch ($type) {
         case 'old':
@@ -51,6 +53,27 @@ protected function renderPrimitives(array $primitives, $rows) {
         case 'new-file':
           $is_old = ($type == 'old' || $type == 'old-file');
 
+          $o_hidden = array();
+          $n_hidden = array();
+
+          for ($look = $k + 1; isset($primitives[$look]); $look++) {
+            $next = $primitives[$look];
+            switch ($next['type']) {
+              case 'inline':
+                $comment = $next['comment'];
+                if ($comment->isHidden()) {
+                  if ($next['right']) {
+                    $n_hidden[] = $comment;
+                  } else {
+                    $o_hidden[] = $comment;
+                  }
+                }
+                break;
+              default:
+                break 2;
+            }
+          }
+
           $cells = array();
           if ($is_old) {
             if ($p['htype']) {
@@ -68,7 +91,13 @@ protected function renderPrimitives(array $primitives, $rows) {
             } else {
               $left_id = null;
             }
-            $cells[] = phutil_tag('th', array('id' => $left_id), $p['line']);
+
+            $line = $p['line'];
+            if ($o_hidden) {
+              $line = array($hidden, $line);
+            }
+
+            $cells[] = phutil_tag('th', array('id' => $left_id), $line);
 
             $cells[] = phutil_tag('th', array());
             $cells[] = $no_copy;
@@ -85,7 +114,13 @@ protected function renderPrimitives(array $primitives, $rows) {
               } else {
                 $left_id = null;
               }
-              $cells[] = phutil_tag('th', array('id' => $left_id), $p['oline']);
+
+              $oline = $p['oline'];
+              if ($o_hidden) {
+                $oline = array($hidden, $oline);
+              }
+
+              $cells[] = phutil_tag('th', array('id' => $left_id), $oline);
             }
 
             if ($type == 'new-file') {
@@ -97,8 +132,13 @@ protected function renderPrimitives(array $primitives, $rows) {
             } else {
               $right_id = null;
             }
-            $cells[] = phutil_tag('th', array('id' => $right_id), $p['line']);
 
+            $line = $p['line'];
+            if ($n_hidden) {
+              $line = array($hidden, $line);
+            }
+
+            $cells[] = phutil_tag('th', array('id' => $right_id), $line);
 
             $cells[] = $no_copy;
             $cells[] = phutil_tag('td', array('class' => $class), $p['render']);