Skip to content

Commit efe187d

Browse files
author
epriestley
committed
Support "Repository's projects" field in Commit and Differential Revision rules
Summary: This also cleans up some code a little bit. Most of the gymnastics are to make sure we call `needProjectPHIDs()` appropriately. Test Plan: Created new commit and revision rules with this field. Ran commits and revisions through the test console. Field behavior seemed correct. Reviewers: btrahan Reviewed By: btrahan CC: aran, dctrwatson Differential Revision: https://secure.phabricator.com/D7923
1 parent c020837 commit efe187d

File tree

5 files changed

+74
-33
lines changed

5 files changed

+74
-33
lines changed

src/applications/herald/adapter/HeraldCommitAdapter.php

Lines changed: 42 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,19 +64,24 @@ public function canTriggerOnObject($object) {
6464
if ($object instanceof PhabricatorRepository) {
6565
return true;
6666
}
67+
if ($object instanceof PhabricatorProject) {
68+
return true;
69+
}
6770
return false;
6871
}
6972

7073
public function getTriggerObjectPHIDs() {
71-
return array(
72-
$this->repository->getPHID(),
73-
$this->getPHID(),
74-
);
74+
return array_merge(
75+
array(
76+
$this->repository->getPHID(),
77+
$this->getPHID(),
78+
),
79+
$this->repository->getProjectPHIDs());
7580
}
7681

7782
public function explainValidTriggerObjects() {
7883
return pht(
79-
'This rule can trigger for **repositories**.');
84+
'This rule can trigger for **repositories** and **projects**.');
8085
}
8186

8287
public function getFieldNameMap() {
@@ -95,6 +100,7 @@ public function getFields() {
95100
self::FIELD_COMMITTER,
96101
self::FIELD_REVIEWER,
97102
self::FIELD_REPOSITORY,
103+
self::FIELD_REPOSITORY_PROJECTS,
98104
self::FIELD_DIFF_FILE,
99105
self::FIELD_DIFF_CONTENT,
100106
self::FIELD_DIFF_ADDED_CONTENT,
@@ -177,6 +183,35 @@ public static function newLegacyAdapter(
177183
return $object;
178184
}
179185

186+
public function setCommit(PhabricatorRepositoryCommit $commit) {
187+
$viewer = PhabricatorUser::getOmnipotentUser();
188+
189+
$repository = id(new PhabricatorRepositoryQuery())
190+
->setViewer($viewer)
191+
->withIDs(array($commit->getRepositoryID()))
192+
->needProjectPHIDs(true)
193+
->executeOne();
194+
if (!$repository) {
195+
throw new Exception(pht('Unable to load repository!'));
196+
}
197+
198+
$data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
199+
'commitID = %d',
200+
$commit->getID());
201+
if (!$data) {
202+
throw new Exception(pht('Unable to load commit data!'));
203+
}
204+
205+
$this->commit = clone $commit;
206+
$this->commit->attachRepository($repository);
207+
$this->commit->attachCommitData($data);
208+
209+
$this->repository = $repository;
210+
$this->commitData = $data;
211+
212+
return $this;
213+
}
214+
180215
public function getPHID() {
181216
return $this->commit->getPHID();
182217
}
@@ -375,6 +410,8 @@ public function getHeraldField($field) {
375410
return $this->loadAffectedPaths();
376411
case self::FIELD_REPOSITORY:
377412
return $this->repository->getPHID();
413+
case self::FIELD_REPOSITORY_PROJECTS:
414+
return $this->repository->getProjectPHIDs();
378415
case self::FIELD_DIFF_CONTENT:
379416
return $this->getDiffContent('*');
380417
case self::FIELD_DIFF_ADDED_CONTENT:

src/applications/herald/adapter/HeraldDifferentialRevisionAdapter.php

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ public function getFields() {
6464
self::FIELD_REVIEWERS,
6565
self::FIELD_CC,
6666
self::FIELD_REPOSITORY,
67+
self::FIELD_REPOSITORY_PROJECTS,
6768
self::FIELD_DIFF_FILE,
6869
self::FIELD_DIFF_CONTENT,
6970
self::FIELD_DIFF_ADDED_CONTENT,
@@ -152,32 +153,36 @@ public function loadRepository() {
152153
if ($this->repository === null) {
153154
$this->repository = false;
154155

155-
// TODO: (T603) Implement policy stuff in Herald.
156156
$viewer = PhabricatorUser::getOmnipotentUser();
157+
$repository_phid = null;
157158

158159
$revision = $this->revision;
159160
if ($revision->getRepositoryPHID()) {
160-
$repositories = id(new PhabricatorRepositoryQuery())
161+
$repository_phid = $revision->getRepositoryPHID();
162+
} else {
163+
$repository = id(new DifferentialRepositoryLookup())
161164
->setViewer($viewer)
162-
->withPHIDs(array($revision->getRepositoryPHID()))
163-
->execute();
164-
if ($repositories) {
165-
$this->repository = head($repositories);
166-
return $this->repository;
165+
->setDiff($this->diff)
166+
->lookupRepository();
167+
if ($repository) {
168+
// We want to get the projects for this repository too, so run a
169+
// full query for it below.
170+
$repository_phid = $repository->getPHID();
167171
}
168172
}
169173

170-
$repository = id(new DifferentialRepositoryLookup())
171-
->setViewer($viewer)
172-
->setDiff($this->diff)
173-
->lookupRepository();
174-
if ($repository) {
175-
$this->repository = $repository;
176-
return $this->repository;
174+
if ($repository_phid) {
175+
$repository = id(new PhabricatorRepositoryQuery())
176+
->setViewer($viewer)
177+
->withPHIDs(array($repository_phid))
178+
->needProjectPHIDs(true)
179+
->executeOne();
180+
if ($repository) {
181+
$this->repository = $repository;
182+
}
177183
}
178-
179-
$repository = false;
180184
}
185+
181186
return $this->repository;
182187
}
183188

@@ -345,6 +350,12 @@ public function getHeraldField($field) {
345350
return null;
346351
}
347352
return $repository->getPHID();
353+
case self::FIELD_REPOSITORY_PROJECTS:
354+
$repository = $this->loadRepository();
355+
if (!$repository) {
356+
return null;
357+
}
358+
return $repository->getProjectPHIDs();
348359
case self::FIELD_DIFF_CONTENT:
349360
return $this->loadContentDictionary();
350361
case self::FIELD_DIFF_ADDED_CONTENT:

src/applications/herald/controller/HeraldTestConsoleController.php

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -39,13 +39,8 @@ public function processRequest() {
3939
$object,
4040
$object->loadActiveDiff());
4141
} else if ($object instanceof PhabricatorRepositoryCommit) {
42-
$data = id(new PhabricatorRepositoryCommitData())->loadOneWhere(
43-
'commitID = %d',
44-
$object->getID());
45-
$adapter = HeraldCommitAdapter::newLegacyAdapter(
46-
$object->getRepository(),
47-
$object,
48-
$data);
42+
$adapter = id(new HeraldCommitAdapter())
43+
->setCommit($object);
4944
} else if ($object instanceof ManiphestTask) {
5045
$adapter = id(new HeraldManiphestTaskAdapter())
5146
->setTask($object);

src/applications/herald/storage/HeraldRule.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ final class HeraldRule extends HeraldDAO
1717
protected $isDisabled = 0;
1818
protected $triggerObjectPHID;
1919

20-
protected $configVersion = 26;
20+
protected $configVersion = 27;
2121

2222
// phids for which this rule has been applied
2323
private $ruleApplied = self::ATTACHABLE;

src/applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -42,10 +42,8 @@ private function applyHeraldRules(
4242
'or no longer exists.'));
4343
}
4444

45-
$adapter = HeraldCommitAdapter::newLegacyAdapter(
46-
$repository,
47-
$commit,
48-
$data);
45+
$adapter = id(new HeraldCommitAdapter())
46+
->setCommit($commit);
4947

5048
$rules = id(new HeraldRuleQuery())
5149
->setViewer(PhabricatorUser::getOmnipotentUser())

0 commit comments

Comments
 (0)