diff --git a/src/applications/differential/constants/DifferentialAction.php b/src/applications/differential/constants/DifferentialAction.php --- a/src/applications/differential/constants/DifferentialAction.php +++ b/src/applications/differential/constants/DifferentialAction.php @@ -3,6 +3,8 @@ final class DifferentialAction { const ACTION_CLOSE = 'commit'; + // this is the "automagical" close from committing + const ACTION_COMMIT_CLOSE = 'commit_close'; const ACTION_COMMENT = 'none'; const ACTION_ACCEPT = 'accept'; const ACTION_REJECT = 'reject'; @@ -22,83 +24,84 @@ public static function getBasicStoryText($action, $author_name) { switch ($action) { - 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); - break; - 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); - break; - 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); - break; - 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); - break; - 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); - break; - 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); - break; - 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); - break; - 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); - break; - 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.'); - break; - } + 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); + break; + 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); + break; + case DifferentialAction::ACTION_ABANDON: + $title = pht('%s abandoned this revision.', + $author_name); + break; + case DifferentialAction::ACTION_COMMIT_CLOSE: + 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); + break; + 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); + break; + 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); + break; + 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); + break; + 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); + break; + 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); + break; + 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; } @@ -115,6 +118,7 @@ self::ACTION_ADDREVIEWERS => pht('Add Reviewers'), self::ACTION_ADDCCS => pht('Add Subscribers'), self::ACTION_CLOSE => pht('Close Revision'), + self::ACTION_COMMIT_CLOSE => pht('Close Revision'), self::ACTION_CLAIM => pht('Commandeer Revision'), self::ACTION_REOPEN => pht('Reopen'), ); diff --git a/src/applications/differential/editor/DifferentialTransactionEditor.php b/src/applications/differential/editor/DifferentialTransactionEditor.php --- a/src/applications/differential/editor/DifferentialTransactionEditor.php +++ b/src/applications/differential/editor/DifferentialTransactionEditor.php @@ -148,6 +148,7 @@ return $will_add_reviewer; case DifferentialAction::ACTION_CLOSE: + case DifferentialAction::ACTION_COMMIT_CLOSE: return ($object->getStatus() != $status_closed); case DifferentialAction::ACTION_ABANDON: return ($object->getStatus() != $status_abandoned); @@ -235,6 +236,7 @@ $object->setStatus($status_review); return; case DifferentialAction::ACTION_CLOSE: + case DifferentialAction::ACTION_COMMIT_CLOSE: $object->setStatus(ArcanistDifferentialRevisionStatus::CLOSED); return; case DifferentialAction::ACTION_CLAIM: diff --git a/src/applications/differential/storage/DifferentialTransaction.php b/src/applications/differential/storage/DifferentialTransaction.php --- a/src/applications/differential/storage/DifferentialTransaction.php +++ b/src/applications/differential/storage/DifferentialTransaction.php @@ -102,6 +102,17 @@ $new = $this->getNewValue(); switch ($this->getTransactionType()) { + case self::TYPE_ACTION: + if ($new == DifferentialAction::ACTION_COMMIT_CLOSE) { + $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; @@ -234,7 +245,40 @@ $author_handle); } case self::TYPE_ACTION: - return DifferentialAction::getBasicStoryText($new, $author_handle); + switch ($new) { + case DifferentialAction::ACTION_COMMIT_CLOSE: + $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: @@ -300,6 +344,38 @@ '%s closed %s.', $author_link, $object_link); + case DifferentialAction::ACTION_COMMIT_CLOSE: + $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.', @@ -366,6 +442,7 @@ case self::TYPE_ACTION: switch ($this->getNewValue()) { case DifferentialAction::ACTION_CLOSE: + case DifferentialAction::ACTION_COMMIT_CLOSE: return 'fa-check'; case DifferentialAction::ACTION_ACCEPT: return 'fa-check-circle-o'; @@ -433,6 +510,7 @@ case self::TYPE_ACTION: switch ($this->getNewValue()) { case DifferentialAction::ACTION_CLOSE: + case DifferentialAction::ACTION_COMMIT_CLOSE: return PhabricatorTransactions::COLOR_BLUE; case DifferentialAction::ACTION_ACCEPT: return PhabricatorTransactions::COLOR_GREEN; @@ -472,6 +550,7 @@ case DifferentialTransaction::TYPE_ACTION: switch ($this->getNewValue()) { case DifferentialAction::ACTION_CLOSE: + case DifferentialAction::ACTION_COMMIT_CLOSE: return pht('This revision is already closed.'); case DifferentialAction::ACTION_ABANDON: return pht('This revision has already been abandoned.'); diff --git a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php --- a/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php +++ b/src/applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php @@ -140,31 +140,28 @@ $should_autoclose; if ($should_close) { - $commit_name = $repository->formatCommitName( - $commit->getCommitIdentifier()); - - $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 = id(new DifferentialTransaction()) + ->setTransactionType(DifferentialTransaction::TYPE_ACTION) + ->setNewValue(DifferentialAction::ACTION_COMMIT_CLOSE); + + $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 +178,11 @@ } $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)); + $xactions[] = $commit_close_xaction; $content_source = PhabricatorContentSource::newForSource( PhabricatorContentSource::SOURCE_DAEMON, @@ -237,18 +223,6 @@ 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 +495,8 @@ ->setActingAsPHID($acting_as) ->setContinueOnNoEffect(true) ->setContinueOnMissingFields(true) + ->setUnmentionablePHIDMap( + array($commit->getPHID() => $commit->getPHID())) ->setContentSource($content_source); $editor->applyTransactions($task, $xactions);