From 42e46bbe5a917f1de7f3ea068d67dfb0d607ffd1 Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 3 Feb 2020 05:04:01 -0800 Subject: [PATCH 01/16] Fix an issue where Herald rules could fail to evaluate at post-commit time Summary: Ref T13480. Some Herald fields need audit information, which recent changes to Herald adapters discarded. For now, just load it unconditionally. Test Plan: Triggered an Audit-related rule locally. Maniphest Tasks: T13480 Differential Revision: https://secure.phabricator.com/D20962 --- src/applications/diffusion/herald/HeraldCommitAdapter.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/applications/diffusion/herald/HeraldCommitAdapter.php b/src/applications/diffusion/herald/HeraldCommitAdapter.php index d285d23f9b..59508d77fb 100644 --- a/src/applications/diffusion/herald/HeraldCommitAdapter.php +++ b/src/applications/diffusion/herald/HeraldCommitAdapter.php @@ -52,6 +52,7 @@ final class HeraldCommitAdapter ->withPHIDs(array($commit_phid)) ->needCommitData(true) ->needIdentities(true) + ->needAuditRequests(true) ->executeOne(); if (!$commit) { throw new Exception( From c42c5983aa6af18d4c26f1b5bd8677732ecc0ebf Mon Sep 17 00:00:00 2001 From: epriestley Date: Mon, 3 Feb 2020 08:40:57 -0800 Subject: [PATCH 02/16] Fix an issue where loading a mangled project graph could fail too abruptly Summary: Ref T13484. If you load a subproject S which has a mangled/invalid `parentPath`, the query currently tries to execute an empty edge query and fatals. Instead, we want to deny-by-default in the policy layer but not fail the query. The subproject should become restricted but not fatal anything related to it. See T13484 for a future refinement where we could identify "broken / data integrity issue" objects explicilty. Test Plan: - Modified the `projectPath` of some subproject in the database to `QQQQ...`. - Loaded that project page. - Before patch: fatal after issuing bad edge query. - After patch: "functionally correct" policy layer failure, although an explicit "data integrity issue" failure would be better. Maniphest Tasks: T13484 Differential Revision: https://secure.phabricator.com/D20963 --- .../project/query/PhabricatorProjectQuery.php | 19 +++++++++++++++++++ .../edges/query/PhabricatorEdgeQuery.php | 12 +++++++++--- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/src/applications/project/query/PhabricatorProjectQuery.php b/src/applications/project/query/PhabricatorProjectQuery.php index b68c638363..96efcfba4b 100644 --- a/src/applications/project/query/PhabricatorProjectQuery.php +++ b/src/applications/project/query/PhabricatorProjectQuery.php @@ -271,6 +271,25 @@ final class PhabricatorProjectQuery $all_graph = $this->getAllReachableAncestors($projects); + // See T13484. If the graph is damaged (and contains a cycle or an edge + // pointing at a project which has been destroyed), some of the nodes we + // started with may be filtered out by reachability tests. If any of the + // projects we are linking up don't have available ancestors, filter them + // out. + + foreach ($projects as $key => $project) { + $project_phid = $project->getPHID(); + if (!isset($all_graph[$project_phid])) { + $this->didRejectResult($project); + unset($projects[$key]); + continue; + } + } + + if (!$projects) { + return array(); + } + // NOTE: Although we may not need much information about ancestors, we // always need to test if the viewer is a member, because we will return // ancestor projects to the policy filter via ExtendedPolicy calls. If diff --git a/src/infrastructure/edges/query/PhabricatorEdgeQuery.php b/src/infrastructure/edges/query/PhabricatorEdgeQuery.php index 6519c47724..f63e827431 100644 --- a/src/infrastructure/edges/query/PhabricatorEdgeQuery.php +++ b/src/infrastructure/edges/query/PhabricatorEdgeQuery.php @@ -46,6 +46,13 @@ final class PhabricatorEdgeQuery extends PhabricatorQuery { * @task config */ public function withSourcePHIDs(array $source_phids) { + if (!$source_phids) { + throw new Exception( + pht( + 'Edge list passed to "withSourcePHIDs(...)" is empty, but it must '. + 'be nonempty.')); + } + $this->sourcePHIDs = $source_phids; return $this; } @@ -158,11 +165,10 @@ final class PhabricatorEdgeQuery extends PhabricatorQuery { * @task exec */ public function execute() { - if (!$this->sourcePHIDs) { + if ($this->sourcePHIDs === null) { throw new Exception( pht( - 'You must use %s to query edges.', - 'withSourcePHIDs()')); + 'You must use "withSourcePHIDs()" to query edges.')); } $sources = phid_group_by_type($this->sourcePHIDs); From a72ade94757ae38b4760bd6efad67ff30dd0b376 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 26 Nov 2019 14:49:36 -0800 Subject: [PATCH 03/16] Carve out a separate "Modules/Extensions" section of Config Summary: Ref T13362. Config is currently doing a ton of stuff and fairly overwhelming. Separate out "Modules/Extensions" so it can live in its own section. (This stuff is mostly useful for development and normal users rarely need to end up here.) Test Plan: Visited seciton, clicked around. This is just a visual change. Maniphest Tasks: T13362 Differential Revision: https://secure.phabricator.com/D20930 --- .../PhabricatorConfigApplication.php | 2 +- .../PhabricatorConfigController.php | 7 ------ .../PhabricatorConfigModuleController.php | 23 +++++++++++++++++-- 3 files changed, 22 insertions(+), 10 deletions(-) diff --git a/src/applications/config/application/PhabricatorConfigApplication.php b/src/applications/config/application/PhabricatorConfigApplication.php index e7540ad9de..a8c7f2ff5b 100644 --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -63,7 +63,7 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { 'purge/' => 'PhabricatorConfigPurgeCacheController', ), 'module/' => array( - '(?P[^/]+)/' => 'PhabricatorConfigModuleController', + '(?:(?P[^/]+)/)?' => 'PhabricatorConfigModuleController', ), 'cluster/' => array( 'databases/' => 'PhabricatorConfigClusterDatabasesController', diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php index 1b6a5af8b5..0b2a67a17f 100644 --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -42,13 +42,6 @@ abstract class PhabricatorConfigController extends PhabricatorController { pht('Repository Servers'), null, 'fa-code'); $nav->addFilter('cluster/search/', pht('Search Servers'), null, 'fa-search'); - $nav->addLabel(pht('Modules')); - - $modules = PhabricatorConfigModule::getAllModules(); - foreach ($modules as $key => $module) { - $nav->addFilter('module/'.$key.'/', - $module->getModuleName(), null, 'fa-puzzle-piece'); - } return $nav; } diff --git a/src/applications/config/controller/PhabricatorConfigModuleController.php b/src/applications/config/controller/PhabricatorConfigModuleController.php index fe919c57e4..aaa873f5d2 100644 --- a/src/applications/config/controller/PhabricatorConfigModuleController.php +++ b/src/applications/config/controller/PhabricatorConfigModuleController.php @@ -8,6 +8,11 @@ final class PhabricatorConfigModuleController $key = $request->getURIData('module'); $all_modules = PhabricatorConfigModule::getAllModules(); + + if (!strlen($key)) { + $key = head_key($all_modules); + } + if (empty($all_modules[$key])) { return new Aphront404Response(); } @@ -16,13 +21,27 @@ final class PhabricatorConfigModuleController $content = $module->renderModuleStatus($request); $title = $module->getModuleName(); - $nav = $this->buildSideNavView(); - $nav->selectFilter('module/'.$key.'/'); + $nav = new AphrontSideNavFilterView(); + $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); + + $modules_uri = $this->getApplicationURI('module/'); + + $modules = PhabricatorConfigModule::getAllModules(); + + foreach ($modules as $module_key => $module) { + $nav->newLink($module_key) + ->setName($module->getModuleName()) + ->setHref(urisprintf('%s%s/', $modules_uri, $module_key)) + ->setIcon('fa-puzzle-piece'); + } + + $nav->selectFilter($key); $header = $this->buildHeaderView($title); $view = $this->buildConfigBoxView($title, $content); $crumbs = $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Extensions/Modules'), $modules_uri) ->addTextCrumb($title) ->setBorder(true); From cb481f36c54b1473b5e4c5b59daa4d97e7055c7e Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 26 Nov 2019 15:25:58 -0800 Subject: [PATCH 04/16] Carve out a separate "Services" section of Config Summary: Depends on D20930. Ref T13362. Put all the "Services" parts of Config in their own section. Test Plan: Clicked through each section. This is just an organization / UI change with no significant behavioral impact. Maniphest Tasks: T13362 Differential Revision: https://secure.phabricator.com/D20931 --- src/__phutil_library_map__.php | 30 ++++---- .../PhabricatorConfigController.php | 17 ----- .../PhabricatorConfigCacheController.php | 15 ++-- ...icatorConfigClusterDatabasesController.php | 12 ++-- ...orConfigClusterNotificationsController.php | 14 ++-- ...torConfigClusterRepositoriesController.php | 14 ++-- ...abricatorConfigClusterSearchController.php | 14 ++-- .../PhabricatorConfigDatabaseController.php | 2 +- ...abricatorConfigDatabaseIssueController.php | 7 +- ...bricatorConfigDatabaseStatusController.php | 6 +- .../PhabricatorConfigServicesController.php | 69 +++++++++++++++++++ 11 files changed, 122 insertions(+), 78 deletions(-) rename src/applications/config/controller/{ => services}/PhabricatorConfigCacheController.php (93%) rename src/applications/config/controller/{ => services}/PhabricatorConfigClusterDatabasesController.php (95%) rename src/applications/config/controller/{ => services}/PhabricatorConfigClusterNotificationsController.php (94%) rename src/applications/config/controller/{ => services}/PhabricatorConfigClusterRepositoriesController.php (97%) rename src/applications/config/controller/{ => services}/PhabricatorConfigClusterSearchController.php (92%) rename src/applications/config/controller/{ => services}/PhabricatorConfigDatabaseController.php (95%) rename src/applications/config/controller/{ => services}/PhabricatorConfigDatabaseIssueController.php (96%) rename src/applications/config/controller/{ => services}/PhabricatorConfigDatabaseStatusController.php (99%) create mode 100644 src/applications/config/controller/services/PhabricatorConfigServicesController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 9b042e024a..da9c6e1fb5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2801,22 +2801,22 @@ phutil_register_library_map(array( 'PhabricatorConfigAllController' => 'applications/config/controller/PhabricatorConfigAllController.php', 'PhabricatorConfigApplication' => 'applications/config/application/PhabricatorConfigApplication.php', 'PhabricatorConfigApplicationController' => 'applications/config/controller/PhabricatorConfigApplicationController.php', - 'PhabricatorConfigCacheController' => 'applications/config/controller/PhabricatorConfigCacheController.php', - 'PhabricatorConfigClusterDatabasesController' => 'applications/config/controller/PhabricatorConfigClusterDatabasesController.php', - 'PhabricatorConfigClusterNotificationsController' => 'applications/config/controller/PhabricatorConfigClusterNotificationsController.php', - 'PhabricatorConfigClusterRepositoriesController' => 'applications/config/controller/PhabricatorConfigClusterRepositoriesController.php', - 'PhabricatorConfigClusterSearchController' => 'applications/config/controller/PhabricatorConfigClusterSearchController.php', + 'PhabricatorConfigCacheController' => 'applications/config/controller/services/PhabricatorConfigCacheController.php', + 'PhabricatorConfigClusterDatabasesController' => 'applications/config/controller/services/PhabricatorConfigClusterDatabasesController.php', + 'PhabricatorConfigClusterNotificationsController' => 'applications/config/controller/services/PhabricatorConfigClusterNotificationsController.php', + 'PhabricatorConfigClusterRepositoriesController' => 'applications/config/controller/services/PhabricatorConfigClusterRepositoriesController.php', + 'PhabricatorConfigClusterSearchController' => 'applications/config/controller/services/PhabricatorConfigClusterSearchController.php', 'PhabricatorConfigCollectorsModule' => 'applications/config/module/PhabricatorConfigCollectorsModule.php', 'PhabricatorConfigColumnSchema' => 'applications/config/schema/PhabricatorConfigColumnSchema.php', 'PhabricatorConfigConfigPHIDType' => 'applications/config/phid/PhabricatorConfigConfigPHIDType.php', 'PhabricatorConfigConstants' => 'applications/config/constants/PhabricatorConfigConstants.php', 'PhabricatorConfigController' => 'applications/config/controller/PhabricatorConfigController.php', 'PhabricatorConfigCoreSchemaSpec' => 'applications/config/schema/PhabricatorConfigCoreSchemaSpec.php', - 'PhabricatorConfigDatabaseController' => 'applications/config/controller/PhabricatorConfigDatabaseController.php', - 'PhabricatorConfigDatabaseIssueController' => 'applications/config/controller/PhabricatorConfigDatabaseIssueController.php', + 'PhabricatorConfigDatabaseController' => 'applications/config/controller/services/PhabricatorConfigDatabaseController.php', + 'PhabricatorConfigDatabaseIssueController' => 'applications/config/controller/services/PhabricatorConfigDatabaseIssueController.php', 'PhabricatorConfigDatabaseSchema' => 'applications/config/schema/PhabricatorConfigDatabaseSchema.php', 'PhabricatorConfigDatabaseSource' => 'infrastructure/env/PhabricatorConfigDatabaseSource.php', - 'PhabricatorConfigDatabaseStatusController' => 'applications/config/controller/PhabricatorConfigDatabaseStatusController.php', + 'PhabricatorConfigDatabaseStatusController' => 'applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php', 'PhabricatorConfigDefaultSource' => 'infrastructure/env/PhabricatorConfigDefaultSource.php', 'PhabricatorConfigDictionarySource' => 'infrastructure/env/PhabricatorConfigDictionarySource.php', 'PhabricatorConfigEdgeModule' => 'applications/config/module/PhabricatorConfigEdgeModule.php', @@ -2861,6 +2861,7 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaQuery' => 'applications/config/schema/PhabricatorConfigSchemaQuery.php', 'PhabricatorConfigSchemaSpec' => 'applications/config/schema/PhabricatorConfigSchemaSpec.php', 'PhabricatorConfigServerSchema' => 'applications/config/schema/PhabricatorConfigServerSchema.php', + 'PhabricatorConfigServicesController' => 'applications/config/controller/services/PhabricatorConfigServicesController.php', 'PhabricatorConfigSetupCheckModule' => 'applications/config/module/PhabricatorConfigSetupCheckModule.php', 'PhabricatorConfigSiteModule' => 'applications/config/module/PhabricatorConfigSiteModule.php', 'PhabricatorConfigSiteSource' => 'infrastructure/env/PhabricatorConfigSiteSource.php', @@ -9134,18 +9135,18 @@ phutil_register_library_map(array( 'PhabricatorConfigAllController' => 'PhabricatorConfigController', 'PhabricatorConfigApplication' => 'PhabricatorApplication', 'PhabricatorConfigApplicationController' => 'PhabricatorConfigController', - 'PhabricatorConfigCacheController' => 'PhabricatorConfigController', - 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigController', - 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigController', - 'PhabricatorConfigClusterRepositoriesController' => 'PhabricatorConfigController', - 'PhabricatorConfigClusterSearchController' => 'PhabricatorConfigController', + 'PhabricatorConfigCacheController' => 'PhabricatorConfigServicesController', + 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigServicesController', + 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigServicesController', + 'PhabricatorConfigClusterRepositoriesController' => 'PhabricatorConfigServicesController', + 'PhabricatorConfigClusterSearchController' => 'PhabricatorConfigServicesController', 'PhabricatorConfigCollectorsModule' => 'PhabricatorConfigModule', 'PhabricatorConfigColumnSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigConfigPHIDType' => 'PhabricatorPHIDType', 'PhabricatorConfigConstants' => 'Phobject', 'PhabricatorConfigController' => 'PhabricatorController', 'PhabricatorConfigCoreSchemaSpec' => 'PhabricatorConfigSchemaSpec', - 'PhabricatorConfigDatabaseController' => 'PhabricatorConfigController', + 'PhabricatorConfigDatabaseController' => 'PhabricatorConfigServicesController', 'PhabricatorConfigDatabaseIssueController' => 'PhabricatorConfigDatabaseController', 'PhabricatorConfigDatabaseSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigDatabaseSource' => 'PhabricatorConfigProxySource', @@ -9198,6 +9199,7 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaQuery' => 'Phobject', 'PhabricatorConfigSchemaSpec' => 'Phobject', 'PhabricatorConfigServerSchema' => 'PhabricatorConfigStorageSchema', + 'PhabricatorConfigServicesController' => 'PhabricatorConfigController', 'PhabricatorConfigSetupCheckModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteSource' => 'PhabricatorConfigProxySource', diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php index 0b2a67a17f..1aa9946236 100644 --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -25,23 +25,6 @@ abstract class PhabricatorConfigController extends PhabricatorController { pht('Setup Issues'), null, 'fa-warning'); $nav->addFilter(null, pht('Installation Guide'), $guide_href, 'fa-book'); - $nav->addLabel(pht('Database')); - $nav->addFilter('database/', - pht('Database Status'), null, 'fa-heartbeat'); - $nav->addFilter('dbissue/', - pht('Database Issues'), null, 'fa-exclamation-circle'); - $nav->addLabel(pht('Cache')); - $nav->addFilter('cache/', - pht('Cache Status'), null, 'fa-home'); - $nav->addLabel(pht('Cluster')); - $nav->addFilter('cluster/databases/', - pht('Database Servers'), null, 'fa-database'); - $nav->addFilter('cluster/notifications/', - pht('Notification Servers'), null, 'fa-bell-o'); - $nav->addFilter('cluster/repositories/', - pht('Repository Servers'), null, 'fa-code'); - $nav->addFilter('cluster/search/', - pht('Search Servers'), null, 'fa-search'); return $nav; } diff --git a/src/applications/config/controller/PhabricatorConfigCacheController.php b/src/applications/config/controller/services/PhabricatorConfigCacheController.php similarity index 93% rename from src/applications/config/controller/PhabricatorConfigCacheController.php rename to src/applications/config/controller/services/PhabricatorConfigCacheController.php index 36642657c9..e79eac07bb 100644 --- a/src/applications/config/controller/PhabricatorConfigCacheController.php +++ b/src/applications/config/controller/services/PhabricatorConfigCacheController.php @@ -1,13 +1,11 @@ getViewer(); - $nav = $this->buildSideNavView(); - $nav->selectFilter('cache/'); $purge_button = id(new PHUIButtonView()) ->setText(pht('Purge Caches')) @@ -27,14 +25,15 @@ final class PhabricatorConfigCacheController $data_box, ); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) ->setFooter($page); + $nav = $this->newNavigation('cache'); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) @@ -92,10 +91,12 @@ final class PhabricatorConfigCacheController 'n', 'n', )); + + $table = $this->buildConfigBoxView(pht('Cache Storage'), $table); } $properties = $this->buildConfigBoxView(pht('Data Cache'), $properties); - $table = $this->buildConfigBoxView(pht('Cache Storage'), $table); + return array($properties, $table); } diff --git a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php b/src/applications/config/controller/services/PhabricatorConfigClusterDatabasesController.php similarity index 95% rename from src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php rename to src/applications/config/controller/services/PhabricatorConfigClusterDatabasesController.php index 417fa9d3a1..ca22698212 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php +++ b/src/applications/config/controller/services/PhabricatorConfigClusterDatabasesController.php @@ -1,13 +1,12 @@ buildSideNavView(); - $nav->selectFilter('cluster/databases/'); + $nav = $this->newNavigation('database-servers'); - $title = pht('Cluster Database Status'); + $title = pht('Database Servers'); $doc_href = PhabricatorEnv::getDoclink('Cluster: Databases'); $button = id(new PHUIButtonView()) ->setIcon('fa-book') @@ -20,9 +19,8 @@ final class PhabricatorConfigClusterDatabasesController $database_status = $this->buildClusterDatabaseStatus(); $status = $this->buildConfigBoxView(pht('Status'), $database_status); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) diff --git a/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php b/src/applications/config/controller/services/PhabricatorConfigClusterNotificationsController.php similarity index 94% rename from src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php rename to src/applications/config/controller/services/PhabricatorConfigClusterNotificationsController.php index e9f64d411a..ba4185984a 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php +++ b/src/applications/config/controller/services/PhabricatorConfigClusterNotificationsController.php @@ -1,13 +1,10 @@ buildSideNavView(); - $nav->selectFilter('cluster/notifications/'); - - $title = pht('Cluster Notifications'); + $title = pht('Notification Servers'); $doc_href = PhabricatorEnv::getDoclink('Cluster: Notifications'); $button = id(new PHUIButtonView()) ->setIcon('fa-book') @@ -22,14 +19,15 @@ final class PhabricatorConfigClusterNotificationsController pht('Notifications Status'), $notification_status); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) ->setFooter($status); + $nav = $this->newNavigation('notification-servers'); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) diff --git a/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php b/src/applications/config/controller/services/PhabricatorConfigClusterRepositoriesController.php similarity index 97% rename from src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php rename to src/applications/config/controller/services/PhabricatorConfigClusterRepositoriesController.php index eb83a28a2a..d6ebc314f2 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php +++ b/src/applications/config/controller/services/PhabricatorConfigClusterRepositoriesController.php @@ -1,13 +1,10 @@ buildSideNavView(); - $nav->selectFilter('cluster/repositories/'); - - $title = pht('Cluster Repository Status'); + $title = pht('Repository Services'); $doc_href = PhabricatorEnv::getDoclink('Cluster: Repositories'); $button = id(new PHUIButtonView()) @@ -26,9 +23,8 @@ final class PhabricatorConfigClusterRepositoriesController $repo_errors = $this->buildConfigBoxView( pht('Repository Errors'), $repository_errors); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) @@ -38,6 +34,8 @@ final class PhabricatorConfigClusterRepositoriesController $repo_errors, )); + $nav = $this->newNavigation('repository-servers'); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) diff --git a/src/applications/config/controller/PhabricatorConfigClusterSearchController.php b/src/applications/config/controller/services/PhabricatorConfigClusterSearchController.php similarity index 92% rename from src/applications/config/controller/PhabricatorConfigClusterSearchController.php rename to src/applications/config/controller/services/PhabricatorConfigClusterSearchController.php index 55caeb1cad..5e877d6b95 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterSearchController.php +++ b/src/applications/config/controller/services/PhabricatorConfigClusterSearchController.php @@ -1,13 +1,10 @@ buildSideNavView(); - $nav->selectFilter('cluster/search/'); - - $title = pht('Cluster Search'); + $title = pht('Search Servers'); $doc_href = PhabricatorEnv::getDoclink('Cluster: Search'); $button = id(new PHUIButtonView()) @@ -20,14 +17,15 @@ final class PhabricatorConfigClusterSearchController $search_status = $this->buildClusterSearchStatus(); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) ->setFooter($search_status); + $nav = $this->newNavigation('search-servers'); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseController.php b/src/applications/config/controller/services/PhabricatorConfigDatabaseController.php similarity index 95% rename from src/applications/config/controller/PhabricatorConfigDatabaseController.php rename to src/applications/config/controller/services/PhabricatorConfigDatabaseController.php index 53af9a6b92..eb08164636 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseController.php +++ b/src/applications/config/controller/services/PhabricatorConfigDatabaseController.php @@ -1,7 +1,7 @@ buildHeaderView($title); - $nav = $this->buildSideNavView(); - $nav->selectFilter('dbissue/'); + $nav = $this->newNavigation('schemata-issues'); $view = $this->buildConfigBoxView(pht('Issues'), $table); - $crumbs = $this->buildApplicationCrumbs() + $crumbs = $this->newCrumbs() ->addTextCrumb($title) ->setBorder(true); diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php similarity index 99% rename from src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php rename to src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php index 6831a048d5..09f4f344b5 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/services/PhabricatorConfigDatabaseStatusController.php @@ -71,8 +71,7 @@ final class PhabricatorConfigDatabaseStatusController } private function buildResponse($title, $body) { - $nav = $this->buildSideNavView(); - $nav->selectFilter('database/'); + $nav = $this->newNavigation('schemata'); if (!$title) { $title = pht('Database Status'); @@ -118,8 +117,7 @@ final class PhabricatorConfigDatabaseStatusController ); } - $crumbs = $this->buildApplicationCrumbs(); - $crumbs->setBorder(true); + $crumbs = $this->newCrumbs(); $last_key = last_key($links); foreach ($links as $link_key => $link) { diff --git a/src/applications/config/controller/services/PhabricatorConfigServicesController.php b/src/applications/config/controller/services/PhabricatorConfigServicesController.php new file mode 100644 index 0000000000..376849d6b7 --- /dev/null +++ b/src/applications/config/controller/services/PhabricatorConfigServicesController.php @@ -0,0 +1,69 @@ +getApplicationURI(); + + $nav = id(new AphrontSideNavFilterView()) + ->setBaseURI(new PhutilURI($services_uri)); + + $nav->addLabel(pht('Databases')); + + $nav->newLink('database-servers') + ->setName(pht('Database Servers')) + ->setIcon('fa-database') + ->setHref(urisprintf('%s%s/', $services_uri, 'cluster/databases')); + + $nav->newLink('schemata') + ->setName(pht('Database Schemata')) + ->setIcon('fa-table') + ->setHref(urisprintf('%s%s/', $services_uri, 'database')); + + $nav->newLink('schemata-issues') + ->setName(pht('Schemata Issues')) + ->setIcon('fa-exclamation-circle') + ->setHref(urisprintf('%s%s/', $services_uri, 'dbissue')); + + + $nav->addLabel(pht('Cache')); + + $nav->newLink('cache') + ->setName(pht('Cache Status')) + ->setIcon('fa-archive') + ->setHref(urisprintf('%s%s/', $services_uri, 'cache')); + + $nav->addLabel(pht('Other Services')); + + $nav->newLink('notification-servers') + ->setName(pht('Notification Servers')) + ->setIcon('fa-bell-o') + ->setHref(urisprintf('%s%s/', $services_uri, 'cluster/notifications')); + + $nav->newLink('repository-servers') + ->setName(pht('Repository Servers')) + ->setIcon('fa-code') + ->setHref(urisprintf('%s%s/', $services_uri, 'cluster/repositories')); + + $nav->newLink('search-servers') + ->setName(pht('Search Servers')) + ->setIcon('fa-search') + ->setHref(urisprintf('%s%s/', $services_uri, 'cluster/search')); + + if ($select_filter) { + $nav->selectFilter($select_filter); + } + + return $nav; + } + + public function newCrumbs() { + $services_uri = $this->getApplicationURI('cluster/databases/'); + + return $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Services')) + ->setBorder(true); + } + +} From 26c2a1ba68b15c3e8a00abca025e55d40ac315fb Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 30 Nov 2019 10:28:22 -0800 Subject: [PATCH 05/16] Move existing "Console" interfaces away from "setFixed(...)" on "TwoColumnView" Summary: Depends on D20931. Ref T13362. Move all "Console"-style interfaces to use a consistent layout based on a new "LauncherView" which just centers the content. Test Plan: Viewed all affected interfaces. Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T13362 Differential Revision: https://secure.phabricator.com/D20933 --- resources/celerity/map.php | 6 +++--- src/__phutil_library_map__.php | 2 ++ .../controller/AlmanacConsoleController.php | 20 +++++++++++-------- .../PhabricatorDashboardConsoleController.php | 6 ++++-- .../DiffusionRepositoryEditController.php | 8 +++++--- .../controller/DrydockConsoleController.php | 6 ++++-- src/view/phui/PHUILauncherView.php | 20 +++++++++++++++++++ .../rsrc/css/phui/phui-two-column-view.css | 5 +++++ 8 files changed, 55 insertions(+), 18 deletions(-) create mode 100644 src/view/phui/PHUILauncherView.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 9b8013c826..d3d95f832c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => 'ad8fc332', + 'core.pkg.css' => '6d9a0ba6', 'core.pkg.js' => '705aec2c', 'differential.pkg.css' => '607c84be', 'differential.pkg.js' => '1b97518d', @@ -176,7 +176,7 @@ return array( 'rsrc/css/phui/phui-status.css' => 'e5ff8be0', 'rsrc/css/phui/phui-tag-view.css' => '8519160a', 'rsrc/css/phui/phui-timeline-view.css' => '1e348e4b', - 'rsrc/css/phui/phui-two-column-view.css' => '0a876b9e', + 'rsrc/css/phui/phui-two-column-view.css' => 'f96d319f', 'rsrc/css/phui/workboards/phui-workboard-color.css' => 'e86de308', 'rsrc/css/phui/workboards/phui-workboard.css' => '74fc9d98', 'rsrc/css/phui/workboards/phui-workcard.css' => '913441b6', @@ -873,7 +873,7 @@ return array( 'phui-tag-view-css' => '8519160a', 'phui-theme-css' => '35883b37', 'phui-timeline-view-css' => '1e348e4b', - 'phui-two-column-view-css' => '0a876b9e', + 'phui-two-column-view-css' => 'f96d319f', 'phui-workboard-color-css' => 'e86de308', 'phui-workboard-view-css' => '74fc9d98', 'phui-workcard-view-css' => '913441b6', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index da9c6e1fb5..ebefb1db5d 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2037,6 +2037,7 @@ phutil_register_library_map(array( 'PHUIInfoView' => 'view/phui/PHUIInfoView.php', 'PHUIInvisibleCharacterTestCase' => 'view/phui/__tests__/PHUIInvisibleCharacterTestCase.php', 'PHUIInvisibleCharacterView' => 'view/phui/PHUIInvisibleCharacterView.php', + 'PHUILauncherView' => 'view/phui/PHUILauncherView.php', 'PHUILeftRightExample' => 'applications/uiexample/examples/PHUILeftRightExample.php', 'PHUILeftRightView' => 'view/phui/PHUILeftRightView.php', 'PHUIListExample' => 'applications/uiexample/examples/PHUIListExample.php', @@ -8236,6 +8237,7 @@ phutil_register_library_map(array( 'PHUIInfoView' => 'AphrontTagView', 'PHUIInvisibleCharacterTestCase' => 'PhabricatorTestCase', 'PHUIInvisibleCharacterView' => 'AphrontView', + 'PHUILauncherView' => 'AphrontTagView', 'PHUILeftRightExample' => 'PhabricatorUIExample', 'PHUILeftRightView' => 'AphrontTagView', 'PHUIListExample' => 'PhabricatorUIExample', diff --git a/src/applications/almanac/controller/AlmanacConsoleController.php b/src/applications/almanac/controller/AlmanacConsoleController.php index 74c57c384f..9c27461ac7 100644 --- a/src/applications/almanac/controller/AlmanacConsoleController.php +++ b/src/applications/almanac/controller/AlmanacConsoleController.php @@ -10,13 +10,15 @@ final class AlmanacConsoleController extends AlmanacController { $viewer = $request->getViewer(); $menu = id(new PHUIObjectItemListView()) - ->setUser($viewer); + ->setUser($viewer) + ->setBig(true); $menu->addItem( id(new PHUIObjectItemView()) ->setHeader(pht('Devices')) ->setHref($this->getApplicationURI('device/')) ->setImageIcon('fa-server') + ->setClickable(true) ->addAttribute( pht( 'Create an inventory of physical and virtual hosts and '. @@ -27,6 +29,7 @@ final class AlmanacConsoleController extends AlmanacController { ->setHeader(pht('Services')) ->setHref($this->getApplicationURI('service/')) ->setImageIcon('fa-plug') + ->setClickable(true) ->addAttribute( pht( 'Create and update services, and map them to interfaces on '. @@ -37,6 +40,7 @@ final class AlmanacConsoleController extends AlmanacController { ->setHeader(pht('Networks')) ->setHref($this->getApplicationURI('network/')) ->setImageIcon('fa-globe') + ->setClickable(true) ->addAttribute( pht( 'Manage public and private networks.'))); @@ -46,6 +50,7 @@ final class AlmanacConsoleController extends AlmanacController { ->setHeader(pht('Namespaces')) ->setHref($this->getApplicationURI('namespace/')) ->setImageIcon('fa-asterisk') + ->setClickable(true) ->addAttribute( pht('Control who can create new named services and devices.'))); @@ -57,6 +62,7 @@ final class AlmanacConsoleController extends AlmanacController { ->setHeader(pht('Documentation')) ->setHref($docs_uri) ->setImageIcon('fa-book') + ->setClickable(true) ->addAttribute(pht('Browse documentation for Almanac.'))); $crumbs = $this->buildApplicationCrumbs(); @@ -64,17 +70,15 @@ final class AlmanacConsoleController extends AlmanacController { $crumbs->setBorder(true); $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Almanac Console')) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) ->setObjectList($menu); - $header = id(new PHUIHeaderView()) - ->setHeader(pht('Almanac Console')) - ->setHeaderIcon('fa-server'); + $launcher_view = id(new PHUILauncherView()) + ->appendChild($box); $view = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter(array( - $box, - )); + ->setFooter($launcher_view); return $this->newPage() ->setTitle(pht('Almanac Console')) diff --git a/src/applications/dashboard/controller/PhabricatorDashboardConsoleController.php b/src/applications/dashboard/controller/PhabricatorDashboardConsoleController.php index 1caceec1e4..e63cedb1da 100644 --- a/src/applications/dashboard/controller/PhabricatorDashboardConsoleController.php +++ b/src/applications/dashboard/controller/PhabricatorDashboardConsoleController.php @@ -59,9 +59,11 @@ final class PhabricatorDashboardConsoleController ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) ->setObjectList($menu); + $launch_view = id(new PHUILauncherView()) + ->appendChild($box); + $view = id(new PHUITwoColumnView()) - ->setFixed(true) - ->setFooter($box); + ->setFooter($launch_view); return $this->newPage() ->setTitle($title) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditController.php index bf3f389806..af05f6719f 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditController.php @@ -78,14 +78,16 @@ final class DiffusionRepositoryEditController ->addClass('diffusion-create-repo') ->appendChild($layout); - $view = id(new PHUITwoColumnView()) - ->setFixed(true) - ->setFooter( + $launcher_view = id(new PHUILauncherView()) + ->appendChild( array( $layout, $hints, )); + $view = id(new PHUITwoColumnView()) + ->setFooter($launcher_view); + return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) diff --git a/src/applications/drydock/controller/DrydockConsoleController.php b/src/applications/drydock/controller/DrydockConsoleController.php index d0ae358768..2d7ccfb8a0 100644 --- a/src/applications/drydock/controller/DrydockConsoleController.php +++ b/src/applications/drydock/controller/DrydockConsoleController.php @@ -76,9 +76,11 @@ final class DrydockConsoleController extends DrydockController { ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) ->setObjectList($menu); + $launcher_view = id(new PHUILauncherView()) + ->appendChild($box); + $view = id(new PHUITwoColumnView()) - ->setFixed(true) - ->setFooter($box); + ->setFooter($launcher_view); return $this->newPage() ->setTitle($title) diff --git a/src/view/phui/PHUILauncherView.php b/src/view/phui/PHUILauncherView.php new file mode 100644 index 0000000000..fbb029b9d3 --- /dev/null +++ b/src/view/phui/PHUILauncherView.php @@ -0,0 +1,20 @@ + implode(' ', $classes), + ); + } + +} diff --git a/webroot/rsrc/css/phui/phui-two-column-view.css b/webroot/rsrc/css/phui/phui-two-column-view.css index 3e58f58173..c2aa0c5fbd 100644 --- a/webroot/rsrc/css/phui/phui-two-column-view.css +++ b/webroot/rsrc/css/phui/phui-two-column-view.css @@ -2,6 +2,11 @@ * @provides phui-two-column-view-css */ +.phui-launcher-view { + max-width: 1140px; + margin: 0 auto; +} + .phui-two-column-fixed { max-width: 1140px; margin: 0 auto; From 530145ba3be62b6c54b8f3bc8aca09f2f369697d Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 30 Nov 2019 11:23:34 -0800 Subject: [PATCH 06/16] Give "Config" a full-width, hierarchical layout Summary: Depends on D20933. Ref T13362. This reorganizes Config a bit and attempts to simplify it. Subsections are now in a landing page console and groupings have been removed. We "only" have 75 values you can edit from the web UI nowadays, which is still a lot, but less overwhelming than it was in the past. And the trend is generally downward, as config is removed/simplified or moved into application settings. This also gets rid of the "gigantic blobs of JSON in the UI". Test Plan: Browsed all Config sections. Maniphest Tasks: T13362 Differential Revision: https://secure.phabricator.com/D20934 --- src/__phutil_library_map__.php | 36 ++- .../controller/AlmanacConsoleController.php | 3 +- .../PhabricatorConfigApplication.php | 9 +- .../check/PhabricatorAuthSetupCheck.php | 21 +- .../check/PhabricatorSecuritySetupCheck.php | 3 +- .../PhabricatorConfigAllController.php | 77 ------ ...PhabricatorConfigApplicationController.php | 57 ----- ...=> PhabricatorConfigConsoleController.php} | 221 +++++++++++++----- .../PhabricatorConfigController.php | 27 --- .../PhabricatorConfigGroupController.php | 112 --------- .../PhabricatorConfigListController.php | 57 ----- .../PhabricatorConfigIgnoreController.php | 0 .../PhabricatorConfigIssueListController.php | 60 ++--- .../PhabricatorConfigIssuePanelController.php | 0 .../PhabricatorConfigIssueViewController.php | 13 +- .../PhabricatorConfigModuleController.php | 0 .../PhabricatorConfigPurgeCacheController.php | 0 .../PhabricatorConfigEditController.php | 27 +-- .../PhabricatorConfigSettingsController.php | 51 ++++ ...icatorConfigSettingsHistoryController.php} | 12 +- ...habricatorConfigSettingsListController.php | 107 +++++++++ .../setup/PhabricatorAphlictSetupCheck.php | 4 + 22 files changed, 401 insertions(+), 496 deletions(-) delete mode 100644 src/applications/config/controller/PhabricatorConfigAllController.php delete mode 100644 src/applications/config/controller/PhabricatorConfigApplicationController.php rename src/applications/config/controller/{PhabricatorConfigVersionController.php => PhabricatorConfigConsoleController.php} (52%) delete mode 100644 src/applications/config/controller/PhabricatorConfigGroupController.php delete mode 100644 src/applications/config/controller/PhabricatorConfigListController.php rename src/applications/config/controller/{ => issue}/PhabricatorConfigIgnoreController.php (100%) rename src/applications/config/controller/{ => issue}/PhabricatorConfigIssueListController.php (64%) rename src/applications/config/controller/{ => issue}/PhabricatorConfigIssuePanelController.php (100%) rename src/applications/config/controller/{ => issue}/PhabricatorConfigIssueViewController.php (88%) rename src/applications/config/controller/{ => module}/PhabricatorConfigModuleController.php (100%) rename src/applications/config/controller/{ => services}/PhabricatorConfigPurgeCacheController.php (100%) rename src/applications/config/controller/{ => settings}/PhabricatorConfigEditController.php (94%) create mode 100644 src/applications/config/controller/settings/PhabricatorConfigSettingsController.php rename src/applications/config/controller/{PhabricatorConfigHistoryController.php => settings/PhabricatorConfigSettingsHistoryController.php} (78%) create mode 100644 src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ebefb1db5d..d6c1499150 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2799,9 +2799,7 @@ phutil_register_library_map(array( 'PhabricatorConduitTokenQuery' => 'applications/conduit/query/PhabricatorConduitTokenQuery.php', 'PhabricatorConduitTokenTerminateController' => 'applications/conduit/controller/PhabricatorConduitTokenTerminateController.php', 'PhabricatorConduitTokensSettingsPanel' => 'applications/conduit/settings/PhabricatorConduitTokensSettingsPanel.php', - 'PhabricatorConfigAllController' => 'applications/config/controller/PhabricatorConfigAllController.php', 'PhabricatorConfigApplication' => 'applications/config/application/PhabricatorConfigApplication.php', - 'PhabricatorConfigApplicationController' => 'applications/config/controller/PhabricatorConfigApplicationController.php', 'PhabricatorConfigCacheController' => 'applications/config/controller/services/PhabricatorConfigCacheController.php', 'PhabricatorConfigClusterDatabasesController' => 'applications/config/controller/services/PhabricatorConfigClusterDatabasesController.php', 'PhabricatorConfigClusterNotificationsController' => 'applications/config/controller/services/PhabricatorConfigClusterNotificationsController.php', @@ -2810,6 +2808,7 @@ phutil_register_library_map(array( 'PhabricatorConfigCollectorsModule' => 'applications/config/module/PhabricatorConfigCollectorsModule.php', 'PhabricatorConfigColumnSchema' => 'applications/config/schema/PhabricatorConfigColumnSchema.php', 'PhabricatorConfigConfigPHIDType' => 'applications/config/phid/PhabricatorConfigConfigPHIDType.php', + 'PhabricatorConfigConsoleController' => 'applications/config/controller/PhabricatorConfigConsoleController.php', 'PhabricatorConfigConstants' => 'applications/config/constants/PhabricatorConfigConstants.php', 'PhabricatorConfigController' => 'applications/config/controller/PhabricatorConfigController.php', 'PhabricatorConfigCoreSchemaSpec' => 'applications/config/schema/PhabricatorConfigCoreSchemaSpec.php', @@ -2821,24 +2820,21 @@ phutil_register_library_map(array( 'PhabricatorConfigDefaultSource' => 'infrastructure/env/PhabricatorConfigDefaultSource.php', 'PhabricatorConfigDictionarySource' => 'infrastructure/env/PhabricatorConfigDictionarySource.php', 'PhabricatorConfigEdgeModule' => 'applications/config/module/PhabricatorConfigEdgeModule.php', - 'PhabricatorConfigEditController' => 'applications/config/controller/PhabricatorConfigEditController.php', + 'PhabricatorConfigEditController' => 'applications/config/controller/settings/PhabricatorConfigEditController.php', 'PhabricatorConfigEditor' => 'applications/config/editor/PhabricatorConfigEditor.php', 'PhabricatorConfigEntry' => 'applications/config/storage/PhabricatorConfigEntry.php', 'PhabricatorConfigEntryDAO' => 'applications/config/storage/PhabricatorConfigEntryDAO.php', 'PhabricatorConfigEntryQuery' => 'applications/config/query/PhabricatorConfigEntryQuery.php', 'PhabricatorConfigFileSource' => 'infrastructure/env/PhabricatorConfigFileSource.php', 'PhabricatorConfigGroupConstants' => 'applications/config/constants/PhabricatorConfigGroupConstants.php', - 'PhabricatorConfigGroupController' => 'applications/config/controller/PhabricatorConfigGroupController.php', 'PhabricatorConfigHTTPParameterTypesModule' => 'applications/config/module/PhabricatorConfigHTTPParameterTypesModule.php', - 'PhabricatorConfigHistoryController' => 'applications/config/controller/PhabricatorConfigHistoryController.php', - 'PhabricatorConfigIgnoreController' => 'applications/config/controller/PhabricatorConfigIgnoreController.php', - 'PhabricatorConfigIssueListController' => 'applications/config/controller/PhabricatorConfigIssueListController.php', - 'PhabricatorConfigIssuePanelController' => 'applications/config/controller/PhabricatorConfigIssuePanelController.php', - 'PhabricatorConfigIssueViewController' => 'applications/config/controller/PhabricatorConfigIssueViewController.php', + 'PhabricatorConfigIgnoreController' => 'applications/config/controller/issue/PhabricatorConfigIgnoreController.php', + 'PhabricatorConfigIssueListController' => 'applications/config/controller/issue/PhabricatorConfigIssueListController.php', + 'PhabricatorConfigIssuePanelController' => 'applications/config/controller/issue/PhabricatorConfigIssuePanelController.php', + 'PhabricatorConfigIssueViewController' => 'applications/config/controller/issue/PhabricatorConfigIssueViewController.php', 'PhabricatorConfigJSON' => 'applications/config/json/PhabricatorConfigJSON.php', 'PhabricatorConfigJSONOptionType' => 'applications/config/custom/PhabricatorConfigJSONOptionType.php', 'PhabricatorConfigKeySchema' => 'applications/config/schema/PhabricatorConfigKeySchema.php', - 'PhabricatorConfigListController' => 'applications/config/controller/PhabricatorConfigListController.php', 'PhabricatorConfigLocalSource' => 'infrastructure/env/PhabricatorConfigLocalSource.php', 'PhabricatorConfigManagementDeleteWorkflow' => 'applications/config/management/PhabricatorConfigManagementDeleteWorkflow.php', 'PhabricatorConfigManagementDoneWorkflow' => 'applications/config/management/PhabricatorConfigManagementDoneWorkflow.php', @@ -2849,12 +2845,12 @@ phutil_register_library_map(array( 'PhabricatorConfigManagementWorkflow' => 'applications/config/management/PhabricatorConfigManagementWorkflow.php', 'PhabricatorConfigManualActivity' => 'applications/config/storage/PhabricatorConfigManualActivity.php', 'PhabricatorConfigModule' => 'applications/config/module/PhabricatorConfigModule.php', - 'PhabricatorConfigModuleController' => 'applications/config/controller/PhabricatorConfigModuleController.php', + 'PhabricatorConfigModuleController' => 'applications/config/controller/module/PhabricatorConfigModuleController.php', 'PhabricatorConfigOption' => 'applications/config/option/PhabricatorConfigOption.php', 'PhabricatorConfigOptionType' => 'applications/config/custom/PhabricatorConfigOptionType.php', 'PhabricatorConfigPHIDModule' => 'applications/config/module/PhabricatorConfigPHIDModule.php', 'PhabricatorConfigProxySource' => 'infrastructure/env/PhabricatorConfigProxySource.php', - 'PhabricatorConfigPurgeCacheController' => 'applications/config/controller/PhabricatorConfigPurgeCacheController.php', + 'PhabricatorConfigPurgeCacheController' => 'applications/config/controller/services/PhabricatorConfigPurgeCacheController.php', 'PhabricatorConfigRegexOptionType' => 'applications/config/custom/PhabricatorConfigRegexOptionType.php', 'PhabricatorConfigRemarkupRule' => 'infrastructure/markup/rule/PhabricatorConfigRemarkupRule.php', 'PhabricatorConfigRequestExceptionHandlerModule' => 'applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php', @@ -2863,6 +2859,9 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaSpec' => 'applications/config/schema/PhabricatorConfigSchemaSpec.php', 'PhabricatorConfigServerSchema' => 'applications/config/schema/PhabricatorConfigServerSchema.php', 'PhabricatorConfigServicesController' => 'applications/config/controller/services/PhabricatorConfigServicesController.php', + 'PhabricatorConfigSettingsController' => 'applications/config/controller/settings/PhabricatorConfigSettingsController.php', + 'PhabricatorConfigSettingsHistoryController' => 'applications/config/controller/settings/PhabricatorConfigSettingsHistoryController.php', + 'PhabricatorConfigSettingsListController' => 'applications/config/controller/settings/PhabricatorConfigSettingsListController.php', 'PhabricatorConfigSetupCheckModule' => 'applications/config/module/PhabricatorConfigSetupCheckModule.php', 'PhabricatorConfigSiteModule' => 'applications/config/module/PhabricatorConfigSiteModule.php', 'PhabricatorConfigSiteSource' => 'infrastructure/env/PhabricatorConfigSiteSource.php', @@ -2874,7 +2873,6 @@ phutil_register_library_map(array( 'PhabricatorConfigTransactionQuery' => 'applications/config/query/PhabricatorConfigTransactionQuery.php', 'PhabricatorConfigType' => 'applications/config/type/PhabricatorConfigType.php', 'PhabricatorConfigValidationException' => 'applications/config/exception/PhabricatorConfigValidationException.php', - 'PhabricatorConfigVersionController' => 'applications/config/controller/PhabricatorConfigVersionController.php', 'PhabricatorConpherenceApplication' => 'applications/conpherence/application/PhabricatorConpherenceApplication.php', 'PhabricatorConpherenceColumnMinimizeSetting' => 'applications/settings/setting/PhabricatorConpherenceColumnMinimizeSetting.php', 'PhabricatorConpherenceColumnVisibleSetting' => 'applications/settings/setting/PhabricatorConpherenceColumnVisibleSetting.php', @@ -9134,9 +9132,7 @@ phutil_register_library_map(array( 'PhabricatorConduitTokenQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorConduitTokenTerminateController' => 'PhabricatorConduitController', 'PhabricatorConduitTokensSettingsPanel' => 'PhabricatorSettingsPanel', - 'PhabricatorConfigAllController' => 'PhabricatorConfigController', 'PhabricatorConfigApplication' => 'PhabricatorApplication', - 'PhabricatorConfigApplicationController' => 'PhabricatorConfigController', 'PhabricatorConfigCacheController' => 'PhabricatorConfigServicesController', 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigServicesController', 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigServicesController', @@ -9145,6 +9141,7 @@ phutil_register_library_map(array( 'PhabricatorConfigCollectorsModule' => 'PhabricatorConfigModule', 'PhabricatorConfigColumnSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigConfigPHIDType' => 'PhabricatorPHIDType', + 'PhabricatorConfigConsoleController' => 'PhabricatorConfigController', 'PhabricatorConfigConstants' => 'Phobject', 'PhabricatorConfigController' => 'PhabricatorController', 'PhabricatorConfigCoreSchemaSpec' => 'PhabricatorConfigSchemaSpec', @@ -9156,7 +9153,7 @@ phutil_register_library_map(array( 'PhabricatorConfigDefaultSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigDictionarySource' => 'PhabricatorConfigSource', 'PhabricatorConfigEdgeModule' => 'PhabricatorConfigModule', - 'PhabricatorConfigEditController' => 'PhabricatorConfigController', + 'PhabricatorConfigEditController' => 'PhabricatorConfigSettingsController', 'PhabricatorConfigEditor' => 'PhabricatorApplicationTransactionEditor', 'PhabricatorConfigEntry' => array( 'PhabricatorConfigEntryDAO', @@ -9167,9 +9164,7 @@ phutil_register_library_map(array( 'PhabricatorConfigEntryQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorConfigFileSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigGroupConstants' => 'PhabricatorConfigConstants', - 'PhabricatorConfigGroupController' => 'PhabricatorConfigController', 'PhabricatorConfigHTTPParameterTypesModule' => 'PhabricatorConfigModule', - 'PhabricatorConfigHistoryController' => 'PhabricatorConfigController', 'PhabricatorConfigIgnoreController' => 'PhabricatorConfigController', 'PhabricatorConfigIssueListController' => 'PhabricatorConfigController', 'PhabricatorConfigIssuePanelController' => 'PhabricatorConfigController', @@ -9177,7 +9172,6 @@ phutil_register_library_map(array( 'PhabricatorConfigJSON' => 'Phobject', 'PhabricatorConfigJSONOptionType' => 'PhabricatorConfigOptionType', 'PhabricatorConfigKeySchema' => 'PhabricatorConfigStorageSchema', - 'PhabricatorConfigListController' => 'PhabricatorConfigController', 'PhabricatorConfigLocalSource' => 'PhabricatorConfigProxySource', 'PhabricatorConfigManagementDeleteWorkflow' => 'PhabricatorConfigManagementWorkflow', 'PhabricatorConfigManagementDoneWorkflow' => 'PhabricatorConfigManagementWorkflow', @@ -9202,6 +9196,9 @@ phutil_register_library_map(array( 'PhabricatorConfigSchemaSpec' => 'Phobject', 'PhabricatorConfigServerSchema' => 'PhabricatorConfigStorageSchema', 'PhabricatorConfigServicesController' => 'PhabricatorConfigController', + 'PhabricatorConfigSettingsController' => 'PhabricatorConfigController', + 'PhabricatorConfigSettingsHistoryController' => 'PhabricatorConfigSettingsController', + 'PhabricatorConfigSettingsListController' => 'PhabricatorConfigSettingsController', 'PhabricatorConfigSetupCheckModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteModule' => 'PhabricatorConfigModule', 'PhabricatorConfigSiteSource' => 'PhabricatorConfigProxySource', @@ -9213,7 +9210,6 @@ phutil_register_library_map(array( 'PhabricatorConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorConfigType' => 'Phobject', 'PhabricatorConfigValidationException' => 'Exception', - 'PhabricatorConfigVersionController' => 'PhabricatorConfigController', 'PhabricatorConpherenceApplication' => 'PhabricatorApplication', 'PhabricatorConpherenceColumnMinimizeSetting' => 'PhabricatorInternalSetting', 'PhabricatorConpherenceColumnVisibleSetting' => 'PhabricatorInternalSetting', diff --git a/src/applications/almanac/controller/AlmanacConsoleController.php b/src/applications/almanac/controller/AlmanacConsoleController.php index 9c27461ac7..73f9e4799e 100644 --- a/src/applications/almanac/controller/AlmanacConsoleController.php +++ b/src/applications/almanac/controller/AlmanacConsoleController.php @@ -10,7 +10,7 @@ final class AlmanacConsoleController extends AlmanacController { $viewer = $request->getViewer(); $menu = id(new PHUIObjectItemListView()) - ->setUser($viewer) + ->setViewer($viewer) ->setBig(true); $menu->addItem( @@ -84,7 +84,6 @@ final class AlmanacConsoleController extends AlmanacController { ->setTitle(pht('Almanac Console')) ->setCrumbs($crumbs) ->appendChild($view); - } } diff --git a/src/applications/config/application/PhabricatorConfigApplication.php b/src/applications/config/application/PhabricatorConfigApplication.php index a8c7f2ff5b..0e5e5972e1 100644 --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -37,13 +37,12 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { public function getRoutes() { return array( '/config/' => array( - '' => 'PhabricatorConfigListController', + '' => 'PhabricatorConfigConsoleController', 'application/' => 'PhabricatorConfigApplicationController', 'all/' => 'PhabricatorConfigAllController', 'history/' => 'PhabricatorConfigHistoryController', 'edit/(?P[\w\.\-]+)/' => 'PhabricatorConfigEditController', 'group/(?P[^/]+)/' => 'PhabricatorConfigGroupController', - 'version/' => 'PhabricatorConfigVersionController', 'database/'. '(?:(?P[^/]+)/'. '(?:(?P[^/]+)/'. @@ -71,6 +70,12 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { 'repositories/' => 'PhabricatorConfigClusterRepositoriesController', 'search/' => 'PhabricatorConfigClusterSearchController', ), + 'settings/' => array( + '' => 'PhabricatorConfigSettingsListController', + '(?Padvanced|all)/' + => 'PhabricatorConfigSettingsListController', + 'history/' => 'PhabricatorConfigSettingsHistoryController', + ), ), ); } diff --git a/src/applications/config/check/PhabricatorAuthSetupCheck.php b/src/applications/config/check/PhabricatorAuthSetupCheck.php index 59bc17ecce..e2b076944e 100644 --- a/src/applications/config/check/PhabricatorAuthSetupCheck.php +++ b/src/applications/config/check/PhabricatorAuthSetupCheck.php @@ -53,20 +53,23 @@ final class PhabricatorAuthSetupCheck extends PhabricatorSetupCheck { "\n\n". 'Leaving your authentication provider configuration unlocked '. 'increases the damage that a compromised administrator account can '. - 'do to your install, by, for example, changing the authentication '. - 'provider to a server they control and intercepting usernames and '. + 'do to your install. For example, an attacker who compromises an '. + 'administrator account can change authentication providers to point '. + 'at a server they control and attempt to intercept usernames and '. 'passwords.'. "\n\n". - 'To prevent this attack, you should configure your authentication '. - 'providers, and then lock the configuration by doing `%s` '. - 'from the command line. This will prevent changing the '. - 'authentication provider config without first doing `%s`.', - 'bin/auth lock', - 'bin/auth unlock'); + 'To prevent this attack, you should configure authentication, and '. + 'then lock the configuration by running "bin/auth lock" from the '. + 'command line. This will prevent changing the authentication config '. + 'without first running "bin/auth unlock".'); $this ->newIssue('auth.config-unlocked') ->setShortName(pht('Auth Config Unlocked')) - ->setName(pht('Authenticaton Provider Configuration Unlocked')) + ->setName(pht('Authenticaton Configuration Unlocked')) + ->setSummary( + pht( + 'Authentication configuration is currently unlocked. Once you '. + 'finish configuring authentication, you should lock it.')) ->setMessage($message) ->addRelatedPhabricatorConfig('auth.lock-config') ->addCommand( diff --git a/src/applications/config/check/PhabricatorSecuritySetupCheck.php b/src/applications/config/check/PhabricatorSecuritySetupCheck.php index 518f6c7d54..0cd1b41504 100644 --- a/src/applications/config/check/PhabricatorSecuritySetupCheck.php +++ b/src/applications/config/check/PhabricatorSecuritySetupCheck.php @@ -58,8 +58,7 @@ final class PhabricatorSecuritySetupCheck extends PhabricatorSetupCheck { ->setName(pht('Alternate File Domain Not Configured')) ->setSummary( pht( - 'Increase security (and improve performance) by configuring '. - 'a CDN or alternate file domain.')) + 'Improve security by configuring an alternate file domain.')) ->setMessage( pht( 'Phabricator is currently configured to serve user uploads '. diff --git a/src/applications/config/controller/PhabricatorConfigAllController.php b/src/applications/config/controller/PhabricatorConfigAllController.php deleted file mode 100644 index 7452b29e21..0000000000 --- a/src/applications/config/controller/PhabricatorConfigAllController.php +++ /dev/null @@ -1,77 +0,0 @@ -getViewer(); - - $db_values = id(new PhabricatorConfigEntry()) - ->loadAllWhere('namespace = %s', 'default'); - $db_values = mpull($db_values, null, 'getConfigKey'); - - $rows = array(); - $options = PhabricatorApplicationConfigOptions::loadAllOptions(); - ksort($options); - foreach ($options as $option) { - $key = $option->getKey(); - - if ($option->getHidden()) { - $value = phutil_tag('em', array(), pht('Hidden')); - } else { - $value = PhabricatorEnv::getEnvConfig($key); - $value = PhabricatorConfigJSON::prettyPrintJSON($value); - } - - $db_value = idx($db_values, $key); - $rows[] = array( - phutil_tag( - 'a', - array( - 'href' => $this->getApplicationURI('edit/'.$key.'/'), - ), - $key), - $value, - $db_value && !$db_value->getIsDeleted() ? pht('Customized') : '', - ); - } - $table = id(new AphrontTableView($rows)) - ->setColumnClasses( - array( - '', - 'wide', - )) - ->setHeaders( - array( - pht('Key'), - pht('Value'), - pht('Customized'), - )); - - $title = pht('Current Settings'); - $header = $this->buildHeaderView($title); - - $nav = $this->buildSideNavView(); - $nav->selectFilter('all/'); - - $view = $this->buildConfigBoxView( - pht('All Settings'), - $table); - - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); - - $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($view); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->setNavigation($nav) - ->appendChild($content); - - } - -} diff --git a/src/applications/config/controller/PhabricatorConfigApplicationController.php b/src/applications/config/controller/PhabricatorConfigApplicationController.php deleted file mode 100644 index a6b8cd38c6..0000000000 --- a/src/applications/config/controller/PhabricatorConfigApplicationController.php +++ /dev/null @@ -1,57 +0,0 @@ -getViewer(); - - $nav = $this->buildSideNavView(); - $nav->selectFilter('application/'); - - $groups = PhabricatorApplicationConfigOptions::loadAll(); - $apps_list = $this->buildConfigOptionsList($groups, 'apps'); - $apps_list = $this->buildConfigBoxView(pht('Applications'), $apps_list); - - $title = pht('Application Settings'); - $header = $this->buildHeaderView($title); - - $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($apps_list); - - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->setNavigation($nav) - ->appendChild($content); - } - - private function buildConfigOptionsList(array $groups, $type) { - assert_instances_of($groups, 'PhabricatorApplicationConfigOptions'); - - $list = new PHUIObjectItemListView(); - $list->setBig(true); - $groups = msort($groups, 'getName'); - foreach ($groups as $group) { - if ($group->getGroup() == $type) { - $icon = id(new PHUIIconView()) - ->setIcon($group->getIcon()) - ->setBackground('bg-violet'); - $item = id(new PHUIObjectItemView()) - ->setHeader($group->getName()) - ->setHref('/config/group/'.$group->getKey().'/') - ->addAttribute($group->getDescription()) - ->setImageIcon($icon); - $list->addItem($item); - } - } - - return $list; - } - -} diff --git a/src/applications/config/controller/PhabricatorConfigVersionController.php b/src/applications/config/controller/PhabricatorConfigConsoleController.php similarity index 52% rename from src/applications/config/controller/PhabricatorConfigVersionController.php rename to src/applications/config/controller/PhabricatorConfigConsoleController.php index 153d363062..e09cecec79 100644 --- a/src/applications/config/controller/PhabricatorConfigVersionController.php +++ b/src/applications/config/controller/PhabricatorConfigConsoleController.php @@ -1,92 +1,139 @@ getViewer(); - $title = pht('Version Information'); - $versions = $this->renderModuleStatus($viewer); + $menu = id(new PHUIObjectItemListView()) + ->setViewer($viewer) + ->setBig(true); - $nav = $this->buildSideNavView(); - $nav->selectFilter('version/'); - $header = $this->buildHeaderView($title); + $menu->addItem( + id(new PHUIObjectItemView()) + ->setHeader(pht('Settings')) + ->setHref($this->getApplicationURI('settings/')) + ->setImageIcon('fa-wrench') + ->setClickable(true) + ->addAttribute( + pht( + 'Review and modify configuration settings.'))); - $view = $this->buildConfigBoxView( - pht('Installed Versions'), - $versions); + $menu->addItem( + id(new PHUIObjectItemView()) + ->setHeader(pht('Setup Issues')) + ->setHref($this->getApplicationURI('issue/')) + ->setImageIcon('fa-exclamation-triangle') + ->setClickable(true) + ->addAttribute( + pht( + 'Show unresolved issues with setup and configuration.'))); + + $menu->addItem( + id(new PHUIObjectItemView()) + ->setHeader(pht('Services')) + ->setHref($this->getApplicationURI('cluster/databases/')) + ->setImageIcon('fa-server') + ->setClickable(true) + ->addAttribute( + pht( + 'View status information for databases, caches, repositories, '. + 'and other services.'))); + + $menu->addItem( + id(new PHUIObjectItemView()) + ->setHeader(pht('Extensions/Modules')) + ->setHref($this->getApplicationURI('module/')) + ->setImageIcon('fa-gear') + ->setClickable(true) + ->addAttribute( + pht( + 'Show installed extensions and modules.'))); $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) + ->addTextCrumb(pht('Console')) ->setBorder(true); - $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($view); + $box = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Phabricator Configuation')) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setObjectList($menu); + + $versions = $this->newLibraryVersionTable($viewer); + $binary_versions = $this->newBinaryVersionTable(); + + $launcher_view = id(new PHUILauncherView()) + ->appendChild($box) + ->appendChild($versions) + ->appendChild($binary_versions); + + $view = id(new PHUITwoColumnView()) + ->setFooter($launcher_view); return $this->newPage() - ->setTitle($title) + ->setTitle(pht('Phabricator Configuation')) ->setCrumbs($crumbs) - ->setNavigation($nav) - ->appendChild($content); + ->appendChild($view); } - public function renderModuleStatus($viewer) { + public function newLibraryVersionTable() { + $viewer = $this->getViewer(); + $versions = $this->loadVersions($viewer); - $version_property_list = id(new PHUIPropertyListView()); + $rows = array(); foreach ($versions as $name => $info) { - $version = $info['version']; - - if ($info['branchpoint']) { - $display = pht( - '%s (branched from %s on %s)', - $version, - $info['branchpoint'], - $info['upstream']); + $branchpoint = $info['branchpoint']; + if (strlen($branchpoint)) { + $branchpoint = substr($branchpoint, 0, 12); } else { - $display = $version; + $branchpoint = null; } - $version_property_list->addProperty($name, $display); - } - - $phabricator_root = dirname(phutil_get_library_root('phabricator')); - $version_path = $phabricator_root.'/conf/local/VERSION'; - if (Filesystem::pathExists($version_path)) { - $version_from_file = Filesystem::readFile($version_path); - $version_property_list->addProperty( - pht('Local Version'), - $version_from_file); - } - - $version_property_list->addProperty('php', phpversion()); - - $binaries = PhutilBinaryAnalyzer::getAllBinaries(); - foreach ($binaries as $binary) { - if (!$binary->isBinaryAvailable()) { - $binary_info = pht('Not Available'); + $version = $info['hash']; + if (strlen($version)) { + $version = substr($version, 0, 12); } else { - $version = $binary->getBinaryVersion(); - $path = $binary->getBinaryPath(); - if ($path === null && $version === null) { - $binary_info = pht('-'); - } else if ($path === null) { - $binary_info = $version; - } else if ($version === null) { - $binary_info = pht('- at %s', $path); - } else { - $binary_info = pht('%s at %s', $version, $path); - } + $version = pht('Unknown'); } - $version_property_list->addProperty( - $binary->getBinaryName(), - $binary_info); + + $epoch = $info['epoch']; + if ($epoch) { + $epoch = phabricator_date($epoch, $viewer); + } else { + $epoch = null; + } + + $rows[] = array( + $name, + $version, + $epoch, + $branchpoint, + ); } - return $version_property_list; + $table_view = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Library'), + pht('Version'), + pht('Date'), + pht('Branchpoint'), + )) + ->setColumnClasses( + array( + 'pri', + null, + null, + 'wide', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Phabricator Version Information')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($table_view); } private function loadVersions(PhabricatorUser $viewer) { @@ -207,13 +254,14 @@ final class PhabricatorConfigVersionController list($err, $stdout) = $future->resolve(); if (!$err) { list($hash, $epoch) = explode(' ', $stdout); - $version = pht('%s (%s)', $hash, phabricator_date($epoch, $viewer)); } else { - $version = pht('Unknown'); + $hash = null; + $epoch = null; } $result = array( - 'version' => $version, + 'hash' => $hash, + 'epoch' => $epoch, 'upstream' => null, 'branchpoint' => null, ); @@ -239,4 +287,51 @@ final class PhabricatorConfigVersionController return $results; } + private function newBinaryVersionTable() { + $rows = array(); + + $rows[] = array( + 'php', + phpversion(), + php_sapi_name(), + ); + + $binaries = PhutilBinaryAnalyzer::getAllBinaries(); + foreach ($binaries as $binary) { + if (!$binary->isBinaryAvailable()) { + $binary_version = pht('Not Available'); + $binary_path = null; + } else { + $binary_version = $binary->getBinaryVersion(); + $binary_path = $binary->getBinaryPath(); + } + + $rows[] = array( + $binary->getBinaryName(), + $binary_version, + $binary_path, + ); + } + + $table_view = id(new AphrontTableView($rows)) + ->setHeaders( + array( + pht('Binary'), + pht('Version'), + pht('Path'), + )) + ->setColumnClasses( + array( + 'pri', + null, + 'wide', + )); + + return id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Other Version Information')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->appendChild($table_view); + } + + } diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php index 1aa9946236..bcdaf968fc 100644 --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -6,33 +6,6 @@ abstract class PhabricatorConfigController extends PhabricatorController { return true; } - public function buildSideNavView($filter = null, $for_app = false) { - $guide_href = new PhutilURI('/guides/'); - $nav = new AphrontSideNavFilterView(); - $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); - $nav->addFilter('/', - pht('Core Settings'), null, 'fa-gear'); - $nav->addFilter('application/', - pht('Application Settings'), null, 'fa-globe'); - $nav->addFilter('history/', - pht('Settings History'), null, 'fa-history'); - $nav->addFilter('version/', - pht('Version Information'), null, 'fa-download'); - $nav->addFilter('all/', - pht('All Settings'), null, 'fa-list-ul'); - $nav->addLabel(pht('Setup')); - $nav->addFilter('issue/', - pht('Setup Issues'), null, 'fa-warning'); - $nav->addFilter(null, - pht('Installation Guide'), $guide_href, 'fa-book'); - - return $nav; - } - - public function buildApplicationMenu() { - return $this->buildSideNavView(null, true)->getMenu(); - } - public function buildHeaderView($text, $action = null) { $viewer = $this->getViewer(); diff --git a/src/applications/config/controller/PhabricatorConfigGroupController.php b/src/applications/config/controller/PhabricatorConfigGroupController.php deleted file mode 100644 index f981c1c1a1..0000000000 --- a/src/applications/config/controller/PhabricatorConfigGroupController.php +++ /dev/null @@ -1,112 +0,0 @@ -getViewer(); - $group_key = $request->getURIData('key'); - - $groups = PhabricatorApplicationConfigOptions::loadAll(); - $options = idx($groups, $group_key); - if (!$options) { - return new Aphront404Response(); - } - - $group_uri = PhabricatorConfigGroupConstants::getGroupFullURI( - $options->getGroup()); - $group_name = PhabricatorConfigGroupConstants::getGroupShortName( - $options->getGroup()); - - $nav = $this->buildSideNavView(); - $nav->selectFilter($group_uri); - - $title = pht('%s Configuration', $options->getName()); - $header = $this->buildHeaderView($title); - $list = $this->buildOptionList($options->getOptions()); - $group_url = phutil_tag('a', array('href' => $group_uri), $group_name); - - $box_header = pht("%s \xC2\xBB %s", $group_url, $options->getName()); - $view = $this->buildConfigBoxView($box_header, $list); - - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($group_name, $group_uri) - ->addTextCrumb($options->getName()) - ->setBorder(true); - - $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($view); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->setNavigation($nav) - ->appendChild($content); - } - - private function buildOptionList(array $options) { - assert_instances_of($options, 'PhabricatorConfigOption'); - - require_celerity_resource('config-options-css'); - - $db_values = array(); - if ($options) { - $db_values = id(new PhabricatorConfigEntry())->loadAllWhere( - 'configKey IN (%Ls) AND namespace = %s', - mpull($options, 'getKey'), - 'default'); - $db_values = mpull($db_values, null, 'getConfigKey'); - } - - $list = new PHUIObjectItemListView(); - $list->setBig(true); - foreach ($options as $option) { - $summary = $option->getSummary(); - - $item = id(new PHUIObjectItemView()) - ->setHeader($option->getKey()) - ->setHref('/config/edit/'.$option->getKey().'/') - ->addAttribute($summary); - - $color = null; - $db_value = idx($db_values, $option->getKey()); - if ($db_value && !$db_value->getIsDeleted()) { - $item->setEffect('visited'); - $color = 'violet'; - } - - if ($option->getHidden()) { - $item->setStatusIcon('fa-eye-slash grey', pht('Hidden')); - $item->setDisabled(true); - } else if ($option->getLocked()) { - $item->setStatusIcon('fa-lock '.$color, pht('Locked')); - } else if ($color) { - $item->setStatusIcon('fa-pencil '.$color, pht('Editable')); - } else { - $item->setStatusIcon('fa-pencil-square-o '.$color, pht('Editable')); - } - - if (!$option->getHidden()) { - $current_value = PhabricatorEnv::getEnvConfig($option->getKey()); - $current_value = PhabricatorConfigJSON::prettyPrintJSON( - $current_value); - $current_value = phutil_tag( - 'div', - array( - 'class' => 'config-options-current-value '.$color, - ), - array( - $current_value, - )); - - $item->setSideColumn($current_value); - } - - $list->addItem($item); - } - - return $list; - } - -} diff --git a/src/applications/config/controller/PhabricatorConfigListController.php b/src/applications/config/controller/PhabricatorConfigListController.php deleted file mode 100644 index 38a0afc328..0000000000 --- a/src/applications/config/controller/PhabricatorConfigListController.php +++ /dev/null @@ -1,57 +0,0 @@ -getViewer(); - - $nav = $this->buildSideNavView(); - $nav->selectFilter('/'); - - $groups = PhabricatorApplicationConfigOptions::loadAll(); - $core_list = $this->buildConfigOptionsList($groups, 'core'); - $core_list = $this->buildConfigBoxView(pht('Core'), $core_list); - - $title = pht('Core Settings'); - $header = $this->buildHeaderView($title); - - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); - - $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($core_list); - - return $this->newPage() - ->setTitle($title) - ->setCrumbs($crumbs) - ->setNavigation($nav) - ->appendChild($content); - } - - private function buildConfigOptionsList(array $groups, $type) { - assert_instances_of($groups, 'PhabricatorApplicationConfigOptions'); - - $list = new PHUIObjectItemListView(); - $list->setBig(true); - $groups = msort($groups, 'getName'); - foreach ($groups as $group) { - if ($group->getGroup() == $type) { - $icon = id(new PHUIIconView()) - ->setIcon($group->getIcon()) - ->setBackground('bg-blue'); - $item = id(new PHUIObjectItemView()) - ->setHeader($group->getName()) - ->setHref('/config/group/'.$group->getKey().'/') - ->addAttribute($group->getDescription()) - ->setImageIcon($icon); - $list->addItem($item); - } - } - - return $list; - } - -} diff --git a/src/applications/config/controller/PhabricatorConfigIgnoreController.php b/src/applications/config/controller/issue/PhabricatorConfigIgnoreController.php similarity index 100% rename from src/applications/config/controller/PhabricatorConfigIgnoreController.php rename to src/applications/config/controller/issue/PhabricatorConfigIgnoreController.php diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/issue/PhabricatorConfigIssueListController.php similarity index 64% rename from src/applications/config/controller/PhabricatorConfigIssueListController.php rename to src/applications/config/controller/issue/PhabricatorConfigIssueListController.php index 6518ccec97..b1f00fc013 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueListController.php +++ b/src/applications/config/controller/issue/PhabricatorConfigIssueListController.php @@ -6,9 +6,6 @@ final class PhabricatorConfigIssueListController public function handleRequest(AphrontRequest $request) { $viewer = $request->getViewer(); - $nav = $this->buildSideNavView(); - $nav->selectFilter('issue/'); - $engine = new PhabricatorSetupEngine(); $response = $engine->execute(); if ($response) { @@ -34,7 +31,6 @@ final class PhabricatorConfigIssueListController 'fa-question-circle'); $title = pht('Setup Issues'); - $header = $this->buildHeaderView($title); if (!$issues) { $issue_list = id(new PHUIInfoView()) @@ -50,21 +46,24 @@ final class PhabricatorConfigIssueListController $other, ); - $issue_list = $this->buildConfigBoxView(pht('Issues'), $issue_list); + $issue_list = $this->buildConfigBoxView( + pht('Unresolved Setup Issues'), + $issue_list); } $crumbs = $this->buildApplicationCrumbs() ->addTextCrumb($title) ->setBorder(true); + $launcher_view = id(new PHUILauncherView()) + ->appendChild($issue_list); + $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($issue_list); + ->setFooter($launcher_view); return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->setNavigation($nav) ->appendChild($content); } @@ -76,27 +75,30 @@ final class PhabricatorConfigIssueListController $items = 0; foreach ($issues as $issue) { - if ($issue->getGroup() == $group) { - $items++; - $href = $this->getApplicationURI('/issue/'.$issue->getIssueKey().'/'); - $item = id(new PHUIObjectItemView()) - ->setHeader($issue->getName()) - ->setHref($href) - ->addAttribute($issue->getSummary()); - if (!$issue->getIsIgnored()) { - $icon = id(new PHUIIconView()) - ->setIcon($fonticon) - ->setBackground('bg-sky'); - $item->setImageIcon($icon); - $list->addItem($item); - } else { - $icon = id(new PHUIIconView()) - ->setIcon('fa-eye-slash') - ->setBackground('bg-grey'); - $item->setDisabled(true); - $item->setImageIcon($icon); - $ignored_items[] = $item; - } + if ($issue->getGroup() != $group) { + continue; + } + + $items++; + $href = $this->getApplicationURI('/issue/'.$issue->getIssueKey().'/'); + $item = id(new PHUIObjectItemView()) + ->setHeader($issue->getName()) + ->setHref($href) + ->setClickable(true) + ->addAttribute($issue->getSummary()); + if (!$issue->getIsIgnored()) { + $icon = id(new PHUIIconView()) + ->setIcon($fonticon) + ->setBackground('bg-sky'); + $item->setImageIcon($icon); + $list->addItem($item); + } else { + $icon = id(new PHUIIconView()) + ->setIcon('fa-eye-slash') + ->setBackground('bg-grey'); + $item->setDisabled(true); + $item->setImageIcon($icon); + $ignored_items[] = $item; } } diff --git a/src/applications/config/controller/PhabricatorConfigIssuePanelController.php b/src/applications/config/controller/issue/PhabricatorConfigIssuePanelController.php similarity index 100% rename from src/applications/config/controller/PhabricatorConfigIssuePanelController.php rename to src/applications/config/controller/issue/PhabricatorConfigIssuePanelController.php diff --git a/src/applications/config/controller/PhabricatorConfigIssueViewController.php b/src/applications/config/controller/issue/PhabricatorConfigIssueViewController.php similarity index 88% rename from src/applications/config/controller/PhabricatorConfigIssueViewController.php rename to src/applications/config/controller/issue/PhabricatorConfigIssueViewController.php index 2967169e38..d42d9701c6 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueViewController.php +++ b/src/applications/config/controller/issue/PhabricatorConfigIssueViewController.php @@ -14,9 +14,6 @@ final class PhabricatorConfigIssueViewController } $issues = $engine->getIssues(); - $nav = $this->buildSideNavView(); - $nav->selectFilter('issue/'); - if (empty($issues[$issue_key])) { $content = id(new PHUIInfoView()) ->setSeverity(PHUIInfoView::SEVERITY_NOTICE) @@ -36,23 +33,21 @@ final class PhabricatorConfigIssueViewController $title = $issue->getShortName(); } - $header = $this->buildHeaderView($title); - $crumbs = $this ->buildApplicationCrumbs() - ->setBorder(true) ->addTextCrumb(pht('Setup Issues'), $this->getApplicationURI('issue/')) ->addTextCrumb($title, $request->getRequestURI()) ->setBorder(true); + $launcher_view = id(new PHUILauncherView()) + ->appendChild($content); + $content = id(new PHUITwoColumnView()) - ->setHeader($header) - ->setFooter($content); + ->setFooter($launcher_view); return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->setNavigation($nav) ->appendChild($content); } diff --git a/src/applications/config/controller/PhabricatorConfigModuleController.php b/src/applications/config/controller/module/PhabricatorConfigModuleController.php similarity index 100% rename from src/applications/config/controller/PhabricatorConfigModuleController.php rename to src/applications/config/controller/module/PhabricatorConfigModuleController.php diff --git a/src/applications/config/controller/PhabricatorConfigPurgeCacheController.php b/src/applications/config/controller/services/PhabricatorConfigPurgeCacheController.php similarity index 100% rename from src/applications/config/controller/PhabricatorConfigPurgeCacheController.php rename to src/applications/config/controller/services/PhabricatorConfigPurgeCacheController.php diff --git a/src/applications/config/controller/PhabricatorConfigEditController.php b/src/applications/config/controller/settings/PhabricatorConfigEditController.php similarity index 94% rename from src/applications/config/controller/PhabricatorConfigEditController.php rename to src/applications/config/controller/settings/PhabricatorConfigEditController.php index 381b54e046..2cb217db69 100644 --- a/src/applications/config/controller/PhabricatorConfigEditController.php +++ b/src/applications/config/controller/settings/PhabricatorConfigEditController.php @@ -1,7 +1,7 @@ getViewer(); @@ -30,11 +30,9 @@ final class PhabricatorConfigEditController ->setDefault(null) ->setDescription($desc); $group = null; - $group_uri = $this->getApplicationURI(); } else { $option = $options[$key]; $group = $option->getGroup(); - $group_uri = $this->getApplicationURI('group/'.$group->getKey().'/'); } $issue = $request->getStr('issue'); @@ -42,7 +40,7 @@ final class PhabricatorConfigEditController // If the user came here from an open setup issue, send them back. $done_uri = $this->getApplicationURI('issue/'.$issue.'/'); } else { - $done_uri = $group_uri; + $done_uri = $this->getApplicationURI('settings/'); } // Check if the config key is already stored in the database. @@ -205,23 +203,10 @@ final class PhabricatorConfigEditController $title = $key; $box_header = array(); - if ($group) { - $box_header[] = phutil_tag( - 'a', - array( - 'href' => $group_uri, - ), - $group->getName()); - $box_header[] = " \xC2\xBB "; - } $box_header[] = $key; - $crumbs = $this->buildApplicationCrumbs(); - if ($group) { - $crumbs->addTextCrumb($group->getName(), $group_uri); - } - $crumbs->addTextCrumb($key, '/config/edit/'.$key); - $crumbs->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($key, '/config/edit/'.$key); $form_box = $this->buildConfigBoxView($box_header, $form, $tag); @@ -230,9 +215,6 @@ final class PhabricatorConfigEditController new PhabricatorConfigTransactionQuery()); $timeline->setShouldTerminate(true); - $nav = $this->buildSideNavView(); - $nav->selectFilter($group_uri); - $header = $this->buildHeaderView($title); $view = id(new PHUITwoColumnView()) @@ -249,7 +231,6 @@ final class PhabricatorConfigEditController return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->setNavigation($nav) ->appendChild($view); } diff --git a/src/applications/config/controller/settings/PhabricatorConfigSettingsController.php b/src/applications/config/controller/settings/PhabricatorConfigSettingsController.php new file mode 100644 index 0000000000..7000fa3613 --- /dev/null +++ b/src/applications/config/controller/settings/PhabricatorConfigSettingsController.php @@ -0,0 +1,51 @@ +getApplicationURI('settings/'); + + $nav = id(new AphrontSideNavFilterView()) + ->setBaseURI(new PhutilURI($settings_uri)); + + $nav->addLabel(pht('Configuration')); + + $nav->newLink('settings') + ->setName(pht('Core Settings')) + ->setIcon('fa-wrench') + ->setHref($settings_uri); + + $nav->newLink('advanced') + ->setName(pht('Advanced Settings')) + ->setIcon('fa-cogs') + ->setHref(urisprintf('%s%s/', $settings_uri, 'advanced')); + + $nav->newLink('all') + ->setName(pht('All Settings')) + ->setIcon('fa-list') + ->setHref(urisprintf('%s%s/', $settings_uri, 'all')); + + $nav->addLabel(pht('History')); + + $nav->newLink('history') + ->setName(pht('View History')) + ->setIcon('fa-history') + ->setHref(urisprintf('%s%s/', $settings_uri, 'history')); + + if ($select_filter) { + $nav->selectFilter($select_filter); + } + + return $nav; + } + + public function newCrumbs() { + $settings_uri = $this->getApplicationURI('settings/'); + + return $this->buildApplicationCrumbs() + ->addTextCrumb(pht('Settings'), $settings_uri) + ->setBorder(true); + } + +} diff --git a/src/applications/config/controller/PhabricatorConfigHistoryController.php b/src/applications/config/controller/settings/PhabricatorConfigSettingsHistoryController.php similarity index 78% rename from src/applications/config/controller/PhabricatorConfigHistoryController.php rename to src/applications/config/controller/settings/PhabricatorConfigSettingsHistoryController.php index 495102b6a2..86f6c50773 100644 --- a/src/applications/config/controller/PhabricatorConfigHistoryController.php +++ b/src/applications/config/controller/settings/PhabricatorConfigSettingsHistoryController.php @@ -1,7 +1,7 @@ getViewer(); @@ -27,12 +27,10 @@ final class PhabricatorConfigHistoryController $title = pht('Settings History'); $header = $this->buildHeaderView($title); - $nav = $this->buildSideNavView(); - $nav->selectFilter('history/'); + $nav = $this->newNavigation('history'); - $crumbs = $this->buildApplicationCrumbs() - ->addTextCrumb($title) - ->setBorder(true); + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); $content = id(new PHUITwoColumnView()) ->setHeader($header) diff --git a/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php b/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php new file mode 100644 index 0000000000..8513150134 --- /dev/null +++ b/src/applications/config/controller/settings/PhabricatorConfigSettingsListController.php @@ -0,0 +1,107 @@ +getViewer(); + + $filter = $request->getURIData('filter'); + if (!strlen($filter)) { + $filter = 'settings'; + } + + $is_core = ($filter === 'settings'); + $is_advanced = ($filter === 'advanced'); + $is_all = ($filter === 'all'); + + $show_core = ($is_core || $is_all); + $show_advanced = ($is_advanced || $is_all); + + if ($is_core) { + $title = pht('Core Settings'); + } else if ($is_advanced) { + $title = pht('Advanced Settings'); + } else { + $title = pht('All Settings'); + } + + $db_values = id(new PhabricatorConfigEntry()) + ->loadAllWhere('namespace = %s', 'default'); + $db_values = mpull($db_values, null, 'getConfigKey'); + + $list = id(new PHUIObjectItemListView()) + ->setBig(true) + ->setFlush(true); + + $rows = array(); + $options = PhabricatorApplicationConfigOptions::loadAllOptions(); + ksort($options); + foreach ($options as $option) { + $key = $option->getKey(); + + $is_advanced = (bool)$option->getLocked(); + if ($is_advanced && !$show_advanced) { + continue; + } + + if (!$is_advanced && !$show_core) { + continue; + } + + $db_value = idx($db_values, $key); + + $item = $this->newConfigOptionView($option, $db_value); + $list->addItem($item); + } + + $header = $this->buildHeaderView($title); + + $crumbs = $this->newCrumbs() + ->addTextCrumb($title); + + $content = id(new PHUITwoColumnView()) + ->setHeader($header) + ->setFooter($list); + + $nav = $this->newNavigation($filter); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->setNavigation($nav) + ->appendChild($content); + } + + private function newConfigOptionView( + PhabricatorConfigOption $option, + PhabricatorConfigEntry $stored_value = null) { + + $summary = $option->getSummary(); + + $item = id(new PHUIObjectItemView()) + ->setHeader($option->getKey()) + ->setClickable(true) + ->setHref('/config/edit/'.$option->getKey().'/') + ->addAttribute($summary); + + $color = null; + if ($stored_value && !$stored_value->getIsDeleted()) { + $item->setEffect('visited'); + $color = 'violet'; + } + + if ($option->getHidden()) { + $item->setStatusIcon('fa-eye-slash', pht('Hidden')); + } else if ($option->getLocked()) { + $item->setStatusIcon('fa-lock '.$color, pht('Locked')); + } else if ($color) { + $item->setStatusIcon('fa-pencil '.$color, pht('Editable')); + } else { + $item->setStatusIcon('fa-circle-o grey', pht('Default')); + } + + return $item; + } + +} diff --git a/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php b/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php index b42ff85c2e..7a00673ae1 100644 --- a/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php +++ b/src/applications/notification/setup/PhabricatorAphlictSetupCheck.php @@ -24,6 +24,10 @@ final class PhabricatorAphlictSetupCheck extends PhabricatorSetupCheck { $this->newIssue('aphlict.connect') ->setShortName(pht('Notification Server Down')) ->setName(pht('Unable to Connect to Notification Server')) + ->setSummary( + pht( + 'Phabricator is configured to use a notification server, '. + 'but is not able to connect to it.')) ->setMessage($message) ->addRelatedPhabricatorConfig('notification.servers') ->addCommand( From 4904d7711ea14383a35c9c33249551771ba64f97 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 30 Jan 2020 10:06:35 -0800 Subject: [PATCH 07/16] When publishing a commit, copy "Related Tasks" from the associated revision (if one exists) Summary: Fixes T13463. Currently, if you use the web UI to set "Related Tasks" for a revision, the resulting commit does not link to the tasks. If you use "Ref ..." in the message instead, the resulting commit does link to the tasks. Broadly, this should all be cleaner (see T3577) but we can step toward better behavior by just copying these edges when commits are published. Test Plan: - Created a revision. - Used the web UI to edit "Related Tasks". - Landed the revision. - Saw the commit link to the tasks as though I'd used "Ref ..." in the message. Maniphest Tasks: T13463 Differential Revision: https://secure.phabricator.com/D20961 --- .../audit/editor/PhabricatorAuditEditor.php | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/src/applications/audit/editor/PhabricatorAuditEditor.php b/src/applications/audit/editor/PhabricatorAuditEditor.php index 7fdb586f56..887f29a3d8 100644 --- a/src/applications/audit/editor/PhabricatorAuditEditor.php +++ b/src/applications/audit/editor/PhabricatorAuditEditor.php @@ -405,6 +405,31 @@ final class PhabricatorAuditEditor $phid_map[] = $reverted_phids; } + // See T13463. Copy "related task" edges from the associated revision, if + // one exists. + + $revision = DiffusionCommitRevisionQuery::loadRevisionForCommit( + $actor, + $object); + if ($revision) { + $task_phids = PhabricatorEdgeQuery::loadDestinationPHIDs( + $revision->getPHID(), + DifferentialRevisionHasTaskEdgeType::EDGECONST); + $task_phids = array_fuse($task_phids); + + if ($task_phids) { + $related_edge = DiffusionCommitHasTaskEdgeType::EDGECONST; + $result[] = id(new PhabricatorAuditTransaction()) + ->setTransactionType(PhabricatorTransactions::TYPE_EDGE) + ->setMetadataValue('edge:type', $related_edge) + ->setNewValue(array('+' => $task_phids)); + } + + // Mark these objects as unmentionable, since the explicit relationship + // is stronger and any mentions are redundant. + $phid_map[] = $task_phids; + } + $phid_map = array_mergev($phid_map); $this->addUnmentionablePHIDs($phid_map); From 6d4c6924d6d942bd75e5a306d1a845907268b54e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 29 Jan 2020 11:33:31 -0800 Subject: [PATCH 08/16] Update Herald rule creation workflow to use more modern UI elements Summary: Ref T13480. Creating a rule in Herald currently uses the older radio-button flow. Update it to the "clickable menu" flow to simplify it a little bit. Test Plan: Created new personal, object, and global rules. Hit the object rule error conditions. Maniphest Tasks: T13480 Differential Revision: https://secure.phabricator.com/D20956 --- resources/celerity/map.php | 6 +- .../herald/adapter/HeraldAdapter.php | 6 + .../herald/controller/HeraldNewController.php | 523 ++++++++++-------- src/view/phui/PHUIObjectBoxView.php | 20 + webroot/rsrc/css/phui/phui-object-box.css | 6 + 5 files changed, 315 insertions(+), 246 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index d3d95f832c..3cd5709405 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -9,7 +9,7 @@ return array( 'names' => array( 'conpherence.pkg.css' => '3c8a0668', 'conpherence.pkg.js' => '020aebcf', - 'core.pkg.css' => '6d9a0ba6', + 'core.pkg.css' => '5edb4679', 'core.pkg.js' => '705aec2c', 'differential.pkg.css' => '607c84be', 'differential.pkg.js' => '1b97518d', @@ -165,7 +165,7 @@ return array( 'rsrc/css/phui/phui-left-right.css' => '68513c34', 'rsrc/css/phui/phui-lightbox.css' => '4ebf22da', 'rsrc/css/phui/phui-list.css' => 'b05144dd', - 'rsrc/css/phui/phui-object-box.css' => 'f434b6be', + 'rsrc/css/phui/phui-object-box.css' => 'b8d7eea0', 'rsrc/css/phui/phui-pager.css' => 'd022c7ad', 'rsrc/css/phui/phui-pinboard-view.css' => '1f08f5d8', 'rsrc/css/phui/phui-policy-section-view.css' => '139fdc64', @@ -855,7 +855,7 @@ return array( 'phui-left-right-css' => '68513c34', 'phui-lightbox-css' => '4ebf22da', 'phui-list-view-css' => 'b05144dd', - 'phui-object-box-css' => 'f434b6be', + 'phui-object-box-css' => 'b8d7eea0', 'phui-oi-big-ui-css' => 'fa74cc35', 'phui-oi-color-css' => 'b517bfa0', 'phui-oi-drag-ui-css' => 'da15d3dc', diff --git a/src/applications/herald/adapter/HeraldAdapter.php b/src/applications/herald/adapter/HeraldAdapter.php index 76955751c3..58e0143799 100644 --- a/src/applications/herald/adapter/HeraldAdapter.php +++ b/src/applications/herald/adapter/HeraldAdapter.php @@ -243,6 +243,12 @@ abstract class HeraldAdapter extends Phobject { abstract public function getAdapterApplicationClass(); abstract public function getObject(); + public function getAdapterContentIcon() { + $application_class = $this->getAdapterApplicationClass(); + $application = newv($application_class, array()); + return $application->getIcon(); + } + /** * Return a new characteristic object for this adapter. * diff --git a/src/applications/herald/controller/HeraldNewController.php b/src/applications/herald/controller/HeraldNewController.php index f571aeb395..09b7e62c6e 100644 --- a/src/applications/herald/controller/HeraldNewController.php +++ b/src/applications/herald/controller/HeraldNewController.php @@ -3,314 +3,351 @@ final class HeraldNewController extends HeraldController { public function handleRequest(AphrontRequest $request) { - $viewer = $request->getViewer(); + $viewer = $this->getViewer(); - $content_type_map = HeraldAdapter::getEnabledAdapterMap($viewer); - $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); + $adapter_type_map = HeraldAdapter::getEnabledAdapterMap($viewer); + $adapter_type = $request->getStr('adapter'); - $errors = array(); + if (!isset($adapter_type_map[$adapter_type])) { + $title = pht('Create Herald Rule'); + $content = $this->newAdapterMenu($title); + } else { + $adapter = HeraldAdapter::getAdapterForContentType($adapter_type); - $e_type = null; - $e_rule = null; - $e_object = null; + $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); + $rule_type = $request->getStr('type'); - $step = $request->getInt('step'); - if ($request->isFormPost()) { - $content_type = $request->getStr('content_type'); - if (empty($content_type_map[$content_type])) { - $errors[] = pht('You must choose a content type for this rule.'); - $e_type = pht('Required'); - $step = 0; - } + if (!isset($rule_type_map[$rule_type])) { + $title = pht( + 'Create Herald Rule: %s', + $adapter->getAdapterContentName()); - if (!$errors && $step > 1) { - $rule_type = $request->getStr('rule_type'); - if (empty($rule_type_map[$rule_type])) { - $errors[] = pht('You must choose a rule type for this rule.'); - $e_rule = pht('Required'); - $step = 1; - } - } + $content = $this->newTypeMenu($adapter, $title); + } else { + if ($rule_type !== HeraldRuleTypeConfig::RULE_TYPE_OBJECT) { + $target_phid = null; + $target_okay = true; + } else { + $object_name = $request->getStr('objectName'); + $target_okay = false; - if (!$errors && $step >= 2) { - $target_phid = null; - $object_name = $request->getStr('objectName'); - $done = false; - if ($rule_type != HeraldRuleTypeConfig::RULE_TYPE_OBJECT) { - $done = true; - } else if (strlen($object_name)) { - $target_object = id(new PhabricatorObjectQuery()) - ->setViewer($viewer) - ->withNames(array($object_name)) - ->executeOne(); - if ($target_object) { - $can_edit = PhabricatorPolicyFilter::hasCapability( - $viewer, - $target_object, - PhabricatorPolicyCapability::CAN_EDIT); - if (!$can_edit) { - $errors[] = pht( - 'You can not create a rule for that object, because you do '. - 'not have permission to edit it. You can only create rules '. - 'for objects you can edit.'); - $e_object = pht('Not Editable'); - $step = 2; - } else { - $adapter = HeraldAdapter::getAdapterForContentType($content_type); - if (!$adapter->canTriggerOnObject($target_object)) { - $errors[] = pht( - 'This object is not of an allowed type for the rule. '. - 'Rules can only trigger on certain objects.'); - $e_object = pht('Invalid'); - $step = 2; + $errors = array(); + $e_object = null; + + if ($request->isFormPost()) { + if (strlen($object_name)) { + $target_object = id(new PhabricatorObjectQuery()) + ->setViewer($viewer) + ->withNames(array($object_name)) + ->executeOne(); + if ($target_object) { + $can_edit = PhabricatorPolicyFilter::hasCapability( + $viewer, + $target_object, + PhabricatorPolicyCapability::CAN_EDIT); + if (!$can_edit) { + $errors[] = pht( + 'You can not create a rule for that object, because you '. + 'do not have permission to edit it. You can only create '. + 'rules for objects you can edit.'); + $e_object = pht('Not Editable'); + } else { + if (!$adapter->canTriggerOnObject($target_object)) { + $errors[] = pht( + 'This object is not of an allowed type for the rule. '. + 'Rules can only trigger on certain objects.'); + $e_object = pht('Invalid'); + } else { + $target_phid = $target_object->getPHID(); + } + } } else { - $target_phid = $target_object->getPHID(); - $done = true; + $errors[] = pht('No object exists by that name.'); + $e_object = pht('Invalid'); } + } else { + $errors[] = pht( + 'You must choose an object to associate this rule with.'); + $e_object = pht('Required'); } - } else { - $errors[] = pht('No object exists by that name.'); - $e_object = pht('Invalid'); - $step = 2; + + $target_okay = !$errors; } - } else if ($step > 2) { - $errors[] = pht( - 'You must choose an object to associate this rule with.'); - $e_object = pht('Required'); - $step = 2; } - if (!$errors && $done) { + if (!$target_okay) { + $title = pht('Choose Object'); + $content = $this->newTargetForm( + $adapter, + $rule_type, + $object_name, + $errors, + $e_object, + $title); + } else { $params = array( - 'content_type' => $content_type, + 'content_type' => $adapter_type, 'rule_type' => $rule_type, 'targetPHID' => $target_phid, ); - $uri = new PhutilURI('edit/', $params); - $uri = $this->getApplicationURI($uri); - return id(new AphrontRedirectResponse())->setURI($uri); + $edit_uri = $this->getApplicationURI('edit/'); + $edit_uri = new PhutilURI($edit_uri, $params); + + return id(new AphrontRedirectResponse()) + ->setURI($edit_uri); } } } - $content_type = $request->getStr('content_type'); - $rule_type = $request->getStr('rule_type'); - - $form = id(new AphrontFormView()) - ->setUser($viewer) - ->setAction($this->getApplicationURI('new/')); - - switch ($step) { - case 0: - default: - $content_types = $this->renderContentTypeControl( - $content_type_map, - $e_type); - - $form - ->addHiddenInput('step', 1) - ->appendChild($content_types); - - $cancel_text = null; - $cancel_uri = $this->getApplicationURI(); - $title = pht('Create Herald Rule'); - break; - case 1: - $rule_types = $this->renderRuleTypeControl( - $rule_type_map, - $e_rule); - - $form - ->addHiddenInput('content_type', $content_type) - ->addHiddenInput('step', 2) - ->appendChild($rule_types); - - $params = array( - 'content_type' => $content_type, - 'step' => '0', - ); - - $cancel_text = pht('Back'); - $cancel_uri = new PhutilURI('new/', $params); - $cancel_uri = $this->getApplicationURI($cancel_uri); - $title = pht('Create Herald Rule: %s', - idx($content_type_map, $content_type)); - break; - case 2: - $adapter = HeraldAdapter::getAdapterForContentType($content_type); - $form - ->addHiddenInput('content_type', $content_type) - ->addHiddenInput('rule_type', $rule_type) - ->addHiddenInput('step', 3) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel(pht('Rule for')) - ->setValue( - phutil_tag( - 'strong', - array(), - idx($content_type_map, $content_type)))) - ->appendChild( - id(new AphrontFormStaticControl()) - ->setLabel(pht('Rule Type')) - ->setValue( - phutil_tag( - 'strong', - array(), - idx($rule_type_map, $rule_type)))) - ->appendRemarkupInstructions( - pht( - 'Choose the object this rule will act on (for example, enter '. - '`rX` to act on the `rX` repository, or `#project` to act on '. - 'a project).')) - ->appendRemarkupInstructions( - $adapter->explainValidTriggerObjects()) - ->appendChild( - id(new AphrontFormTextControl()) - ->setName('objectName') - ->setError($e_object) - ->setValue($request->getStr('objectName')) - ->setLabel(pht('Object'))); - - $params = array( - 'content_type' => $content_type, - 'rule_type' => $rule_type, - 'step' => 1, - ); - - $cancel_text = pht('Back'); - $cancel_uri = new PhutilURI('new/', $params); - $cancel_uri = $this->getApplicationURI($cancel_uri); - $title = pht('Create Herald Rule: %s', - idx($content_type_map, $content_type)); - break; - } - - $form - ->appendChild( - id(new AphrontFormSubmitControl()) - ->setValue(pht('Continue')) - ->addCancelButton($cancel_uri, $cancel_text)); - - $form_box = id(new PHUIObjectBoxView()) - ->setHeaderText($title) - ->setFormErrors($errors) - ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) - ->setForm($form); - $crumbs = $this ->buildApplicationCrumbs() ->addTextCrumb(pht('Create Rule')) ->setBorder(true); $view = id(new PHUITwoColumnView()) - ->setFooter($form_box); + ->setFooter($content); return $this->newPage() ->setTitle($title) ->setCrumbs($crumbs) - ->appendChild( - array( - $view, - )); + ->appendChild($view); } - private function renderContentTypeControl(array $content_type_map, $e_type) { - $request = $this->getRequest(); + private function newAdapterMenu($title) { + $viewer = $this->getViewer(); - $radio = id(new AphrontFormRadioButtonControl()) - ->setLabel(pht('New Rule for')) - ->setName('content_type') - ->setValue($request->getStr('content_type')) - ->setError($e_type); + $types = HeraldAdapter::getEnabledAdapterMap($viewer); - foreach ($content_type_map as $value => $name) { - $adapter = HeraldAdapter::getAdapterForContentType($value); - $radio->addButton( - $value, - $name, - phutil_escape_html_newlines($adapter->getAdapterContentDescription())); + foreach ($types as $key => $type) { + $types[$key] = HeraldAdapter::getAdapterForContentType($key); } - return $radio; + $types = msort($types, 'getAdapterContentName'); + + $base_uri = $this->getApplicationURI('create/'); + + $menu = id(new PHUIObjectItemListView()) + ->setViewer($viewer) + ->setBig(true); + + foreach ($types as $key => $adapter) { + $adapter_uri = id(new PhutilURI($base_uri)) + ->replaceQueryParam('adapter', $key); + + $description = $adapter->getAdapterContentDescription(); + $description = phutil_escape_html_newlines($description); + + $item = id(new PHUIObjectItemView()) + ->setHeader($adapter->getAdapterContentName()) + ->setImageIcon($adapter->getAdapterContentIcon()) + ->addAttribute($description) + ->setHref($adapter_uri) + ->setClickable(true); + + $menu->addItem($item); + } + + $box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setObjectList($menu); + + return id(new PHUILauncherView()) + ->appendChild($box); } + private function newTypeMenu(HeraldAdapter $adapter, $title) { + $viewer = $this->getViewer(); - private function renderRuleTypeControl(array $rule_type_map, $e_rule) { - $request = $this->getRequest(); + $global_capability = HeraldManageGlobalRulesCapability::CAPABILITY; + $can_global = $this->hasApplicationCapability($global_capability); - // Reorder array to put less powerful rules first. - $rule_type_map = array_select_keys( - $rule_type_map, - array( - HeraldRuleTypeConfig::RULE_TYPE_PERSONAL, - HeraldRuleTypeConfig::RULE_TYPE_OBJECT, - HeraldRuleTypeConfig::RULE_TYPE_GLOBAL, - )) + $rule_type_map; + if ($can_global) { + $global_note = pht( + 'You have permission to create and manage global rules.'); + } else { + $global_note = pht( + 'You do not have permission to create or manage global rules.'); + } + $global_note = phutil_tag('em', array(), $global_note); - list($can_global, $global_link) = $this->explainApplicationCapability( - HeraldManageGlobalRulesCapability::CAPABILITY, - pht('You have permission to create and manage global rules.'), - pht('You do not have permission to create or manage global rules.')); - - $captions = array( - HeraldRuleTypeConfig::RULE_TYPE_PERSONAL => - pht( + $specs = array( + HeraldRuleTypeConfig::RULE_TYPE_PERSONAL => array( + 'name' => pht('Personal Rule'), + 'icon' => 'fa-user', + 'help' => pht( 'Personal rules notify you about events. You own them, but they can '. 'only affect you. Personal rules only trigger for objects you have '. 'permission to see.'), - HeraldRuleTypeConfig::RULE_TYPE_OBJECT => - pht( + 'enabled' => true, + ), + HeraldRuleTypeConfig::RULE_TYPE_OBJECT => array( + 'name' => pht('Object Rule'), + 'icon' => 'fa-cube', + 'help' => pht( 'Object rules notify anyone about events. They are bound to an '. 'object (like a repository) and can only act on that object. You '. 'must be able to edit an object to create object rules for it. '. 'Other users who can edit the object can edit its rules.'), - HeraldRuleTypeConfig::RULE_TYPE_GLOBAL => - array( + 'enabled' => true, + ), + HeraldRuleTypeConfig::RULE_TYPE_GLOBAL => array( + 'name' => pht('Global Rule'), + 'icon' => 'fa-globe', + 'help' => array( pht( 'Global rules notify anyone about events. Global rules can '. 'bypass access control policies and act on any object.'), - $global_link, + $global_note, ), + 'enabled' => $can_global, + ), ); - $radio = id(new AphrontFormRadioButtonControl()) - ->setLabel(pht('Rule Type')) - ->setName('rule_type') - ->setValue($request->getStr('rule_type')) - ->setError($e_rule); + $adapter_type = $adapter->getAdapterContentType(); - $adapter = HeraldAdapter::getAdapterForContentType( - $request->getStr('content_type')); + $base_uri = new PhutilURI($this->getApplicationURI('create/')); - foreach ($rule_type_map as $value => $name) { - $caption = idx($captions, $value); - $disabled = ($value == HeraldRuleTypeConfig::RULE_TYPE_GLOBAL) && - (!$can_global); + $adapter_uri = id(clone $base_uri) + ->replaceQueryParam('adapter', $adapter_type); - if (!$adapter->supportsRuleType($value)) { - $disabled = true; - $caption = array( - $caption, - "\n\n", - phutil_tag( + $menu = id(new PHUIObjectItemListView()) + ->setUser($viewer) + ->setBig(true); + + foreach ($specs as $rule_type => $spec) { + $type_uri = id(clone $adapter_uri) + ->replaceQueryParam('type', $rule_type); + + $name = $spec['name']; + $icon = $spec['icon']; + + $description = $spec['help']; + $description = (array)$description; + + $enabled = $spec['enabled']; + if ($enabled) { + $enabled = $adapter->supportsRuleType($rule_type); + if (!$enabled) { + $description[] = phutil_tag( 'em', array(), pht( - 'This rule type is not supported by the selected content type.')), - ); + 'This rule type is not supported by the selected '. + 'content type.')); + } } - $radio->addButton( - $value, - $name, - phutil_escape_html_newlines($caption), - $disabled ? 'disabled' : null, - $disabled); + $description = phutil_implode_html( + array( + phutil_tag('br'), + phutil_tag('br'), + ), + $description); + + $item = id(new PHUIObjectItemView()) + ->setHeader($name) + ->setImageIcon($icon) + ->addAttribute($description); + + if ($enabled) { + $item + ->setHref($type_uri) + ->setClickable(true); + } else { + $item->setDisabled(true); + } + + $menu->addItem($item); } - return $radio; + $box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setObjectList($menu); + + $box->newTailButton() + ->setText(pht('Back to Content Types')) + ->setIcon('fa-chevron-left') + ->setHref($base_uri); + + return id(new PHUILauncherView()) + ->appendChild($box); + } + + + private function newTargetForm( + HeraldAdapter $adapter, + $rule_type, + $object_name, + $errors, + $e_object, + $title) { + + $viewer = $this->getViewer(); + $content_type = $adapter->getAdapterContentType(); + $rule_type_map = HeraldRuleTypeConfig::getRuleTypeMap(); + + $params = array( + 'adapter' => $content_type, + 'type' => $rule_type, + ); + + $form = id(new AphrontFormView()) + ->setViewer($viewer) + ->appendChild( + id(new AphrontFormStaticControl()) + ->setLabel(pht('Rule for')) + ->setValue( + phutil_tag( + 'strong', + array(), + $adapter->getAdapterContentName()))) + ->appendChild( + id(new AphrontFormStaticControl()) + ->setLabel(pht('Rule Type')) + ->setValue( + phutil_tag( + 'strong', + array(), + idx($rule_type_map, $rule_type)))) + ->appendRemarkupInstructions( + pht( + 'Choose the object this rule will act on (for example, enter '. + '`rX` to act on the `rX` repository, or `#project` to act on '. + 'a project).')) + ->appendRemarkupInstructions( + $adapter->explainValidTriggerObjects()) + ->appendChild( + id(new AphrontFormTextControl()) + ->setName('objectName') + ->setError($e_object) + ->setValue($object_name) + ->setLabel(pht('Object'))); + + foreach ($params as $key => $value) { + $form->addHiddenInput($key, $value); + } + + $cancel_params = $params; + unset($cancel_params['type']); + + $cancel_uri = $this->getApplicationURI('new/'); + $cancel_uri = new PhutilURI($cancel_uri, $params); + + $form->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue(pht('Continue')) + ->addCancelButton($cancel_uri, pht('Back'))); + + $form_box = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setFormErrors($errors) + ->setBackground(PHUIObjectBoxView::WHITE_CONFIG) + ->setForm($form); + + return $form_box; } } diff --git a/src/view/phui/PHUIObjectBoxView.php b/src/view/phui/PHUIObjectBoxView.php index f5fdbfffc4..39c6d4f5b0 100644 --- a/src/view/phui/PHUIObjectBoxView.php +++ b/src/view/phui/PHUIObjectBoxView.php @@ -27,6 +27,7 @@ final class PHUIObjectBoxView extends AphrontTagView { private $showHideOpen; private $propertyLists = array(); + private $tailButtons = array(); const COLOR_RED = 'red'; const COLOR_BLUE = 'blue'; @@ -153,6 +154,16 @@ final class PHUIObjectBoxView extends AphrontTagView { return $this; } + public function newTailButton() { + $button = id(new PHUIButtonView()) + ->setTag('a') + ->setColor(PHUIButtonView::GREY); + + $this->tailButtons[] = $button; + + return $button; + } + protected function getTagAttributes() { $classes = array(); $classes[] = 'phui-box'; @@ -329,6 +340,15 @@ final class PHUIObjectBoxView extends AphrontTagView { $content[] = $this->objectList; } + if ($this->tailButtons) { + $content[] = phutil_tag( + 'div', + array( + 'class' => 'phui-object-box-tail-buttons', + ), + $this->tailButtons); + } + return $content; } } diff --git a/webroot/rsrc/css/phui/phui-object-box.css b/webroot/rsrc/css/phui/phui-object-box.css index f95e36eedc..4ab6732e29 100644 --- a/webroot/rsrc/css/phui/phui-object-box.css +++ b/webroot/rsrc/css/phui/phui-object-box.css @@ -62,6 +62,12 @@ div.phui-object-box.phui-object-box-flush { font-size: {$normalfontsize}; } +.phui-object-box-tail-buttons { + padding: 8px; + background: {$lightgreybackground}; + border-top: 1px solid {$lightgreyborder}; +} + /* - Object Box Colors ------------------------------------------------------ */ .phui-box-border.phui-object-box-green { From 0f1acb6cef1d11341e9a3e561b2fb33ba4d74e66 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 07:49:01 -0800 Subject: [PATCH 09/16] Update GitHub API calls to use "Authorization" header instead of "access_token" URI parameter Summary: Fixes T13485. GitHub has deprecated the "access_token" URI parameter for API authentication. Update to "Authorization: token ...". Test Plan: Linked and unlinked a GitHub account locally. Maniphest Tasks: T13485 Differential Revision: https://secure.phabricator.com/D20964 --- src/applications/auth/adapter/PhutilGitHubAuthAdapter.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/applications/auth/adapter/PhutilGitHubAuthAdapter.php b/src/applications/auth/adapter/PhutilGitHubAuthAdapter.php index 7fa8a660fa..a9d4c849d5 100644 --- a/src/applications/auth/adapter/PhutilGitHubAuthAdapter.php +++ b/src/applications/auth/adapter/PhutilGitHubAuthAdapter.php @@ -51,13 +51,17 @@ final class PhutilGitHubAuthAdapter extends PhutilOAuthAuthAdapter { protected function loadOAuthAccountData() { $uri = new PhutilURI('https://api.github.com/user'); - $uri->replaceQueryParam('access_token', $this->getAccessToken()); $future = new HTTPSFuture($uri); // NOTE: GitHub requires a User-Agent string. $future->addHeader('User-Agent', __CLASS__); + // See T13485. Circa early 2020, GitHub has deprecated use of the + // "access_token" URI parameter. + $token_header = sprintf('token %s', $this->getAccessToken()); + $future->addHeader('Authorization', $token_header); + list($body) = $future->resolvex(); try { From 84fd5cd5bb4ed9c9ba8e6d734a0a648dcf33ecd7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 08:03:28 -0800 Subject: [PATCH 10/16] Fix an issue where intracontent empty lines were incorrectly trimmed in quoted blocks Summary: Fixes T13335. When processing quoted blocks, we remove leading empty lines. This logic incorrectly continued after encountering a nonempty line. Test Plan: Added a test, made it pass. Previewed blocks in web UI. Maniphest Tasks: T13335 Differential Revision: https://secure.phabricator.com/D20965 --- src/infrastructure/markup/PhabricatorMarkupEngine.php | 2 +- .../blockrule/PhutilRemarkupQuotedBlockRule.php | 2 ++ .../remarkup/__tests__/remarkup/quoted-paragraphs.txt | 11 +++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 src/infrastructure/markup/remarkup/__tests__/remarkup/quoted-paragraphs.txt diff --git a/src/infrastructure/markup/PhabricatorMarkupEngine.php b/src/infrastructure/markup/PhabricatorMarkupEngine.php index 35de846650..5422870b68 100644 --- a/src/infrastructure/markup/PhabricatorMarkupEngine.php +++ b/src/infrastructure/markup/PhabricatorMarkupEngine.php @@ -42,7 +42,7 @@ final class PhabricatorMarkupEngine extends Phobject { private $objects = array(); private $viewer; private $contextObject; - private $version = 19; + private $version = 20; private $engineCaches = array(); private $auxiliaryConfig = array(); diff --git a/src/infrastructure/markup/blockrule/PhutilRemarkupQuotedBlockRule.php b/src/infrastructure/markup/blockrule/PhutilRemarkupQuotedBlockRule.php index ed87f7063d..9f2bd7a297 100644 --- a/src/infrastructure/markup/blockrule/PhutilRemarkupQuotedBlockRule.php +++ b/src/infrastructure/markup/blockrule/PhutilRemarkupQuotedBlockRule.php @@ -66,6 +66,8 @@ abstract class PhutilRemarkupQuotedBlockRule foreach ($text as $key => $line) { if (!strlen(trim($line))) { unset($text[$key]); + } else { + break; } } diff --git a/src/infrastructure/markup/remarkup/__tests__/remarkup/quoted-paragraphs.txt b/src/infrastructure/markup/remarkup/__tests__/remarkup/quoted-paragraphs.txt new file mode 100644 index 0000000000..de597b5e0a --- /dev/null +++ b/src/infrastructure/markup/remarkup/__tests__/remarkup/quoted-paragraphs.txt @@ -0,0 +1,11 @@ +> x +> +> y +~~~~~~~~~~ +

x

+ +

y

+~~~~~~~~~~ +> x +> +> y From 2a92fef8798d73a90bb046a334becff8334d53d0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 09:21:29 -0800 Subject: [PATCH 11/16] Improve wrapping and overflow behavior for curtain panels containing long usernames Summary: Ref T13486. When a curtain element like "Author" in Maniphest has a very long username, the wrapping and overflow behavior is poor: the date is obscured. Adjust curtain elements which contain lists of references to other objects to improve wrapping behavior (put the date on a separate line) and overflow behavior (so we get a "..." when a name overflows). Test Plan: {F7179376} Maniphest Tasks: T13486 Differential Revision: https://secure.phabricator.com/D20966 --- resources/celerity/map.php | 2 + src/__phutil_library_map__.php | 4 + .../ManiphestTaskDetailController.php | 36 ++-- .../phui/PHUICurtainObjectRefListView.php | 46 ++++++ src/view/phui/PHUICurtainObjectRefView.php | 155 ++++++++++++++++++ .../css/phui/phui-curtain-object-ref-view.css | 48 ++++++ 6 files changed, 269 insertions(+), 22 deletions(-) create mode 100644 src/view/phui/PHUICurtainObjectRefListView.php create mode 100644 src/view/phui/PHUICurtainObjectRefView.php create mode 100644 webroot/rsrc/css/phui/phui-curtain-object-ref-view.css diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 3cd5709405..3bb89e7178 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -146,6 +146,7 @@ return array( 'rsrc/css/phui/phui-comment-form.css' => '68a2d99a', 'rsrc/css/phui/phui-comment-panel.css' => 'ec4e31c0', 'rsrc/css/phui/phui-crumbs-view.css' => '614f43cf', + 'rsrc/css/phui/phui-curtain-object-ref-view.css' => 'e3331b60', 'rsrc/css/phui/phui-curtain-view.css' => '68c5efb6', 'rsrc/css/phui/phui-document-pro.css' => 'b9613a10', 'rsrc/css/phui/phui-document-summary.css' => 'b068eed1', @@ -833,6 +834,7 @@ return array( 'phui-comment-form-css' => '68a2d99a', 'phui-comment-panel-css' => 'ec4e31c0', 'phui-crumbs-view-css' => '614f43cf', + 'phui-curtain-object-ref-view-css' => 'e3331b60', 'phui-curtain-view-css' => '68c5efb6', 'phui-document-summary-view-css' => 'b068eed1', 'phui-document-view-css' => '52b748a5', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index d6c1499150..0948e69e06 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1991,6 +1991,8 @@ phutil_register_library_map(array( 'PHUICrumbView' => 'view/phui/PHUICrumbView.php', 'PHUICrumbsView' => 'view/phui/PHUICrumbsView.php', 'PHUICurtainExtension' => 'view/extension/PHUICurtainExtension.php', + 'PHUICurtainObjectRefListView' => 'view/phui/PHUICurtainObjectRefListView.php', + 'PHUICurtainObjectRefView' => 'view/phui/PHUICurtainObjectRefView.php', 'PHUICurtainPanelView' => 'view/layout/PHUICurtainPanelView.php', 'PHUICurtainView' => 'view/layout/PHUICurtainView.php', 'PHUIDiffGraphView' => 'infrastructure/diff/view/PHUIDiffGraphView.php', @@ -8189,6 +8191,8 @@ phutil_register_library_map(array( 'PHUICrumbView' => 'AphrontView', 'PHUICrumbsView' => 'AphrontView', 'PHUICurtainExtension' => 'Phobject', + 'PHUICurtainObjectRefListView' => 'AphrontTagView', + 'PHUICurtainObjectRefView' => 'AphrontTagView', 'PHUICurtainPanelView' => 'AphrontTagView', 'PHUICurtainView' => 'AphrontTagView', 'PHUIDiffGraphView' => 'Phobject', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index b6985268db..6cd0b967e0 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -340,37 +340,29 @@ final class ManiphestTaskDetailController extends ManiphestController { $author_phid = $task->getAuthorPHID(); $handles = $viewer->loadHandles(array($owner_phid, $author_phid)); + $assigned_refs = id(new PHUICurtainObjectRefListView()) + ->setViewer($viewer) + ->setEmptyMessage(pht('None')); + if ($owner_phid) { - $image_uri = $handles[$owner_phid]->getImageURI(); - $image_href = $handles[$owner_phid]->getURI(); - $owner = $viewer->renderHandle($owner_phid)->render(); - $content = phutil_tag('strong', array(), $owner); - $assigned_to = id(new PHUIHeadThingView()) - ->setImage($image_uri) - ->setImageHref($image_href) - ->setContent($content); - } else { - $assigned_to = phutil_tag('em', array(), pht('None')); + $assigned_ref = $assigned_refs->newObjectRefView() + ->setHandle($handles[$owner_phid]); } $curtain->newPanel() ->setHeaderText(pht('Assigned To')) - ->appendChild($assigned_to); + ->appendChild($assigned_refs); - $author_uri = $handles[$author_phid]->getImageURI(); - $author_href = $handles[$author_phid]->getURI(); - $author = $viewer->renderHandle($author_phid)->render(); - $content = phutil_tag('strong', array(), $author); - $date = phabricator_date($task->getDateCreated(), $viewer); - $content = pht('%s, %s', $content, $date); - $authored_by = id(new PHUIHeadThingView()) - ->setImage($author_uri) - ->setImageHref($author_href) - ->setContent($content); + $author_refs = id(new PHUICurtainObjectRefListView()) + ->setViewer($viewer); + + $author_ref = $author_refs->newObjectRefView() + ->setHandle($handles[$author_phid]) + ->setEpoch($task->getDateCreated()); $curtain->newPanel() ->setHeaderText(pht('Authored By')) - ->appendChild($authored_by); + ->appendChild($author_refs); return $curtain; } diff --git a/src/view/phui/PHUICurtainObjectRefListView.php b/src/view/phui/PHUICurtainObjectRefListView.php new file mode 100644 index 0000000000..f58f97c98a --- /dev/null +++ b/src/view/phui/PHUICurtainObjectRefListView.php @@ -0,0 +1,46 @@ + 'phui-curtain-object-ref-list-view', + ); + } + + public function setEmptyMessage($empty_message) { + $this->emptyMessage = $empty_message; + return $this; + } + + protected function getTagContent() { + $refs = $this->refs; + + if (!$refs) { + if ($this->emptyMessage) { + return phutil_tag( + 'div', + array( + 'class' => 'phui-curtain-object-ref-list-view-empty', + ), + $this->emptyMessage); + } + } + + return $refs; + } + + public function newObjectRefView() { + $ref_view = id(new PHUICurtainObjectRefView()) + ->setViewer($this->getViewer()); + + $this->refs[] = $ref_view; + + return $ref_view; + } + +} diff --git a/src/view/phui/PHUICurtainObjectRefView.php b/src/view/phui/PHUICurtainObjectRefView.php new file mode 100644 index 0000000000..357e4eec9b --- /dev/null +++ b/src/view/phui/PHUICurtainObjectRefView.php @@ -0,0 +1,155 @@ +handle = $handle; + return $this; + } + + public function setEpoch($epoch) { + $this->epoch = $epoch; + return $this; + } + + protected function getTagAttributes() { + return array( + 'class' => 'phui-curtain-object-ref-view', + ); + } + + protected function getTagContent() { + require_celerity_resource('phui-curtain-object-ref-view-css'); + + $viewer = $this->getViewer(); + $handle = $this->handle; + + $more_rows = array(); + + $epoch = $this->epoch; + if ($epoch !== null) { + $epoch_view = phabricator_datetime($epoch, $viewer); + + $epoch_cells = array(); + + $epoch_cells[] = phutil_tag( + 'td', + array( + 'class' => 'phui-curtain-object-ref-view-epoch-cell', + ), + $epoch_view); + + $more_rows[] = phutil_tag('tr', array(), $epoch_cells); + } + + $header_cells = array(); + + $image_view = $this->newImage(); + + if ($more_rows) { + $row_count = 1 + count($more_rows); + } else { + $row_count = null; + } + + $header_cells[] = phutil_tag( + 'td', + array( + 'rowspan' => $row_count, + 'class' => 'phui-curtain-object-ref-view-image-cell', + ), + $image_view); + + $title_view = $this->newTitle(); + + $header_cells[] = phutil_tag( + 'td', + array( + 'class' => 'phui-curtain-object-ref-view-title-cell', + ), + $title_view); + + $rows = array(); + + if (!$more_rows) { + $title_row_class = 'phui-curtain-object-ref-view-without-content'; + } else { + $title_row_class = 'phui-curtain-object-ref-view-with-content'; + } + + $rows[] = phutil_tag( + 'tr', + array( + 'class' => $title_row_class, + ), + $header_cells); + + $body = phutil_tag( + 'tbody', + array(), + array( + $rows, + $more_rows, + )); + + return phutil_tag('table', array(), $body); + } + + private function newTitle() { + $title_view = null; + $handle = $this->handle; + + if ($handle) { + $title_view = $handle->renderLink(); + } + + return $title_view; + } + + private function newImage() { + $image_uri = $this->getImageURI(); + $target_uri = $this->getTargetURI(); + + if ($image_uri !== null) { + $image_view = javelin_tag( + 'a', + array( + 'style' => sprintf('background-image: url(%s)', $image_uri), + 'href' => $target_uri, + 'aural' => false, + )); + } else { + $image_view = null; + } + + return $image_view; + } + + private function getTargetURI() { + $target_uri = null; + $handle = $this->handle; + + if ($handle) { + $target_uri = $handle->getURI(); + } + + return $target_uri; + } + + private function getImageURI() { + $image_uri = null; + $handle = $this->handle; + + if ($handle) { + $image_uri = $handle->getImageURI(); + } + + return $image_uri; + } + + +} diff --git a/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css b/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css new file mode 100644 index 0000000000..b8ba7e1a6f --- /dev/null +++ b/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css @@ -0,0 +1,48 @@ +/** + * @provides phui-curtain-object-ref-view-css + */ + +.phui-curtain-object-ref-list-view-empty { + font-style: italic; + color: {$greytext}; +} + +.phui-curtain-object-ref-view-image-cell { + min-width: 32px; + min-height: 32px; +} + +.phui-curtain-object-ref-view-image-cell > a { + height: 24px; + width: 24px; + background-size: 100%; + border-radius: 3px; + display: block; +} + +.phui-curtain-object-ref-view-title-cell { + font-weight: bold; + text-overflow: ellipsis; + overflow: hidden; + + /* This is forcing "text-overflow: ellipsis" to actually work. */ + max-width: 225px; +} + +.phui-curtain-object-ref-view-without-content > + .phui-curtain-object-ref-view-title-cell { + vertical-align: middle; +} + +.phui-curtain-object-ref-view-with-content > + .phui-curtain-object-ref-view-image-cell > a { + margin-top: 4px; +} + +.phui-curtain-object-ref-view-title-cell > a { + color: {$darkgreytext}; +} + +.phui-curtain-object-ref-view-epoch-cell { + color: {$greytext}; +} From 0e82bd024a8cf30d4513ec5697db92404fa24803 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 12:05:11 -0800 Subject: [PATCH 12/16] Use the new "CurtainObjectRefList" UI element for subscribers Summary: Depends on D20966. Ref T13486. Curtains currently render subscribers in a plain text list, but the new ref list element is a good fit for this. Also, improve the sorting and ordering behavior. This makes the subscriber list take up a bit more space, but it should make it a lot easier to read at a glance. Test Plan: Viewed object subscriber lists at varying limits and subscriber counts, saw sensible subscriber lists. Maniphest Tasks: T13486 Differential Revision: https://secure.phabricator.com/D20967 --- resources/celerity/map.php | 4 +- src/__phutil_library_map__.php | 2 + .../ManiphestTaskDetailController.php | 7 +- ...abricatorSubscriptionsCurtainExtension.php | 120 ++++++++++++++++-- .../PhabricatorUSEnglishTranslation.php | 5 + .../phui/PHUICurtainObjectRefListView.php | 42 ++++-- src/view/phui/PHUICurtainObjectRefView.php | 41 +++++- src/view/phui/PHUILinkView.php | 50 ++++++++ .../css/phui/phui-curtain-object-ref-view.css | 40 +++++- 9 files changed, 286 insertions(+), 25 deletions(-) create mode 100644 src/view/phui/PHUILinkView.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 3bb89e7178..d1a24e4ddd 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -146,7 +146,7 @@ return array( 'rsrc/css/phui/phui-comment-form.css' => '68a2d99a', 'rsrc/css/phui/phui-comment-panel.css' => 'ec4e31c0', 'rsrc/css/phui/phui-crumbs-view.css' => '614f43cf', - 'rsrc/css/phui/phui-curtain-object-ref-view.css' => 'e3331b60', + 'rsrc/css/phui/phui-curtain-object-ref-view.css' => '12404744', 'rsrc/css/phui/phui-curtain-view.css' => '68c5efb6', 'rsrc/css/phui/phui-document-pro.css' => 'b9613a10', 'rsrc/css/phui/phui-document-summary.css' => 'b068eed1', @@ -834,7 +834,7 @@ return array( 'phui-comment-form-css' => '68a2d99a', 'phui-comment-panel-css' => 'ec4e31c0', 'phui-crumbs-view-css' => '614f43cf', - 'phui-curtain-object-ref-view-css' => 'e3331b60', + 'phui-curtain-object-ref-view-css' => '12404744', 'phui-curtain-view-css' => '68c5efb6', 'phui-document-summary-view-css' => 'b068eed1', 'phui-document-view-css' => '52b748a5', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 0948e69e06..ddd25bf7e5 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2042,6 +2042,7 @@ phutil_register_library_map(array( 'PHUILauncherView' => 'view/phui/PHUILauncherView.php', 'PHUILeftRightExample' => 'applications/uiexample/examples/PHUILeftRightExample.php', 'PHUILeftRightView' => 'view/phui/PHUILeftRightView.php', + 'PHUILinkView' => 'view/phui/PHUILinkView.php', 'PHUIListExample' => 'applications/uiexample/examples/PHUIListExample.php', 'PHUIListItemView' => 'view/phui/PHUIListItemView.php', 'PHUIListView' => 'view/phui/PHUIListView.php', @@ -8242,6 +8243,7 @@ phutil_register_library_map(array( 'PHUILauncherView' => 'AphrontTagView', 'PHUILeftRightExample' => 'PhabricatorUIExample', 'PHUILeftRightView' => 'AphrontTagView', + 'PHUILinkView' => 'AphrontTagView', 'PHUIListExample' => 'PhabricatorUIExample', 'PHUIListItemView' => 'AphrontTagView', 'PHUIListView' => 'AphrontTagView', diff --git a/src/applications/maniphest/controller/ManiphestTaskDetailController.php b/src/applications/maniphest/controller/ManiphestTaskDetailController.php index 6cd0b967e0..e542cb017a 100644 --- a/src/applications/maniphest/controller/ManiphestTaskDetailController.php +++ b/src/applications/maniphest/controller/ManiphestTaskDetailController.php @@ -336,6 +336,7 @@ final class ManiphestTaskDetailController extends ManiphestController { $curtain->addAction($relationship_submenu); } + $viewer_phid = $viewer->getPHID(); $owner_phid = $task->getOwnerPHID(); $author_phid = $task->getAuthorPHID(); $handles = $viewer->loadHandles(array($owner_phid, $author_phid)); @@ -346,7 +347,8 @@ final class ManiphestTaskDetailController extends ManiphestController { if ($owner_phid) { $assigned_ref = $assigned_refs->newObjectRefView() - ->setHandle($handles[$owner_phid]); + ->setHandle($handles[$owner_phid]) + ->setHighlighted($owner_phid === $viewer_phid); } $curtain->newPanel() @@ -358,7 +360,8 @@ final class ManiphestTaskDetailController extends ManiphestController { $author_ref = $author_refs->newObjectRefView() ->setHandle($handles[$author_phid]) - ->setEpoch($task->getDateCreated()); + ->setEpoch($task->getDateCreated()) + ->setHighlighted($author_phid === $viewer_phid); $curtain->newPanel() ->setHeaderText(pht('Authored By')) diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php index 6467141de2..639e96eb86 100644 --- a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php @@ -15,25 +15,129 @@ final class PhabricatorSubscriptionsCurtainExtension public function buildCurtainPanel($object) { $viewer = $this->getViewer(); + $viewer_phid = $viewer->getPHID(); $object_phid = $object->getPHID(); + $max_handles = 100; + $max_visible = 8; + + // TODO: We should limit the number of subscriber PHIDs we'll load, so + // we degrade gracefully when objects have thousands of subscribers. + $subscriber_phids = PhabricatorSubscribersQuery::loadSubscribersForPHID( $object_phid); + $subscriber_count = count($subscriber_phids); - $handles = $viewer->loadHandles($subscriber_phids); + $subscriber_phids = $this->sortSubscriberPHIDs( + $subscriber_phids, + null); - // TODO: This class can't accept a HandleList yet. - $handles = iterator_to_array($handles); + // If we have fewer subscribers than the maximum number of handles we're + // willing to load, load all the handles and then sort the list based on + // complete handle data. - $susbscribers_view = id(new SubscriptionListStringBuilder()) - ->setObjectPHID($object_phid) - ->setHandles($handles) - ->buildPropertyString(); + // If we have too many PHIDs, we'll skip this step and accept a less + // useful ordering. + $handles = null; + if ($subscriber_count <= $max_handles) { + $handles = $viewer->loadHandles($subscriber_phids); + + $subscriber_phids = $this->sortSubscriberPHIDs( + $subscriber_phids, + $handles); + } + + // If we have more PHIDs to show than visible slots, slice the list. + if ($subscriber_count > $max_visible) { + $visible_phids = array_slice($subscriber_phids, 0, $max_visible - 1); + $show_all = true; + } else { + $visible_phids = $subscriber_phids; + $show_all = false; + } + + // If we didn't load handles earlier because we had too many PHIDs, + // load them now. + if ($handles === null) { + $handles = $viewer->loadHandles($visible_phids); + } + + $ref_list = id(new PHUICurtainObjectRefListView()) + ->setViewer($viewer) + ->setEmptyMessage(pht('None')); + + foreach ($visible_phids as $phid) { + $ref = $ref_list->newObjectRefView() + ->setHandle($handles[$phid]); + + if ($phid === $viewer_phid) { + $ref->setHighlighted(true); + } + } + + if ($show_all) { + $view_all_uri = urisprintf( + '/subscriptions/list/%s/', + $object_phid); + + $ref_list->newTailLink() + ->setURI($view_all_uri) + ->setText(pht('View All %d Subscriber(s)', $subscriber_count)) + ->setWorkflow(true); + } return $this->newPanel() ->setHeaderText(pht('Subscribers')) ->setOrder(20000) - ->appendChild($susbscribers_view); + ->appendChild($ref_list); + } + + private function sortSubscriberPHIDs(array $subscriber_phids, $handles) { + + // Sort subscriber PHIDs with or without handle data. If we have handles, + // we can sort results more comprehensively. + + $viewer = $this->getViewer(); + + $user_type = PhabricatorPeopleUserPHIDType::TYPECONST; + $viewer_phid = $viewer->getPHID(); + + $type_order_map = array( + PhabricatorPeopleUserPHIDType::TYPECONST => 0, + PhabricatorProjectProjectPHIDType::TYPECONST => 1, + PhabricatorOwnersPackagePHIDType::TYPECONST => 2, + ); + $default_type_order = count($type_order_map); + + $subscriber_map = array(); + foreach ($subscriber_phids as $subscriber_phid) { + $is_viewer = ($viewer_phid === $subscriber_phid); + + $subscriber_type = phid_get_type($subscriber_phid); + $type_order = idx($type_order_map, $subscriber_type, $default_type_order); + + $sort_name = ''; + $is_complete = false; + if ($handles) { + if (isset($handles[$subscriber_phid])) { + $handle = $handles[$subscriber_phid]; + if ($handle->isComplete()) { + $is_complete = true; + $sort_name = $handle->getLinkName(); + } + } + } + + $subscriber_map[$subscriber_phid] = id(new PhutilSortVector()) + ->addInt($is_viewer ? 0 : 1) + ->addInt($is_complete ? 0 : 1) + ->addInt($type_order) + ->addString($sort_name); + } + + $subscriber_map = msortv($subscriber_map, 'getSelf'); + + return array_keys($subscriber_map); } } diff --git a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php index ec07d018b7..9c814cb45c 100644 --- a/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php +++ b/src/infrastructure/internationalization/translation/PhabricatorUSEnglishTranslation.php @@ -1718,6 +1718,11 @@ final class PhabricatorUSEnglishTranslation 'then try again.', ), + 'View All %d Subscriber(s)' => array( + 'View Subscriber', + 'View All %d Subscribers', + ), + ); } diff --git a/src/view/phui/PHUICurtainObjectRefListView.php b/src/view/phui/PHUICurtainObjectRefListView.php index f58f97c98a..d42a34452f 100644 --- a/src/view/phui/PHUICurtainObjectRefListView.php +++ b/src/view/phui/PHUICurtainObjectRefListView.php @@ -5,6 +5,7 @@ final class PHUICurtainObjectRefListView private $refs = array(); private $emptyMessage; + private $tail = array(); protected function getTagAttributes() { return array( @@ -20,18 +21,31 @@ final class PHUICurtainObjectRefListView protected function getTagContent() { $refs = $this->refs; - if (!$refs) { - if ($this->emptyMessage) { - return phutil_tag( - 'div', - array( - 'class' => 'phui-curtain-object-ref-list-view-empty', - ), - $this->emptyMessage); - } + if (!$refs && ($this->emptyMessage !== null)) { + $view = phutil_tag( + 'div', + array( + 'class' => 'phui-curtain-object-ref-list-view-empty', + ), + $this->emptyMessage); + } else { + $view = $refs; } - return $refs; + $tail = null; + if ($this->tail) { + $tail = phutil_tag( + 'div', + array( + 'class' => 'phui-curtain-object-ref-list-view-tail', + ), + $this->tail); + } + + return array( + $view, + $tail, + ); } public function newObjectRefView() { @@ -43,4 +57,12 @@ final class PHUICurtainObjectRefListView return $ref_view; } + public function newTailLink() { + $link = new PHUILinkView(); + + $this->tail[] = $link; + + return $link; + } + } diff --git a/src/view/phui/PHUICurtainObjectRefView.php b/src/view/phui/PHUICurtainObjectRefView.php index 357e4eec9b..f8a04796ca 100644 --- a/src/view/phui/PHUICurtainObjectRefView.php +++ b/src/view/phui/PHUICurtainObjectRefView.php @@ -5,6 +5,7 @@ final class PHUICurtainObjectRefView private $handle; private $epoch; + private $highlighted; public function setHandle(PhabricatorObjectHandle $handle) { $this->handle = $handle; @@ -16,9 +17,22 @@ final class PHUICurtainObjectRefView return $this; } + public function setHighlighted($highlighted) { + $this->highlighted = $highlighted; + return $this; + } + protected function getTagAttributes() { + $classes = array(); + $classes[] = 'phui-curtain-object-ref-view'; + + if ($this->highlighted) { + $classes[] = 'phui-curtain-object-ref-view-highlighted'; + } + $classes = implode(' ', $classes); + return array( - 'class' => 'phui-curtain-object-ref-view', + 'class' => $classes, ); } @@ -114,6 +128,11 @@ final class PHUICurtainObjectRefView $image_uri = $this->getImageURI(); $target_uri = $this->getTargetURI(); + $icon_view = null; + if ($image_uri == null) { + $icon_view = $this->newIconView(); + } + if ($image_uri !== null) { $image_view = javelin_tag( 'a', @@ -122,6 +141,15 @@ final class PHUICurtainObjectRefView 'href' => $target_uri, 'aural' => false, )); + } else if ($icon_view !== null) { + $image_view = javelin_tag( + 'a', + array( + 'href' => $target_uri, + 'class' => 'phui-curtain-object-ref-view-icon-image', + 'aural' => false, + ), + $icon_view); } else { $image_view = null; } @@ -151,5 +179,16 @@ final class PHUICurtainObjectRefView return $image_uri; } + private function newIconView() { + $handle = $this->handle; + + if ($handle) { + $icon_view = id(new PHUIIconView()) + ->setIcon($handle->getIcon()); + } + + return $icon_view; + } + } diff --git a/src/view/phui/PHUILinkView.php b/src/view/phui/PHUILinkView.php new file mode 100644 index 0000000000..33843c2ae9 --- /dev/null +++ b/src/view/phui/PHUILinkView.php @@ -0,0 +1,50 @@ +uri = $uri; + return $this; + } + + public function getURI() { + return $this->uri; + } + + public function setText($text) { + $this->text = $text; + return $this; + } + + public function setWorkflow($workflow) { + $this->workflow = $workflow; + return $this; + } + + protected function getTagName() { + return 'a'; + } + + protected function getTagAttributes() { + $sigil = array(); + + if ($this->workflow) { + $sigil[] = 'workflow'; + } + + return array( + 'href' => $this->getURI(), + 'sigil' => $sigil, + ); + } + + protected function getTagContent() { + return $this->text; + } + +} diff --git a/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css b/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css index b8ba7e1a6f..3becc1ca84 100644 --- a/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css +++ b/webroot/rsrc/css/phui/phui-curtain-object-ref-view.css @@ -7,9 +7,14 @@ color: {$greytext}; } +.phui-curtain-object-ref-view { + padding: 4px 6px; + border-radius: 3px; +} + .phui-curtain-object-ref-view-image-cell { min-width: 32px; - min-height: 32px; + padding-bottom: 24px; } .phui-curtain-object-ref-view-image-cell > a { @@ -18,6 +23,24 @@ background-size: 100%; border-radius: 3px; display: block; + position: absolute; +} + +.phui-curtain-object-ref-view-image-cell .phui-icon-view { + font-size: 16px; + line-height: 16px; + vertical-align: middle; + text-align: center; + width: 24px; + height: 24px; + top: 3px; + display: block; + position: absolute; + color: #ffffff; +} + +.phui-curtain-object-ref-view-icon-image { + background-color: {$backdrop}; } .phui-curtain-object-ref-view-title-cell { @@ -26,7 +49,7 @@ overflow: hidden; /* This is forcing "text-overflow: ellipsis" to actually work. */ - max-width: 225px; + max-width: 210px; } .phui-curtain-object-ref-view-without-content > @@ -46,3 +69,16 @@ .phui-curtain-object-ref-view-epoch-cell { color: {$greytext}; } + +.phui-curtain-object-ref-list-view-tail { + text-align: center; + margin-top: 8px; + padding: 4px; + background: {$lightgreybackground}; + border-top: 1px dashed {$thinblueborder}; + box-shadow: inset 0 2px 3px rgba(0, 0, 0, 0.04); +} + +.phui-curtain-object-ref-view-highlighted { + background: {$bluebackground}; +} From fdbe9ba149b6fa1fa2f6b746dc0363a823d13150 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 14:19:04 -0800 Subject: [PATCH 13/16] Improve Remarkup parsing performance for certain large input blocks Summary: Fixes T13487. In PHI1628, an install has a 4MB remarkup corpus which takes a long time to render. This is broadly expected, but a few reasonable improvements fell out of running it through the profiler. Test Plan: - Saw local cold-cache end-to-end rendering time drop from 12s to 4s for the highly secret input corpus. - Verified output has the same hashes before/after. - Ran all remarkup unit tests. Maniphest Tasks: T13487 Differential Revision: https://secure.phabricator.com/D20968 --- .../blockrule/PhutilRemarkupNoteBlockRule.php | 34 +++-- .../markup/remarkup/PhutilRemarkupEngine.php | 122 ++++++++++++------ .../rule/PhabricatorObjectRemarkupRule.php | 61 +++++---- 3 files changed, 143 insertions(+), 74 deletions(-) diff --git a/src/infrastructure/markup/blockrule/PhutilRemarkupNoteBlockRule.php b/src/infrastructure/markup/blockrule/PhutilRemarkupNoteBlockRule.php index e80f3e1953..ae2ef1fe34 100644 --- a/src/infrastructure/markup/blockrule/PhutilRemarkupNoteBlockRule.php +++ b/src/infrastructure/markup/blockrule/PhutilRemarkupNoteBlockRule.php @@ -100,22 +100,28 @@ final class PhutilRemarkupNoteBlockRule extends PhutilRemarkupBlockRule { } private function getRegEx() { - $words = array( - 'NOTE', - 'IMPORTANT', - 'WARNING', - ); + static $regex; - foreach ($words as $k => $word) { - $words[$k] = preg_quote($word, '/'); + if ($regex === null) { + $words = array( + 'NOTE', + 'IMPORTANT', + 'WARNING', + ); + + foreach ($words as $k => $word) { + $words[$k] = preg_quote($word, '/'); + } + $words = implode('|', $words); + + $regex = + '/^(?:'. + '(?:\((?P'.$words.')\))'. + '|'. + '(?:(?P'.$words.'):))\s*'. + '/'; } - $words = implode('|', $words); - return - '/^(?:'. - '(?:\((?P'.$words.')\))'. - '|'. - '(?:(?P'.$words.'):))\s*'. - '/'; + return $regex; } } diff --git a/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php b/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php index 0b01fd6a69..5ba60d9b8e 100644 --- a/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php +++ b/src/infrastructure/markup/remarkup/PhutilRemarkupEngine.php @@ -153,33 +153,54 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine { $block_rules = $this->blockRules; $blocks = array(); $cursor = 0; - $prev_block = array(); + + $can_merge = array(); + foreach ($block_rules as $key => $block_rule) { + if ($block_rule instanceof PhutilRemarkupDefaultBlockRule) { + $can_merge[$key] = true; + } + } + + $last_block = null; + $last_block_key = -1; + + // See T13487. For very large inputs, block separation can dominate + // runtime. This is written somewhat clumsily to attempt to handle + // very large inputs as gracefully as is practical. while (isset($text[$cursor])) { $starting_cursor = $cursor; - foreach ($block_rules as $block_rule) { + foreach ($block_rules as $block_key => $block_rule) { $num_lines = $block_rule->getMatchingLineCount($text, $cursor); if ($num_lines) { - if ($blocks) { - $prev_block = last($blocks); - } - - $curr_block = array( + $current_block = array( 'start' => $cursor, 'num_lines' => $num_lines, 'rule' => $block_rule, - 'is_empty' => self::isEmptyBlock($text, $cursor, $num_lines), + 'empty' => self::isEmptyBlock($text, $cursor, $num_lines), 'children' => array(), + 'merge' => isset($can_merge[$block_key]), ); - if ($prev_block - && self::shouldMergeBlocks($text, $prev_block, $curr_block)) { - $blocks[last_key($blocks)]['num_lines'] += $curr_block['num_lines']; - $blocks[last_key($blocks)]['is_empty'] = - $blocks[last_key($blocks)]['is_empty'] && $curr_block['is_empty']; + $should_merge = self::shouldMergeParagraphBlocks( + $text, + $last_block, + $current_block); + + if ($should_merge) { + $last_block['num_lines'] = + ($last_block['num_lines'] + $current_block['num_lines']); + + $last_block['empty'] = + ($last_block['empty'] && $current_block['empty']); + + $blocks[$last_block_key] = $last_block; } else { - $blocks[] = $curr_block; + $blocks[] = $current_block; + + $last_block = $current_block; + $last_block_key++; } $cursor += $num_lines; @@ -192,9 +213,20 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine { } } + // See T13487. It's common for blocks to be small, and this loop seems to + // measure as faster if we manually concatenate blocks than if we + // "array_slice()" and "implode()" blocks. This is a bit muddy. + foreach ($blocks as $key => $block) { - $lines = array_slice($text, $block['start'], $block['num_lines']); - $blocks[$key]['text'] = implode('', $lines); + $min = $block['start']; + $max = $min + $block['num_lines']; + + $lines = ''; + for ($ii = $min; $ii < $max; $ii++) { + $lines .= $text[$ii]; + } + + $blocks[$key]['text'] = $lines; } // Stop splitting child blocks apart if we get too deep. This arrests @@ -246,30 +278,48 @@ final class PhutilRemarkupEngine extends PhutilMarkupEngine { return $output; } - private static function shouldMergeBlocks($text, $prev_block, $curr_block) { - $block_rules = ipull(array($prev_block, $curr_block), 'rule'); + private static function shouldMergeParagraphBlocks( + $text, + $last_block, + $current_block) { - $default_rule = 'PhutilRemarkupDefaultBlockRule'; - try { - assert_instances_of($block_rules, $default_rule); + // If we're at the beginning of the input, we can't merge. + if ($last_block === null) { + return false; + } - // If the last block was empty keep merging - if ($prev_block['is_empty']) { - return true; - } + // If the previous block wasn't a default block, we can't merge. + if (!$last_block['merge']) { + return false; + } - // If this line is blank keep merging - if ($curr_block['is_empty']) { - return true; - } + // If the current block isn't a default block, we can't merge. + if (!$current_block['merge']) { + return false; + } - // If the current line and the last line have content, keep merging - if (strlen(trim($text[$curr_block['start'] - 1]))) { - if (strlen(trim($text[$curr_block['start']]))) { - return true; - } - } - } catch (Exception $e) {} + // If the last block was empty, we definitely want to merge. + if ($last_block['empty']) { + return true; + } + + // If this block is empty, we definitely want to merge. + if ($current_block['empty']) { + return true; + } + + // Check if the last line of the previous block or the first line of this + // block have any non-whitespace text. If they both do, we're going to + // merge. + + // If either of them are a blank line or a line with only whitespace, we + // do not merge: this means we've found a paragraph break. + + $tail = $text[$current_block['start'] - 1]; + $head = $text[$current_block['start']]; + if (strlen(trim($tail)) && strlen(trim($head))) { + return true; + } return false; } diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php index 31b4e52250..7fb997b2c5 100644 --- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php @@ -2,6 +2,9 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { + private $referencePattern; + private $embedPattern; + const KEY_RULE_OBJECT = 'rule.object'; const KEY_MENTIONED_OBJECTS = 'rule.object.mentioned'; @@ -192,38 +195,48 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { } private function getObjectEmbedPattern() { - $prefix = $this->getObjectNamePrefix(); - $prefix = preg_quote($prefix); - $id = $this->getObjectIDPattern(); + if ($this->embedPattern === null) { + $prefix = $this->getObjectNamePrefix(); + $prefix = preg_quote($prefix); + $id = $this->getObjectIDPattern(); - return '(\B{'.$prefix.'('.$id.')([,\s](?:[^}\\\\]|\\\\.)*)?}\B)u'; + $this->embedPattern = + '(\B{'.$prefix.'('.$id.')([,\s](?:[^}\\\\]|\\\\.)*)?}\B)u'; + } + + return $this->embedPattern; } private function getObjectReferencePattern() { - $prefix = $this->getObjectNamePrefix(); - $prefix = preg_quote($prefix); + if ($this->referencePattern === null) { + $prefix = $this->getObjectNamePrefix(); + $prefix = preg_quote($prefix); - $id = $this->getObjectIDPattern(); + $id = $this->getObjectIDPattern(); - // If the prefix starts with a word character (like "D"), we want to - // require a word boundary so that we don't match "XD1" as "D1". If the - // prefix does not start with a word character, we want to require no word - // boundary for the same reasons. Test if the prefix starts with a word - // character. - if ($this->getObjectNamePrefixBeginsWithWordCharacter()) { - $boundary = '\\b'; - } else { - $boundary = '\\B'; + // If the prefix starts with a word character (like "D"), we want to + // require a word boundary so that we don't match "XD1" as "D1". If the + // prefix does not start with a word character, we want to require no word + // boundary for the same reasons. Test if the prefix starts with a word + // character. + if ($this->getObjectNamePrefixBeginsWithWordCharacter()) { + $boundary = '\\b'; + } else { + $boundary = '\\B'; + } + + // The "(?referencePattern = + '((?referencePattern; } From fd46c597aec5cc44707dc99e153da749c5389907 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 4 Feb 2020 15:03:17 -0800 Subject: [PATCH 14/16] When sorting subscriber references for display in the curtain UI, sort without case sensitivity Summary: Ref T13486. Currently, "Zarbo" sorts above "alice", but this isn't expected for a list of (mostly) human usernames. Test Plan: Loaded a task with subscribers with mixed-case usernames. Maniphest Tasks: T13486 Differential Revision: https://secure.phabricator.com/D20969 --- .../PhabricatorSubscriptionsCurtainExtension.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php index 639e96eb86..a6df97b095 100644 --- a/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php +++ b/src/applications/subscriptions/engineextension/PhabricatorSubscriptionsCurtainExtension.php @@ -123,7 +123,9 @@ final class PhabricatorSubscriptionsCurtainExtension $handle = $handles[$subscriber_phid]; if ($handle->isComplete()) { $is_complete = true; + $sort_name = $handle->getLinkName(); + $sort_name = phutil_utf8_strtolower($sort_name); } } } From 9d1af762d5182723282f62c8fb5c1bb69b7bf96d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 5 Feb 2020 07:36:31 -0800 Subject: [PATCH 15/16] In summary interfaces, don't render very large inline remarkup details for unit test messages Summary: Ref T10635. An install with large blocks of remarkup (4MB) in test details is reporting slow page rendering. This is expected, but I've mostly given up on fighting this unless I absolutely have to. Degrade the interface more aggressively. Test Plan: - Submitted a large block of test details in remarkup format. - Before patch: they rendered inline. - After patch: degraded display. - Verified small blocks are not changed. {F7180727} {F7180728} Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam Maniphest Tasks: T10635 Differential Revision: https://secure.phabricator.com/D20970 --- .../build/HarbormasterBuildUnitMessage.php | 84 +++++++++++++++++-- .../view/HarbormasterUnitPropertyView.php | 6 +- 2 files changed, 81 insertions(+), 9 deletions(-) diff --git a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php index 9e437efaba..f2632153c3 100644 --- a/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php +++ b/src/applications/harbormaster/storage/build/HarbormasterBuildUnitMessage.php @@ -179,18 +179,23 @@ final class HarbormasterBuildUnitMessage $is_text = ($format !== self::FORMAT_REMARKUP); $is_remarkup = ($format === self::FORMAT_REMARKUP); + $message = null; $full_details = $this->getUnitMessageDetails(); + $byte_length = strlen($full_details); - if (!strlen($full_details)) { + $text_limit = 1024 * 2; + $remarkup_limit = 1024 * 8; + + if (!$byte_length) { if ($summarize) { return null; } - $details = phutil_tag('em', array(), pht('No details provided.')); + $message = phutil_tag('em', array(), pht('No details provided.')); } else if ($summarize) { if ($is_text) { $details = id(new PhutilUTF8StringTruncator()) - ->setMaximumBytes(2048) + ->setMaximumBytes($text_limit) ->truncateString($full_details); $details = phutil_split_lines($details); @@ -201,7 +206,34 @@ final class HarbormasterBuildUnitMessage $details = implode('', $details); } else { - $details = $full_details; + if ($byte_length > $remarkup_limit) { + $uri = $this->getURI(); + + if ($uri) { + $link = phutil_tag( + 'a', + array( + 'href' => $uri, + 'target' => '_blank', + ), + pht('View Details')); + } else { + $link = null; + } + + $message = array(); + $message[] = phutil_tag( + 'em', + array(), + pht('This test has too much data to display inline.')); + if ($link) { + $message[] = $link; + } + + $message = phutil_implode_html(" \xC2\xB7 ", $message); + } else { + $details = $full_details; + } } } else { $details = $full_details; @@ -212,19 +244,47 @@ final class HarbormasterBuildUnitMessage $classes = array(); $classes[] = 'harbormaster-unit-details'; - if ($is_remarkup) { + if ($message !== null) { + // If we have a message, show that instead of rendering any test details. + $details = $message; + } else if ($is_remarkup) { $details = new PHUIRemarkupView($viewer, $details); } else { $classes[] = 'harbormaster-unit-details-text'; $classes[] = 'PhabricatorMonospaced'; } - return phutil_tag( + $warning = null; + if (!$summarize) { + $warnings = array(); + + if ($is_remarkup && ($byte_length > $remarkup_limit)) { + $warnings[] = pht( + 'This test result has %s bytes of Remarkup test details. Remarkup '. + 'blocks longer than %s bytes are not rendered inline when showing '. + 'test summaries.', + new PhutilNumber($byte_length), + new PhutilNumber($remarkup_limit)); + } + + if ($warnings) { + $warning = id(new PHUIInfoView()) + ->setSeverity(PHUIInfoView::SEVERITY_WARNING) + ->setErrors($warnings); + } + } + + $content = phutil_tag( 'div', array( 'class' => implode(' ', $classes), ), $details); + + return array( + $warning, + $content, + ); } public function getUnitMessageDisplayName() { @@ -262,6 +322,18 @@ final class HarbormasterBuildUnitMessage return implode("\0", $parts); } + public function getURI() { + $id = $this->getID(); + + if (!$id) { + return null; + } + + return urisprintf( + '/harbormaster/unit/view/%d/', + $id); + } + public function save() { if ($this->nameIndex === null) { $this->nameIndex = HarbormasterString::newIndex($this->getName()); diff --git a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php index ee8b80a7a7..2306f5ba64 100644 --- a/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php +++ b/src/applications/harbormaster/view/HarbormasterUnitPropertyView.php @@ -72,13 +72,13 @@ final class HarbormasterUnitPropertyView extends AphrontView { } $name = $message->getUnitMessageDisplayName(); - $id = $message->getID(); + $uri = $message->getURI(); - if ($id) { + if ($uri) { $name = phutil_tag( 'a', array( - 'href' => "/harbormaster/unit/view/{$id}/", + 'href' => $uri, ), $name); } From 2327578adc946eefb1864ea6ebe0522b5290d6a0 Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 6 Feb 2020 14:48:08 -0800 Subject: [PATCH 16/16] Respect linebreaks in full HTML tables in Remarkup Summary: Fixes T5427. See PHI1630. See also T13160 and D20568. In the full HTML table syntax with "", respect linebreaks as literals inside "
" cells. Test Plan: Previewed some full-HTML tables with and without linebreaks, saw what seemed like sensible rendering behavior. Maniphest Tasks: T5427 Differential Revision: https://secure.phabricator.com/D20971 --- .../blockrule/PhutilRemarkupTableBlockRule.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php b/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php index 57f2fe1f1d..72e61881ce 100644 --- a/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php +++ b/src/infrastructure/markup/blockrule/PhutilRemarkupTableBlockRule.php @@ -121,8 +121,19 @@ final class PhutilRemarkupTableBlockRule extends PhutilRemarkupBlockRule { return $table->newRawString(); } + // Respect newlines in table cells as literal linebreaks. + $content = $cell->newRawContentString(); - $content = $this->applyRules($content); + $content = trim($content, "\r\n"); + + $lines = phutil_split_lines($content, $retain_endings = false); + foreach ($lines as $key => $line) { + $lines[$key] = $this->applyRules($line); + } + + $content = phutil_implode_html( + phutil_tag('br'), + $lines); $cell_specs[] = array( 'type' => $cell->getTagName(),