From 46d496b8cc061db11954bcd2c03b882a5573eaad Mon Sep 17 00:00:00 2001 From: lkassianik Date: Tue, 5 Dec 2017 19:24:57 +0000 Subject: [PATCH] Fix error for URL's that could mean several commits Summary: Ref T13001, URLs that return multiple commits should show a list of those commits. Not sure if the actual list looks very pretty this way, but was wondering if this approach was vaguely correct. Test Plan: - Navigate to `install/rPbd3c23` - User should see a list view providing links to `install/rPbd3c2355e8e2b220ae5e3cbfe4a057c8088c6a38` and `install/rPbd3c239d5aada68a31db5742bbb8ec099074a561` Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin Maniphest Tasks: T13001 Differential Revision: https://secure.phabricator.com/D18816 --- .../controller/DiffusionCommitController.php | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index cdce8702ca..11463ef97d 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -39,20 +39,23 @@ final class DiffusionCommitController extends DiffusionController { return $this->buildRawDiffResponse($drequest); } - $commit = id(new DiffusionCommitQuery()) + $commits = id(new DiffusionCommitQuery()) ->setViewer($viewer) ->withRepository($repository) ->withIdentifiers(array($commit_identifier)) ->needCommitData(true) ->needAuditRequests(true) - ->executeOne(); + ->setLimit(100) + ->execute(); + + $multiple_results = count($commits) > 1; $crumbs = $this->buildCrumbs(array( - 'commit' => true, + 'commit' => !$multiple_results, )); $crumbs->setBorder(true); - if (!$commit) { + if (!$commits) { if (!$this->getCommitExists()) { return new Aphront404Response(); } @@ -70,7 +73,40 @@ final class DiffusionCommitController extends DiffusionController { ->setTitle($title) ->setCrumbs($crumbs) ->appendChild($error); + } else if ($multiple_results) { + $warning_message = + pht( + 'The identifier %s is ambiguous and matches more than one commit.', + phutil_tag( + 'strong', + array(), + $commit_identifier)); + + $error = id(new PHUIInfoView()) + ->setTitle(pht('Ambiguous Commit')) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->appendChild($warning_message); + + $list = id(new DiffusionCommitListView()) + ->setViewer($viewer) + ->setCommits($commits) + ->setNoDataString(pht('No recent commits.')); + + $crumbs->addTextCrumb(pht('Ambiguous Commit')); + + $matched_commits = id(new PHUITwoColumnView()) + ->setFooter(array( + $error, + $list, + )); + + return $this->newPage() + ->setTitle(pht('Ambiguous Commit')) + ->setCrumbs($crumbs) + ->appendChild($matched_commits); + } else { + $commit = head($commits); } $audit_requests = $commit->getAudits();