When users mark their own inline comments as "Done", suppress the timeline/mail stories

Summary:
Depends on D19858. Ref T13222. See PHI995. In D19635 and related revisions, inline behavior changed to allow you to pre-mark your own inlines as done (as a reviewer) and to pre-mark your inlines for you (as an author).

These actions generate low-value stories in the timeline, like "alice marked 3 comments done." when an author adds some notes to their own revision. These aren't helpful and can be a little misleading.

Instead, just don't count it when someone marks their own inlines as "done". If we throw away all the marks after throwing away the self-marks, hide the whole story.

This happens in three cases:

  # You comment on your own revision, and don't uncheck the "Done" checkbox.
  # You comment on someone else's revision, and check the "Done" checkbox before submitting.
  # You leave a not-"Done" inline on your own revision, then "Done" it later.

Cases (1) and (2) seem unambiguously good/clear. Case (3) is a little more questionable, but I think this still isn't very useful for reviewers.

If there's still a clarity issue around case (3), we could change the story text to "alice marked 3 inline comments by other users as done.", but I think this is probably needlessly verbose and that no one will be confused by the behavior as written here.

(Also note that this story is never shown in feed.)

Test Plan: Created and marked a bunch of inlines as "Done" in Differential and Diffusion, as the author and reviewer/auditor. My own marks didn't generate timeline stories; marking others' comments still does.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13222

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

src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php

@@ -4658,9 +4658,21 @@ abstract class PhabricatorApplicationTransactionEditor
       $new_value[$key] = $state_map[$state];
     }
 
+    // See PHI995. Copy some information about the inlines into the transaction
+    // so we can tailor rendering behavior. In particular, we don't want to
+    // render transactions about users marking their own inlines as "Done".
+
+    $inline_details = array();
+    foreach ($inlines as $inline) {
+      $inline_details[$inline->getPHID()] = array(
+        'authorPHID' => $inline->getAuthorPHID(),
+      );
+    }
+
     return $object->getApplicationTransactionTemplate()
       ->setTransactionType(PhabricatorTransactions::TYPE_INLINESTATE)
       ->setIgnoreOnNoEffect(true)
+      ->setMetadataValue('inline.details', $inline_details)
       ->setOldValue($old_value)
       ->setNewValue($new_value);
   }

src/applications/transactions/storage/PhabricatorApplicationTransaction.php

@@ -664,6 +664,16 @@ abstract class PhabricatorApplicationTransaction
             break;
         }
         break;
+
+      case PhabricatorTransactions::TYPE_INLINESTATE:
+        list($done, $undone) = $this->getInterestingInlineStateChangeCounts();
+
+        if (!$done && !$undone) {
+          return true;
+        }
+
+        break;
+
     }
 
     return false;
@@ -1007,15 +1017,7 @@ abstract class PhabricatorApplicationTransaction
         }
 
       case PhabricatorTransactions::TYPE_INLINESTATE:
-        $done = 0;
-        $undone = 0;
-        foreach ($new as $phid => $state) {
-          if ($state == PhabricatorInlineCommentInterface::STATE_DONE) {
-            $done++;
-          } else {
-            $undone++;
-          }
-        }
+        list($done, $undone) = $this->getInterestingInlineStateChangeCounts();
         if ($done && $undone) {
           return pht(
             '%s marked %s inline comment(s) as done and %s inline comment(s) '.
@@ -1582,6 +1584,49 @@ abstract class PhabricatorApplicationTransaction
     return $moves;
   }
 
+  private function getInterestingInlineStateChangeCounts() {
+    // See PHI995. Newer inline state transactions have additional details
+    // which we use to tailor the rendering behavior. These details are not
+    // present on older transactions.
+    $details = $this->getMetadataValue('inline.details', array());
+
+    $new = $this->getNewValue();
+
+    $done = 0;
+    $undone = 0;
+    foreach ($new as $phid => $state) {
+      $is_done = ($state == PhabricatorInlineCommentInterface::STATE_DONE);
+
+      // See PHI995. If you're marking your own inline comments as "Done",
+      // don't count them when rendering a timeline story. In the case where
+      // you're only affecting your own comments, this will hide the
+      // "alice marked X comments as done" story entirely.
+
+      // Usually, this happens when you pre-mark inlines as "done" and submit
+      // them yourself. We'll still generate an "alice added inline comments"
+      // story (in most cases/contexts), but the state change story is largely
+      // just clutter and slightly confusing/misleading.
+
+      $inline_details = idx($details, $phid, array());
+      $inline_author_phid = idx($inline_details, 'authorPHID');
+      if ($inline_author_phid) {
+        if ($inline_author_phid == $this->getAuthorPHID()) {
+          if ($is_done) {
+            continue;
+          }
+        }
+      }
+
+      if ($is_done) {
+        $done++;
+      } else {
+        $undone++;
+      }
+    }
+
+    return array($done, $undone);
+  }
+
 
 /* -(  PhabricatorPolicyInterface Implementation  )-------------------------- */