From 8692d673c8ab1ebec062f662aab17023b65cd061 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 7 Jun 2017 19:04:23 -0700
Subject: [PATCH] Fix minor inline comment header button behaviors

Summary:
Fixes T12806. Ref T12733.

  - Don't count synthetic (lint) comments as anything.
  - When you begin writing an inline then cancel it, don't count it as anything.
  - When we would show "0 / X", just show "X".

Test Plan:
  - Viewed a diff with synthetic comments, no button.
  - Wrote, then cancelled an inline. No "X comments".
  - Clicked / unlicked "Done", saw "X" -> "1 / X".

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12806, T12733

Differential Revision: https://secure.phabricator.com/D18103
---
 .../view/PHUIDiffInlineCommentDetailView.php  | 11 +++---
 .../js/application/diff/DiffChangesetList.js  | 37 +++++++++++++++----
 .../rsrc/js/application/diff/DiffInline.js    |  6 +++
 3 files changed, 42 insertions(+), 12 deletions(-)

diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
index a42efa932a..46853e185c 100644
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php
@@ -107,6 +107,11 @@ final class PHUIDiffInlineCommentDetailView
         break;
     }
 
+    $is_synthetic = false;
+    if ($inline->getSyntheticAuthor()) {
+      $is_synthetic = true;
+    }
+
     $metadata = array(
       'id' => $inline->getID(),
       'phid' => $inline->getPHID(),
@@ -120,6 +125,7 @@ final class PHUIDiffInlineCommentDetailView
       'isDraft' => $inline->isDraft(),
       'isFixed' => $is_fixed,
       'isGhost' => $inline->getIsGhost(),
+      'isSynthetic' => $is_synthetic,
     );
 
     $sigil = 'differential-inline-comment';
@@ -136,11 +142,6 @@ final class PHUIDiffInlineCommentDetailView
 
     $links = array();
 
-    $is_synthetic = false;
-    if ($inline->getSyntheticAuthor()) {
-      $is_synthetic = true;
-    }
-
     $draft_text = null;
     if (!$is_synthetic) {
       // This display is controlled by CSS
diff --git a/webroot/rsrc/js/application/diff/DiffChangesetList.js b/webroot/rsrc/js/application/diff/DiffChangesetList.js
index 400b828a9a..b476b8c168 100644
--- a/webroot/rsrc/js/application/diff/DiffChangesetList.js
+++ b/webroot/rsrc/js/application/diff/DiffChangesetList.js
@@ -1352,8 +1352,16 @@ JX.install('DiffChangesetList', {
             continue;
           }
 
+          if (inline.isSynthetic()) {
+            continue;
+          }
+
           if (inline.isEditing()) {
             unsaved.push(inline);
+          } else if (!inline.getID()) {
+            // These are new comments which have been cancelled, and do not
+            // count as anything.
+            continue;
           } else if (inline.isDraft()) {
             unsubmitted.push(inline);
           } else if (!inline.isDone()) {
@@ -1395,13 +1403,28 @@ JX.install('DiffChangesetList', {
       }
 
       if (done.length || undone.length) {
-        done_button.setText([
-          done.length,
-          ' / ',
-          (done.length + undone.length),
-          ' ',
-          pht('Comments')
-        ]);
+        // If you haven't marked any comments as "Done", we just show text
+        // like "3 Comments". If you've marked at least one done, we show
+        // "1 / 3 Comments".
+
+        var done_text;
+        if (done.length) {
+          done_text = [
+            done.length,
+            ' / ',
+            (done.length + undone.length),
+            ' ',
+            pht('Comments')
+          ];
+        } else {
+          done_text = [
+            undone.length,
+            ' ',
+            pht('Comments')
+          ];
+        }
+
+        done_button.setText(done_text);
 
         JX.DOM.show(done_button.getNode());
 
diff --git a/webroot/rsrc/js/application/diff/DiffInline.js b/webroot/rsrc/js/application/diff/DiffInline.js
index 6d897c27ae..baee53a7a4 100644
--- a/webroot/rsrc/js/application/diff/DiffInline.js
+++ b/webroot/rsrc/js/application/diff/DiffInline.js
@@ -34,6 +34,7 @@ JX.install('DiffInline', {
     _isFixed: null,
     _isEditing: false,
     _isNew: false,
+    _isSynthetic: false,
 
     bindToRow: function(row) {
       this._row = row;
@@ -71,6 +72,7 @@ JX.install('DiffInline', {
       this._isDraft = data.isDraft;
       this._isFixed = data.isFixed;
       this._isGhost = data.isGhost;
+      this._isSynthetic = data.isSynthetic;
 
       this._changesetID = data.changesetID;
       this._isNew = false;
@@ -97,6 +99,10 @@ JX.install('DiffInline', {
       return this._isDeleted;
     },
 
+    isSynthetic: function() {
+      return this._isSynthetic;
+    },
+
     bindToRange: function(data) {
       this._displaySide = data.displaySide;
       this._number = parseInt(data.number, 10);
-- 
2.51.2