1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-13 02:12:41 +01:00

Show the difference between a committer and an author

Summary:
Git and hg (supposedly..) differentiate between an author (who wrote the patch)
and a committer (who applied the patch).

This patch allows Phabricator to note when a patch is committed
by someone other than the Author.

Test Plan:
Created 2 accounts,
 - U (Account with a PHID)
 - U' (Account without a PHID)
and had them create and commit commits

testing if their username/real name would be displayed correctly in Diffusion,
  - BrowserTable
  - HistoryTable
  - Code revision

Teztz,
A(uthor)/C(ommitter)
If it's A/A then Author committed

UL = User link (<a href="/p/username">username</a>)
UN = User name ("Firstname Lastname")

Tezt | Expected in table  | Got
-------------------------------------------
A/A   | UL/UL             | UL/UL
A'/C  | UN/UL             | UN/UL
A/C'  | UL/UN             | UL/UN
A'/C' | UN/UN             | UN/UN

Reviewers: epriestley

Reviewed By: epriestley

CC: aran, Koolvin

Maniphest Tasks: T688

Differential Revision: https://secure.phabricator.com/D2541
This commit is contained in:
Hafsteinn Baldvinsson 2012-05-23 08:34:36 -07:00 committed by epriestley
parent e4e56bb431
commit a438c87c52
8 changed files with 95 additions and 7 deletions

View file

@ -287,6 +287,9 @@ final class DiffusionCommitController extends DiffusionController {
if ($data->getCommitDetail('reviewerPHID')) { if ($data->getCommitDetail('reviewerPHID')) {
$phids[] = $data->getCommitDetail('reviewerPHID'); $phids[] = $data->getCommitDetail('reviewerPHID');
} }
if ($data->getCommitDetail('committerPHID')) {
$phids[] = $data->getCommitDetail('committerPHID');
}
if ($data->getCommitDetail('differential.revisionPHID')) { if ($data->getCommitDetail('differential.revisionPHID')) {
$phids[] = $data->getCommitDetail('differential.revisionPHID'); $phids[] = $data->getCommitDetail('differential.revisionPHID');
} }
@ -330,6 +333,17 @@ final class DiffusionCommitController extends DiffusionController {
$props['Reviewer'] = phutil_escape_html($reviewer_name); $props['Reviewer'] = phutil_escape_html($reviewer_name);
} }
$committer = $data->getCommitDetail('committer');
if ($committer) {
$committer_phid = $data->getCommitDetail('committerPHID');
if ($data->getCommitDetail('committerPHID')) {
$props['Committer'] = $handles[$committer_phid]->renderLink();
} else {
$props['Committer'] = phutil_escape_html($committer);
}
}
$revision_phid = $data->getCommitDetail('differential.revisionPHID'); $revision_phid = $data->getCommitDetail('differential.revisionPHID');
if ($revision_phid) { if ($revision_phid) {
$props['Differential Revision'] = $handles[$revision_phid]->renderLink(); $props['Differential Revision'] = $handles[$revision_phid]->renderLink();

View file

@ -27,10 +27,16 @@ final class DiffusionLastModifiedController extends DiffusionController {
list($commit, $commit_data) = $modified_query->loadLastModification(); list($commit, $commit_data) = $modified_query->loadLastModification();
$phids = array(); $phids = array();
if ($commit_data && $commit_data->getCommitDetail('authorPHID')) { if ($commit_data) {
$phids = array($commit_data->getCommitDetail('authorPHID')); if ($commit_data->getCommitDetail('authorPHID')) {
$phids[$commit_data->getCommitDetail('authorPHID')] = true;
}
if ($commit_data->getCommitDetail('committerPHID')) {
$phids[$commit_data->getCommitDetail('committerPHID')] = true;
}
} }
$phids = array_keys($phids);
$handles = id(new PhabricatorObjectHandleData($phids))->loadHandles(); $handles = id(new PhabricatorObjectHandleData($phids))->loadHandles();
$output = DiffusionBrowseTableView::renderLastModifiedColumns( $output = DiffusionBrowseTableView::renderLastModifiedColumns(

View file

@ -45,6 +45,9 @@ final class DiffusionRepositoryController extends DiffusionController {
if ($data->getCommitDetail('authorPHID')) { if ($data->getCommitDetail('authorPHID')) {
$phids[$data->getCommitDetail('authorPHID')] = true; $phids[$data->getCommitDetail('authorPHID')] = true;
} }
if ($data->getCommitDetail('committerPHID')) {
$phids[$data->getCommitDetail('committerPHID')] = true;
}
} }
} }
@ -54,6 +57,9 @@ final class DiffusionRepositoryController extends DiffusionController {
if ($data->getCommitDetail('authorPHID')) { if ($data->getCommitDetail('authorPHID')) {
$phids[$data->getCommitDetail('authorPHID')] = true; $phids[$data->getCommitDetail('authorPHID')] = true;
} }
if ($data->getCommitDetail('committerPHID')) {
$phids[$data->getCommitDetail('committerPHID')] = true;
}
} }
} }

View file

@ -60,11 +60,25 @@ final class DiffusionBrowseTableView extends DiffusionView {
} else { } else {
$author = phutil_escape_html($data->getAuthorName()); $author = phutil_escape_html($data->getAuthorName());
} }
$committer = $data->getCommitDetail('committer');
if ($committer) {
$committer_phid = $data->getCommitDetail('committerPHID');
if ($committer_phid && isset($handles[$committer_phid])) {
$committer = $handles[$committer_phid]->renderLink();
} else {
$committer = phutil_escape_html($data->getCommitDetail('committer'));
}
} else {
$committer = $author;
}
$details = AphrontTableView::renderSingleDisplayLine( $details = AphrontTableView::renderSingleDisplayLine(
phutil_escape_html($data->getSummary())); phutil_escape_html($data->getSummary()));
} else { } else {
$author = ''; $author = '';
$details = ''; $details = '';
$committer = '';
} }
return array( return array(
@ -72,6 +86,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
'date' => $date, 'date' => $date,
'time' => $time, 'time' => $time,
'author' => $author, 'author' => $author,
'committer' => $committer,
'details' => $details, 'details' => $details,
); );
} }
@ -140,6 +155,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
'date' => celerity_generate_unique_node_id(), 'date' => celerity_generate_unique_node_id(),
'time' => celerity_generate_unique_node_id(), 'time' => celerity_generate_unique_node_id(),
'author' => celerity_generate_unique_node_id(), 'author' => celerity_generate_unique_node_id(),
'committer' => celerity_generate_unique_node_id(),
'details' => celerity_generate_unique_node_id(), 'details' => celerity_generate_unique_node_id(),
); );
@ -162,6 +178,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
$dict['date'], $dict['date'],
$dict['time'], $dict['time'],
$dict['author'], $dict['author'],
$dict['committer'],
$dict['details'], $dict['details'],
); );
} }
@ -179,6 +196,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
'Date', 'Date',
'Time', 'Time',
'Author', 'Author',
'Committer',
'Details', 'Details',
)); ));
$view->setColumnClasses( $view->setColumnClasses(
@ -189,6 +207,7 @@ final class DiffusionBrowseTableView extends DiffusionView {
'', '',
'right', 'right',
'', '',
'',
'wide', 'wide',
)); ));
return $view->render(); return $view->render();

View file

@ -43,6 +43,9 @@ final class DiffusionHistoryTableView extends DiffusionView {
if ($data->getCommitDetail('authorPHID')) { if ($data->getCommitDetail('authorPHID')) {
$phids[$data->getCommitDetail('authorPHID')] = true; $phids[$data->getCommitDetail('authorPHID')] = true;
} }
if ($data->getCommitDetail('committerPHID')) {
$phids[$data->getCommitDetail('committerPHID')] = true;
}
} }
} }
return array_keys($phids); return array_keys($phids);
@ -82,9 +85,11 @@ final class DiffusionHistoryTableView extends DiffusionView {
} }
$data = $history->getCommitData(); $data = $history->getCommitData();
$author_phid = null; $author_phid = $committer = $committer_phid = null;
if ($data) { if ($data) {
$author_phid = $data->getCommitDetail('authorPHID'); $author_phid = $data->getCommitDetail('authorPHID');
$committer_phid = $data->getCommitDetail('committerPHID');
$committer = $data->getCommitDetail('committer');
} }
if ($author_phid && isset($handles[$author_phid])) { if ($author_phid && isset($handles[$author_phid])) {
@ -93,6 +98,18 @@ final class DiffusionHistoryTableView extends DiffusionView {
$author = phutil_escape_html($history->getAuthorName()); $author = phutil_escape_html($history->getAuthorName());
} }
if ($committer) {
if ($committer_phid && isset($handles[$committer_phid])) {
$committer = $handles[$committer_phid]->renderLink();
} else {
$committer = phutil_escape_html($committer);
}
}
else {
$committer = $author;
}
$commit = $history->getCommit(); $commit = $history->getCommit();
if ($commit && !$commit->getIsUnparsed() && $data) { if ($commit && !$commit->getIsUnparsed() && $data) {
$change = $this->linkChange( $change = $this->linkChange(
@ -118,6 +135,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
$date, $date,
$time, $time,
$author, $author,
$committer,
AphrontTableView::renderSingleDisplayLine( AphrontTableView::renderSingleDisplayLine(
phutil_escape_html($history->getSummary())), phutil_escape_html($history->getSummary())),
// TODO: etc etc // TODO: etc etc
@ -134,6 +152,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
'Date', 'Date',
'Time', 'Time',
'Author', 'Author',
'Committer',
'Details', 'Details',
)); ));
$view->setColumnClasses( $view->setColumnClasses(
@ -145,6 +164,7 @@ final class DiffusionHistoryTableView extends DiffusionView {
'', '',
'right', 'right',
'', '',
'',
'wide', 'wide',
)); ));
$view->setColumnVisibility( $view->setColumnVisibility(

View file

@ -82,6 +82,15 @@ class PhabricatorRepositoryDefaultCommitMessageDetailParser
unset($details['authorPHID']); unset($details['authorPHID']);
} }
if (isset($details['committer'])) {
$committer_phid = $this->resolveUserPHID($details['committer']);
if ($committer_phid) {
$details['committerPHID'] = $committer_phid;
} else {
unset($details['committerPHID']);
}
}
$data->setCommitDetails($details); $data->setCommitDetails($details);
} }

View file

@ -23,7 +23,9 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
PhabricatorRepository $repository, PhabricatorRepository $repository,
PhabricatorRepositoryCommit $commit); PhabricatorRepositoryCommit $commit);
final protected function updateCommitData($author, $message) { final protected function updateCommitData($author, $message,
$committer = null) {
$commit = $this->commit; $commit = $this->commit;
$data = id(new PhabricatorRepositoryCommitData())->loadOneWhere( $data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
@ -36,6 +38,12 @@ abstract class PhabricatorRepositoryCommitMessageParserWorker
$data->setAuthorName($author); $data->setAuthorName($author);
$data->setCommitMessage($message); $data->setCommitMessage($message);
if ($committer) {
$details = $data->getCommitDetails();
$details['committer'] = $committer;
$data->setCommitDetails($details);
}
$repository = $this->repository; $repository = $this->repository;
$detail_parser = $repository->getDetail( $detail_parser = $repository->getDetail(
'detail-parser', 'detail-parser',

View file

@ -26,17 +26,23 @@ final class PhabricatorRepositoryGitCommitMessageParserWorker
// NOTE: %B was introduced somewhat recently in git's history, so pull // NOTE: %B was introduced somewhat recently in git's history, so pull
// commit message information with %s and %b instead. // commit message information with %s and %b instead.
list($info) = $repository->execxLocalCommand( list($info) = $repository->execxLocalCommand(
"log -n 1 --encoding='UTF-8' --pretty=format:%%an%%x00%%s%%n%%n%%b %s", "log -n 1 --encoding='UTF-8' " .
"--pretty=format:%%cn%%x00%%an%%x00%%s%%n%%n%%b %s",
$commit->getCommitIdentifier()); $commit->getCommitIdentifier());
list($author, $message) = explode("\0", $info); list($committer, $author, $message) = explode("\0", $info);
// Make sure these are valid UTF-8. // Make sure these are valid UTF-8.
$committer = phutil_utf8ize($committer);
$author = phutil_utf8ize($author); $author = phutil_utf8ize($author);
$message = phutil_utf8ize($message); $message = phutil_utf8ize($message);
$message = trim($message); $message = trim($message);
$this->updateCommitData($author, $message); if ($committer == $author) {
$committer = null;
}
$this->updateCommitData($author, $message, $committer);
if ($this->shouldQueueFollowupTasks()) { if ($this->shouldQueueFollowupTasks()) {
$task = new PhabricatorWorkerTask(); $task = new PhabricatorWorkerTask();