Page MenuHomePhabricator

D8201.id18555.diff
No OneTemporary

D8201.id18555.diff

Index: src/applications/differential/editor/DifferentialCommentEditor.php
===================================================================
--- src/applications/differential/editor/DifferentialCommentEditor.php
+++ src/applications/differential/editor/DifferentialCommentEditor.php
@@ -462,16 +462,8 @@
$added_ccs = $this->filterAddedCCs($added_ccs);
if ($added_ccs) {
- foreach ($added_ccs as $cc) {
- DifferentialRevisionEditor::addCC(
- $revision,
- $cc,
- $actor_phid);
- }
-
$key = DifferentialComment::METADATA_ADDED_CCS;
$metadata[$key] = $added_ccs;
-
} else {
if ($user_tried_to_add == 0) {
throw new DifferentialActionHasNoEffectException(
@@ -564,69 +556,102 @@
$event->setUser($actor);
PhutilEventEngine::dispatchEvent($event);
- $comment = id(new DifferentialComment())
+ $template = id(new DifferentialComment())
->setAuthorPHID($actor_phid)
- ->setRevision($revision)
- ->setAction($action)
- ->setContent((string)$this->message)
- ->setMetadata($metadata);
+ ->setRevision($revision);
if ($this->contentSource) {
- $comment->setContentSource($this->contentSource);
+ $template->setContentSource($this->contentSource);
}
- $comment->save();
+ $comments = array();
- $changesets = array();
- if ($inline_comments) {
- $load_ids = mpull($inline_comments, 'getChangesetID');
- if ($load_ids) {
- $load_ids = array_unique($load_ids);
- $changesets = id(new DifferentialChangeset())->loadAllWhere(
- 'id in (%Ld)',
- $load_ids);
- }
- foreach ($inline_comments as $inline) {
- $inline->setCommentID($comment->getID());
- $inline->save();
- }
+ // If this edit performs a meaningful action, save a transaction for the
+ // action. Do this first, since the mail currently assumes the first
+ // transaction is the strongest.
+ if ($action != DifferentialAction::ACTION_COMMENT &&
+ $action != DifferentialAction::ACTION_ADDREVIEWERS &&
+ $action != DifferentialAction::ACTION_ADDCCS) {
+ $comments[] = id(clone $template)
+ ->setAction($action);
}
- // Find any "@mentions" in the comment blocks.
- $content_blocks = array($comment->getContent());
+ // If this edit adds reviewers, save a transaction for those changes.
+ $rev_add = DifferentialComment::METADATA_ADDED_REVIEWERS;
+ $rev_rem = DifferentialComment::METADATA_REMOVED_REVIEWERS;
+ if (idx($metadata, $rev_add) || idx($metadata, $rev_rem)) {
+ $reviewer_meta = array_select_keys($metadata, array($rev_add, $rev_rem));
+
+ $comments[] = id(clone $template)
+ ->setAction(DifferentialAction::ACTION_ADDREVIEWERS)
+ ->setMetadata($reviewer_meta);
+ }
+
+ // If this edit adds CCs, save a transaction for the new CCs. We build this
+ // for either explicit CCs, or for @mentions. First, find any "@mentions" in
+ // the comment blocks.
+ $content_blocks = array($this->message);
foreach ($inline_comments as $inline) {
$content_blocks[] = $inline->getContent();
}
$mention_ccs = PhabricatorMarkupEngine::extractPHIDsFromMentions(
$content_blocks);
- if ($mention_ccs) {
- $mention_ccs = $this->filterAddedCCs($mention_ccs);
- if ($mention_ccs) {
- $metadata = $comment->getMetadata();
- $metacc = idx(
- $metadata,
- DifferentialComment::METADATA_ADDED_CCS,
- array());
- foreach ($mention_ccs as $cc_phid) {
+
+ // Now, build a comment if we have explicit action CCs or mention CCs.
+ $cc_add = DifferentialComment::METADATA_ADDED_CCS;
+ if (idx($metadata, $cc_add) || $mention_ccs) {
+ $all_adds = array_merge(
+ idx($metadata, $cc_add, array()),
+ $mention_ccs);
+ $all_adds = $this->filterAddedCCs($all_adds);
+ if ($all_adds) {
+ $cc_meta = array(
+ DifferentialComment::METADATA_ADDED_CCS => $all_adds,
+ );
+
+ foreach ($all_adds as $cc_phid) {
DifferentialRevisionEditor::addCC(
$revision,
$cc_phid,
$actor_phid);
- $metacc[] = $cc_phid;
}
- $metadata[DifferentialComment::METADATA_ADDED_CCS] = $metacc;
- $comment->setMetadata($metadata);
- $comment->save();
+ $comments[] = id(clone $template)
+ ->setAction(DifferentialAction::ACTION_ADDCCS)
+ ->setMetadata($cc_meta);
+ }
+ }
+
+ // If this edit has comments or inline comments, save a transaction for
+ // the comment content.
+ if (strlen($this->message) || $inline_comments) {
+ $comments[] = id(clone $template)
+ ->setAction(DifferentialAction::ACTION_COMMENT)
+ ->setContent((string)$this->message);
+ }
+
+ foreach ($comments as $comment) {
+ $comment->save();
+ }
+
+ $last_comment = last($comments);
- $event = new PhabricatorEvent(
- PhabricatorEventType::TYPE_DIFFERENTIAL_DIDEDITREVISION,
- array(
- 'revision' => $revision,
- 'new' => $is_new,
- ));
- $event->setUser($actor);
- PhutilEventEngine::dispatchEvent($event);
+ $changesets = array();
+ if ($inline_comments) {
+ $load_ids = mpull($inline_comments, 'getChangesetID');
+ if ($load_ids) {
+ $load_ids = array_unique($load_ids);
+ $changesets = id(new DifferentialChangeset())->loadAllWhere(
+ 'id in (%Ld)',
+ $load_ids);
+ }
+ foreach ($inline_comments as $inline) {
+ // For now, attach inlines to the last comment. We'll eventually give
+ // them their own transactions, but this would be fairly gross during
+ // the storage transition and we'll have to do special thing with these
+ // during migration anyway.
+ $inline->setCommentID($last_comment->getID());
+ $inline->save();
}
}
@@ -647,7 +672,7 @@
$mail = id(new DifferentialCommentMail(
$revision,
$actor_handle,
- array($comment),
+ $comments,
$changesets,
$inline_comments))
->setActor($actor)
@@ -665,18 +690,19 @@
$mailed_phids = $mail->getRawMail()->buildRecipientList();
}
+
$event_data = array(
'revision_id' => $revision->getID(),
'revision_phid' => $revision->getPHID(),
'revision_name' => $revision->getTitle(),
'revision_author_phid' => $revision->getAuthorPHID(),
- 'action' => $comment->getAction(),
- 'feedback_content' => $comment->getContent(),
+ 'action' => head($comments)->getAction(),
+ 'feedback_content' => $this->message,
'actor_phid' => $actor_phid,
// NOTE: Don't use this, it will be removed after ApplicationTransactions.
// For now, it powers inline comment rendering over the Asana brdige.
- 'temporaryCommentID' => $comment->getID(),
+ 'temporaryCommentID' => $last_comment->getID(),
);
id(new PhabricatorFeedStoryPublisher())
@@ -701,8 +727,6 @@
id(new PhabricatorSearchIndexer())
->queueDocumentForIndexing($revision->getPHID());
-
- return $comment;
}
private function filterAddedCCs(array $ccs) {
Index: src/applications/differential/mail/DifferentialReplyHandler.php
===================================================================
--- src/applications/differential/mail/DifferentialReplyHandler.php
+++ src/applications/differential/mail/DifferentialReplyHandler.php
@@ -142,10 +142,7 @@
$editor->setParentMessageID($this->receivedMail->getMessageID());
}
$editor->setMessage($body);
- $comment = $editor->save();
-
- return $comment->getID();
-
+ $editor->save();
} catch (Exception $ex) {
if ($this->receivedMail) {
$error_body = $this->receivedMail->getRawTextBody();
Index: src/applications/differential/storage/DifferentialComment.php
===================================================================
--- src/applications/differential/storage/DifferentialComment.php
+++ src/applications/differential/storage/DifferentialComment.php
@@ -127,7 +127,7 @@
$this->openTransaction();
$result = parent::save();
- if (strlen($this->getContent())) {
+ if ($this->getContent() !== null) {
$content_source = PhabricatorContentSource::newForSource(
PhabricatorContentSource::SOURCE_LEGACY,
array());

File Metadata

Mime Type
text/plain
Expires
Sun, Mar 16, 3:21 PM (2 w, 2 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7706647
Default Alt Text
D8201.id18555.diff (8 KB)

Event Timeline