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 @@ -46,6 +46,7 @@ ->needCommitData(true) ->needAuditRequests(true) ->setLimit(100) + ->needIdentities(true) ->execute(); $multiple_results = count($commits) > 1; @@ -504,15 +505,13 @@ $phids = $edge_query->getDestinationPHIDs(array($commit_phid)); - if ($data->getCommitDetail('authorPHID')) { - $phids[] = $data->getCommitDetail('authorPHID'); - } + if ($data->getCommitDetail('reviewerPHID')) { $phids[] = $data->getCommitDetail('reviewerPHID'); } - if ($data->getCommitDetail('committerPHID')) { - $phids[] = $data->getCommitDetail('committerPHID'); - } + + $phids[] = $commit->getCommitterDisplayPHID(); + $phids[] = $commit->getAuthorDisplayPHID(); // NOTE: We should never normally have more than a single push log, but // it can occur naturally if a commit is pushed, then the branch it was @@ -573,24 +572,11 @@ } } - $author_phid = $data->getCommitDetail('authorPHID'); - $author_name = $data->getAuthorName(); $author_epoch = $data->getCommitDetail('authorEpoch'); $committed_info = id(new PHUIStatusItemView()) - ->setNote(phabricator_datetime($commit->getEpoch(), $viewer)); - - $committer_phid = $data->getCommitDetail('committerPHID'); - $committer_name = $data->getCommitDetail('committer'); - if ($committer_phid) { - $committed_info->setTarget($handles[$committer_phid]->renderLink()); - } else if (strlen($committer_name)) { - $committed_info->setTarget($committer_name); - } else if ($author_phid) { - $committed_info->setTarget($handles[$author_phid]->renderLink()); - } else if (strlen($author_name)) { - $committed_info->setTarget($author_name); - } + ->setNote(phabricator_datetime($commit->getEpoch(), $viewer)) + ->setTarget($commit->renderAnyCommitter($viewer, $handles)); $committed_list = new PHUIStatusListView(); $committed_list->addItem($committed_info); @@ -716,7 +702,7 @@ return null; } - $author_phid = $data->getCommitDetail('authorPHID'); + $author_phid = $commit->getAuthorDisplayPHID(); $author_name = $data->getAuthorName(); $author_epoch = $data->getCommitDetail('authorEpoch'); $date = null; @@ -748,10 +734,8 @@ ->setImage($image_uri) ->setImageHref($image_href) ->setContent($content); - } - private function buildComments(PhabricatorRepositoryCommit $commit) { $timeline = $this->buildTransactionTimeline( $commit, 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 @@ -439,6 +439,77 @@ 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, array $handles) { + $committer = $this->renderCommitter($viewer, $handles); + if ($committer) { + return $committer; + } + + return $this->renderAuthor($viewer, $handles); + } + + public function renderCommitter(PhabricatorUser $viewer, array $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 $committer_name; + } + + return null; + } + + public function renderAuthor(PhabricatorUser $viewer, array $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 $author_name; + } + + return null; + } + + public function hasCommitterIdentity() { + return ($this->getCommitterIdentity() !== null); + } + + public function hasAuthorIdentity() { + return ($this->getAuthorIdentity() !== null); + } + + public function getCommitterDisplayPHID() { + if ($this->hasCommitterIdentity()) { + return $this->getCommitterIdentity()->getIdentityDisplayPHID(); + } + + $data = $this->getCommitData(); + return $data->getCommitDetail('committerPHID'); + } + + public function getAuthorDisplayPHID() { + if ($this->hasAuthorIdentity()) { + return $this->getAuthorIdentity()->getIdentityDisplayPHID(); + } + + $data = $this->getCommitData(); + return $data->getCommitDetail('authorPHID'); + } /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/repository/storage/PhabricatorRepositoryIdentity.php b/src/applications/repository/storage/PhabricatorRepositoryIdentity.php --- a/src/applications/repository/storage/PhabricatorRepositoryIdentity.php +++ b/src/applications/repository/storage/PhabricatorRepositoryIdentity.php @@ -78,6 +78,14 @@ return ($this->currentEffectiveUserPHID != null); } + public function getIdentityDisplayPHID() { + if ($this->hasEffectiveUser()) { + return $this->getCurrentEffectiveUserPHID(); + } else { + return $this->getPHID(); + } + } + public function save() { if ($this->manuallySetUserPHID) { $this->currentEffectiveUserPHID = $this->manuallySetUserPHID;