diff --git a/src/applications/differential/constants/DifferentialAction.php b/src/applications/differential/constants/DifferentialAction.php index 57038959de..9238990161 100644 --- a/src/applications/differential/constants/DifferentialAction.php +++ b/src/applications/differential/constants/DifferentialAction.php @@ -22,83 +22,83 @@ final class DifferentialAction { public static function getBasicStoryText($action, $author_name) { switch ($action) { - case DifferentialAction::ACTION_COMMENT: - $title = pht('%s commented on this revision.', - $author_name); + case DifferentialAction::ACTION_COMMENT: + $title = pht('%s commented on this revision.', + $author_name); break; - case DifferentialAction::ACTION_ACCEPT: - $title = pht('%s accepted this revision.', - $author_name); + case DifferentialAction::ACTION_ACCEPT: + $title = pht('%s accepted this revision.', + $author_name); break; - case DifferentialAction::ACTION_REJECT: - $title = pht('%s requested changes to this revision.', - $author_name); + case DifferentialAction::ACTION_REJECT: + $title = pht('%s requested changes to this revision.', + $author_name); break; - case DifferentialAction::ACTION_RETHINK: - $title = pht('%s planned changes to this revision.', - $author_name); + case DifferentialAction::ACTION_RETHINK: + $title = pht('%s planned changes to this revision.', + $author_name); break; - case DifferentialAction::ACTION_ABANDON: - $title = pht('%s abandoned this revision.', - $author_name); + case DifferentialAction::ACTION_ABANDON: + $title = pht('%s abandoned this revision.', + $author_name); break; - case DifferentialAction::ACTION_CLOSE: - $title = pht('%s closed this revision.', - $author_name); + case DifferentialAction::ACTION_CLOSE: + $title = pht('%s closed this revision.', + $author_name); break; - case DifferentialAction::ACTION_REQUEST: - $title = pht('%s requested a review of this revision.', - $author_name); + case DifferentialAction::ACTION_REQUEST: + $title = pht('%s requested a review of this revision.', + $author_name); break; - case DifferentialAction::ACTION_RECLAIM: - $title = pht('%s reclaimed this revision.', - $author_name); + case DifferentialAction::ACTION_RECLAIM: + $title = pht('%s reclaimed this revision.', + $author_name); break; - case DifferentialAction::ACTION_UPDATE: - $title = pht('%s updated this revision.', - $author_name); + case DifferentialAction::ACTION_UPDATE: + $title = pht('%s updated this revision.', + $author_name); break; - case DifferentialAction::ACTION_RESIGN: - $title = pht('%s resigned from this revision.', - $author_name); + case DifferentialAction::ACTION_RESIGN: + $title = pht('%s resigned from this revision.', + $author_name); break; - case DifferentialAction::ACTION_SUMMARIZE: - $title = pht('%s summarized this revision.', - $author_name); + case DifferentialAction::ACTION_SUMMARIZE: + $title = pht('%s summarized this revision.', + $author_name); break; - case DifferentialAction::ACTION_TESTPLAN: - $title = pht('%s explained the test plan for this revision.', - $author_name); + case DifferentialAction::ACTION_TESTPLAN: + $title = pht('%s explained the test plan for this revision.', + $author_name); break; - case DifferentialAction::ACTION_CREATE: - $title = pht('%s created this revision.', - $author_name); + case DifferentialAction::ACTION_CREATE: + $title = pht('%s created this revision.', + $author_name); break; - case DifferentialAction::ACTION_ADDREVIEWERS: - $title = pht('%s added reviewers to this revision.', - $author_name); + case DifferentialAction::ACTION_ADDREVIEWERS: + $title = pht('%s added reviewers to this revision.', + $author_name); break; - case DifferentialAction::ACTION_ADDCCS: - $title = pht('%s added CCs to this revision.', - $author_name); + case DifferentialAction::ACTION_ADDCCS: + $title = pht('%s added CCs to this revision.', + $author_name); break; - case DifferentialAction::ACTION_CLAIM: - $title = pht('%s commandeered this revision.', - $author_name); + case DifferentialAction::ACTION_CLAIM: + $title = pht('%s commandeered this revision.', + $author_name); break; - case DifferentialAction::ACTION_REOPEN: - $title = pht('%s reopened this revision.', - $author_name); + case DifferentialAction::ACTION_REOPEN: + $title = pht('%s reopened this revision.', + $author_name); break; - case DifferentialTransaction::TYPE_INLINE: - $title = pht( - '%s added an inline comment.', - $author_name); - break; - default: - $title = pht('Ghosts happened to this revision.'); + case DifferentialTransaction::TYPE_INLINE: + $title = pht( + '%s added an inline comment.', + $author_name); break; - } + default: + $title = pht('Ghosts happened to this revision.'); + break; + } return $title; } diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php index 5b7fce871b..55c589b6e3 100644 --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -102,6 +102,18 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction { $new = $this->getNewValue(); switch ($this->getTransactionType()) { + case self::TYPE_ACTION: + if ($new == DifferentialAction::ACTION_CLOSE && + $this->getMetadataValue('isCommitClose')) { + $phids[] = $this->getMetadataValue('commitPHID'); + if ($this->getMetadataValue('committerPHID')) { + $phids[] = $this->getMetadataValue('committerPHID'); + } + if ($this->getMetadataValue('authorPHID')) { + $phids[] = $this->getMetadataValue('authorPHID'); + } + } + break; case self::TYPE_UPDATE: if ($new) { $phids[] = $new; @@ -216,7 +228,11 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction { '%s added inline comments.', $author_handle); case self::TYPE_UPDATE: - if ($new) { + if ($this->getMetadataValue('isCommitUpdate')) { + return pht( + 'This revision was automatically updated to reflect the '. + 'committed changes.'); + } else if ($new) { // TODO: Migrate to PHIDs and use handles here? if (phid_get_type($new) == DifferentialDiffPHIDType::TYPECONST) { return pht( @@ -234,7 +250,45 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction { $author_handle); } case self::TYPE_ACTION: - return DifferentialAction::getBasicStoryText($new, $author_handle); + switch ($new) { + case DifferentialAction::ACTION_CLOSE: + if (!$this->getMetadataValue('isCommitClose')) { + return DifferentialAction::getBasicStoryText( + $new, + $author_handle); + } + $commit_name = $this->renderHandleLink( + $this->getMetadataValue('commitPHID')); + $committer_phid = $this->getMetadataValue('committerPHID'); + $author_phid = $this->getMetadataValue('authorPHID'); + if ($this->getHandleIfExists($committer_phid)) { + $committer_name = $this->renderHandleLink($committer_phid); + } else { + $committer_name = $this->getMetadataValue('committerName'); + } + if ($this->getHandleIfExists($author_phid)) { + $author_name = $this->renderHandleLink($author_phid); + } else { + $author_name = $this->getMetadataValue('authorName'); + } + + if ($committer_name && ($committer_name != $author_name)) { + return pht( + 'Closed by commit %s (authored by %s, committed by %s).', + $commit_name, + $author_name, + $committer_name); + } else { + return pht( + 'Closed by commit %s (authored by %s).', + $commit_name, + $author_name); + } + break; + default: + return DifferentialAction::getBasicStoryText($new, $author_handle); + } + break; case self::TYPE_STATUS: switch ($this->getNewValue()) { case ArcanistDifferentialRevisionStatus::ACCEPTED: @@ -296,10 +350,44 @@ final class DifferentialTransaction extends PhabricatorApplicationTransaction { $author_link, $object_link); case DifferentialAction::ACTION_CLOSE: - return pht( - '%s closed %s.', - $author_link, - $object_link); + if (!$this->getMetadataValue('isCommitClose')) { + return pht( + '%s closed %s.', + $author_link, + $object_link); + } else { + $commit_name = $this->renderHandleLink( + $this->getMetadataValue('commitPHID')); + $committer_phid = $this->getMetadataValue('committerPHID'); + $author_phid = $this->getMetadataValue('authorPHID'); + if ($this->getHandleIfExists($committer_phid)) { + $committer_name = $this->renderHandleLink($committer_phid); + } else { + $committer_name = $this->getMetadataValue('committerName'); + } + if ($this->getHandleIfExists($author_phid)) { + $author_name = $this->renderHandleLink($author_phid); + } else { + $author_name = $this->getMetadataValue('authorName'); + } + + if ($committer_name && ($committer_name != $author_name)) { + return pht( + '%s closed %s by commit %s (authored by %s).', + $author_link, + $object_link, + $commit_name, + $author_name); + } else { + return pht( + '%s closed %s by commit %s.', + $author_link, + $object_link, + $commit_name); + } + } + break; + case DifferentialAction::ACTION_REQUEST: return pht( '%s requested review of %s.', diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php index 5d6b786f36..a432e84853 100644 --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -140,31 +140,29 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker $should_autoclose; if ($should_close) { - $commit_name = $repository->formatCommitName( - $commit->getCommitIdentifier()); + $commit_close_xaction = id(new DifferentialTransaction()) + ->setTransactionType(DifferentialTransaction::TYPE_ACTION) + ->setNewValue(DifferentialAction::ACTION_CLOSE) + ->setMetadataValue('isCommitClose', true); - $committer_name = $this->loadUserName( - $committer_phid, - $data->getCommitDetail('committer'), - $actor); - - $author_name = $this->loadUserName( - $author_phid, - $data->getAuthorName(), - $actor); - - if ($committer_name && ($committer_name != $author_name)) { - $revision_update_comment = pht( - 'Closed by commit %s (authored by %s, committed by %s).', - $commit_name, - $author_name, - $committer_name); - } else { - $revision_update_comment = pht( - 'Closed by commit %s (authored by %s).', - $commit_name, - $author_name); - } + $commit_close_xaction->setMetadataValue( + 'commitPHID', + $commit->getPHID()); + $commit_close_xaction->setMetadataValue( + 'committerPHID', + $committer_phid); + $commit_close_xaction->setMetadataValue( + 'committerName', + $data->getCommitDetail('committer')); + $commit_close_xaction->setMetadataValue( + 'authorPHID', + $author_phid); + $commit_close_xaction->setMetadataValue( + 'authorName', + $data->getAuthorName()); + $commit_close_xaction->setMetadataValue( + 'commitHashes', + $hashes); $diff = $this->generateFinalDiff($revision, $acting_as_phid); @@ -181,22 +179,12 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker } $xactions = array(); - - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(DifferentialTransaction::TYPE_ACTION) - ->setNewValue(DifferentialAction::ACTION_CLOSE); - $xactions[] = id(new DifferentialTransaction()) ->setTransactionType(DifferentialTransaction::TYPE_UPDATE) ->setIgnoreOnNoEffect(true) - ->setNewValue($diff->getPHID()); - - $xactions[] = id(new DifferentialTransaction()) - ->setTransactionType(PhabricatorTransactions::TYPE_COMMENT) - ->setIgnoreOnNoEffect(true) - ->attachComment( - id(new DifferentialTransactionComment()) - ->setContent($revision_update_comment)); + ->setNewValue($diff->getPHID()) + ->setMetadataValue('isCommitUpdate', true); + $xactions[] = $commit_close_xaction; $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_DAEMON, @@ -237,18 +225,6 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker PhabricatorRepositoryCommit::IMPORTED_MESSAGE); } - private function loadUserName($user_phid, $default, PhabricatorUser $actor) { - if (!$user_phid) { - return $default; - } - $handle = id(new PhabricatorHandleQuery()) - ->setViewer($actor) - ->withPHIDs(array($user_phid)) - ->executeOne(); - - return '@'.$handle->getName(); - } - private function generateFinalDiff( DifferentialRevision $revision, $actor_phid) { @@ -521,6 +497,8 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker ->setActingAsPHID($acting_as) ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true) + ->setUnmentionablePHIDMap( + array($commit->getPHID() => $commit->getPHID())) ->setContentSource($content_source); $editor->applyTransactions($task, $xactions);