1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-22 06:42:42 +01:00

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
This commit is contained in:
epriestley 2021-03-25 10:31:51 -07:00
parent d30c3a961c
commit 428fff2e58
3 changed files with 35 additions and 7 deletions

View file

@ -718,6 +718,9 @@ abstract class DifferentialChangesetRenderer extends Phobject {
foreach ($views as $key => $view) { foreach ($views as $key => $view) {
$scaffold = $this->getRowScaffoldForInline($view); $scaffold = $this->getRowScaffoldForInline($view);
$scaffold->setIsUndoTemplate(true);
$views[$key] = id(new PHUIDiffInlineCommentTableScaffold()) $views[$key] = id(new PHUIDiffInlineCommentTableScaffold())
->addRowScaffold($scaffold); ->addRowScaffold($scaffold);
} }

View file

@ -10,6 +10,16 @@
abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView { abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
private $views = array(); 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() { public function getInlineViews() {
return $this->views; return $this->views;
@ -21,11 +31,28 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
} }
protected function getRowAttributes() { protected function getRowAttributes() {
$is_undo_template = $this->getIsUndoTemplate();
$is_hidden = false; $is_hidden = false;
foreach ($this->getInlineViews() as $view) { if ($is_undo_template) {
if ($view->isHidden()) {
$is_hidden = true; // 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(); $classes = array();
@ -37,9 +64,7 @@ abstract class PHUIDiffInlineCommentRowScaffold extends AphrontView {
$result = array( $result = array(
'class' => implode(' ', $classes), 'class' => implode(' ', $classes),
'sigil' => 'inline-row', 'sigil' => 'inline-row',
'meta' => array( 'meta' => $metadata,
'hidden' => $is_hidden,
),
); );
return $result; return $result;

View file

@ -27,7 +27,7 @@ final class PHUIDiffInlineCommentUndoView
array( array(
'class' => 'differential-inline-undo', 'class' => 'differential-inline-undo',
), ),
array(pht('Changes discarded. '), $link)); array(pht('Changes discarded.'), ' ', $link));
} }
} }