mirror of
https://we.phorge.it/source/phorge.git
synced 2024-12-30 01:10:58 +01:00
Make Herald less ambitious about resolving repositories for revisions
Summary: Fixes T4636. If a user manually deletes a "repository" setting from a revision, Herald attempts to resolve it. Instead, Herald should now just trust Differential. Generally, the new logic is: - When diffs are created, figure out repository information. - When revisions are updated, copy info from diffs. - Everywhere else, just trust the revision field. Test Plan: - Created revisions. - Used Herald to dry-run revisions before and after a manual edit to remove the repository setting. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4636 Differential Revision: https://secure.phabricator.com/D8576
This commit is contained in:
parent
70ed1ff7d0
commit
03c6bf0d09
3 changed files with 22 additions and 22 deletions
|
@ -40,6 +40,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
|
||||||
}
|
}
|
||||||
|
|
||||||
protected function execute(ConduitAPIRequest $request) {
|
protected function execute(ConduitAPIRequest $request) {
|
||||||
|
$viewer = $request->getUser();
|
||||||
$change_data = $request->getValue('changes');
|
$change_data = $request->getValue('changes');
|
||||||
|
|
||||||
$changes = array();
|
$changes = array();
|
||||||
|
@ -53,7 +54,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
|
||||||
|
|
||||||
$diff->setBranch($request->getValue('branch'));
|
$diff->setBranch($request->getValue('branch'));
|
||||||
$diff->setCreationMethod($request->getValue('creationMethod'));
|
$diff->setCreationMethod($request->getValue('creationMethod'));
|
||||||
$diff->setAuthorPHID($request->getUser()->getPHID());
|
$diff->setAuthorPHID($viewer->getPHID());
|
||||||
$diff->setBookmark($request->getValue('bookmark'));
|
$diff->setBookmark($request->getValue('bookmark'));
|
||||||
|
|
||||||
// TODO: Remove this eventually; for now continue writing the UUID. Note
|
// TODO: Remove this eventually; for now continue writing the UUID. Note
|
||||||
|
@ -64,7 +65,7 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
|
||||||
$repository_phid = $request->getValue('repositoryPHID');
|
$repository_phid = $request->getValue('repositoryPHID');
|
||||||
if ($repository_phid) {
|
if ($repository_phid) {
|
||||||
$repository = id(new PhabricatorRepositoryQuery())
|
$repository = id(new PhabricatorRepositoryQuery())
|
||||||
->setViewer($request->getUser())
|
->setViewer($viewer)
|
||||||
->withPHIDs(array($repository_phid))
|
->withPHIDs(array($repository_phid))
|
||||||
->executeOne();
|
->executeOne();
|
||||||
if ($repository) {
|
if ($repository) {
|
||||||
|
@ -142,6 +143,22 @@ final class ConduitAPI_differential_creatediff_Method extends ConduitAPIMethod {
|
||||||
|
|
||||||
$diff->save();
|
$diff->save();
|
||||||
|
|
||||||
|
// If we didn't get an explicit `repositoryPHID` (which means the client is
|
||||||
|
// old, or couldn't figure out which repository the working copy belongs
|
||||||
|
// to), apply heuristics to try to figure it out.
|
||||||
|
|
||||||
|
if (!$repository_phid) {
|
||||||
|
$repository = id(new DifferentialRepositoryLookup())
|
||||||
|
->setDiff($diff)
|
||||||
|
->setViewer($viewer)
|
||||||
|
->lookupRepository();
|
||||||
|
if ($repository) {
|
||||||
|
$diff->setRepositoryPHID($repository->getPHID());
|
||||||
|
$diff->setRepositoryUUID($repository->getUUID());
|
||||||
|
$diff->save();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
$path = '/differential/diff/'.$diff->getID().'/';
|
$path = '/differential/diff/'.$diff->getID().'/';
|
||||||
$uri = PhabricatorEnv::getURI($path);
|
$uri = PhabricatorEnv::getURI($path);
|
||||||
|
|
||||||
|
|
|
@ -498,6 +498,7 @@ final class DifferentialTransactionEditor
|
||||||
|
|
||||||
$object->setLineCount($diff->getLineCount());
|
$object->setLineCount($diff->getLineCount());
|
||||||
$object->setRepositoryPHID($diff->getRepositoryPHID());
|
$object->setRepositoryPHID($diff->getRepositoryPHID());
|
||||||
|
$object->setArcanistProjectPHID($diff->getArcanistProjectPHID());
|
||||||
|
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
|
@ -157,28 +157,10 @@ final class HeraldDifferentialRevisionAdapter extends HeraldAdapter {
|
||||||
public function loadRepository() {
|
public function loadRepository() {
|
||||||
if ($this->repository === null) {
|
if ($this->repository === null) {
|
||||||
$this->repository = false;
|
$this->repository = false;
|
||||||
|
$repository_phid = $this->getObject()->getRepositoryPHID();
|
||||||
$viewer = PhabricatorUser::getOmnipotentUser();
|
|
||||||
$repository_phid = null;
|
|
||||||
|
|
||||||
$revision = $this->revision;
|
|
||||||
if ($revision->getRepositoryPHID()) {
|
|
||||||
$repository_phid = $revision->getRepositoryPHID();
|
|
||||||
} else {
|
|
||||||
$repository = id(new DifferentialRepositoryLookup())
|
|
||||||
->setViewer($viewer)
|
|
||||||
->setDiff($this->diff)
|
|
||||||
->lookupRepository();
|
|
||||||
if ($repository) {
|
|
||||||
// We want to get the projects for this repository too, so run a
|
|
||||||
// full query for it below.
|
|
||||||
$repository_phid = $repository->getPHID();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
if ($repository_phid) {
|
if ($repository_phid) {
|
||||||
$repository = id(new PhabricatorRepositoryQuery())
|
$repository = id(new PhabricatorRepositoryQuery())
|
||||||
->setViewer($viewer)
|
->setViewer(PhabricatorUser::getOmnipotentUser())
|
||||||
->withPHIDs(array($repository_phid))
|
->withPHIDs(array($repository_phid))
|
||||||
->needProjectPHIDs(true)
|
->needProjectPHIDs(true)
|
||||||
->executeOne();
|
->executeOne();
|
||||||
|
|
Loading…
Reference in a new issue