Skip to content

Commit c13b7da

Browse files
author
jungejason
committed
Add Related Commits for Owners
Summary: For each commit, find the affected packages, and provide a way to search by package. Test Plan: create commits that touch and don't touch two packages, and verify that they display correctly in all the UI pages. Reviewers: epriestley, blair, nh, tuomaspelkonen Reviewed By: epriestley CC: benmathews, aran, epriestley, btrahan, jungejason, mpodobnik, prithvi Maniphest Tasks: T83 Differential Revision: 1208
1 parent d739d8c commit c13b7da

File tree

18 files changed

+524
-32
lines changed

18 files changed

+524
-32
lines changed

‎resources/sql/patches/085.packagecommitrelationship.sql‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
CREATE TABLE IF NOT EXISTS phabricator_owners.owners_packagecommitrelationship (
2+
`id` int(10) unsigned NOT NULL AUTO_INCREMENT,
3+
`packagePHID` varchar(64) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL,
4+
`commitPHID` varchar(64) CHARACTER SET latin1 COLLATE latin1_bin NOT NULL,
5+
PRIMARY KEY (`id`),
6+
KEY `packagePHID` (`packagePHID`),
7+
KEY `commitPHID` (`commitPHID`)
8+
) ENGINE=InnoDB DEFAULT CHARSET=latin1;

‎scripts/repository/reparse.php‎

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@
2626
$reparse_message = false;
2727
$reparse_change = false;
2828
$reparse_herald = false;
29+
$reparse_owners = false;
2930
$reparse_what = false;
31+
$force = false;
3032

3133
$args = array_slice($argv, 1);
3234
foreach ($args as $arg) {
@@ -47,6 +49,12 @@
4749
case 'herald':
4850
$reparse_herald = true;
4951
break;
52+
case 'owners':
53+
$reparse_owners = true;
54+
break;
55+
case 'force':
56+
$force = true;
57+
break;
5058
case 'trace':
5159
PhutilServiceProfiler::installEchoListener();
5260
break;
@@ -67,9 +75,21 @@
6775
if (!$reparse_what) {
6876
usage("Specify a commit or repository to reparse.");
6977
}
70-
if (!$reparse_message && !$reparse_change && !$reparse_herald) {
71-
usage("Specify what information to reparse with --message, --change, and/or ".
72-
"--herald.");
78+
if (!$reparse_message && !$reparse_change && !$reparse_herald &&
79+
!$reparse_owners) {
80+
usage("Specify what information to reparse with --message, --change, ".
81+
"--herald, and/or --owners");
82+
}
83+
if ($reparse_owners && !$force) {
84+
echo phutil_console_wrap(
85+
"You are about to recreate the relationship entries between the commits ".
86+
"and the packages they touch. This might delete some existing ".
87+
"relationship entries for some old commits.");
88+
89+
if (!phutil_console_confirm('Are you ready to continue?')) {
90+
echo "Cancelled.\n";
91+
exit(1);
92+
}
7393
}
7494

7595
$commits = array();
@@ -157,6 +177,10 @@
157177
$classes[] = 'PhabricatorRepositoryCommitHeraldWorker';
158178
}
159179

180+
if ($reparse_owners) {
181+
$classes[] = 'PhabricatorRepositoryCommitOwnersWorker';
182+
}
183+
160184
$spec = array(
161185
'commitID' => $commit->getID(),
162186
'only' => true,
@@ -194,7 +218,7 @@ function help() {
194218
$help = <<<EOHELP
195219
**SUMMARY**
196220
197-
**reparse.php** __what__ __which_parts__ [--trace]
221+
**reparse.php** __what__ __which_parts__ [--trace] [--force]
198222
199223
Rerun the Diffusion parser on specific commits and repositories. Mostly
200224
useful for debugging changes to Diffusion.
@@ -221,6 +245,11 @@ function help() {
221245
__--herald__
222246
Reevaluate Herald rules (may send huge amounts of email!)
223247
248+
__--owners__
249+
Reevaluate related commits for owners packages (may delete existing
250+
relationship entries between your package and some old commits!)
251+
252+
__--force__: act noninteractively, without prompting
224253
__--trace__: run with debug tracing
225254
__--help__: show this help
226255

‎src/__phutil_library_map__.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,8 @@
526526
'PhabricatorObjectHandleData' => 'applications/phid/handle/data',
527527
'PhabricatorObjectHandleStatus' => 'applications/phid/handle/const/status',
528528
'PhabricatorObjectSelectorDialog' => 'view/control/objectselector',
529+
'PhabricatorOwnerPathQuery' => 'applications/owners/query/path',
530+
'PhabricatorOwnerRelatedListController' => 'applications/owners/controller/relatedlist',
529531
'PhabricatorOwnersController' => 'applications/owners/controller/base',
530532
'PhabricatorOwnersDAO' => 'applications/owners/storage/base',
531533
'PhabricatorOwnersDeleteController' => 'applications/owners/controller/delete',
@@ -534,6 +536,7 @@
534536
'PhabricatorOwnersListController' => 'applications/owners/controller/list',
535537
'PhabricatorOwnersOwner' => 'applications/owners/storage/owner',
536538
'PhabricatorOwnersPackage' => 'applications/owners/storage/package',
539+
'PhabricatorOwnersPackageCommitRelationship' => 'applications/owners/storage/packagecommitrelationship',
537540
'PhabricatorOwnersPath' => 'applications/owners/storage/path',
538541
'PhabricatorPHID' => 'applications/phid/storage/phid',
539542
'PhabricatorPHIDConstants' => 'applications/phid/constants',
@@ -590,6 +593,7 @@
590593
'PhabricatorRepositoryCommitHeraldWorker' => 'applications/repository/worker/herald',
591594
'PhabricatorRepositoryCommitMessageDetailParser' => 'applications/repository/parser/base',
592595
'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/base',
596+
'PhabricatorRepositoryCommitOwnersWorker' => 'applications/repository/worker/owner',
593597
'PhabricatorRepositoryCommitParserWorker' => 'applications/repository/worker/base',
594598
'PhabricatorRepositoryCommitTaskDaemon' => 'applications/repository/daemon/committask',
595599
'PhabricatorRepositoryController' => 'applications/repository/controller/base',
@@ -1157,6 +1161,7 @@
11571161
'PhabricatorOAuthUnlinkController' => 'PhabricatorAuthController',
11581162
'PhabricatorObjectGraph' => 'AbstractDirectedGraph',
11591163
'PhabricatorObjectHandleStatus' => 'PhabricatorObjectHandleConstants',
1164+
'PhabricatorOwnerRelatedListController' => 'PhabricatorOwnersController',
11601165
'PhabricatorOwnersController' => 'PhabricatorController',
11611166
'PhabricatorOwnersDAO' => 'PhabricatorLiskDAO',
11621167
'PhabricatorOwnersDeleteController' => 'PhabricatorOwnersController',
@@ -1165,6 +1170,7 @@
11651170
'PhabricatorOwnersListController' => 'PhabricatorOwnersController',
11661171
'PhabricatorOwnersOwner' => 'PhabricatorOwnersDAO',
11671172
'PhabricatorOwnersPackage' => 'PhabricatorOwnersDAO',
1173+
'PhabricatorOwnersPackageCommitRelationship' => 'PhabricatorOwnersDAO',
11681174
'PhabricatorOwnersPath' => 'PhabricatorOwnersDAO',
11691175
'PhabricatorPHID' => 'PhabricatorPHIDDAO',
11701176
'PhabricatorPHIDController' => 'PhabricatorController',
@@ -1218,6 +1224,7 @@
12181224
'PhabricatorRepositoryCommitDiscoveryDaemon' => 'PhabricatorRepositoryDaemon',
12191225
'PhabricatorRepositoryCommitHeraldWorker' => 'PhabricatorRepositoryCommitParserWorker',
12201226
'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker',
1227+
'PhabricatorRepositoryCommitOwnersWorker' => 'PhabricatorRepositoryCommitParserWorker',
12211228
'PhabricatorRepositoryCommitParserWorker' => 'PhabricatorWorker',
12221229
'PhabricatorRepositoryCommitTaskDaemon' => 'PhabricatorRepositoryDaemon',
12231230
'PhabricatorRepositoryController' => 'PhabricatorController',

‎src/aphront/default/configuration/AphrontDefaultApplicationConfiguration.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,6 +296,10 @@ public function getURIMap() {
296296
'new/$' => 'PhabricatorOwnersEditController',
297297
'package/(?P<id>\d+)/$' => 'PhabricatorOwnersDetailController',
298298
'delete/(?P<id>\d+)/$' => 'PhabricatorOwnersDeleteController',
299+
'related/' => array(
300+
'$' => 'PhabricatorOwnerRelatedListController',
301+
'view/(?P<view>[^/]+)/$' => 'PhabricatorOwnerRelatedListController',
302+
),
299303
),
300304

301305
'/xhpast/' => array(

‎src/applications/herald/adapter/commit/HeraldCommitAdapter.php‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -62,19 +62,8 @@ public function getHeraldTypeName() {
6262

6363
public function loadAffectedPaths() {
6464
if ($this->affectedPaths === null) {
65-
$drequest = $this->buildDiffusionRequest();
66-
$path_query = DiffusionPathChangeQuery::newFromDiffusionRequest(
67-
$drequest);
68-
$paths = $path_query->loadChanges();
69-
70-
$result = array();
71-
foreach ($paths as $path) {
72-
$basic_path = '/'.$path->getPath();
73-
if ($path->getFileType() == DifferentialChangeType::FILE_DIRECTORY) {
74-
$basic_path = rtrim($basic_path, '/').'/';
75-
}
76-
$result[] = $basic_path;
77-
}
65+
$result = PhabricatorOwnerPathQuery::loadAffectedPaths(
66+
$this->repository, $this->commit);
7867
$this->affectedPaths = $result;
7968
}
8069
return $this->affectedPaths;
@@ -106,14 +95,6 @@ public function loadDifferentialRevision() {
10695
return $this->affectedRevision;
10796
}
10897

109-
private function buildDiffusionRequest() {
110-
return DiffusionRequest::newFromAphrontRequestDictionary(
111-
array(
112-
'callsign' => $this->repository->getCallsign(),
113-
'commit' => $this->commit->getCommitIdentifier(),
114-
));
115-
}
116-
11798
public function getHeraldField($field) {
11899
$data = $this->commitData;
119100
switch ($field) {

‎src/applications/herald/adapter/commit/__init__.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,13 @@
66

77

88

9-
phutil_require_module('phabricator', 'applications/differential/constants/changetype');
109
phutil_require_module('phabricator', 'applications/differential/storage/revision');
11-
phutil_require_module('phabricator', 'applications/diffusion/query/pathchange/base');
12-
phutil_require_module('phabricator', 'applications/diffusion/request/base');
1310
phutil_require_module('phabricator', 'applications/herald/adapter/base');
1411
phutil_require_module('phabricator', 'applications/herald/config/action');
1512
phutil_require_module('phabricator', 'applications/herald/config/contenttype');
1613
phutil_require_module('phabricator', 'applications/herald/config/field');
1714
phutil_require_module('phabricator', 'applications/herald/storage/transcript/apply');
15+
phutil_require_module('phabricator', 'applications/owners/query/path');
1816
phutil_require_module('phabricator', 'applications/owners/storage/owner');
1917
phutil_require_module('phabricator', 'applications/owners/storage/package');
2018

‎src/applications/owners/controller/base/PhabricatorOwnersController.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,10 @@ public function buildStandardPageResponse($view, array $data) {
3131
'href' => '/owners/',
3232
'name' => 'Package Index',
3333
),
34+
'related' => array(
35+
'href' => '/owners/related/',
36+
'name' => 'Related Commits',
37+
),
3438
),
3539
idx($data, 'tab'));
3640
$page->setGlyph("\xE2\x98\x81");

‎src/applications/owners/controller/detail/PhabricatorOwnersDetailController.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,17 @@ public function processRequest() {
7676
'Owners',
7777
$owner_links);
7878

79+
$rows[] = array(
80+
'Related Commits',
81+
phutil_render_tag(
82+
'a',
83+
array(
84+
'href' => '/owners/related/view/all/?phid='.$package->getPHID(),
85+
),
86+
phutil_escape_html('Related Commits'))
87+
);
88+
89+
7990
$path_links = array();
8091
foreach ($paths as $path) {
8192
$callsign = $handles[$path->getRepositoryPHID()]->getName();

‎src/applications/owners/controller/list/PhabricatorOwnersListController.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,12 @@ private function renderPackageTable(array $packages, $header, $nodata) {
257257
phutil_escape_html($package->getName())),
258258
$pkg_owners,
259259
$pkg_paths,
260+
phutil_render_tag(
261+
'a',
262+
array(
263+
'href' => '/owners/related/view/all/?phid='.$package->getPHID(),
264+
),
265+
phutil_escape_html('Related Commits'))
260266
);
261267
}
262268

@@ -266,12 +272,14 @@ private function renderPackageTable(array $packages, $header, $nodata) {
266272
'Name',
267273
'Owners',
268274
'Paths',
275+
'Related Commits',
269276
));
270277
$table->setColumnClasses(
271278
array(
272279
'pri',
273280
'',
274281
'wide wrap',
282+
'narrow',
275283
));
276284

277285
$panel = new AphrontPanelView();

0 commit comments

Comments
 (0)