diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php index 3884f5c3ae..6c9f75f845 100644 --- a/src/applications/differential/editor/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/DifferentialCommentEditor.php @@ -1,771 +1,771 @@ revision = $revision; $this->action = $action; } public function setParentMessageID($parent_message_id) { $this->parentMessageID = $parent_message_id; return $this; } public function setMessage($message) { $this->message = $message; return $this; } public function setAttachInlineComments($attach) { $this->attachInlineComments = $attach; return $this; } public function setChangedByCommit($changed_by_commit) { $this->changedByCommit = $changed_by_commit; return $this; } public function getChangedByCommit() { return $this->changedByCommit; } public function setAddedReviewers(array $added_reviewers) { $this->addedReviewers = $added_reviewers; return $this; } public function getAddedReviewers() { return $this->addedReviewers; } public function setRemovedReviewers(array $removeded_reviewers) { $this->removedReviewers = $removeded_reviewers; return $this; } public function getRemovedReviewers() { return $this->removedReviewers; } public function setAddedCCs($added_ccs) { $this->addedCCs = $added_ccs; return $this; } public function getAddedCCs() { return $this->addedCCs; } public function setContentSource(PhabricatorContentSource $content_source) { $this->contentSource = $content_source; return $this; } public function setIsDaemonWorkflow($is_daemon) { $this->isDaemonWorkflow = $is_daemon; return $this; } public function setNoEmail($no_email) { $this->noEmail = $no_email; return $this; } public function save() { $actor = $this->requireActor(); // Reload the revision to pick up reviewer status, until we can lift this // out of here. $this->revision = id(new DifferentialRevisionQuery()) ->setViewer($actor) ->withIDs(array($this->revision->getID())) ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); $revision = $this->revision; $action = $this->action; $actor_phid = $actor->getPHID(); $actor_is_author = ($actor_phid == $revision->getAuthorPHID()); $allow_self_accept = PhabricatorEnv::getEnvConfig( 'differential.allow-self-accept'); $always_allow_close = PhabricatorEnv::getEnvConfig( 'differential.always-allow-close'); $allow_reopen = PhabricatorEnv::getEnvConfig( 'differential.allow-reopen'); $revision_status = $revision->getStatus(); $update_accepted_status = false; $reviewer_phids = $revision->getReviewers(); if ($reviewer_phids) { $reviewer_phids = array_fuse($reviewer_phids); } $metadata = array(); $inline_comments = array(); if ($this->attachInlineComments) { $inline_comments = id(new DifferentialInlineCommentQuery()) ->withDraftComments($actor_phid, $revision->getID()) ->execute(); } switch ($action) { case DifferentialAction::ACTION_COMMENT: if (!$this->message && !$inline_comments) { throw new DifferentialActionHasNoEffectException( "You are submitting an empty comment with no action: ". "you must act on the revision or post a comment."); } // If the actor is a reviewer, and their status is "added" (that is, // they haven't accepted or requested changes to the revision), // upgrade their status to "commented". If they have a stronger status // already, don't overwrite it. if (isset($reviewer_phids[$actor_phid])) { $status_added = DifferentialReviewerStatus::STATUS_ADDED; $reviewer_status = $revision->getReviewerStatus(); foreach ($reviewer_status as $reviewer) { if ($reviewer->getReviewerPHID() == $actor_phid) { if ($reviewer->getStatus() == $status_added) { DifferentialRevisionEditor::updateReviewerStatus( $revision, $actor, $actor_phid, DifferentialReviewerStatus::STATUS_COMMENTED); } } } } break; case DifferentialAction::ACTION_RESIGN: if ($actor_is_author) { throw new Exception('You can not resign from your own revision!'); } if (empty($reviewer_phids[$actor_phid])) { throw new DifferentialActionHasNoEffectException( "You can not resign from this revision because you are not ". "a reviewer."); } list($added_reviewers, $ignored) = $this->alterReviewers(); if ($added_reviewers) { $key = DifferentialComment::METADATA_ADDED_REVIEWERS; $metadata[$key] = $added_reviewers; } DifferentialRevisionEditor::updateReviewers( $revision, $actor, array(), array($actor_phid)); // If you are a blocking reviewer, your presence as a reviewer may be // the only thing keeping a revision from transitioning to "accepted". // Recalculate state after removing the resigning reviewer. switch ($revision_status) { case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $update_accepted_status = true; break; } break; case DifferentialAction::ACTION_ABANDON: if (!$actor_is_author) { throw new Exception('You can only abandon your own revisions.'); } if ($revision_status == ArcanistDifferentialRevisionStatus::CLOSED) { throw new DifferentialActionHasNoEffectException( "You can not abandon this revision because it has already ". "been closed."); } if ($revision_status == ArcanistDifferentialRevisionStatus::ABANDONED) { throw new DifferentialActionHasNoEffectException( "You can not abandon this revision because it has already ". "been abandoned."); } $revision->setStatus(ArcanistDifferentialRevisionStatus::ABANDONED); break; case DifferentialAction::ACTION_ACCEPT: if ($actor_is_author && !$allow_self_accept) { throw new Exception('You can not accept your own revision.'); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ABANDONED: throw new DifferentialActionHasNoEffectException( "You can not accept this revision because it has been ". "abandoned."); case ArcanistDifferentialRevisionStatus::CLOSED: throw new DifferentialActionHasNoEffectException( "You can not accept this revision because it has already ". "been closed."); case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::ACCEPTED: // We expect "Accept" from these states. break; default: throw new Exception( "Unexpected revision state '{$revision_status}'!"); } $was_reviewer_already = false; foreach ($revision->getReviewerStatus() as $reviewer) { if ($reviewer->hasAuthority($actor)) { DifferentialRevisionEditor::updateReviewerStatus( $revision, $actor, $reviewer->getReviewerPHID(), DifferentialReviewerStatus::STATUS_ACCEPTED); if ($reviewer->getReviewerPHID() == $actor_phid) { $was_reviewer_already = true; } } } if (!$was_reviewer_already) { DifferentialRevisionEditor::updateReviewerStatus( $revision, $actor, $actor_phid, DifferentialReviewerStatus::STATUS_ACCEPTED); } $update_accepted_status = true; break; case DifferentialAction::ACTION_REQUEST: if (!$actor_is_author) { throw new Exception('You must own a revision to request review.'); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: $revision->setStatus( ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); break; case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: throw new DifferentialActionHasNoEffectException( "You can not request review of this revision because it has ". "been abandoned."); case ArcanistDifferentialRevisionStatus::ABANDONED: throw new DifferentialActionHasNoEffectException( "You can not request review of this revision because it has ". "been abandoned."); case ArcanistDifferentialRevisionStatus::CLOSED: throw new DifferentialActionHasNoEffectException( "You can not request review of this revision because it has ". "already been closed."); default: throw new Exception( "Unexpected revision state '{$revision_status}'!"); } list($added_reviewers, $ignored) = $this->alterReviewers(); if ($added_reviewers) { $key = DifferentialComment::METADATA_ADDED_REVIEWERS; $metadata[$key] = $added_reviewers; } break; case DifferentialAction::ACTION_REJECT: if ($actor_is_author) { throw new Exception( 'You can not request changes to your own revision.'); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: // We expect rejects from these states. break; case ArcanistDifferentialRevisionStatus::ABANDONED: throw new DifferentialActionHasNoEffectException( "You can not request changes to this revision because it has ". "been abandoned."); case ArcanistDifferentialRevisionStatus::CLOSED: throw new DifferentialActionHasNoEffectException( "You can not request changes to this revision because it has ". "already been closed."); default: throw new Exception( "Unexpected revision state '{$revision_status}'!"); } DifferentialRevisionEditor::updateReviewerStatus( $revision, $actor, $actor_phid, DifferentialReviewerStatus::STATUS_REJECTED); $revision ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVISION); break; case DifferentialAction::ACTION_RETHINK: if (!$actor_is_author) { throw new Exception( "You can not plan changes to somebody else's revision"); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: // We expect accepts from these states. break; case ArcanistDifferentialRevisionStatus::ABANDONED: throw new DifferentialActionHasNoEffectException( "You can not plan changes to this revision because it has ". "been abandoned."); case ArcanistDifferentialRevisionStatus::CLOSED: throw new DifferentialActionHasNoEffectException( "You can not plan changes to this revision because it has ". "already been closed."); default: throw new Exception( "Unexpected revision state '{$revision_status}'!"); } $revision ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVISION); break; case DifferentialAction::ACTION_RECLAIM: if (!$actor_is_author) { throw new Exception('You can not reclaim a revision you do not own.'); } if ($revision_status != ArcanistDifferentialRevisionStatus::ABANDONED) { throw new DifferentialActionHasNoEffectException( "You can not reclaim this revision because it is not abandoned."); } $revision ->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); $update_accepted_status = true; break; case DifferentialAction::ACTION_CLOSE: // NOTE: The daemons can mark things closed from any state. We treat // them as completely authoritative. if (!$this->isDaemonWorkflow) { if (!$actor_is_author && !$always_allow_close) { throw new Exception( "You can not mark a revision you don't own as closed."); } $status_closed = ArcanistDifferentialRevisionStatus::CLOSED; $status_accepted = ArcanistDifferentialRevisionStatus::ACCEPTED; if ($revision_status == $status_closed) { throw new DifferentialActionHasNoEffectException( "You can not mark this revision as closed because it has ". "already been marked as closed."); } if ($revision_status != $status_accepted) { throw new DifferentialActionHasNoEffectException( "You can not mark this revision as closed because it is ". "has not been accepted."); } } if (!$revision->getDateCommitted()) { $revision->setDateCommitted(time()); } $revision->setStatus(ArcanistDifferentialRevisionStatus::CLOSED); break; case DifferentialAction::ACTION_REOPEN: if (!$allow_reopen) { throw new Exception( "You cannot reopen a revision when this action is disabled."); } if ($revision_status != ArcanistDifferentialRevisionStatus::CLOSED) { throw new Exception( "You cannot reopen a revision that is not currently closed."); } $revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); break; case DifferentialAction::ACTION_ADDREVIEWERS: list($added_reviewers, $ignored) = $this->alterReviewers(); if ($added_reviewers) { $key = DifferentialComment::METADATA_ADDED_REVIEWERS; $metadata[$key] = $added_reviewers; } else { $user_tried_to_add = count($this->getAddedReviewers()); if ($user_tried_to_add == 0) { throw new DifferentialActionHasNoEffectException( "You can not add reviewers, because you did not specify any ". "reviewers."); } else if ($user_tried_to_add == 1) { throw new DifferentialActionHasNoEffectException( "You can not add that reviewer, because they are already an ". "author or reviewer."); } else { throw new DifferentialActionHasNoEffectException( "You can not add those reviewers, because they are all already ". "authors or reviewers."); } } break; case DifferentialAction::ACTION_ADDCCS: $added_ccs = $this->getAddedCCs(); $user_tried_to_add = count($added_ccs); $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( "You can not add CCs, because you did not specify any ". "CCs."); } else if ($user_tried_to_add == 1) { throw new DifferentialActionHasNoEffectException( "You can not add that CC, because they are already an ". "author, reviewer or CC."); } else { throw new DifferentialActionHasNoEffectException( "You can not add those CCs, because they are all already ". "authors, reviewers or CCs."); } } break; case DifferentialAction::ACTION_CLAIM: if ($actor_is_author) { throw new Exception("You can not commandeer your own revision."); } switch ($revision_status) { case ArcanistDifferentialRevisionStatus::CLOSED: throw new DifferentialActionHasNoEffectException( "You can not commandeer this revision because it has ". "already been closed."); break; } $this->setAddedReviewers(array($revision->getAuthorPHID())); $this->setRemovedReviewers(array($actor_phid)); // NOTE: Set the new author PHID before calling addReviewers(), since it // doesn't permit the author to become a reviewer. $revision->setAuthorPHID($actor_phid); list($added_reviewers, $removed_reviewers) = $this->alterReviewers(); if ($added_reviewers) { $key = DifferentialComment::METADATA_ADDED_REVIEWERS; $metadata[$key] = $added_reviewers; } if ($removed_reviewers) { $key = DifferentialComment::METADATA_REMOVED_REVIEWERS; $metadata[$key] = $removed_reviewers; } break; default: throw new Exception('Unsupported action.'); } // Update information about reviewer in charge. if ($action == DifferentialAction::ACTION_ACCEPT || $action == DifferentialAction::ACTION_REJECT) { $revision->setLastReviewerPHID($actor_phid); } // TODO: Call beginReadLocking() prior to loading the revision. $revision->openTransaction(); // Always save the revision (even if we didn't actually change any of its // properties) so that it jumps to the top of the revision list when sorted // by "updated". Notably, this allows "ping" comments to push it to the // top of the action list. $revision->save(); if ($update_accepted_status) { $revision = DifferentialRevisionEditor::updateAcceptedStatus( $actor, $revision); } if ($action != DifferentialAction::ACTION_RESIGN) { DifferentialRevisionEditor::addCC( $revision, $actor_phid, $actor_phid); } $is_new = !$revision->getID(); $event = new PhabricatorEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_WILLEDITREVISION, array( 'revision' => $revision, 'new' => $is_new, )); $event->setUser($actor); PhutilEventEngine::dispatchEvent($event); $comment = id(new DifferentialComment()) ->setAuthorPHID($actor_phid) ->setRevision($revision) ->setAction($action) ->setContent((string)$this->message) ->setMetadata($metadata); if ($this->contentSource) { $comment->setContentSource($this->contentSource); } $comment->save(); $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(); } } // Find any "@mentions" in the comment blocks. $content_blocks = array($comment->getContent()); 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) { DifferentialRevisionEditor::addCC( $revision, $cc_phid, $actor_phid); $metacc[] = $cc_phid; } $metadata[DifferentialComment::METADATA_ADDED_CCS] = $metacc; $comment->setMetadata($metadata); $comment->save(); $event = new PhabricatorEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_DIDEDITREVISION, array( 'revision' => $revision, 'new' => $is_new, )); $event->setUser($actor); PhutilEventEngine::dispatchEvent($event); } } $revision->saveTransaction(); $phids = array($actor_phid); $handles = id(new PhabricatorHandleQuery()) ->setViewer($actor) ->withPHIDs($phids) ->execute(); $actor_handle = $handles[$actor_phid]; $xherald_header = HeraldTranscript::loadXHeraldRulesHeader( $revision->getPHID()); $mailed_phids = array(); if (!$this->noEmail) { $mail = id(new DifferentialCommentMail( $revision, $actor_handle, - $comment, + array($comment), $changesets, $inline_comments)) ->setActor($actor) ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->setToPHIDs( array_merge( $revision->getReviewers(), array($revision->getAuthorPHID()))) ->setCCPHIDs($revision->getCCPHIDs()) ->setChangedByCommit($this->getChangedByCommit()) ->setXHeraldRulesHeader($xherald_header) ->setParentMessageID($this->parentMessageID) ->send(); $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(), '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(), ); id(new PhabricatorFeedStoryPublisher()) ->setStoryType('PhabricatorFeedStoryDifferential') ->setStoryData($event_data) ->setStoryTime(time()) ->setStoryAuthorPHID($actor_phid) ->setRelatedPHIDs( array( $revision->getPHID(), $actor_phid, $revision->getAuthorPHID(), )) ->setPrimaryObjectPHID($revision->getPHID()) ->setSubscribedPHIDs( array_merge( array($revision->getAuthorPHID()), $revision->getReviewers(), $revision->getCCPHIDs())) ->setMailRecipientPHIDs($mailed_phids) ->publish(); id(new PhabricatorSearchIndexer()) ->queueDocumentForIndexing($revision->getPHID()); return $comment; } private function filterAddedCCs(array $ccs) { $revision = $this->revision; $current_ccs = $revision->getCCPHIDs(); $current_ccs = array_fill_keys($current_ccs, true); $reviewer_phids = $revision->getReviewers(); $reviewer_phids = array_fill_keys($reviewer_phids, true); foreach ($ccs as $key => $cc) { if (isset($current_ccs[$cc])) { unset($ccs[$key]); } if (isset($reviewer_phids[$cc])) { unset($ccs[$key]); } if ($cc == $revision->getAuthorPHID()) { unset($ccs[$key]); } } return $ccs; } private function alterReviewers() { $actor_phid = $this->getActor()->getPHID(); $revision = $this->revision; $added_reviewers = $this->getAddedReviewers(); $removed_reviewers = $this->getRemovedReviewers(); $reviewer_phids = $revision->getReviewers(); $allow_self_accept = PhabricatorEnv::getEnvConfig( 'differential.allow-self-accept'); $reviewer_phids_map = array_fill_keys($reviewer_phids, true); foreach ($added_reviewers as $k => $user_phid) { if (!$allow_self_accept && $user_phid == $revision->getAuthorPHID()) { unset($added_reviewers[$k]); } if (isset($reviewer_phids_map[$user_phid])) { unset($added_reviewers[$k]); } } foreach ($removed_reviewers as $k => $user_phid) { if (!isset($reviewer_phids_map[$user_phid])) { unset($removed_reviewers[$k]); } } $added_reviewers = array_unique($added_reviewers); $removed_reviewers = array_unique($removed_reviewers); if ($added_reviewers) { DifferentialRevisionEditor::updateReviewers( $revision, $this->getActor(), $added_reviewers, $removed_reviewers); } return array($added_reviewers, $removed_reviewers); } } diff --git a/src/applications/differential/editor/DifferentialRevisionEditor.php b/src/applications/differential/editor/DifferentialRevisionEditor.php index 97a533b7c3..6e3b99ac23 100644 --- a/src/applications/differential/editor/DifferentialRevisionEditor.php +++ b/src/applications/differential/editor/DifferentialRevisionEditor.php @@ -1,1114 +1,1109 @@ aphrontRequestForEventDispatch = $request; return $this; } public function getAphrontRequestForEventDispatch() { return $this->aphrontRequestForEventDispatch; } public function __construct(DifferentialRevision $revision) { $this->revision = $revision; $this->isCreate = !($revision->getID()); } public static function newRevisionFromConduitWithDiff( array $fields, DifferentialDiff $diff, PhabricatorUser $actor) { $revision = DifferentialRevision::initializeNewRevision($actor); $revision->setPHID($revision->generatePHID()); $editor = new DifferentialRevisionEditor($revision); $editor->setActor($actor); $editor->addDiff($diff, null); $editor->copyFieldsFromConduit($fields); $editor->save(); return $revision; } public function copyFieldsFromConduit(array $fields) { $actor = $this->getActor(); $revision = $this->revision; $revision->loadRelationships(); $all_fields = DifferentialFieldSelector::newSelector() ->getFieldSpecifications(); $aux_fields = array(); foreach ($all_fields as $aux_field) { $aux_field->setRevision($revision); $aux_field->setDiff($this->diff); $aux_field->setUser($actor); if ($aux_field->shouldAppearOnCommitMessage()) { $aux_fields[$aux_field->getCommitMessageKey()] = $aux_field; } } foreach ($fields as $field => $value) { if (empty($aux_fields[$field])) { throw new Exception( "Parsed commit message contains unrecognized field '{$field}'."); } $aux_fields[$field]->setValueFromParsedCommitMessage($value); } foreach ($aux_fields as $aux_field) { $aux_field->validateField(); } $this->setAuxiliaryFields($all_fields); } public function setAuxiliaryFields(array $auxiliary_fields) { assert_instances_of($auxiliary_fields, 'DifferentialFieldSpecification'); $this->auxiliaryFields = $auxiliary_fields; return $this; } public function getRevision() { return $this->revision; } public function setReviewers(array $reviewers) { $this->reviewers = $reviewers; return $this; } public function setCCPHIDs(array $cc) { $this->cc = $cc; return $this; } public function setContentSource(PhabricatorContentSource $content_source) { $this->contentSource = $content_source; return $this; } public function addDiff(DifferentialDiff $diff, $comments) { if ($diff->getRevisionID() && $diff->getRevisionID() != $this->getRevision()->getID()) { $diff_id = (int)$diff->getID(); $targ_id = (int)$this->getRevision()->getID(); $real_id = (int)$diff->getRevisionID(); throw new Exception( "Can not attach diff #{$diff_id} to Revision D{$targ_id}, it is ". "already attached to D{$real_id}."); } $this->diff = $diff; $this->comments = $comments; $repository = id(new DifferentialRepositoryLookup()) ->setViewer($this->getActor()) ->setDiff($diff) ->lookupRepository(); if ($repository) { $this->getRevision()->setRepositoryPHID($repository->getPHID()); } return $this; } protected function getDiff() { return $this->diff; } protected function getComments() { return $this->comments; } protected function getActorPHID() { return $this->getActor()->getPHID(); } public function isNewRevision() { return !$this->getRevision()->getID(); } /** * A silent update does not trigger Herald rules or send emails. This is used * for auto-amends at commit time. */ public function setSilentUpdate($silent) { $this->silentUpdate = $silent; return $this; } public function save() { $revision = $this->getRevision(); $is_new = $this->isNewRevision(); $revision->loadRelationships(); $this->willWriteRevision(); if ($this->reviewers === null) { $this->reviewers = $revision->getReviewers(); } if ($this->cc === null) { $this->cc = $revision->getCCPHIDs(); } if ($is_new) { $content_blocks = array(); foreach ($this->auxiliaryFields as $field) { if ($field->shouldExtractMentions()) { $content_blocks[] = $field->renderValueForCommitMessage(false); } } $phids = PhabricatorMarkupEngine::extractPHIDsFromMentions( $content_blocks); $this->cc = array_unique(array_merge($this->cc, $phids)); } $diff = $this->getDiff(); if ($diff) { $revision->setLineCount($diff->getLineCount()); } // Save the revision, to generate its ID and PHID if it is new. We need // the ID/PHID in order to record them in Herald transcripts, but don't // want to hold a transaction open while running Herald because it is // potentially somewhat slow. The downside is that we may end up with a // saved revision/diff pair without appropriate CCs. We could be better // about this -- for example: // // - Herald can't affect reviewers, so we could compute them before // opening the transaction and then save them in the transaction. // - Herald doesn't *really* need PHIDs to compute its effects, we could // run it before saving these objects and then hand over the PHIDs later. // // But this should address the problem of orphaned revisions, which is // currently the only problem we experience in practice. $revision->openTransaction(); if ($diff) { $revision->setBranchName($diff->getBranch()); $revision->setArcanistProjectPHID($diff->getArcanistProjectPHID()); } $revision->save(); if ($diff) { $diff->setRevisionID($revision->getID()); $diff->save(); } $revision->saveTransaction(); // We're going to build up three dictionaries: $add, $rem, and $stable. The // $add dictionary has added reviewers/CCs. The $rem dictionary has // reviewers/CCs who have been removed, and the $stable array is // reviewers/CCs who haven't changed. We're going to send new reviewers/CCs // a different ("welcome") email than we send stable reviewers/CCs. $old = array( 'rev' => array_fill_keys($revision->getReviewers(), true), 'ccs' => array_fill_keys($revision->getCCPHIDs(), true), ); $xscript_header = null; $xscript_uri = null; $new = array( 'rev' => array_fill_keys($this->reviewers, true), 'ccs' => array_fill_keys($this->cc, true), ); $rem_ccs = array(); $xscript_phid = null; if ($diff) { $adapter = HeraldDifferentialRevisionAdapter::newLegacyAdapter( $revision, $diff); $adapter->setExplicitCCs($new['ccs']); $adapter->setExplicitReviewers($new['rev']); $adapter->setForbiddenCCs($revision->loadUnsubscribedPHIDs()); $adapter->setIsNewObject($is_new); $xscript = HeraldEngine::loadAndApplyRules($adapter); $xscript_uri = '/herald/transcript/'.$xscript->getID().'/'; $xscript_phid = $xscript->getPHID(); $xscript_header = $xscript->getXHeraldRulesHeader(); $xscript_header = HeraldTranscript::saveXHeraldRulesHeader( $revision->getPHID(), $xscript_header); $sub = array( 'rev' => $adapter->getReviewersAddedByHerald(), 'ccs' => $adapter->getCCsAddedByHerald(), ); $rem_ccs = $adapter->getCCsRemovedByHerald(); $blocking_reviewers = array_keys( $adapter->getBlockingReviewersAddedByHerald()); HarbormasterBuildable::applyBuildPlans( $diff->getPHID(), $revision->getPHID(), $adapter->getBuildPlans()); } else { $sub = array( 'rev' => array(), 'ccs' => array(), ); $blocking_reviewers = array(); } // Remove any CCs which are prevented by Herald rules. $sub['ccs'] = array_diff_key($sub['ccs'], $rem_ccs); $new['ccs'] = array_diff_key($new['ccs'], $rem_ccs); $add = array(); $rem = array(); $stable = array(); foreach (array('rev', 'ccs') as $key) { $add[$key] = array(); if ($new[$key] !== null) { $add[$key] += array_diff_key($new[$key], $old[$key]); } $add[$key] += array_diff_key($sub[$key], $old[$key]); $combined = $sub[$key]; if ($new[$key] !== null) { $combined += $new[$key]; } $rem[$key] = array_diff_key($old[$key], $combined); $stable[$key] = array_diff_key($old[$key], $add[$key] + $rem[$key]); } // Prevent Herald rules from adding a revision's owner as a reviewer. unset($add['rev'][$revision->getAuthorPHID()]); self::updateReviewers( $revision, $this->getActor(), array_keys($add['rev']), array_keys($rem['rev']), $blocking_reviewers); // We want to attribute new CCs to a "reasonPHID", representing the reason // they were added. This is either a user (if some user explicitly CCs // them, or uses "Add CCs...") or a Herald transcript PHID, indicating that // they were added by a Herald rule. if ($add['ccs'] || $rem['ccs']) { $reasons = array(); foreach ($add['ccs'] as $phid => $ignored) { if (empty($new['ccs'][$phid])) { $reasons[$phid] = $xscript_phid; } else { $reasons[$phid] = $this->getActorPHID(); } } foreach ($rem['ccs'] as $phid => $ignored) { if (empty($new['ccs'][$phid])) { $reasons[$phid] = $this->getActorPHID(); } else { $reasons[$phid] = $xscript_phid; } } } else { $reasons = $this->getActorPHID(); } self::alterCCs( $revision, $this->cc, array_keys($rem['ccs']), array_keys($add['ccs']), $reasons); $this->updateAuxiliaryFields(); // Add the author and users included from Herald rules to the relevant set // of users so they get a copy of the email. if (!$this->silentUpdate) { if ($is_new) { $add['rev'][$this->getActorPHID()] = true; if ($diff) { $add['rev'] += $adapter->getEmailPHIDsAddedByHerald(); } } else { $stable['rev'][$this->getActorPHID()] = true; if ($diff) { $stable['rev'] += $adapter->getEmailPHIDsAddedByHerald(); } } } $mail = array(); $phids = array($this->getActorPHID()); $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->getActor()) ->withPHIDs($phids) ->execute(); $actor_handle = $handles[$this->getActorPHID()]; $changesets = null; - $comment = null; $old_status = $revision->getStatus(); if ($diff) { $changesets = $diff->loadChangesets(); // TODO: This should probably be in DifferentialFeedbackEditor? if (!$is_new) { - $comment = $this->createComment(); - } - if ($comment) { + $this->createComment(); $mail[] = id(new DifferentialNewDiffMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) - ->setIsFirstMailAboutRevision($is_new) - ->setIsFirstMailToRecipients($is_new) - ->setComments($this->getComments()) + ->setIsFirstMailAboutRevision(false) + ->setIsFirstMailToRecipients(false) + ->setCommentText($this->getComments()) ->setToPHIDs(array_keys($stable['rev'])) ->setCCPHIDs(array_keys($stable['ccs'])); } // Save the changes we made above. $diff->setDescription(preg_replace('/\n.*/s', '', $this->getComments())); $diff->save(); $this->updateAffectedPathTable($revision, $diff, $changesets); $this->updateRevisionHashTable($revision, $diff); // An updated diff should require review, as long as it's not closed // or accepted. The "accepted" status is "sticky" to encourage courtesy // re-diffs after someone accepts with minor changes/suggestions. $status = $revision->getStatus(); if ($status != ArcanistDifferentialRevisionStatus::CLOSED && $status != ArcanistDifferentialRevisionStatus::ACCEPTED) { $revision->setStatus(ArcanistDifferentialRevisionStatus::NEEDS_REVIEW); } } else { $diff = $revision->loadActiveDiff(); if ($diff) { $changesets = $diff->loadChangesets(); } else { $changesets = array(); } } $revision->save(); // If the actor just deleted all the blocking/rejected reviewers, we may // be able to put the revision into "accepted". switch ($revision->getStatus()) { case ArcanistDifferentialRevisionStatus::NEEDS_REVISION: case ArcanistDifferentialRevisionStatus::NEEDS_REVIEW: $revision = self::updateAcceptedStatus( $this->getActor(), $revision); break; } $this->didWriteRevision(); $event_data = array( 'revision_id' => $revision->getID(), 'revision_phid' => $revision->getPHID(), 'revision_name' => $revision->getTitle(), 'revision_author_phid' => $revision->getAuthorPHID(), 'action' => $is_new ? DifferentialAction::ACTION_CREATE : DifferentialAction::ACTION_UPDATE, 'feedback_content' => $is_new ? phutil_utf8_shorten($revision->getSummary(), 140) : $this->getComments(), 'actor_phid' => $revision->getAuthorPHID(), ); $mailed_phids = array(); if (!$this->silentUpdate) { $revision->loadRelationships(); if ($add['rev']) { $message = id(new DifferentialNewDiffMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) ->setIsFirstMailAboutRevision($is_new) ->setIsFirstMailToRecipients(true) ->setToPHIDs(array_keys($add['rev'])); if ($is_new) { // The first time we send an email about a revision, put the CCs in // the "CC:" field of the same "Review Requested" email that reviewers // get, so you don't get two initial emails if you're on a list that // is CC'd. $message->setCCPHIDs(array_keys($add['ccs'])); } $mail[] = $message; } // If we added CCs, we want to send them an email, but only if they were // not already a reviewer and were not added as one (in these cases, they // got a "NewDiff" mail, either in the past or just a moment ago). You can // still get two emails, but only if a revision is updated and you are // added as a reviewer at the same time a list you are on is added as a // CC, which is rare and reasonable. $implied_ccs = self::getImpliedCCs($revision); $implied_ccs = array_fill_keys($implied_ccs, true); $add['ccs'] = array_diff_key($add['ccs'], $implied_ccs); if (!$is_new && $add['ccs']) { $mail[] = id(new DifferentialCCWelcomeMail( $revision, $actor_handle, $changesets)) ->setActor($this->getActor()) ->setIsFirstMailToRecipients(true) ->setToPHIDs(array_keys($add['ccs'])); } foreach ($mail as $message) { $message->setHeraldTranscriptURI($xscript_uri); $message->setXHeraldRulesHeader($xscript_header); $message->send(); $mailed_phids[] = $message->getRawMail()->buildRecipientList(); } $mailed_phids = array_mergev($mailed_phids); } id(new PhabricatorFeedStoryPublisher()) ->setStoryType('PhabricatorFeedStoryDifferential') ->setStoryData($event_data) ->setStoryTime(time()) ->setStoryAuthorPHID($revision->getAuthorPHID()) ->setRelatedPHIDs( array( $revision->getPHID(), $revision->getAuthorPHID(), )) ->setPrimaryObjectPHID($revision->getPHID()) ->setSubscribedPHIDs( array_merge( array($revision->getAuthorPHID()), $revision->getReviewers(), $revision->getCCPHIDs())) ->setMailRecipientPHIDs($mailed_phids) ->publish(); id(new PhabricatorSearchIndexer()) ->queueDocumentForIndexing($revision->getPHID()); } public static function addCCAndUpdateRevision( $revision, $phid, PhabricatorUser $actor) { self::addCC($revision, $phid, $actor->getPHID()); $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER; id(new PhabricatorEdgeEditor()) ->setActor($actor) ->removeEdge($revision->getPHID(), $type, $phid) ->save(); } public static function removeCCAndUpdateRevision( $revision, $phid, PhabricatorUser $actor) { self::removeCC($revision, $phid, $actor->getPHID()); $type = PhabricatorEdgeConfig::TYPE_OBJECT_HAS_UNSUBSCRIBER; id(new PhabricatorEdgeEditor()) ->setActor($actor) ->addEdge($revision->getPHID(), $type, $phid) ->save(); } public static function addCC( DifferentialRevision $revision, $phid, $reason) { return self::alterCCs( $revision, $revision->getCCPHIDs(), $rem = array(), $add = array($phid), $reason); } public static function removeCC( DifferentialRevision $revision, $phid, $reason) { return self::alterCCs( $revision, $revision->getCCPHIDs(), $rem = array($phid), $add = array(), $reason); } protected static function alterCCs( DifferentialRevision $revision, array $stable_phids, array $rem_phids, array $add_phids, $reason_phid) { $dont_add = self::getImpliedCCs($revision); $add_phids = array_diff($add_phids, $dont_add); return self::alterRelationships( $revision, $stable_phids, $rem_phids, $add_phids, $reason_phid, DifferentialRevision::RELATION_SUBSCRIBED); } private static function getImpliedCCs(DifferentialRevision $revision) { return array_merge( $revision->getReviewers(), array($revision->getAuthorPHID())); } public static function updateReviewers( DifferentialRevision $revision, PhabricatorUser $actor, array $add_phids, array $remove_phids, array $blocking_phids = array()) { $reviewers = $revision->getReviewers(); $editor = id(new PhabricatorEdgeEditor()) ->setActor($actor); $reviewer_phids_map = array_fill_keys($reviewers, true); $blocking_phids = array_fuse($blocking_phids); foreach ($add_phids as $phid) { // Adding an already existing edge again would have cause memory loss // That is, the previous state for that reviewer would be lost if (isset($reviewer_phids_map[$phid])) { // TODO: If we're writing a blocking edge, we should overwrite an // existing weaker edge (like "added" or "commented"), just not a // stronger existing edge. continue; } if (isset($blocking_phids[$phid])) { $status = DifferentialReviewerStatus::STATUS_BLOCKING; } else { $status = DifferentialReviewerStatus::STATUS_ADDED; } $options = array( 'data' => array( 'status' => $status, ) ); $editor->addEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $phid, $options); } foreach ($remove_phids as $phid) { $editor->removeEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $phid); } $editor->save(); } public static function updateReviewerStatus( DifferentialRevision $revision, PhabricatorUser $actor, $reviewer_phid, $status) { $options = array( 'data' => array( 'status' => $status ) ); $active_diff = $revision->loadActiveDiff(); if ($active_diff) { $options['data']['diff'] = $active_diff->getID(); } id(new PhabricatorEdgeEditor()) ->setActor($actor) ->addEdge( $revision->getPHID(), PhabricatorEdgeConfig::TYPE_DREV_HAS_REVIEWER, $reviewer_phid, $options) ->save(); } private static function alterRelationships( DifferentialRevision $revision, array $stable_phids, array $rem_phids, array $add_phids, $reason_phid, $relation_type) { $rem_map = array_fill_keys($rem_phids, true); $add_map = array_fill_keys($add_phids, true); $seq_map = array_values($stable_phids); $seq_map = array_flip($seq_map); foreach ($rem_map as $phid => $ignored) { if (!isset($seq_map[$phid])) { $seq_map[$phid] = count($seq_map); } } foreach ($add_map as $phid => $ignored) { if (!isset($seq_map[$phid])) { $seq_map[$phid] = count($seq_map); } } $raw = $revision->getRawRelations($relation_type); $raw = ipull($raw, null, 'objectPHID'); $sequence = count($seq_map); foreach ($raw as $phid => $ignored) { if (isset($seq_map[$phid])) { $raw[$phid]['sequence'] = $seq_map[$phid]; } else { $raw[$phid]['sequence'] = $sequence++; } } $raw = isort($raw, 'sequence'); foreach ($raw as $phid => $ignored) { if (isset($rem_map[$phid])) { unset($raw[$phid]); } } foreach ($add_phids as $add) { $reason = is_array($reason_phid) ? idx($reason_phid, $add) : $reason_phid; $raw[$add] = array( 'objectPHID' => $add, 'sequence' => idx($seq_map, $add, $sequence++), 'reasonPHID' => $reason, ); } $conn_w = $revision->establishConnection('w'); $sql = array(); foreach ($raw as $relation) { $sql[] = qsprintf( $conn_w, '(%d, %s, %s, %d, %s)', $revision->getID(), $relation_type, $relation['objectPHID'], $relation['sequence'], $relation['reasonPHID']); } $conn_w->openTransaction(); queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d AND relation = %s', DifferentialRevision::RELATIONSHIP_TABLE, $revision->getID(), $relation_type); if ($sql) { queryfx( $conn_w, 'INSERT INTO %T (revisionID, relation, objectPHID, sequence, reasonPHID) VALUES %Q', DifferentialRevision::RELATIONSHIP_TABLE, implode(', ', $sql)); } $conn_w->saveTransaction(); $revision->loadRelationships(); } private function createComment() { $comment = id(new DifferentialComment()) ->setAuthorPHID($this->getActorPHID()) ->setRevision($this->revision) ->setContent($this->getComments()) ->setAction(DifferentialAction::ACTION_UPDATE) ->setMetadata( array( DifferentialComment::METADATA_DIFF_ID => $this->getDiff()->getID(), )); if ($this->contentSource) { $comment->setContentSource($this->contentSource); } $comment->save(); - - return $comment; } private function updateAuxiliaryFields() { $aux_map = array(); foreach ($this->auxiliaryFields as $aux_field) { $key = $aux_field->getStorageKey(); if ($key !== null) { $val = $aux_field->getValueForStorage(); $aux_map[$key] = $val; } } if (!$aux_map) { return; } $revision = $this->revision; $fields = id(new DifferentialAuxiliaryField())->loadAllWhere( 'revisionPHID = %s AND name IN (%Ls)', $revision->getPHID(), array_keys($aux_map)); $fields = mpull($fields, null, 'getName'); foreach ($aux_map as $key => $val) { $obj = idx($fields, $key); if (!strlen($val)) { // If the new value is empty, just delete the old row if one exists and // don't add a new row if it doesn't. if ($obj) { $obj->delete(); } } else { if (!$obj) { $obj = new DifferentialAuxiliaryField(); $obj->setRevisionPHID($revision->getPHID()); $obj->setName($key); } if ($obj->getValue() !== $val) { $obj->setValue($val); $obj->save(); } } } } private function willWriteRevision() { foreach ($this->auxiliaryFields as $aux_field) { $aux_field->willWriteRevision($this); } $this->dispatchEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_WILLEDITREVISION); } private function didWriteRevision() { foreach ($this->auxiliaryFields as $aux_field) { $aux_field->didWriteRevision($this); } $this->dispatchEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_DIDEDITREVISION); } private function dispatchEvent($type) { $event = new PhabricatorEvent( $type, array( 'revision' => $this->revision, 'new' => $this->isCreate, )); $event->setUser($this->getActor()); $request = $this->getAphrontRequestForEventDispatch(); if ($request) { $event->setAphrontRequest($request); } PhutilEventEngine::dispatchEvent($event); } /** * Update the table which links Differential revisions to paths they affect, * so Diffusion can efficiently find pending revisions for a given file. */ private function updateAffectedPathTable( DifferentialRevision $revision, DifferentialDiff $diff, array $changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); $project = $diff->loadArcanistProject(); if (!$project) { // Probably an old revision from before projects. return; } $repository = $project->loadRepository(); if (!$repository) { // Probably no project <-> repository link, or the repository where the // project lives is untracked. return; } $path_prefix = null; $local_root = $diff->getSourceControlPath(); if ($local_root) { // We're in a working copy which supports subdirectory checkouts (e.g., // SVN) so we need to figure out what prefix we should add to each path // (e.g., trunk/projects/example/) to get the absolute path from the // root of the repository. DVCS systems like Git and Mercurial are not // affected. // Normalize both paths and check if the repository root is a prefix of // the local root. If so, throw it away. Note that this correctly handles // the case where the remote path is "/". $local_root = id(new PhutilURI($local_root))->getPath(); $local_root = rtrim($local_root, '/'); $repo_root = id(new PhutilURI($repository->getRemoteURI()))->getPath(); $repo_root = rtrim($repo_root, '/'); if (!strncmp($repo_root, $local_root, strlen($repo_root))) { $path_prefix = substr($local_root, strlen($repo_root)); } } $paths = array(); foreach ($changesets as $changeset) { $paths[] = $path_prefix.'/'.$changeset->getFilename(); } // Mark this as also touching all parent paths, so you can see all pending // changes to any file within a directory. $all_paths = array(); foreach ($paths as $local) { foreach (DiffusionPathIDQuery::expandPathToRoot($local) as $path) { $all_paths[$path] = true; } } $all_paths = array_keys($all_paths); $path_ids = PhabricatorRepositoryCommitChangeParserWorker::lookupOrCreatePaths( $all_paths); $table = new DifferentialAffectedPath(); $conn_w = $table->establishConnection('w'); $sql = array(); foreach ($path_ids as $path_id) { $sql[] = qsprintf( $conn_w, '(%d, %d, %d, %d)', $repository->getID(), $path_id, time(), $revision->getID()); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', $table->getTableName(), $revision->getID()); foreach (array_chunk($sql, 256) as $chunk) { queryfx( $conn_w, 'INSERT INTO %T (repositoryID, pathID, epoch, revisionID) VALUES %Q', $table->getTableName(), implode(', ', $chunk)); } } /** * Update the table connecting revisions to DVCS local hashes, so we can * identify revisions by commit/tree hashes. */ private function updateRevisionHashTable( DifferentialRevision $revision, DifferentialDiff $diff) { $vcs = $diff->getSourceControlSystem(); if ($vcs == DifferentialRevisionControlSystem::SVN) { // Subversion has no local commit or tree hash information, so we don't // have to do anything. return; } $property = id(new DifferentialDiffProperty())->loadOneWhere( 'diffID = %d AND name = %s', $diff->getID(), 'local:commits'); if (!$property) { return; } $hashes = array(); $data = $property->getData(); switch ($vcs) { case DifferentialRevisionControlSystem::GIT: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_COMMIT, $commit['commit'], ); $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_GIT_TREE, $commit['tree'], ); } break; case DifferentialRevisionControlSystem::MERCURIAL: foreach ($data as $commit) { $hashes[] = array( ArcanistDifferentialRevisionHash::HASH_MERCURIAL_COMMIT, $commit['rev'], ); } break; } $conn_w = $revision->establishConnection('w'); $sql = array(); foreach ($hashes as $info) { list($type, $hash) = $info; $sql[] = qsprintf( $conn_w, '(%d, %s, %s)', $revision->getID(), $type, $hash); } queryfx( $conn_w, 'DELETE FROM %T WHERE revisionID = %d', ArcanistDifferentialRevisionHash::TABLE_NAME, $revision->getID()); if ($sql) { queryfx( $conn_w, 'INSERT INTO %T (revisionID, type, hash) VALUES %Q', ArcanistDifferentialRevisionHash::TABLE_NAME, implode(', ', $sql)); } } /** * Try to move a revision to "accepted". We look for: * * - at least one accepting reviewer who is a user; and * - no rejects; and * - no blocking reviewers. */ public static function updateAcceptedStatus( PhabricatorUser $viewer, DifferentialRevision $revision) { $revision = id(new DifferentialRevisionQuery()) ->setViewer($viewer) ->withIDs(array($revision->getID())) ->needRelationships(true) ->needReviewerStatus(true) ->needReviewerAuthority(true) ->executeOne(); $has_user_accept = false; foreach ($revision->getReviewerStatus() as $reviewer) { $status = $reviewer->getStatus(); if ($status == DifferentialReviewerStatus::STATUS_BLOCKING) { // We have a blocking reviewer, so just leave the revision in its // existing state. return $revision; } if ($status == DifferentialReviewerStatus::STATUS_REJECTED) { // We have a rejecting reviewer, so leave the revisoin as is. return $revision; } if ($reviewer->isUser()) { if ($status == DifferentialReviewerStatus::STATUS_ACCEPTED) { $has_user_accept = true; } } } if ($has_user_accept) { $revision ->setStatus(ArcanistDifferentialRevisionStatus::ACCEPTED) ->save(); } return $revision; } } diff --git a/src/applications/differential/mail/DifferentialCommentMail.php b/src/applications/differential/mail/DifferentialCommentMail.php index ccf4020b63..2c7c3c07f8 100644 --- a/src/applications/differential/mail/DifferentialCommentMail.php +++ b/src/applications/differential/mail/DifferentialCommentMail.php @@ -1,205 +1,221 @@ changedByCommit = $changed_by_commit; return $this; } public function getChangedByCommit() { return $this->changedByCommit; } public function __construct( DifferentialRevision $revision, PhabricatorObjectHandle $actor, - DifferentialComment $comment, + array $comments, array $changesets, array $inline_comments) { + + assert_instances_of($comments, 'DifferentialComment'); assert_instances_of($changesets, 'DifferentialChangeset'); assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); $this->setRevision($revision); $this->setActorHandle($actor); - $this->setComment($comment); + $this->setComments($comments); $this->setChangesets($changesets); $this->setInlineComments($inline_comments); } protected function getMailTags() { - $tags = array(); - $comment = $this->getComment(); - $action = $comment->getAction(); - - switch ($action) { - case DifferentialAction::ACTION_ADDCCS: - $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CC; - break; - case DifferentialAction::ACTION_CLOSE: - $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CLOSED; - break; - case DifferentialAction::ACTION_ADDREVIEWERS: - $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEWERS; - break; - case DifferentialAction::ACTION_COMMENT: - // this is a comment which we will check separately below for content - break; - default: - $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER; - break; - } + $tags = array(); - $has_comment = strlen(trim($comment->getContent())); - $has_inlines = (bool)$this->getInlineComments(); + foreach ($this->getComments() as $comment) { + $action = $comment->getAction(); - if ($has_comment || $has_inlines) { switch ($action) { + case DifferentialAction::ACTION_ADDCCS: + $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CC; + break; case DifferentialAction::ACTION_CLOSE: - // Commit comments are auto-generated and not especially interesting, - // so don't tag them as having a comment. + $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_CLOSED; + break; + case DifferentialAction::ACTION_ADDREVIEWERS: + $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEWERS; + break; + case DifferentialAction::ACTION_COMMENT: + // this is a comment which we will check separately below for content break; default: - $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT; + $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER; break; } + + $has_comment = strlen(trim($comment->getContent())); + $has_inlines = (bool)$this->getInlineComments(); + + if ($has_comment || $has_inlines) { + switch ($action) { + case DifferentialAction::ACTION_CLOSE: + // Commit comments are auto-generated and not especially + // interesting, so don't tag them as having a comment. + break; + default: + $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_COMMENT; + break; + } + } } if (!$tags) { $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_OTHER; } return $tags; } protected function renderVaryPrefix() { $verb = ucwords($this->getVerb()); return "[{$verb}]"; } protected function getVerb() { - $comment = $this->getComment(); + // NOTE: Eventually, this will use getStrongestAction() transaction logic. + // For now, pick the first comment. + $comment = head($this->getComments()); $action = $comment->getAction(); $verb = DifferentialAction::getActionPastTenseVerb($action); return $verb; } protected function prepareBody() { parent::prepareBody(); // If the commented added reviewers or CCs, list them explicitly. - $meta = $this->getComment()->getMetadata(); - $m_reviewers = idx( - $meta, - DifferentialComment::METADATA_ADDED_REVIEWERS, - array()); - $m_cc = idx( - $meta, - DifferentialComment::METADATA_ADDED_CCS, - array()); - $load = array_merge($m_reviewers, $m_cc); + $load = array(); + foreach ($this->comments as $comment) { + $meta = $comment->getMetadata(); + $m_reviewers = idx( + $meta, + DifferentialComment::METADATA_ADDED_REVIEWERS, + array()); + $m_cc = idx( + $meta, + DifferentialComment::METADATA_ADDED_CCS, + array()); + $load[] = $m_reviewers; + $load[] = $m_cc; + } + + $load = array_mergev($load); if ($load) { $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->getActor()) ->withPHIDs($load) ->execute(); if ($m_reviewers) { $this->addedReviewers = $this->renderHandleList($handles, $m_reviewers); } if ($m_cc) { $this->addedCCs = $this->renderHandleList($handles, $m_cc); } } } protected function renderBody() { + // TODO: This will be ApplicationTransactions eventually, but split the + // difference for now. - $comment = $this->getComment(); + $comment = head($this->getComments()); $actor = $this->getActorName(); $name = $this->getRevision()->getTitle(); $verb = $this->getVerb(); $body = array(); $body[] = "{$actor} has {$verb} the revision \"{$name}\"."; if ($this->addedReviewers) { $body[] = 'Added Reviewers: '.$this->addedReviewers; } if ($this->addedCCs) { $body[] = 'Added CCs: '.$this->addedCCs; } $body[] = null; - $content = $comment->getContent(); - if (strlen($content)) { - $body[] = $this->formatText($content); - $body[] = null; + foreach ($this->getComments() as $comment) { + $content = $comment->getContent(); + if (strlen($content)) { + $body[] = $this->formatText($content); + $body[] = null; + } } if ($this->getChangedByCommit()) { $body[] = 'CHANGED PRIOR TO COMMIT'; $body[] = ' '.$this->getChangedByCommit(); $body[] = null; } $inlines = $this->getInlineComments(); if ($inlines) { $body[] = 'INLINE COMMENTS'; $changesets = $this->getChangesets(); $hunk_parser = new DifferentialHunkParser(); if (PhabricatorEnv::getEnvConfig( 'metamta.differential.unified-comment-context')) { foreach ($changesets as $changeset) { $changeset->attachHunks($changeset->loadHunks()); } } foreach ($inlines as $inline) { $changeset = $changesets[$inline->getChangesetID()]; if (!$changeset) { throw new Exception('Changeset missing!'); } $file = $changeset->getFilename(); $start = $inline->getLineNumber(); $len = $inline->getLineLength(); if ($len) { $range = $start.'-'.($start + $len); } else { $range = $start; } $inline_content = $inline->getContent(); if (!PhabricatorEnv::getEnvConfig( 'metamta.differential.unified-comment-context')) { $body[] = $this->formatText("{$file}:{$range} {$inline_content}"); } else { $body[] = "================"; $body[] = "Comment at: " . $file . ":" . $range; $body[] = $hunk_parser->makeContextDiff( $changeset->getHunks(), $inline, 1); $body[] = "----------------"; $body[] = $inline_content; $body[] = null; } } $body[] = null; } $body[] = $this->renderAuxFields(DifferentialMailPhase::COMMENT); return implode("\n", $body); } } diff --git a/src/applications/differential/mail/DifferentialMail.php b/src/applications/differential/mail/DifferentialMail.php index 7c4cc563f6..c7ee43e474 100644 --- a/src/applications/differential/mail/DifferentialMail.php +++ b/src/applications/differential/mail/DifferentialMail.php @@ -1,458 +1,458 @@ rawMail) { throw new Exception("Call send() before getRawMail()!"); } return $this->rawMail; } protected function renderSubject() { $revision = $this->getRevision(); $title = $revision->getTitle(); $id = $revision->getID(); return "D{$id}: {$title}"; } abstract protected function renderVaryPrefix(); abstract protected function renderBody(); public function setActorHandle($actor_handle) { $this->actorHandle = $actor_handle; return $this; } public function getActorHandle() { return $this->actorHandle; } protected function getActorName() { $handle = $this->getActorHandle(); if ($handle) { return $handle->getName(); } return '???'; } public function setParentMessageID($parent_message_id) { $this->parentMessageID = $parent_message_id; return $this; } public function setXHeraldRulesHeader($header) { $this->heraldRulesHeader = $header; return $this; } public function send() { $to_phids = $this->getToPHIDs(); if (!$to_phids) { throw new Exception('No "To:" users provided!'); } $cc_phids = $this->getCCPHIDs(); $attachments = $this->buildAttachments(); $template = new PhabricatorMetaMTAMail(); $actor_handle = $this->getActorHandle(); $reply_handler = $this->getReplyHandler(); if ($actor_handle) { $template->setFrom($actor_handle->getPHID()); } $template ->setIsHTML($this->shouldMarkMailAsHTML()) ->setParentMessageID($this->parentMessageID) ->setExcludeMailRecipientPHIDs($this->getExcludeMailRecipientPHIDs()) ->addHeader('Thread-Topic', $this->getThreadTopic()); $template->setAttachments($attachments); $template->setThreadID( $this->getThreadID(), $this->isFirstMailAboutRevision()); if ($this->heraldRulesHeader) { $template->addHeader('X-Herald-Rules', $this->heraldRulesHeader); } $revision = $this->revision; if ($revision) { if ($revision->getAuthorPHID()) { $template->addHeader( 'X-Differential-Author', '<'.$revision->getAuthorPHID().'>'); } $reviewer_phids = $revision->getReviewers(); if ($reviewer_phids) { // Add several headers to support e-mail clients which are not able to // create rules using regular expressions or wildcards (namely Outlook). $template->addPHIDHeaders('X-Differential-Reviewer', $reviewer_phids); // Add it also as a list to allow matching of the first reviewer and // also for backwards compatibility. $template->addHeader( 'X-Differential-Reviewers', '<'.implode('>, <', $reviewer_phids).'>'); } if ($cc_phids) { $template->addPHIDHeaders('X-Differential-CC', $cc_phids); $template->addHeader( 'X-Differential-CCs', '<'.implode('>, <', $cc_phids).'>'); // Determine explicit CCs (those added by humans) and put them in a // header so users can differentiate between Herald CCs and human CCs. $relation_subscribed = DifferentialRevision::RELATION_SUBSCRIBED; $raw = $revision->getRawRelations($relation_subscribed); $reason_phids = ipull($raw, 'reasonPHID'); $reason_handles = id(new PhabricatorHandleQuery()) ->setViewer($this->getActor()) ->withPHIDs($reason_phids) ->execute(); $explicit_cc = array(); foreach ($raw as $relation) { if (!$relation['reasonPHID']) { continue; } $type = $reason_handles[$relation['reasonPHID']]->getType(); if ($type == PhabricatorPeoplePHIDTypeUser::TYPECONST) { $explicit_cc[] = $relation['objectPHID']; } } if ($explicit_cc) { $template->addPHIDHeaders('X-Differential-Explicit-CC', $explicit_cc); $template->addHeader( 'X-Differential-Explicit-CCs', '<'.implode('>, <', $explicit_cc).'>'); } } } $template->setIsBulk(true); $template->setRelatedPHID($this->getRevision()->getPHID()); $mailtags = $this->getMailTags(); if ($mailtags) { $template->setMailTags($mailtags); } $phids = array(); foreach ($to_phids as $phid) { $phids[$phid] = true; } foreach ($cc_phids as $phid) { $phids[$phid] = true; } $phids = array_keys($phids); $handles = id(new PhabricatorHandleQuery()) ->setViewer($this->getActor()) ->withPHIDs($phids) ->execute(); $objects = id(new PhabricatorObjectQuery()) ->setViewer($this->getActor()) ->withPHIDs($phids) ->execute(); $to_handles = array_select_keys($handles, $to_phids); $cc_handles = array_select_keys($handles, $cc_phids); $this->prepareBody(); $this->rawMail = clone $template; $this->rawMail->addTos($to_phids); $this->rawMail->addCCs($cc_phids); $mails = $reply_handler->multiplexMail($template, $to_handles, $cc_handles); $original_translator = PhutilTranslator::getInstance(); if (!PhabricatorMetaMTAMail::shouldMultiplexAllMail()) { $translation = PhabricatorEnv::newObjectFromConfig( 'translation.provider'); $translator = id(new PhutilTranslator()) ->setLanguage($translation->getLanguage()) ->addTranslations($translation->getTranslations()); } try { foreach ($mails as $mail) { if (PhabricatorMetaMTAMail::shouldMultiplexAllMail()) { $translation = newv($mail->getTranslation($objects), array()); $translator = id(new PhutilTranslator()) ->setLanguage($translation->getLanguage()) ->addTranslations($translation->getTranslations()); PhutilTranslator::setInstance($translator); } $body = $this->buildBody()."\n". $reply_handler->getRecipientsSummary($to_handles, $cc_handles); $mail ->setSubject($this->renderSubject()) ->setSubjectPrefix($this->getSubjectPrefix()) ->setVarySubjectPrefix($this->renderVaryPrefix()) ->setBody($body); $event = new PhabricatorEvent( PhabricatorEventType::TYPE_DIFFERENTIAL_WILLSENDMAIL, array( 'mail' => $mail, ) ); PhutilEventEngine::dispatchEvent($event); $mail = $event->getValue('mail'); $mail->saveAndSend(); } } catch (Exception $ex) { PhutilTranslator::setInstance($original_translator); throw $ex; } PhutilTranslator::setInstance($original_translator); return $this; } protected function getMailTags() { return array(); } protected function getSubjectPrefix() { return PhabricatorEnv::getEnvConfig('metamta.differential.subject-prefix'); } protected function shouldMarkMailAsHTML() { return false; } /** * @{method:buildBody} is called once for each e-mail recipient to allow * translating text to his language. This method can be used to load data that * don't need translation and use them later in @{method:buildBody}. * * @param * @return */ protected function prepareBody() { } protected function buildBody() { $main_body = $this->renderBody(); $body = new PhabricatorMetaMTAMailBody(); $body->addRawSection($main_body); $reply_handler = $this->getReplyHandler(); $body->addReplySection($reply_handler->getReplyHandlerInstructions()); if ($this->getHeraldTranscriptURI() && $this->isFirstMailToRecipients()) { $xscript_uri = $this->getHeraldTranscriptURI(); $body->addHeraldSection($xscript_uri); } return $body->render(); } /** * You can override this method in a subclass and return array of attachments * to be sent with the email. Each attachment is an instance of * PhabricatorMetaMTAAttachment. */ protected function buildAttachments() { return array(); } public function getReplyHandler() { if (!$this->replyHandler) { $this->replyHandler = self::newReplyHandlerForRevision($this->getRevision()); } return $this->replyHandler; } public static function newReplyHandlerForRevision( DifferentialRevision $revision) { $reply_handler = PhabricatorEnv::newObjectFromConfig( 'metamta.differential.reply-handler'); $reply_handler->setMailReceiver($revision); return $reply_handler; } protected function formatText($text) { $text = explode("\n", rtrim($text)); foreach ($text as &$line) { $line = rtrim(' '.$line); } unset($line); return implode("\n", $text); } public function setExcludeMailRecipientPHIDs(array $exclude) { $this->excludePHIDs = $exclude; return $this; } public function getExcludeMailRecipientPHIDs() { return $this->excludePHIDs; } public function setToPHIDs(array $to) { $this->to = $to; return $this; } public function setCCPHIDs(array $cc) { $this->cc = $cc; return $this; } protected function getToPHIDs() { return $this->to; } protected function getCCPHIDs() { return $this->cc; } public function setRevision($revision) { $this->revision = $revision; return $this; } public function getRevision() { return $this->revision; } protected function getThreadID() { $phid = $this->getRevision()->getPHID(); return "differential-rev-{$phid}-req"; } protected function getThreadTopic() { $id = $this->getRevision()->getID(); $title = $this->getRevision()->getOriginalTitle(); return "D{$id}: {$title}"; } - public function setComment($comment) { - $this->comment = $comment; + public function setComments(array $comments) { + $this->comments = $comments; return $this; } - public function getComment() { - return $this->comment; + public function getComments() { + return $this->comments; } public function setChangesets($changesets) { $this->changesets = $changesets; return $this; } public function getChangesets() { return $this->changesets; } public function setInlineComments(array $inline_comments) { assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); $this->inlineComments = $inline_comments; return $this; } public function getInlineComments() { return $this->inlineComments; } protected function renderAuxFields($phase) { $selector = DifferentialFieldSelector::newSelector(); $aux_fields = $selector->sortFieldsForMail( $selector->getFieldSpecifications()); $body = array(); foreach ($aux_fields as $field) { $field->setUser($this->getActor()); $field->setRevision($this->getRevision()); // TODO: Introduce and use getRequiredHandlePHIDsForMail() and load all // handles in prepareBody(). $text = $field->renderValueForMail($phase); if ($text !== null) { $body[] = $text; $body[] = null; } } return implode("\n", $body); } public function setIsFirstMailToRecipients($first) { $this->isFirstMailToRecipients = $first; return $this; } public function isFirstMailToRecipients() { return $this->isFirstMailToRecipients; } public function setIsFirstMailAboutRevision($first) { $this->isFirstMailAboutRevision = $first; return $this; } public function isFirstMailAboutRevision() { return $this->isFirstMailAboutRevision; } public function setHeraldTranscriptURI($herald_transcript_uri) { $this->heraldTranscriptURI = $herald_transcript_uri; return $this; } public function getHeraldTranscriptURI() { return $this->heraldTranscriptURI; } protected function renderHandleList(array $handles, array $phids) { assert_instances_of($handles, 'PhabricatorObjectHandle'); $names = array(); foreach ($phids as $phid) { $names[] = $handles[$phid]->getName(); } return implode(', ', $names); } } diff --git a/src/applications/differential/mail/DifferentialReviewRequestMail.php b/src/applications/differential/mail/DifferentialReviewRequestMail.php index 82a58c8c70..a996f8db27 100644 --- a/src/applications/differential/mail/DifferentialReviewRequestMail.php +++ b/src/applications/differential/mail/DifferentialReviewRequestMail.php @@ -1,129 +1,129 @@ comments = $comments; + public function setCommentText($comment_text) { + $this->commentText = $comment_text; return $this; } - public function getComments() { - return $this->comments; + public function getCommentText() { + return $this->commentText; } public function __construct( DifferentialRevision $revision, PhabricatorObjectHandle $actor, array $changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); $this->setRevision($revision); $this->setActorHandle($actor); $this->setChangesets($changesets); } protected function prepareBody() { parent::prepareBody(); $inline_max_length = PhabricatorEnv::getEnvConfig( 'metamta.differential.inline-patches'); if ($inline_max_length) { $patch = $this->buildPatch(); if (count(explode("\n", $patch)) <= $inline_max_length) { $this->patch = $patch; } } } protected function renderReviewRequestBody() { $revision = $this->getRevision(); $body = array(); if (!$this->isFirstMailToRecipients()) { - if (strlen($this->getComments())) { - $body[] = $this->formatText($this->getComments()); + if (strlen($this->getCommentText())) { + $body[] = $this->formatText($this->getCommentText()); $body[] = null; } } $phase = ($this->isFirstMailToRecipients() ? DifferentialMailPhase::WELCOME : DifferentialMailPhase::UPDATE); $body[] = $this->renderAuxFields($phase); $changesets = $this->getChangesets(); if ($changesets) { $body[] = 'AFFECTED FILES'; $max = self::MAX_AFFECTED_FILES; foreach (array_values($changesets) as $i => $changeset) { if ($i == $max) { $body[] = ' ('.(count($changesets) - $max).' more files)'; break; } $body[] = ' '.$changeset->getFilename(); } $body[] = null; } if ($this->patch) { $body[] = 'CHANGE DETAILS'; $body[] = $this->patch; } return implode("\n", $body); } protected function buildAttachments() { $attachments = array(); if (PhabricatorEnv::getEnvConfig('metamta.differential.attach-patches')) { $revision = $this->getRevision(); $revision_id = $revision->getID(); $diffs = id(new DifferentialDiffQuery()) ->setViewer($this->getActor()) ->withRevisionIDs(array($revision_id)) ->execute(); $diff_number = count($diffs); $attachments[] = new PhabricatorMetaMTAAttachment( $this->buildPatch(), "D{$revision_id}.{$diff_number}.patch", 'text/x-patch; charset=utf-8' ); } return $attachments; } private function buildPatch() { $renderer = new DifferentialRawDiffRenderer(); $renderer->setChangesets($this->getChangesets()); $renderer->setFormat( PhabricatorEnv::getEnvConfig('metamta.differential.patch-format')); // TODO: It would be nice to have a real viewer here eventually, but // in the meantime anyone we're sending mail to can certainly see the // patch. $renderer->setViewer(PhabricatorUser::getOmnipotentUser()); return $renderer->buildPatch(); } protected function getMailTags() { $tags = array(); if ($this->isFirstMailToRecipients()) { $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_REVIEW_REQUEST; } else { $tags[] = MetaMTANotificationType::TYPE_DIFFERENTIAL_UPDATED; } return $tags; } }