diff --git a/src/applications/audit/view/PhabricatorAuditListView.php b/src/applications/audit/view/PhabricatorAuditListView.php --- a/src/applications/audit/view/PhabricatorAuditListView.php +++ b/src/applications/audit/view/PhabricatorAuditListView.php @@ -90,11 +90,6 @@ foreach ($commit->getAudits() as $audit) { $phids[] = $audit->getAuditorPHID(); } - - $author_phid = $commit->getAuthorPHID(); - if ($author_phid) { - $phids[] = $author_phid; - } } $handles = $viewer->loadHandles($phids); @@ -126,21 +121,18 @@ $status_color = $status->getColor(); $status_icon = $status->getIcon(); - $author_phid = $commit->getAuthorPHID(); - if ($author_phid) { - $author_name = $handles[$author_phid]->renderLink(); - } else { - $author_name = $commit->getCommitData()->getAuthorName(); - } - $item = id(new PHUIObjectItemView()) ->setObjectName($commit_name) ->setHeader($commit_desc) ->setHref($commit_link) ->setDisabled($commit->isUnreachable()) - ->addByline(pht('Author: %s', $author_name)) ->addIcon('none', $committed); + $author_name = $commit->newCommitAuthorView($viewer); + if ($author_name) { + $item->addByline(pht('Author: %s', $author_name)); + } + if ($show_drafts) { if ($commit->getHasDraft($viewer)) { $item->addAttribute($draft_icon); diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -625,33 +625,50 @@ } } - $author_epoch = $data->getCommitDetail('authorEpoch'); + $provenance_list = new PHUIStatusListView(); + + $author_view = $commit->newCommitAuthorView($viewer); + if ($author_view) { + $author_date = $data->getCommitDetail('authorEpoch'); + $author_date = phabricator_datetime($author_date, $viewer); + + $provenance_list->addItem( + id(new PHUIStatusItemView()) + ->setTarget($author_view) + ->setNote(pht('Authored on %s', $author_date))); + } - $committed_info = id(new PHUIStatusItemView()) - ->setNote(phabricator_datetime($commit->getEpoch(), $viewer)) - ->setTarget($commit->renderAnyCommitter($viewer, $handles)); + if (!$commit->isAuthorSameAsCommitter()) { + $committer_view = $commit->newCommitCommitterView($viewer); + if ($committer_view) { + $committer_date = $commit->getEpoch(); + $committer_date = phabricator_datetime($committer_date, $viewer); - $committed_list = new PHUIStatusListView(); - $committed_list->addItem($committed_info); - $view->addProperty( - pht('Committed'), - $committed_list); + $provenance_list->addItem( + id(new PHUIStatusItemView()) + ->setTarget($committer_view) + ->setNote(pht('Committed on %s', $committer_date))); + } + } if ($push_logs) { $pushed_list = new PHUIStatusListView(); foreach ($push_logs as $push_log) { - $pushed_item = id(new PHUIStatusItemView()) - ->setTarget($handles[$push_log->getPusherPHID()]->renderLink()) - ->setNote(phabricator_datetime($push_log->getEpoch(), $viewer)); - $pushed_list->addItem($pushed_item); - } + $pusher_date = $push_log->getEpoch(); + $pusher_date = phabricator_datetime($pusher_date, $viewer); - $view->addProperty( - pht('Pushed'), - $pushed_list); + $pusher_view = $handles[$push_log->getPusherPHID()]->renderLink(); + + $provenance_list->addItem( + id(new PHUIStatusItemView()) + ->setTarget($pusher_view) + ->setNote(pht('Pushed on %s', $pusher_date))); + } } + $view->addProperty(pht('Provenance'), $provenance_list); + $reviewer_phid = $data->getCommitDetail('reviewerPHID'); if ($reviewer_phid) { $view->addProperty( diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommit.php b/src/applications/repository/storage/PhabricatorRepositoryCommit.php --- a/src/applications/repository/storage/PhabricatorRepositoryCommit.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommit.php @@ -377,52 +377,6 @@ return $repository->formatCommitName($identifier, $local = true); } - /** - * Make a strong effort to find a way to render this commit's committer. - * This currently attempts to use @{PhabricatorRepositoryIdentity}, and - * falls back to examining the commit detail information. After we force - * the migration to using identities, update this method to remove the - * fallback. See T12164 for details. - */ - public function renderAnyCommitter(PhabricatorUser $viewer, $handles) { - $committer = $this->renderCommitter($viewer, $handles); - if ($committer) { - return $committer; - } - - return $this->renderAuthor($viewer, $handles); - } - - public function renderCommitter(PhabricatorUser $viewer, $handles) { - $committer_phid = $this->getCommitterDisplayPHID(); - if ($committer_phid) { - return $handles[$committer_phid]->renderLink(); - } - - $data = $this->getCommitData(); - $committer_name = $data->getCommitDetail('committer'); - if (strlen($committer_name)) { - return DiffusionView::renderName($committer_name); - } - - return null; - } - - public function renderAuthor(PhabricatorUser $viewer, $handles) { - $author_phid = $this->getAuthorDisplayPHID(); - if ($author_phid) { - return $handles[$author_phid]->renderLink(); - } - - $data = $this->getCommitData(); - $author_name = $data->getAuthorName(); - if (strlen($author_name)) { - return DiffusionView::renderName($author_name); - } - - return null; - } - public function loadIdentities(PhabricatorUser $viewer) { if ($this->authorIdentity !== self::ATTACHABLE) { return $this; @@ -511,6 +465,65 @@ return (bool)$this->isPartiallyImported(self::IMPORTED_CLOSEABLE); } + public function newCommitAuthorView(PhabricatorUser $viewer) { + $author_phid = $this->getAuthorDisplayPHID(); + if ($author_phid) { + $handles = $viewer->loadHandles(array($author_phid)); + return $handles[$author_phid]->renderLink(); + } + + $author = $this->getRawAuthorStringForDisplay(); + if (strlen($author)) { + return DiffusionView::renderName($author); + } + + return null; + } + + public function newCommitCommitterView(PhabricatorUser $viewer) { + $committer_phid = $this->getCommitterDisplayPHID(); + if ($committer_phid) { + $handles = $viewer->loadHandles(array($committer_phid)); + return $handles[$committer_phid]->renderLink(); + } + + $committer = $this->getRawCommitterStringForDisplay(); + if (strlen($committer)) { + return DiffusionView::renderName($committer); + } + + return null; + } + + public function isAuthorSameAsCommitter() { + $author_phid = $this->getAuthorDisplayPHID(); + $committer_phid = $this->getCommitterDisplayPHID(); + + if ($author_phid && $committer_phid) { + return ($author_phid === $committer_phid); + } + + if ($author_phid || $committer_phid) { + return false; + } + + $author = $this->getRawAuthorStringForDisplay(); + $committer = $this->getRawCommitterStringForDisplay(); + + return ($author === $committer); + } + + private function getRawAuthorStringForDisplay() { + $data = $this->getCommitData(); + return $data->getAuthorName(); + } + + private function getRawCommitterStringForDisplay() { + $data = $this->getCommitData(); + return $data->getCommitDetail('committer'); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ public function getCapabilities() {