From f5ef341c9e35239a921c6ed497b853dec153c9f1 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Mon, 4 May 2020 09:22:20 -0700
Subject: [PATCH] Don't publish "empty" inline comments

Summary:
Ref T13513. Currently, if you start an inline and then submit overall comments, we publish an empty inline. This is literally faithful to what you did, but almost certainly not the intent.

Instead, simply ignore empty inlines at publishing time (and ignore "done" state changes for those comments).

We could delete them outright, but if we do, they'll break if you have another window open with the empty inline (since the stored comment won't exist anymore). At least for now, leave them in place.

Test Plan: Created empty inlines, submitted comments, no longer saw them publish.

Maniphest Tasks: T13513

Differential Revision: https://secure.phabricator.com/D21211
---
 .../storage/PhabricatorAuditTransactionComment.php |  4 ++++
 .../storage/DifferentialTransactionComment.php     |  4 ++++
 .../PhabricatorApplicationTransactionEditor.php    | 14 +++++++++++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php
index 2a5c2b1ee8..693dc2f9be 100644
--- a/src/applications/audit/storage/PhabricatorAuditTransactionComment.php
+++ b/src/applications/audit/storage/PhabricatorAuditTransactionComment.php
@@ -81,4 +81,8 @@ final class PhabricatorAuditTransactionComment
     return $this;
   }
 
+  public function isEmptyInlineComment() {
+    return !strlen($this->getContent());
+  }
+
 }
diff --git a/src/applications/differential/storage/DifferentialTransactionComment.php b/src/applications/differential/storage/DifferentialTransactionComment.php
index 257b205a58..bbbf1abc5a 100644
--- a/src/applications/differential/storage/DifferentialTransactionComment.php
+++ b/src/applications/differential/storage/DifferentialTransactionComment.php
@@ -127,4 +127,8 @@ final class DifferentialTransactionComment
     return $this;
   }
 
+  public function isEmptyInlineComment() {
+    return !strlen($this->getContent());
+  }
+
 }
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
index 9d000a9c45..4344bc4aa4 100644
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -5033,7 +5033,12 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $xactions = array();
 
-    foreach ($inlines as $inline) {
+    foreach ($inlines as $key => $inline) {
+      if ($inline->isEmptyInlineComment()) {
+        unset($inlines[$key]);
+        continue;
+      }
+
       $xactions[] = $object->getApplicationTransactionTemplate()
         ->setTransactionType($transaction_type)
         ->attachComment($inline);
@@ -5079,6 +5084,13 @@ abstract class PhabricatorApplicationTransactionEditor
 
     $inlines = array_mergev($inlines);
 
+    foreach ($inlines as $key => $inline) {
+      if ($inline->isEmptyInlineComment()) {
+        unset($inlines[$key]);
+        continue;
+      }
+    }
+
     if (!$inlines) {
       return null;
     }