diff --git a/src/applications/differential/editor/DifferentialCommentEditor.php b/src/applications/differential/editor/DifferentialCommentEditor.php index 44d078a93a..30704c87d6 100644 --- a/src/applications/differential/editor/DifferentialCommentEditor.php +++ b/src/applications/differential/editor/DifferentialCommentEditor.php @@ -1,644 +1,669 @@ <?php /* * Copyright 2012 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ final class DifferentialCommentEditor { protected $revision; protected $actorPHID; protected $action; protected $attachInlineComments; protected $message; protected $changedByCommit; protected $addedReviewers = array(); + protected $removedReviewers = array(); private $addedCCs = array(); private $parentMessageID; private $contentSource; private $isDaemonWorkflow; public function __construct( DifferentialRevision $revision, $actor_phid, $action) { $this->revision = $revision; $this->actorPHID = $actor_phid; $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 save() { $revision = $this->revision; $action = $this->action; $actor_phid = $this->actorPHID; $actor = id(new PhabricatorUser())->loadOneWhere('PHID = %s', $actor_phid); $actor_is_author = ($actor_phid == $revision->getAuthorPHID()); $revision_status = $revision->getStatus(); $revision->loadRelationships(); $reviewer_phids = $revision->getReviewers(); if ($reviewer_phids) { $reviewer_phids = array_combine($reviewer_phids, $reviewer_phids); } $metadata = array(); $inline_comments = array(); if ($this->attachInlineComments) { $inline_comments = id(new DifferentialInlineComment())->loadAllWhere( 'authorPHID = %s AND revisionID = %d AND commentID IS NULL', $this->actorPHID, $revision->getID()); } 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."); } 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."); } DifferentialRevisionEditor::alterReviewers( $revision, $reviewer_phids, $rem = array($actor_phid), $add = array(), $actor_phid); 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) { throw new Exception('You can not accept your own revision.'); } if (($revision_status != ArcanistDifferentialRevisionStatus::NEEDS_REVIEW) && ($revision_status != ArcanistDifferentialRevisionStatus::NEEDS_REVISION)) { switch ($revision_status) { case ArcanistDifferentialRevisionStatus::ACCEPTED: throw new DifferentialActionHasNoEffectException( "You can not accept this revision because someone else ". "already accepted it."); 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."); default: throw new Exception( "Unexpected revision state '{$revision_status}'!"); } } $revision ->setStatus(ArcanistDifferentialRevisionStatus::ACCEPTED); if (!isset($reviewer_phids[$actor_phid])) { DifferentialRevisionEditor::alterReviewers( $revision, $reviewer_phids, $rem = array(), $add = array($actor_phid), $actor_phid); } 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}'!"); } - $added_reviewers = $this->addReviewers(); + 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: // NOTE: We allow you to reject an already-rejected revision // because it doesn't create any ambiguity and avoids a rather // needless dialog. 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}'!"); } if (!isset($reviewer_phids[$actor_phid])) { DifferentialRevisionEditor::alterReviewers( $revision, $reviewer_phids, $rem = array(), $add = array($actor_phid), $actor_phid); } $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: 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); 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) { 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_ADDREVIEWERS: - $added_reviewers = $this->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, $this->actorPHID); } $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); - $added_reviewers = $this->addReviewers(); + 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 ($action != DifferentialAction::ACTION_RESIGN) { DifferentialRevisionEditor::addCC( $revision, $this->actorPHID, $this->actorPHID); } $comment = id(new DifferentialComment()) ->setAuthorPHID($this->actorPHID) ->setRevisionID($revision->getID()) ->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, $this->actorPHID); $metacc[] = $cc_phid; } $metadata[DifferentialComment::METADATA_ADDED_CCS] = $metacc; $comment->setMetadata($metadata); $comment->save(); } } $revision->saveTransaction(); $phids = array($this->actorPHID); $handles = id(new PhabricatorObjectHandleData($phids)) ->loadHandles(); $actor_handle = $handles[$this->actorPHID]; $xherald_header = HeraldTranscript::loadXHeraldRulesHeader( $revision->getPHID()); id(new DifferentialCommentMail( $revision, $actor_handle, $comment, $changesets, $inline_comments)) ->setToPHIDs( array_merge( $revision->getReviewers(), array($revision->getAuthorPHID()))) ->setCCPHIDs($revision->getCCPHIDs()) ->setChangedByCommit($this->getChangedByCommit()) ->setXHeraldRulesHeader($xherald_header) ->setParentMessageID($this->parentMessageID) ->send(); $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' => $this->actorPHID, ); id(new PhabricatorTimelineEvent('difx', $event_data)) ->recordEvent(); // TODO: Move to a daemon? id(new PhabricatorFeedStoryPublisher()) ->setStoryType(PhabricatorFeedStoryTypeConstants::STORY_DIFFERENTIAL) ->setStoryData($event_data) ->setStoryTime(time()) ->setStoryAuthorPHID($this->actorPHID) ->setRelatedPHIDs( array( $revision->getPHID(), $this->actorPHID, $revision->getAuthorPHID(), )) ->setPrimaryObjectPHID($revision->getPHID()) ->setSubscribedPHIDs( array_merge( array($revision->getAuthorPHID()), $revision->getReviewers(), $revision->getCCPHIDs())) ->publish(); // TODO: Move to a daemon? PhabricatorSearchDifferentialIndexer::indexRevision($revision); 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 addReviewers() { + private function alterReviewers() { $revision = $this->revision; $added_reviewers = $this->getAddedReviewers(); + $removed_reviewers = $this->getRemovedReviewers(); $reviewer_phids = $revision->getReviewers(); + $reviewer_phids_map = array_fill_keys($reviewer_phids, true); foreach ($added_reviewers as $k => $user_phid) { if ($user_phid == $revision->getAuthorPHID()) { unset($added_reviewers[$k]); } - if (!empty($reviewer_phids[$user_phid])) { + 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::alterReviewers( $revision, $reviewer_phids, - $rem = array(), + $removed_reviewers, $added_reviewers, $this->actorPHID); } - return $added_reviewers; + return array($added_reviewers, $removed_reviewers); } } diff --git a/src/applications/differential/storage/DifferentialComment.php b/src/applications/differential/storage/DifferentialComment.php index 3e59d9e7d3..4cf73d26ab 100644 --- a/src/applications/differential/storage/DifferentialComment.php +++ b/src/applications/differential/storage/DifferentialComment.php @@ -1,50 +1,51 @@ <?php /* * Copyright 2012 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ final class DifferentialComment extends DifferentialDAO { - const METADATA_ADDED_REVIEWERS = 'added-reviewers'; - const METADATA_ADDED_CCS = 'added-ccs'; - const METADATA_DIFF_ID = 'diff-id'; + const METADATA_ADDED_REVIEWERS = 'added-reviewers'; + const METADATA_REMOVED_REVIEWERS = 'removed-reviewers'; + const METADATA_ADDED_CCS = 'added-ccs'; + const METADATA_DIFF_ID = 'diff-id'; protected $authorPHID; protected $revisionID; protected $action; protected $content; protected $cache; protected $metadata = array(); protected $contentSource; public function getConfiguration() { return array( self::CONFIG_SERIALIZATION => array( 'metadata' => self::SERIALIZATION_JSON, ), ) + parent::getConfiguration(); } public function setContentSource(PhabricatorContentSource $content_source) { $this->contentSource = $content_source->serialize(); return $this; } public function getContentSource() { return PhabricatorContentSource::newFromSerialized($this->contentSource); } } diff --git a/src/applications/differential/view/DifferentialRevisionCommentView.php b/src/applications/differential/view/DifferentialRevisionCommentView.php index 745582f353..2f7b795f39 100644 --- a/src/applications/differential/view/DifferentialRevisionCommentView.php +++ b/src/applications/differential/view/DifferentialRevisionCommentView.php @@ -1,309 +1,318 @@ <?php /* * Copyright 2012 Facebook, Inc. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */ final class DifferentialRevisionCommentView extends AphrontView { private $comment; private $handles; private $markupEngine; private $preview; private $inlines; private $changesets; private $target; private $anchorName; private $user; private $versusDiffID; public function setComment($comment) { $this->comment = $comment; return $this; } public function setHandles(array $handles) { assert_instances_of($handles, 'PhabricatorObjectHandle'); $this->handles = $handles; return $this; } public function setMarkupEngine($markup_engine) { $this->markupEngine = $markup_engine; return $this; } public function setPreview($preview) { $this->preview = $preview; return $this; } public function setInlineComments(array $inline_comments) { assert_instances_of($inline_comments, 'PhabricatorInlineCommentInterface'); $this->inlines = $inline_comments; return $this; } public function setChangesets(array $changesets) { assert_instances_of($changesets, 'DifferentialChangeset'); // Ship these in sorted by getSortKey() and keyed by ID... or else! $this->changesets = $changesets; return $this; } public function setTargetDiff($target) { $this->target = $target; return $this; } public function setVersusDiffID($diff_vs) { $this->versusDiffID = $diff_vs; return $this; } public function setAnchorName($anchor_name) { $this->anchorName = $anchor_name; return $this; } public function setUser(PhabricatorUser $user) { $this->user = $user; return $this; } public function render() { if (!$this->user) { throw new Exception("Call setUser() before rendering!"); } require_celerity_resource('phabricator-remarkup-css'); require_celerity_resource('differential-revision-comment-css'); $comment = $this->comment; $action = $comment->getAction(); $action_class = 'differential-comment-action-'.$action; $info = array(); $content = $comment->getContent(); $hide_comments = true; if (strlen(rtrim($content))) { $hide_comments = false; $cache = $comment->getCache(); if (strlen($cache)) { $content = $cache; } else { $content = $this->markupEngine->markupText($content); if ($comment->getID()) { $comment->setCache($content); $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); $comment->save(); unset($unguarded); } } $content = '<div class="phabricator-remarkup">'. $content. '</div>'; } $inline_render = $this->renderInlineComments(); if ($inline_render) { $hide_comments = false; } $author = $this->handles[$comment->getAuthorPHID()]; $author_link = $author->renderLink(); $metadata = $comment->getMetadata(); $added_reviewers = idx( $metadata, DifferentialComment::METADATA_ADDED_REVIEWERS, array()); + $removed_reviewers = idx( + $metadata, + DifferentialComment::METADATA_REMOVED_REVIEWERS, + array()); $added_ccs = idx( $metadata, DifferentialComment::METADATA_ADDED_CCS, array()); $verb = DifferentialAction::getActionPastTenseVerb($comment->getAction()); $verb = phutil_escape_html($verb); $actions = array(); switch ($comment->getAction()) { case DifferentialAction::ACTION_ADDCCS: $actions[] = "{$author_link} added CCs: ". $this->renderHandleList($added_ccs)."."; $added_ccs = null; break; case DifferentialAction::ACTION_ADDREVIEWERS: $actions[] = "{$author_link} added reviewers: ". $this->renderHandleList($added_reviewers)."."; $added_reviewers = null; break; case DifferentialAction::ACTION_UPDATE: $diff_id = idx($metadata, DifferentialComment::METADATA_DIFF_ID); if ($diff_id) { $diff_link = phutil_render_tag( 'a', array( 'href' => '/D'.$comment->getRevisionID().'?id='.$diff_id, ), 'Diff #'.phutil_escape_html($diff_id)); $actions[] = "{$author_link} updated this revision to {$diff_link}."; } else { $actions[] = "{$author_link} {$verb} this revision."; } break; default: $actions[] = "{$author_link} {$verb} this revision."; break; } if ($added_reviewers) { $actions[] = "{$author_link} added reviewers: ". $this->renderHandleList($added_reviewers)."."; } + if ($removed_reviewers) { + $actions[] = "{$author_link} removed reviewers: ". + $this->renderHandleList($removed_reviewers)."."; + } + if ($added_ccs) { $actions[] = "{$author_link} added CCs: ". $this->renderHandleList($added_ccs)."."; } foreach ($actions as $key => $action) { $actions[$key] = '<div>'.$action.'</div>'; } $xaction_view = id(new PhabricatorTransactionView()) ->setUser($this->user) ->setImageURI($author->getImageURI()) ->setContentSource($comment->getContentSource()) ->addClass($action_class) ->setActions($actions); if ($this->preview) { $xaction_view->setIsPreview($this->preview); } else { $xaction_view->setEpoch($comment->getDateCreated()); if ($this->anchorName) { $anchor_name = $this->anchorName; $anchor_text = 'D'.$comment->getRevisionID().'#'.$anchor_name; $xaction_view->setAnchor($anchor_name, $anchor_text); } } if (!$hide_comments) { $xaction_view->appendChild( '<div class="differential-comment-core">'. $content. '</div>'. $this->renderSingleView($inline_render)); } return $xaction_view->render(); } private function renderHandleList(array $phids) { $result = array(); foreach ($phids as $phid) { $result[] = $this->handles[$phid]->renderLink(); } return implode(', ', $result); } private function renderInlineComments() { if (!$this->inlines) { return null; } $inlines = $this->inlines; $changesets = $this->changesets; $inlines_by_changeset = mgroup($inlines, 'getChangesetID'); $inlines_by_changeset = array_select_keys( $inlines_by_changeset, array_keys($this->changesets)); $view = new PhabricatorInlineSummaryView(); foreach ($inlines_by_changeset as $changeset_id => $inlines) { $changeset = $changesets[$changeset_id]; $items = array(); foreach ($inlines as $inline) { $on_target = ($this->target) && ($this->target->getID() == $changeset->getDiffID()); $is_visible = false; if ($inline->getIsNewFile()) { // This comment is on the right side of the versus diff, and visible // on the left side of the page. if ($this->versusDiffID) { if ($changeset->getDiffID() == $this->versusDiffID) { $is_visible = true; } } // This comment is on the right side of the target diff, and visible // on the right side of the page. if ($on_target) { $is_visible = true; } } else { // Ths comment is on the left side of the target diff, and visible // on the left side of the page. if (!$this->versusDiffID) { if ($on_target) { $is_visible = true; } } // TODO: We still get one edge case wrong here, when we have a // versus diff and the file didn't exist in the old version. The // comment is visible because we show the left side of the target // diff when there's no corresponding file in the versus diff, but // we incorrectly link it off-page. } $item = array( 'id' => $inline->getID(), 'line' => $inline->getLineNumber(), 'length' => $inline->getLineLength(), 'content' => PhabricatorInlineSummaryView::renderCommentContent( $inline, $this->markupEngine), ); if (!$is_visible) { $diff_id = $changeset->getDiffID(); $item['where'] = '(On Diff #'.$diff_id.')'; $item['href'] = 'D'.$this->comment->getRevisionID(). '?id='.$diff_id. '#inline-'.$inline->getID(); } $items[] = $item; } $view->addCommentGroup($changeset->getFilename(), $items); } return $view; } }