From 428fff2e58f35b7c3fed22ad3953f7c80f22c1e7 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Thu, 25 Mar 2021 10:31:51 -0700
Subject: [PATCH] Fix an issue when undoing mutiple inline comment deletions

Summary:
Ref T13559. If you create comments A and B, then delete comments A and B, then undo the deletion of A, the UI undoes the deletion of B instead.

This is becasue the undo rows are shipped down with a static scalar metadata reference. When copied multiple times to create multiple undo rows, they reference the same data object.

Preventing this in the general case is a problem with greater scope. For now, just avoid rendering these rows with any metadata so they don't alias a single data object.

Test Plan:
  - Created comments A, B.
  - Deleted comments A, B.
  - Clicked "Undo" on A.
    - Before: Deletion of "B" undone.
    - After: Deletion of "A" undone.

Maniphest Tasks: T13559

Differential Revision: https://secure.phabricator.com/D21650
---
 .../render/DifferentialChangesetRenderer.php  |  3 ++
 .../view/PHUIDiffInlineCommentRowScaffold.php | 37 ++++++++++++++++---
 .../view/PHUIDiffInlineCommentUndoView.php    |  2 +-
 3 files changed, 35 insertions(+), 7 deletions(-)

diff --git a/src/applications/differential/render/DifferentialChangesetRenderer.php b/src/applications/differential/render/DifferentialChangesetRenderer.php
index dcae3b979b..711d7d574b 100644
--- a/src/applications/differential/render/DifferentialChangesetRenderer.php
+++ b/src/applications/differential/render/DifferentialChangesetRenderer.php
@@ -718,6 +718,9 @@ abstract class DifferentialChangesetRenderer extends Phobject {
 
     foreach ($views as $key => $view) {
       $scaffold = $this->getRowScaffoldForInline($view);
+
+      $scaffold->setIsUndoTemplate(true);
+
       $views[$key] = id(new PHUIDiffInlineCommentTableScaffold())
         ->addRowScaffold($scaffold);
     }
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php
index 7c6b1c54b5..9bb68d63ba 100644
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php
@@ -10,6 +10,16 @@
 abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
 
   private $views = array();
+  private $isUndoTemplate;
+
+  final public function setIsUndoTemplate($is_undo_template) {
+    $this->isUndoTemplate = $is_undo_template;
+    return $this;
+  }
+
+  final public function getIsUndoTemplate() {
+    return $this->isUndoTemplate;
+  }
 
   public function getInlineViews() {
     return $this->views;
@@ -21,11 +31,28 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
   }
 
   protected function getRowAttributes() {
+    $is_undo_template = $this->getIsUndoTemplate();
+
     $is_hidden = false;
-    foreach ($this->getInlineViews() as $view) {
-      if ($view->isHidden()) {
-        $is_hidden = true;
+    if ($is_undo_template) {
+
+      // NOTE: When this scaffold is turned into an "undo" template, it is
+      // important it not have any metadata: the metadata reference will be
+      // copied to each instance of the row. This is a complicated mess; for
+      // now, just sneak by without generating metadata when rendering undo
+      // templates.
+
+      $metadata = null;
+    } else {
+      foreach ($this->getInlineViews() as $view) {
+        if ($view->isHidden()) {
+          $is_hidden = true;
+        }
       }
+
+      $metadata = array(
+        'hidden' => $is_hidden,
+      );
     }
 
     $classes = array();
@@ -37,9 +64,7 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
     $result = array(
       'class' => implode(' ', $classes),
       'sigil' => 'inline-row',
-      'meta' => array(
-        'hidden' => $is_hidden,
-      ),
+      'meta' => $metadata,
     );
 
     return $result;
diff --git a/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php
index 4abdb00e0b..a5fd7036d9 100644
--- a/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php
+++ b/src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php
@@ -27,7 +27,7 @@ final class PHUIDiffInlineCommentUndoView
       array(
         'class' => 'differential-inline-undo',
       ),
-      array(pht('Changes discarded. '), $link));
+      array(pht('Changes discarded.'), ' ', $link));
   }
 
 }