1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-26 16:52:41 +01:00

Fix an issue with pulling Subversion blame data

Summary:
Fixes T4067. The way `DiffusionCommitQuery` works prevents it from loading SVN identifiers in some cases without additional constraints, since "12345" might be an SVN revision 12345, or it might be the first 5 characters of a Git commit hash.

Introduce `withRepository()` as a shorthand for `withDefaultRepository()` + `withRepositoryIDs()`. This tells the query to:

  - Only look in the given repository; and
  - use the more liberal identifier resolution rules while doing so.

The practical impact this has is that blame tooltips in SVN work again. The other queries which are fixed here were never run in SVN (which doesn't have first-class branches or tags); I've cleaned them up only for completeness.

Test Plan:
  - Viewed blame in SVN, saw information again instead of empty tooltip.
  - Viewed brnaches/tags in Mercurial and Git.

{F79226}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T4067

Differential Revision: https://secure.phabricator.com/D7523
This commit is contained in:
epriestley 2013-11-07 12:10:43 -08:00
parent 9084f1fe8c
commit f68d67530a
6 changed files with 17 additions and 6 deletions

View file

@ -36,7 +36,7 @@ final class DiffusionBranchTableController extends DiffusionController {
$commits = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier'))
->withRepositoryIDs(array($repository->getID()))
->withRepository($repository)
->execute();
$view = id(new DiffusionBranchTableView())

View file

@ -555,7 +555,7 @@ final class DiffusionBrowseFileController extends DiffusionBrowseController {
if ($commits) {
$commits = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withRepositoryIDs(array($drequest->getRepository()->getID()))
->withRepository($drequest->getRepository())
->withIdentifiers($commits)
->execute();
$commits = mpull($commits, null, 'getCommitIdentifier');

View file

@ -268,7 +268,7 @@ final class DiffusionRepositoryController extends DiffusionController {
$commits = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withIdentifiers(mpull($branches, 'getHeadCommitIdentifier'))
->withRepositoryIDs(array($drequest->getRepository()->getID()))
->withRepository($drequest->getRepository())
->execute();
$table = id(new DiffusionBranchTableView())
@ -332,7 +332,7 @@ final class DiffusionRepositoryController extends DiffusionController {
$commits = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withIdentifiers(mpull($tags, 'getCommitIdentifier'))
->withRepositoryIDs(array($drequest->getRepository()->getID()))
->withRepository($drequest->getRepository())
->needCommitData(true)
->execute();

View file

@ -50,7 +50,7 @@ final class DiffusionTagListController extends DiffusionController {
} else {
$commits = id(new DiffusionCommitQuery())
->setViewer($viewer)
->withRepositoryIDs(array($repository->getID()))
->withRepository($repository)
->withIdentifiers(mpull($tags, 'getCommitIdentifier'))
->needCommitData(true)
->execute();

View file

@ -52,6 +52,17 @@ final class DiffusionCommitQuery
return $this;
}
/**
* Look up commits in a specific repository. This is a shorthand for calling
* @{method:withDefaultRepository} and @{method:withRepositoryIDs}.
*/
public function withRepository(PhabricatorRepository $repository) {
$this->withDefaultRepository($repository);
$this->withRepositoryIDs(array($repository->getID()));
return $this;
}
public function needCommitData($need) {
$this->needCommitData = $need;
return $this;

View file

@ -81,7 +81,7 @@ final class DiffusionRenameHistoryQuery {
$commit = id(new DiffusionCommitQuery())
->setViewer($this->viewer)
->withIdentifiers(array($commit_identifier))
->withDefaultRepository($this->request->getRepository())
->withRepository($this->request->getRepository())
->executeOne();
return $commit->getID();
}