Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F17979906
D16108.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
13 KB
Referenced Files
None
Subscribers
None
D16108.diff
View Options
diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -3591,6 +3591,8 @@
'PhabricatorTokensCurtainExtension' => 'applications/tokens/engineextension/PhabricatorTokensCurtainExtension.php',
'PhabricatorTokensSettingsPanel' => 'applications/settings/panel/PhabricatorTokensSettingsPanel.php',
'PhabricatorTooltipUIExample' => 'applications/uiexample/examples/PhabricatorTooltipUIExample.php',
+ 'PhabricatorTransactionChange' => 'applications/transactions/data/PhabricatorTransactionChange.php',
+ 'PhabricatorTransactionRemarkupChange' => 'applications/transactions/data/PhabricatorTransactionRemarkupChange.php',
'PhabricatorTransactions' => 'applications/transactions/constants/PhabricatorTransactions.php',
'PhabricatorTransactionsApplication' => 'applications/transactions/application/PhabricatorTransactionsApplication.php',
'PhabricatorTransactionsDestructionEngineExtension' => 'applications/transactions/engineextension/PhabricatorTransactionsDestructionEngineExtension.php',
@@ -8397,6 +8399,8 @@
'PhabricatorTokensCurtainExtension' => 'PHUICurtainExtension',
'PhabricatorTokensSettingsPanel' => 'PhabricatorSettingsPanel',
'PhabricatorTooltipUIExample' => 'PhabricatorUIExample',
+ 'PhabricatorTransactionChange' => 'Phobject',
+ 'PhabricatorTransactionRemarkupChange' => 'PhabricatorTransactionChange',
'PhabricatorTransactions' => 'Phobject',
'PhabricatorTransactionsApplication' => 'PhabricatorApplication',
'PhabricatorTransactionsDestructionEngineExtension' => 'PhabricatorDestructionEngineExtension',
diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php
--- a/src/applications/audit/editor/PhabricatorAuditEditor.php
+++ b/src/applications/audit/editor/PhabricatorAuditEditor.php
@@ -542,7 +542,7 @@
protected function expandCustomRemarkupBlockTransactions(
PhabricatorLiskDAO $object,
array $xactions,
- $blocks,
+ array $changes,
PhutilMarkupEngine $engine) {
// we are only really trying to find unmentionable phids here...
@@ -563,7 +563,7 @@
return $result;
}
- $flat_blocks = array_mergev($blocks);
+ $flat_blocks = mpull($changes, 'getNewValue');
$huge_block = implode("\n\n", $flat_blocks);
$phid_map = array();
$phid_map[] = $this->getUnmentionablePHIDMap();
diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php
--- a/src/applications/differential/editor/DifferentialTransactionEditor.php
+++ b/src/applications/differential/editor/DifferentialTransactionEditor.php
@@ -1303,10 +1303,10 @@
protected function expandCustomRemarkupBlockTransactions(
PhabricatorLiskDAO $object,
array $xactions,
- $blocks,
+ array $changes,
PhutilMarkupEngine $engine) {
- $flat_blocks = array_mergev($blocks);
+ $flat_blocks = mpull($changes, 'getNewValue');
$huge_block = implode("\n\n", $flat_blocks);
$task_map = array();
diff --git a/src/applications/maniphest/storage/ManiphestTransaction.php b/src/applications/maniphest/storage/ManiphestTransaction.php
--- a/src/applications/maniphest/storage/ManiphestTransaction.php
+++ b/src/applications/maniphest/storage/ManiphestTransaction.php
@@ -54,16 +54,18 @@
return parent::shouldGenerateOldValue();
}
- public function getRemarkupBlocks() {
- $blocks = parent::getRemarkupBlocks();
+ protected function newRemarkupChanges() {
+ $changes = array();
switch ($this->getTransactionType()) {
case self::TYPE_DESCRIPTION:
- $blocks[] = $this->getNewValue();
+ $changes[] = $this->newRemarkupChange()
+ ->setOldValue($this->getOldValue())
+ ->setNewValue($this->getNewValue());
break;
}
- return $blocks;
+ return $changes;
}
public function getRequiredHandlePHIDs() {
diff --git a/src/applications/transactions/data/PhabricatorTransactionChange.php b/src/applications/transactions/data/PhabricatorTransactionChange.php
new file mode 100644
--- /dev/null
+++ b/src/applications/transactions/data/PhabricatorTransactionChange.php
@@ -0,0 +1,37 @@
+<?php
+
+abstract class PhabricatorTransactionChange extends Phobject {
+
+ private $transaction;
+ private $oldValue;
+ private $newValue;
+
+ final public function setTransaction(
+ PhabricatorApplicationTransaction $xaction) {
+ $this->transaction = $xaction;
+ return $this;
+ }
+
+ final public function getTransaction() {
+ return $this->transaction;
+ }
+
+ final public function setOldValue($old_value) {
+ $this->oldValue = $old_value;
+ return $this;
+ }
+
+ final public function getOldValue() {
+ return $this->oldValue;
+ }
+
+ final public function setNewValue($new_value) {
+ $this->newValue = $new_value;
+ return $this;
+ }
+
+ final public function getNewValue() {
+ return $this->newValue;
+ }
+
+}
diff --git a/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php b/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php
new file mode 100644
--- /dev/null
+++ b/src/applications/transactions/data/PhabricatorTransactionRemarkupChange.php
@@ -0,0 +1,4 @@
+<?php
+
+final class PhabricatorTransactionRemarkupChange
+ extends PhabricatorTransactionChange {}
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionCommentEditor.php
@@ -64,6 +64,9 @@
$comment->setTransactionPHID($xaction->getPHID());
$comment->save();
+ $old_comment = $xaction->getComment();
+ $comment->attachOldComment($old_comment);
+
$xaction->setCommentVersion($new_version);
$xaction->setCommentPHID($comment->getPHID());
$xaction->setViewPolicy($comment->getViewPolicy());
diff --git a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
--- a/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
+++ b/src/applications/transactions/editor/PhabricatorApplicationTransactionEditor.php
@@ -1320,17 +1320,28 @@
private function buildSubscribeTransaction(
PhabricatorLiskDAO $object,
array $xactions,
- array $blocks) {
+ array $changes) {
if (!($object instanceof PhabricatorSubscribableInterface)) {
return null;
}
if ($this->shouldEnableMentions($object, $xactions)) {
- $texts = array_mergev($blocks);
- $phids = PhabricatorMarkupEngine::extractPHIDsFromMentions(
+ // Identify newly mentioned users. We ignore users who were previously
+ // mentioned so that we don't re-subscribe users after an edit of text
+ // which mentions them.
+ $old_texts = mpull($changes, 'getOldValue');
+ $new_texts = mpull($changes, 'getNewValue');
+
+ $old_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions(
+ $this->getActor(),
+ $old_texts);
+
+ $new_phids = PhabricatorMarkupEngine::extractPHIDsFromMentions(
$this->getActor(),
- $texts);
+ $new_texts);
+
+ $phids = array_diff($new_phids, $old_phids);
} else {
$phids = array();
}
@@ -1381,11 +1392,6 @@
return $xaction;
}
- protected function getRemarkupBlocksFromTransaction(
- PhabricatorApplicationTransaction $transaction) {
- return $transaction->getRemarkupBlocks();
- }
-
protected function mergeTransactions(
PhabricatorApplicationTransaction $u,
PhabricatorApplicationTransaction $v) {
@@ -1464,15 +1470,12 @@
$xactions = $this->applyImplicitCC($object, $xactions);
- $blocks = array();
- foreach ($xactions as $key => $xaction) {
- $blocks[$key] = $this->getRemarkupBlocksFromTransaction($xaction);
- }
+ $changes = $this->getRemarkupChanges($xactions);
$subscribe_xaction = $this->buildSubscribeTransaction(
$object,
$xactions,
- $blocks);
+ $changes);
if ($subscribe_xaction) {
$xactions[] = $subscribe_xaction;
}
@@ -1484,7 +1487,7 @@
$block_xactions = $this->expandRemarkupBlockTransactions(
$object,
$xactions,
- $blocks,
+ $changes,
$engine);
foreach ($block_xactions as $xaction) {
@@ -1494,27 +1497,46 @@
return $xactions;
}
+ private function getRemarkupChanges(array $xactions) {
+ $changes = array();
+
+ foreach ($xactions as $key => $xaction) {
+ foreach ($this->getRemarkupChangesFromTransaction($xaction) as $change) {
+ $changes[] = $change;
+ }
+ }
+
+ return $changes;
+ }
+
+ private function getRemarkupChangesFromTransaction(
+ PhabricatorApplicationTransaction $transaction) {
+ return $transaction->getRemarkupChanges();
+ }
+
private function expandRemarkupBlockTransactions(
PhabricatorLiskDAO $object,
array $xactions,
- $blocks,
+ array $changes,
PhutilMarkupEngine $engine) {
$block_xactions = $this->expandCustomRemarkupBlockTransactions(
$object,
$xactions,
- $blocks,
+ $changes,
$engine);
$mentioned_phids = array();
if ($this->shouldEnableMentions($object, $xactions)) {
- foreach ($blocks as $key => $xaction_blocks) {
- foreach ($xaction_blocks as $block) {
- $engine->markupText($block);
- $mentioned_phids += $engine->getTextMetadata(
- PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS,
- array());
- }
+ foreach ($changes as $change) {
+ // Here, we don't care about processing only new mentions after an edit
+ // because there is no way for an object to ever "unmention" itself on
+ // another object, so we can ignore the old value.
+ $engine->markupText($change->getNewValue());
+
+ $mentioned_phids += $engine->getTextMetadata(
+ PhabricatorObjectRemarkupRule::KEY_MENTIONED_OBJECTS,
+ array());
}
}
@@ -1559,7 +1581,7 @@
protected function expandCustomRemarkupBlockTransactions(
PhabricatorLiskDAO $object,
array $xactions,
- $blocks,
+ array $changes,
PhutilMarkupEngine $engine) {
return array();
}
@@ -3096,11 +3118,8 @@
PhabricatorLiskDAO $object,
array $xactions) {
- $blocks = array();
- foreach ($xactions as $xaction) {
- $blocks[] = $this->getRemarkupBlocksFromTransaction($xaction);
- }
- $blocks = array_mergev($blocks);
+ $changes = $this->getRemarkupChanges($xactions);
+ $blocks = mpull($changes, 'getNewValue');
$phids = array();
if ($blocks) {
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
--- a/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransaction.php
@@ -179,6 +179,50 @@
return $this->assertAttached($this->object);
}
+ public function getRemarkupChanges() {
+ $changes = $this->newRemarkupChanges();
+ assert_instances_of($changes, 'PhabricatorTransactionRemarkupChange');
+
+ // Convert older-style remarkup blocks into newer-style remarkup changes.
+ // This builds changes that do not have the correct "old value", so rules
+ // that operate differently against edits (like @user mentions) won't work
+ // properly.
+ foreach ($this->getRemarkupBlocks() as $block) {
+ $changes[] = $this->newRemarkupChange()
+ ->setOldValue(null)
+ ->setNewValue($block);
+ }
+
+ $comment = $this->getComment();
+ if ($comment) {
+ if ($comment->hasOldComment()) {
+ $old_value = $comment->getOldComment()->getContent();
+ } else {
+ $old_value = null;
+ }
+
+ $new_value = $comment->getContent();
+
+ $changes[] = $this->newRemarkupChange()
+ ->setOldValue($old_value)
+ ->setNewValue($new_value);
+ }
+
+ return $changes;
+ }
+
+ protected function newRemarkupChanges() {
+ return array();
+ }
+
+ protected function newRemarkupChange() {
+ return id(new PhabricatorTransactionRemarkupChange())
+ ->setTransaction($this);
+ }
+
+ /**
+ * @deprecated
+ */
public function getRemarkupBlocks() {
$blocks = array();
@@ -195,10 +239,6 @@
break;
}
- if ($this->getComment()) {
- $blocks[] = $this->getComment()->getContent();
- }
-
return $blocks;
}
diff --git a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
--- a/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
+++ b/src/applications/transactions/storage/PhabricatorApplicationTransactionComment.php
@@ -18,6 +18,8 @@
protected $contentSource;
protected $isDeleted = 0;
+ private $oldComment = self::ATTACHABLE;
+
abstract public function getApplicationTransactionObject();
public function generatePHID() {
@@ -85,6 +87,20 @@
return $this;
}
+ public function attachOldComment(
+ PhabricatorApplicationTransactionComment $old_comment) {
+ $this->oldComment = $old_comment;
+ return $this;
+ }
+
+ public function getOldComment() {
+ return $this->assertAttached($this->oldComment);
+ }
+
+ public function hasOldComment() {
+ return ($this->oldComment !== self::ATTACHABLE);
+ }
+
/* -( PhabricatorMarkupInterface )----------------------------------------- */
File Metadata
Details
Attached
Mime Type
text/plain
Expires
Sat, Aug 2, 6:34 PM (4 w, 1 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
8617547
Default Alt Text
D16108.diff (13 KB)
Attached To
Mode
D16108: Don't re-mention users for comment edits
Attached
Detach File
Event Timeline
Log In to Comment