From a529efa5b8554167f8a2c16f1423837ae47cc596 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 23 May 2020 08:13:41 -0700
Subject: [PATCH] Fix an issue where inline comments with only edit suggestions
 are considered empty

Summary:
Ref T13513. An inline is not considered empty if it has a suggestion, but some of the shared transaction code doesn't test for this properly.

Update the shared transaction code to be aware that application comments may have more complex emptiness rules.

Test Plan:
  - Posted an inline with only an edit suggestion, comment went through.
  - Tried to post a normal empty comment, got an appropriate warning.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21287
---
 .../storage/DifferentialTransactionComment.php       | 12 ++++++++++++
 .../storage/PhabricatorApplicationTransaction.php    |  9 +++------
 .../PhabricatorApplicationTransactionComment.php     | 12 ++++++++++++
 3 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php
index 491d89a968..873aecdd41 100644
--- a/src/applications/differential/storage/DifferentialTransactionComment.php
+++ b/src/applications/differential/storage/DifferentialTransactionComment.php
@@ -144,4 +144,16 @@ final class DifferentialTransactionComment
     return $this;
   }
 
+
+  public function isEmptyComment() {
+    if (!parent::isEmptyComment()) {
+      return false;
+    }
+
+    return $this->newInlineCommentObject()
+      ->getContentState()
+      ->isEmptyContentState();
+  }
+
+
 }
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
index b815354d11..95ef1de6c3 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -127,15 +127,12 @@ abstract class PhabricatorApplicationTransaction
   }
 
   public function hasComment() {
-    if (!$this->getComment()) {
+    $comment = $this->getComment();
+    if (!$comment) {
       return false;
     }
 
-    $content = $this->getComment()->getContent();
-
-    // If the content is empty or consists of only whitespace, don't count
-    // this as comment.
-    if (!strlen(trim($content))) {
+    if ($comment->isEmptyComment()) {
       return false;
     }
 
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
index 896b45556a..00cad18d2d 100644
--- a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
@@ -107,6 +107,18 @@ abstract class PhabricatorApplicationTransactionComment
       $this->getTransactionPHID());
   }
 
+  public function isEmptyComment() {
+    $content = $this->getContent();
+
+    // The comment is empty if there's no content, or if the content only has
+    // whitespace.
+    if (!strlen(trim($content))) {
+      return true;
+    }
+
+    return false;
+  }
+
 /* -(  PhabricatorMarkupInterface  )----------------------------------------- */