Transactions - fix bug in differential where older inlines weren't appearing

Summary: Fixes T6699. We need to "loadInlineComments" consistently, though for an unexpected reason - this mutates the $changesets to include all $changesets that have an associated inline comment, which is necessary to make them render properly.

Test Plan: Took a diff with inline comments and updated it, noting the inline comments disappeared. applied this patch and the inlines reappeared.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6699

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

src/applications/differential/controller/DifferentialRevisionViewController.php

@@ -96,9 +96,7 @@ final class DifferentialRevisionViewController extends DifferentialController {
     $props = mpull($props, 'getData', 'getName');
 
     $all_changesets = $changesets;
-    $inlines = $this->loadInlineComments(
+    $inlines = $revision->loadInlineComments($all_changesets);
-      $revision,
-      $all_changesets);
 
     $object_phids = array_merge(
       $revision->getReviewers(),
@@ -644,44 +642,6 @@ final class DifferentialRevisionViewController extends DifferentialController {
     return $actions_dict;
   }
 
-  private function loadInlineComments(
-    DifferentialRevision $revision,
-    array &$changesets) {
-    assert_instances_of($changesets, 'DifferentialChangeset');
-
-    $inline_comments = array();
-
-    $inline_comments = id(new DifferentialInlineCommentQuery())
-      ->withRevisionIDs(array($revision->getID()))
-      ->withNotDraft(true)
-      ->execute();
-
-    $load_changesets = array();
-    foreach ($inline_comments as $inline) {
-      $changeset_id = $inline->getChangesetID();
-      if (isset($changesets[$changeset_id])) {
-        continue;
-      }
-      $load_changesets[$changeset_id] = true;
-    }
-
-    $more_changesets = array();
-    if ($load_changesets) {
-      $changeset_ids = array_keys($load_changesets);
-      $more_changesets += id(new DifferentialChangeset())
-        ->loadAllWhere(
-          'id IN (%Ld)',
-          $changeset_ids);
-    }
-
-    if ($more_changesets) {
-      $changesets += $more_changesets;
-      $changesets = msort($changesets, 'getSortKey');
-    }
-
-    return $inline_comments;
-  }
-
   private function loadChangesetsAndVsMap(
     DifferentialDiff $target,
     DifferentialDiff $diff_vs = null,

src/applications/differential/storage/DifferentialRevision.php

@@ -280,6 +280,44 @@ final class DifferentialRevision extends DifferentialDAO
     return $this;
   }
 
+  public function loadInlineComments(
+    array &$changesets) {
+    assert_instances_of($changesets, 'DifferentialChangeset');
+
+    $inline_comments = array();
+
+    $inline_comments = id(new DifferentialInlineCommentQuery())
+      ->withRevisionIDs(array($this->getID()))
+      ->withNotDraft(true)
+      ->execute();
+
+    $load_changesets = array();
+    foreach ($inline_comments as $inline) {
+      $changeset_id = $inline->getChangesetID();
+      if (isset($changesets[$changeset_id])) {
+        continue;
+      }
+      $load_changesets[$changeset_id] = true;
+    }
+
+    $more_changesets = array();
+    if ($load_changesets) {
+      $changeset_ids = array_keys($load_changesets);
+      $more_changesets += id(new DifferentialChangeset())
+        ->loadAllWhere(
+          'id IN (%Ld)',
+          $changeset_ids);
+    }
+
+    if ($more_changesets) {
+      $changesets += $more_changesets;
+      $changesets = msort($changesets, 'getSortKey');
+    }
+
+    return $inline_comments;
+  }
+
+
   public function getCapabilities() {
     return array(
       PhabricatorPolicyCapability::CAN_VIEW,
@@ -495,6 +533,9 @@ final class DifferentialRevision extends DifferentialDAO
       ->setViewer($request->getUser())
       ->withDiffs(array($right_diff))
       ->execute();
+    // NOTE: this mutates $changesets to include changesets for all inline
+    // comments...!
+    $inlines = $this->loadInlineComments($changesets);
     $changesets = mpull($changesets, null, 'getID');
 
     return $timeline