Skip to content

Commit 57ff0a8

Browse files
author
epriestley
committed
Return applications in application order
Summary: By default, order applications in application order. See discussion in D4708. Principally, this is intended to make sure that application event handlers are registered in order, and thus fire in order. Test Plan: Looked at /applications/, homepage tiles, verified they both still work. I didn't actually test the event handler bit since it's fairly complicated to test blind; D4708 should provide a test case. Reviewers: btrahan, Afaque_Hussain Reviewed By: Afaque_Hussain CC: aran Differential Revision: https://secure.phabricator.com/D4791
1 parent 5459af3 commit 57ff0a8

File tree

3 files changed

+20
-21
lines changed

3 files changed

+20
-21
lines changed

src/applications/base/PhabricatorApplication.php

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ public static function getByClass($class_name) {
244244
}
245245

246246
public static function getAllApplications() {
247-
248247
$classes = id(new PhutilSymbolLoader())
249248
->setAncestorClass(__CLASS__)
250249
->setConcreteOnly(true)
@@ -257,6 +256,13 @@ public static function getAllApplications() {
257256
$apps[] = $app;
258257
}
259258

259+
// Reorder the applications into "application order". Notably, this ensures
260+
// their event handlers register in application order.
261+
$apps = msort($apps, 'getApplicationOrder');
262+
$apps = mgroup($apps, 'getApplicationGroup');
263+
$apps = array_select_keys($apps, self::getApplicationGroups()) + $apps;
264+
$apps = array_mergev($apps);
265+
260266
return $apps;
261267
}
262268

@@ -270,31 +276,24 @@ public static function getAllInstalledApplications() {
270276
PhabricatorEnv::getEnvConfig('phabricator.uninstalled-applications');
271277

272278

273-
274279
if (empty($applications)) {
275-
$classes = id(new PhutilSymbolLoader())
276-
->setAncestorClass(__CLASS__)
277-
->setConcreteOnly(true)
278-
->selectAndLoadSymbols();
279-
280+
$all_applications = self::getAllApplications();
280281
$apps = array();
281-
foreach ($classes as $class) {
282-
283-
if (isset($uninstalled[$class['name']])) {
284-
continue;
285-
}
286-
287-
$app = newv($class['name'], array());
282+
foreach ($all_applications as $app) {
283+
$class = get_class($app);
284+
if (isset($uninstalled[$class])) {
285+
continue;
286+
}
288287

289-
if (!$app->isEnabled()) {
288+
if (!$app->isEnabled()) {
290289
continue;
291-
}
290+
}
292291

293-
if (!$show_beta && $app->isBeta()) {
292+
if (!$show_beta && $app->isBeta()) {
294293
continue;
295-
}
294+
}
296295

297-
$apps[] = $app;
296+
$apps[] = $app;
298297
}
299298

300299
$applications = $apps;

src/applications/directory/controller/PhabricatorDirectoryController.php

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,6 @@ public function buildNav() {
6565
continue;
6666
}
6767

68-
$tile_group = msort($tile_group, 'getApplicationOrder');
69-
7068
$is_small_tiles = ($tile_display == PhabricatorApplication::TILE_SHOW) ||
7169
($tile_display == PhabricatorApplication::TILE_HIDE);
7270

src/applications/meta/controller/PhabricatorApplicationsListController.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ public function processRequest() {
4747
private function buildInstalledApplicationsList(array $applications) {
4848
$list = new PhabricatorObjectItemListView();
4949

50+
$applications = msort($applications, 'getName');
51+
5052
foreach ($applications as $application) {
5153
$item = id(new PhabricatorObjectItemView())
5254
->setHeader($application->getName())

0 commit comments

Comments
 (0)