Skip to content

Commit 428fff2

Browse files
author
epriestley
committed
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
1 parent d30c3a9 commit 428fff2

File tree

3 files changed

+35
-7
lines changed

3 files changed

+35
-7
lines changed

src/applications/differential/render/DifferentialChangesetRenderer.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -718,6 +718,9 @@ public function renderUndoTemplates() {
718718

719719
foreach ($views as $key => $view) {
720720
$scaffold = $this->getRowScaffoldForInline($view);
721+
722+
$scaffold->setIsUndoTemplate(true);
723+
721724
$views[$key] = id(new PHUIDiffInlineCommentTableScaffold())
722725
->addRowScaffold($scaffold);
723726
}

src/infrastructure/diff/view/PHUIDiffInlineCommentRowScaffold.php

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,16 @@
1010
abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
1111

1212
private $views = array();
13+
private $isUndoTemplate;
14+
15+
final public function setIsUndoTemplate($is_undo_template) {
16+
$this->isUndoTemplate = $is_undo_template;
17+
return $this;
18+
}
19+
20+
final public function getIsUndoTemplate() {
21+
return $this->isUndoTemplate;
22+
}
1323

1424
public function getInlineViews() {
1525
return $this->views;
@@ -21,11 +31,28 @@ public function addInlineView(PHUIDiffInlineCommentView $view) {
2131
}
2232

2333
protected function getRowAttributes() {
34+
$is_undo_template = $this->getIsUndoTemplate();
35+
2436
$is_hidden = false;
25-
foreach ($this->getInlineViews() as $view) {
26-
if ($view->isHidden()) {
27-
$is_hidden = true;
37+
if ($is_undo_template) {
38+
39+
// NOTE: When this scaffold is turned into an "undo" template, it is
40+
// important it not have any metadata: the metadata reference will be
41+
// copied to each instance of the row. This is a complicated mess; for
42+
// now, just sneak by without generating metadata when rendering undo
43+
// templates.
44+
45+
$metadata = null;
46+
} else {
47+
foreach ($this->getInlineViews() as $view) {
48+
if ($view->isHidden()) {
49+
$is_hidden = true;
50+
}
2851
}
52+
53+
$metadata = array(
54+
'hidden' => $is_hidden,
55+
);
2956
}
3057

3158
$classes = array();
@@ -37,9 +64,7 @@ protected function getRowAttributes() {
3764
$result = array(
3865
'class' => implode(' ', $classes),
3966
'sigil' => 'inline-row',
40-
'meta' => array(
41-
'hidden' => $is_hidden,
42-
),
67+
'meta' => $metadata,
4368
);
4469

4570
return $result;

src/infrastructure/diff/view/PHUIDiffInlineCommentUndoView.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ public function render() {
2727
array(
2828
'class' => 'differential-inline-undo',
2929
),
30-
array(pht('Changes discarded. '), $link));
30+
array(pht('Changes discarded.'), ' ', $link));
3131
}
3232

3333
}

0 commit comments

Comments
 (0)