From 15ed2b936c374bd84c9c04a65b340860c06b3a55 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Mon, 22 Aug 2016 10:36:23 -0700 Subject: [PATCH 01/27] Update Config Application UI Summary: Switches over to new property UI boxes, splits core and apps into separate pages. Move Versions into "All Settings". I think there is some docs I likely need to update here as well. Test Plan: Click on each item in the sidebar, see new headers. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16429 --- src/__phutil_library_map__.php | 6 +- ...icatorAuthSessionEngineExtensionModule.php | 1 + ...habricatorAuthTemporaryTokenTypeModule.php | 1 + .../PhabricatorConfigApplication.php | 2 + .../PhabricatorConfigAllController.php | 8 ++- ...PhabricatorConfigApplicationController.php | 58 +++++++++++++++++++ .../PhabricatorConfigCacheController.php | 2 + ...icatorConfigClusterDatabasesController.php | 1 + ...orConfigClusterNotificationsController.php | 1 + ...torConfigClusterRepositoriesController.php | 1 + .../PhabricatorConfigController.php | 9 +-- ...abricatorConfigDatabaseIssueController.php | 1 + ...bricatorConfigDatabaseStatusController.php | 9 +++ .../PhabricatorConfigGroupController.php | 1 + .../PhabricatorConfigHistoryController.php | 2 +- .../PhabricatorConfigIssueListController.php | 5 +- .../PhabricatorConfigListController.php | 12 ++-- .../PhabricatorConfigVersionController.php} | 43 ++++++++++---- .../PhabricatorConfigWelcomeController.php | 4 +- .../PhabricatorConfigCollectorsModule.php | 1 + .../module/PhabricatorConfigEdgeModule.php | 1 + ...bricatorConfigHTTPParameterTypesModule.php | 1 + .../module/PhabricatorConfigPHIDModule.php | 1 + ...torConfigRequestExceptionHandlerModule.php | 1 + .../module/PhabricatorConfigSiteModule.php | 1 + ...bricatorHovercardEngineExtensionModule.php | 1 + ...PhabricatorSearchEngineExtensionModule.php | 1 + ...abricatorFulltextEngineExtensionModule.php | 1 + .../PhabricatorIndexEngineExtensionModule.php | 1 + ...icatorDestructionEngineExtensionModule.php | 1 + .../PhabricatorEditEngineExtensionModule.php | 1 + .../PhabricatorContentSourceModule.php | 1 + 32 files changed, 147 insertions(+), 33 deletions(-) create mode 100644 src/applications/config/controller/PhabricatorConfigApplicationController.php rename src/applications/config/{module/PhabricatorConfigVersionsModule.php => controller/PhabricatorConfigVersionController.php} (67%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 93a7bb66c8..a6ab763246 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2140,6 +2140,7 @@ phutil_register_library_map(array( '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/PhabricatorConfigCacheController.php', 'PhabricatorConfigClusterDatabasesController' => 'applications/config/controller/PhabricatorConfigClusterDatabasesController.php', 'PhabricatorConfigClusterNotificationsController' => 'applications/config/controller/PhabricatorConfigClusterNotificationsController.php', @@ -2201,7 +2202,7 @@ phutil_register_library_map(array( 'PhabricatorConfigTransaction' => 'applications/config/storage/PhabricatorConfigTransaction.php', 'PhabricatorConfigTransactionQuery' => 'applications/config/query/PhabricatorConfigTransactionQuery.php', 'PhabricatorConfigValidationException' => 'applications/config/exception/PhabricatorConfigValidationException.php', - 'PhabricatorConfigVersionsModule' => 'applications/config/module/PhabricatorConfigVersionsModule.php', + 'PhabricatorConfigVersionController' => 'applications/config/controller/PhabricatorConfigVersionController.php', 'PhabricatorConfigWelcomeController' => 'applications/config/controller/PhabricatorConfigWelcomeController.php', 'PhabricatorConpherenceApplication' => 'applications/conpherence/application/PhabricatorConpherenceApplication.php', 'PhabricatorConpherenceColumnVisibleSetting' => 'applications/settings/setting/PhabricatorConpherenceColumnVisibleSetting.php', @@ -6877,6 +6878,7 @@ phutil_register_library_map(array( 'PhabricatorConduitTokensSettingsPanel' => 'PhabricatorSettingsPanel', 'PhabricatorConfigAllController' => 'PhabricatorConfigController', 'PhabricatorConfigApplication' => 'PhabricatorApplication', + 'PhabricatorConfigApplicationController' => 'PhabricatorConfigController', 'PhabricatorConfigCacheController' => 'PhabricatorConfigController', 'PhabricatorConfigClusterDatabasesController' => 'PhabricatorConfigController', 'PhabricatorConfigClusterNotificationsController' => 'PhabricatorConfigController', @@ -6945,7 +6947,7 @@ phutil_register_library_map(array( 'PhabricatorConfigTransaction' => 'PhabricatorApplicationTransaction', 'PhabricatorConfigTransactionQuery' => 'PhabricatorApplicationTransactionQuery', 'PhabricatorConfigValidationException' => 'Exception', - 'PhabricatorConfigVersionsModule' => 'PhabricatorConfigModule', + 'PhabricatorConfigVersionController' => 'PhabricatorConfigController', 'PhabricatorConfigWelcomeController' => 'PhabricatorConfigController', 'PhabricatorConpherenceApplication' => 'PhabricatorApplication', 'PhabricatorConpherenceColumnVisibleSetting' => 'PhabricatorInternalSetting', diff --git a/src/applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php b/src/applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php index 9468321d2d..0f8c9fef8c 100644 --- a/src/applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php +++ b/src/applications/auth/engine/PhabricatorAuthSessionEngineExtensionModule.php @@ -43,6 +43,7 @@ final class PhabricatorAuthSessionEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('SessionEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php b/src/applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php index 8f4ad9ea9b..3c44ef01b2 100644 --- a/src/applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php +++ b/src/applications/auth/tokentype/PhabricatorAuthTemporaryTokenTypeModule.php @@ -41,6 +41,7 @@ final class PhabricatorAuthTemporaryTokenTypeModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Temporary Token Types')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/application/PhabricatorConfigApplication.php b/src/applications/config/application/PhabricatorConfigApplication.php index 62fe79f4ce..c9b1c5c345 100644 --- a/src/applications/config/application/PhabricatorConfigApplication.php +++ b/src/applications/config/application/PhabricatorConfigApplication.php @@ -38,10 +38,12 @@ final class PhabricatorConfigApplication extends PhabricatorApplication { return array( '/config/' => array( '' => 'PhabricatorConfigListController', + 'application/' => 'PhabricatorConfigApplicationController', 'all/' => 'PhabricatorConfigAllController', 'history/' => 'PhabricatorConfigHistoryController', 'edit/(?P[\w\.\-]+)/' => 'PhabricatorConfigEditController', 'group/(?P[^/]+)/' => 'PhabricatorConfigGroupController', + 'version/' => 'PhabricatorConfigVersionController', 'welcome/' => 'PhabricatorConfigWelcomeController', 'database/'. '(?:(?P[^/]+)/'. diff --git a/src/applications/config/controller/PhabricatorConfigAllController.php b/src/applications/config/controller/PhabricatorConfigAllController.php index e46db665de..6d661479aa 100644 --- a/src/applications/config/controller/PhabricatorConfigAllController.php +++ b/src/applications/config/controller/PhabricatorConfigAllController.php @@ -52,11 +52,13 @@ final class PhabricatorConfigAllController $crumbs = $this ->buildApplicationCrumbs() + ->addTextCrumb(pht('Configuration'), $this->getApplicationURI()) ->addTextCrumb($title); - $panel = new PHUIObjectBoxView(); - $panel->setHeaderText(pht('Current Settings')); - $panel->setTable($table); + $panel = id(new PHUIObjectBoxView()) + ->setHeaderText(pht('Current Settings')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setTable($table); $nav = $this->buildSideNavView(); $nav->selectFilter('all/'); diff --git a/src/applications/config/controller/PhabricatorConfigApplicationController.php b/src/applications/config/controller/PhabricatorConfigApplicationController.php new file mode 100644 index 0000000000..4a7bccf739 --- /dev/null +++ b/src/applications/config/controller/PhabricatorConfigApplicationController.php @@ -0,0 +1,58 @@ +getViewer(); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('application/'); + + $groups = PhabricatorApplicationConfigOptions::loadAll(); + $apps_list = $this->buildConfigOptionsList($groups, 'apps'); + + $title = pht('Application Configuration'); + + $apps = id(new PHUIObjectBoxView()) + ->setHeaderText($title) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) + ->setObjectList($apps_list); + + $crumbs = $this + ->buildApplicationCrumbs() + ->addTextCrumb(pht('Configuration'), $this->getApplicationURI()) + ->addTextCrumb(pht('Applications')); + + $view = id(new PHUITwoColumnView()) + ->setNavigation($nav) + ->setMainColumn(array( + $apps, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + } + + private function buildConfigOptionsList(array $groups, $type) { + assert_instances_of($groups, 'PhabricatorApplicationConfigOptions'); + + $list = new PHUIObjectItemListView(); + $groups = msort($groups, 'getName'); + foreach ($groups as $group) { + if ($group->getGroup() == $type) { + $item = id(new PHUIObjectItemView()) + ->setHeader($group->getName()) + ->setHref('/config/group/'.$group->getKey().'/') + ->addAttribute($group->getDescription()) + ->setImageIcon($group->getIcon()); + $list->addItem($item); + } + } + + return $list; + } + +} diff --git a/src/applications/config/controller/PhabricatorConfigCacheController.php b/src/applications/config/controller/PhabricatorConfigCacheController.php index ab9ddb3ad0..0f6ee83089 100644 --- a/src/applications/config/controller/PhabricatorConfigCacheController.php +++ b/src/applications/config/controller/PhabricatorConfigCacheController.php @@ -51,6 +51,7 @@ final class PhabricatorConfigCacheController return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); } @@ -102,6 +103,7 @@ final class PhabricatorConfigCacheController return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Data Cache')) ->addPropertyList($properties) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php b/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php index 453ebfe742..33f7f1331e 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php +++ b/src/applications/config/controller/PhabricatorConfigClusterDatabasesController.php @@ -207,6 +207,7 @@ final class PhabricatorConfigClusterDatabasesController return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php b/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php index 7f42c323e6..576d6841d0 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php +++ b/src/applications/config/controller/PhabricatorConfigClusterNotificationsController.php @@ -157,6 +157,7 @@ final class PhabricatorConfigClusterNotificationsController return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php b/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php index bea1a0dec6..c0fafc2c58 100644 --- a/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php +++ b/src/applications/config/controller/PhabricatorConfigClusterRepositoriesController.php @@ -253,6 +253,7 @@ final class PhabricatorConfigClusterRepositoriesController return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/controller/PhabricatorConfigController.php b/src/applications/config/controller/PhabricatorConfigController.php index ac629f85b5..b60418de08 100644 --- a/src/applications/config/controller/PhabricatorConfigController.php +++ b/src/applications/config/controller/PhabricatorConfigController.php @@ -12,11 +12,14 @@ abstract class PhabricatorConfigController extends PhabricatorController { $nav = new AphrontSideNavFilterView(); $nav->setBaseURI(new PhutilURI($this->getApplicationURI())); $nav->addLabel(pht('Configuration')); - $nav->addFilter('/', pht('Browse Settings')); - $nav->addFilter('all/', pht('All Settings')); + $nav->addFilter('/', pht('Core Settings')); + $nav->addFilter('application/', pht('Application Settings')); $nav->addFilter('history/', pht('Settings History')); + $nav->addFilter('version/', pht('Version Information')); + $nav->addFilter('all/', pht('All Settings')); $nav->addLabel(pht('Setup')); $nav->addFilter('issue/', pht('Setup Issues')); + $nav->addFilter('welcome/', pht('Installation Guide')); $nav->addLabel(pht('Database')); $nav->addFilter('database/', pht('Database Status')); $nav->addFilter('dbissue/', pht('Database Issues')); @@ -26,8 +29,6 @@ abstract class PhabricatorConfigController extends PhabricatorController { $nav->addFilter('cluster/databases/', pht('Database Servers')); $nav->addFilter('cluster/notifications/', pht('Notification Servers')); $nav->addFilter('cluster/repositories/', pht('Repository Servers')); - $nav->addLabel(pht('Welcome')); - $nav->addFilter('welcome/', pht('Welcome Screen')); $nav->addLabel(pht('Modules')); $modules = PhabricatorConfigModule::getAllModules(); diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php b/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php index 0366b90b16..775a765d92 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseIssueController.php @@ -149,6 +149,7 @@ final class PhabricatorConfigDatabaseIssueController $table_box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) ->setFormErrors($errors) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); $nav = $this->buildSideNavView(); diff --git a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php index b03ce2a9fc..3e6e9af298 100644 --- a/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php +++ b/src/applications/config/controller/PhabricatorConfigDatabaseStatusController.php @@ -165,10 +165,12 @@ final class PhabricatorConfigDatabaseStatusController $prop_box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); $table_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Databases')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); return $this->buildResponse($title, array($prop_box, $table_box)); @@ -263,10 +265,12 @@ final class PhabricatorConfigDatabaseStatusController $prop_box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); $table_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Database Status')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); return $this->buildResponse($title, array($prop_box, $table_box)); @@ -476,14 +480,17 @@ final class PhabricatorConfigDatabaseStatusController $prop_box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); $table_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Database')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table_view); $key_box = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Keys')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($keys_view); return $this->buildResponse($title, array($prop_box, $table_box, $key_box)); @@ -620,6 +627,7 @@ final class PhabricatorConfigDatabaseStatusController $box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); return $this->buildResponse($title, $box); @@ -713,6 +721,7 @@ final class PhabricatorConfigDatabaseStatusController $box = id(new PHUIObjectBoxView()) ->setHeader($this->buildHeaderWithDocumentationLink($title)) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($properties); return $this->buildResponse($title, $box); diff --git a/src/applications/config/controller/PhabricatorConfigGroupController.php b/src/applications/config/controller/PhabricatorConfigGroupController.php index af5ed41222..c1d46906bc 100644 --- a/src/applications/config/controller/PhabricatorConfigGroupController.php +++ b/src/applications/config/controller/PhabricatorConfigGroupController.php @@ -17,6 +17,7 @@ final class PhabricatorConfigGroupController $list = $this->buildOptionList($options->getOptions()); $box = id(new PHUIObjectBoxView()) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($list); $crumbs = $this diff --git a/src/applications/config/controller/PhabricatorConfigHistoryController.php b/src/applications/config/controller/PhabricatorConfigHistoryController.php index 2709041f94..6599c2d233 100644 --- a/src/applications/config/controller/PhabricatorConfigHistoryController.php +++ b/src/applications/config/controller/PhabricatorConfigHistoryController.php @@ -31,7 +31,7 @@ final class PhabricatorConfigHistoryController $title = pht('Settings History'); $crumbs = $this->buildApplicationCrumbs(); - $crumbs->addTextCrumb('Config', $this->getApplicationURI()); + $crumbs->addTextCrumb('Configuration', $this->getApplicationURI()); $crumbs->addTextCrumb($title, '/config/history/'); $nav = $this->buildSideNavView(); diff --git a/src/applications/config/controller/PhabricatorConfigIssueListController.php b/src/applications/config/controller/PhabricatorConfigIssueListController.php index c3dc7f577c..7eac4c10ee 100644 --- a/src/applications/config/controller/PhabricatorConfigIssueListController.php +++ b/src/applications/config/controller/PhabricatorConfigIssueListController.php @@ -27,25 +27,28 @@ final class PhabricatorConfigIssueListController if ($important) { $setup_issues[] = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Important Setup Issues')) - ->setColor(PHUIObjectBoxView::COLOR_RED) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($important); } if ($php) { $setup_issues[] = id(new PHUIObjectBoxView()) ->setHeaderText(pht('PHP Setup Issues')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($php); } if ($mysql) { $setup_issues[] = id(new PHUIObjectBoxView()) ->setHeaderText(pht('MySQL Setup Issues')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($mysql); } if ($other) { $setup_issues[] = id(new PHUIObjectBoxView()) ->setHeaderText(pht('Other Setup Issues')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($other); } diff --git a/src/applications/config/controller/PhabricatorConfigListController.php b/src/applications/config/controller/PhabricatorConfigListController.php index e6af666931..ed8301ff2f 100644 --- a/src/applications/config/controller/PhabricatorConfigListController.php +++ b/src/applications/config/controller/PhabricatorConfigListController.php @@ -11,27 +11,23 @@ final class PhabricatorConfigListController $groups = PhabricatorApplicationConfigOptions::loadAll(); $core_list = $this->buildConfigOptionsList($groups, 'core'); - $apps_list = $this->buildConfigOptionsList($groups, 'apps'); - $title = pht('Phabricator Configuration'); + $title = pht('Core Configuration'); $core = id(new PHUIObjectBoxView()) ->setHeaderText($title) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setObjectList($core_list); - $apps = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Applications Configuration')) - ->setObjectList($apps_list); - $crumbs = $this ->buildApplicationCrumbs() - ->addTextCrumb(pht('Config'), $this->getApplicationURI()); + ->addTextCrumb(pht('Configuration'), $this->getApplicationURI()) + ->addTextCrumb($title); $view = id(new PHUITwoColumnView()) ->setNavigation($nav) ->setMainColumn(array( $core, - $apps, )); return $this->newPage() diff --git a/src/applications/config/module/PhabricatorConfigVersionsModule.php b/src/applications/config/controller/PhabricatorConfigVersionController.php similarity index 67% rename from src/applications/config/module/PhabricatorConfigVersionsModule.php rename to src/applications/config/controller/PhabricatorConfigVersionController.php index 9a6292a9ef..f7c2b7898f 100644 --- a/src/applications/config/module/PhabricatorConfigVersionsModule.php +++ b/src/applications/config/controller/PhabricatorConfigVersionController.php @@ -1,19 +1,37 @@ getViewer(); + $title = pht('Version Information'); + + $crumbs = $this + ->buildApplicationCrumbs() + ->addTextCrumb(pht('Configuration'), $this->getApplicationURI()) + ->addTextCrumb($title); + + $versions = $this->renderModuleStatus($viewer); + + $nav = $this->buildSideNavView(); + $nav->selectFilter('version/'); + + $view = id(new PHUITwoColumnView()) + ->setNavigation($nav) + ->setMainColumn(array( + $versions, + )); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->appendChild($view); + + } + + public function renderModuleStatus($viewer) { $versions = $this->loadVersions($viewer); $version_property_list = id(new PHUIPropertyListView()); @@ -22,7 +40,8 @@ final class PhabricatorConfigVersionsModule } $object_box = id(new PHUIObjectBoxView()) - ->setHeaderText(pht('Current Versions')) + ->setHeaderText(pht('Version Information')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->addPropertyList($version_property_list); $phabricator_root = dirname(phutil_get_library_root('phabricator')); diff --git a/src/applications/config/controller/PhabricatorConfigWelcomeController.php b/src/applications/config/controller/PhabricatorConfigWelcomeController.php index 6825abba8c..5fa4c65495 100644 --- a/src/applications/config/controller/PhabricatorConfigWelcomeController.php +++ b/src/applications/config/controller/PhabricatorConfigWelcomeController.php @@ -9,11 +9,11 @@ final class PhabricatorConfigWelcomeController $nav = $this->buildSideNavView(); $nav->selectFilter('welcome/'); - $title = pht('Welcome'); + $title = pht('Installation Guide'); $crumbs = $this ->buildApplicationCrumbs() - ->addTextCrumb(pht('Welcome')); + ->addTextCrumb($title); $view = id(new PHUITwoColumnView()) ->setNavigation($nav) diff --git a/src/applications/config/module/PhabricatorConfigCollectorsModule.php b/src/applications/config/module/PhabricatorConfigCollectorsModule.php index f00e9d3489..4a121f483d 100644 --- a/src/applications/config/module/PhabricatorConfigCollectorsModule.php +++ b/src/applications/config/module/PhabricatorConfigCollectorsModule.php @@ -73,6 +73,7 @@ final class PhabricatorConfigCollectorsModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeader($header) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/module/PhabricatorConfigEdgeModule.php b/src/applications/config/module/PhabricatorConfigEdgeModule.php index 8aa5d74664..2791269811 100644 --- a/src/applications/config/module/PhabricatorConfigEdgeModule.php +++ b/src/applications/config/module/PhabricatorConfigEdgeModule.php @@ -41,6 +41,7 @@ final class PhabricatorConfigEdgeModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Edge Types')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/module/PhabricatorConfigHTTPParameterTypesModule.php b/src/applications/config/module/PhabricatorConfigHTTPParameterTypesModule.php index 66a44c8c83..8ea911570b 100644 --- a/src/applications/config/module/PhabricatorConfigHTTPParameterTypesModule.php +++ b/src/applications/config/module/PhabricatorConfigHTTPParameterTypesModule.php @@ -21,6 +21,7 @@ final class PhabricatorConfigHTTPParameterTypesModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('HTTP Parameter Types')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/module/PhabricatorConfigPHIDModule.php b/src/applications/config/module/PhabricatorConfigPHIDModule.php index cd7d0e0092..772ad3ecd5 100644 --- a/src/applications/config/module/PhabricatorConfigPHIDModule.php +++ b/src/applications/config/module/PhabricatorConfigPHIDModule.php @@ -73,6 +73,7 @@ final class PhabricatorConfigPHIDModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('PHID Types')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php b/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php index 22539a1bd3..27b338de6d 100644 --- a/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php +++ b/src/applications/config/module/PhabricatorConfigRequestExceptionHandlerModule.php @@ -41,6 +41,7 @@ final class PhabricatorConfigRequestExceptionHandlerModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Exception Handlers')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/config/module/PhabricatorConfigSiteModule.php b/src/applications/config/module/PhabricatorConfigSiteModule.php index 3f63ae2da4..a1a7bab9d0 100644 --- a/src/applications/config/module/PhabricatorConfigSiteModule.php +++ b/src/applications/config/module/PhabricatorConfigSiteModule.php @@ -40,6 +40,7 @@ final class PhabricatorConfigSiteModule extends PhabricatorConfigModule { return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Sites')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php b/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php index fa7ca4d33a..01f4c5d319 100644 --- a/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php +++ b/src/applications/search/engineextension/PhabricatorHovercardEngineExtensionModule.php @@ -49,6 +49,7 @@ final class PhabricatorHovercardEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('HovercardEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php b/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php index 2dd2697a9d..45879edcb4 100644 --- a/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php +++ b/src/applications/search/engineextension/PhabricatorSearchEngineExtensionModule.php @@ -49,6 +49,7 @@ final class PhabricatorSearchEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('SearchEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php b/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php index 7e42d6e5f5..c0061e9293 100644 --- a/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php +++ b/src/applications/search/index/PhabricatorFulltextEngineExtensionModule.php @@ -38,6 +38,7 @@ final class PhabricatorFulltextEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('FulltextEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php b/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php index e586dc9e3c..712deef5ff 100644 --- a/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php +++ b/src/applications/search/index/PhabricatorIndexEngineExtensionModule.php @@ -38,6 +38,7 @@ final class PhabricatorIndexEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('IndexEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php b/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php index e57375e336..a201e3da54 100644 --- a/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php +++ b/src/applications/system/engine/PhabricatorDestructionEngineExtensionModule.php @@ -38,6 +38,7 @@ final class PhabricatorDestructionEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('DestructionEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php b/src/applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php index 3cc572908a..67bcfae6d9 100644 --- a/src/applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php +++ b/src/applications/transactions/engineextension/PhabricatorEditEngineExtensionModule.php @@ -46,6 +46,7 @@ final class PhabricatorEditEngineExtensionModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('EditEngine Extensions')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } diff --git a/src/infrastructure/contentsource/PhabricatorContentSourceModule.php b/src/infrastructure/contentsource/PhabricatorContentSourceModule.php index c9bd3347f3..2eed665eac 100644 --- a/src/infrastructure/contentsource/PhabricatorContentSourceModule.php +++ b/src/infrastructure/contentsource/PhabricatorContentSourceModule.php @@ -45,6 +45,7 @@ final class PhabricatorContentSourceModule return id(new PHUIObjectBoxView()) ->setHeaderText(pht('Content Sources')) + ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY) ->setTable($table); } From b521f2349e46888514ce55d06ace1d8c5fa6df03 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Mon, 22 Aug 2016 14:35:08 -0500 Subject: [PATCH 02/27] Explain how cats use their time Summary: Added a brand new shiny cat fact Test Plan: Pulled up a project with motivator installed and nothing broke Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16430 --- .../search/profilepanel/PhabricatorMotivatorProfilePanel.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php b/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php index 327290deba..5a645039ec 100644 --- a/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php +++ b/src/applications/search/profilepanel/PhabricatorMotivatorProfilePanel.php @@ -143,7 +143,8 @@ final class PhabricatorMotivatorProfilePanel pht( 'Cats will often bring you their prey because they feel sorry '. 'for your inability to hunt.'), - ); + pht('Cats spend most of their time plotting to kill their owner.'), + ); } private function selectFact(array $facts) { From 5d93290e42d74f05a86d985a89f58e1a8fe87f88 Mon Sep 17 00:00:00 2001 From: epriestley Date: Tue, 23 Aug 2016 14:32:36 -0700 Subject: [PATCH 03/27] Update Celerity map parser for new docblock code Summary: After D16431, listing the same `@annotation` multiple times makes the docblock parser return a list. We have some resources which list `@requires` or `@provides` several times, but don't handle the new parser properly. Make the code more flexible, since this is a reasonable way to specify the annotations. See also D16432. This produces a failure in this form: ``` [2016-08-23 21:10:15] ERROR 2: trim() expects parameter 1 to be string, array given at [/core/data/drydock/workingcopy-74/repo/phabricator/src/applications/celerity/CelerityResourceMapGenerator.php:236] 2 arcanist(head=master, ref.master=89e8b4852384), phabricator(head=6c940fb71b0a8850c6a1b7f5fc642a8f8135a76a, ref.master=b521f2349e46), phutil(head=master, ref.master=237549280f08) 3 #0 trim(array) called at [/src/applications/celerity/CelerityResourceMapGenerator.php:236] 4 #1 CelerityResourceMapGenerator::getProvidesAndRequires(string, string) called at [/src/applications/celerity/CelerityResourceMapGenerator.php:193] 5 #2 CelerityResourceMapGenerator::rebuildTextResources(CelerityPhabricatorResources, CelerityResourceTransformer) called at [/src/applications/celerity/CelerityResourceMapGenerator.php:54] 6 #3 CelerityResourceMapGenerator::generate() called at [/src/__tests__/PhabricatorCelerityTestCase.php:16] 7 #4 PhabricatorCelerityTestCase::testCelerityMaps() 8 #5 call_user_func_array(array, array) called at [/src/unit/engine/phutil/PhutilTestCase.php:492] 9 #6 PhutilTestCase::run() called at [/src/unit/engine/PhutilUnitTestEngine.php:69] 10 #7 PhutilUnitTestEngine::run() called at [/src/unit/engine/ArcanistConfigurationDrivenUnitTestEngine.php:147] 11 #8 ArcanistConfigurationDrivenUnitTestEngine::run() called at [/src/workflow/ArcanistUnitWorkflow.php:167] 12 #9 ArcanistUnitWorkflow::run() called at [/scripts/arcanist.php:394] ``` Test Plan: Ran `bin/celerity map`, no more warnings and no change to the actual map. Reviewers: joshuaspence, chad Reviewed By: chad Differential Revision: https://secure.phabricator.com/D16433 --- .../celerity/CelerityResourceMapGenerator.php | 40 ++++++++++++++++--- 1 file changed, 35 insertions(+), 5 deletions(-) diff --git a/src/applications/celerity/CelerityResourceMapGenerator.php b/src/applications/celerity/CelerityResourceMapGenerator.php index a5bb657bd9..91d0193ade 100644 --- a/src/applications/celerity/CelerityResourceMapGenerator.php +++ b/src/applications/celerity/CelerityResourceMapGenerator.php @@ -232,11 +232,8 @@ EOFILE; list($description, $metadata) = $parser->parse($matches[0]); - $provides = preg_split('/\s+/', trim(idx($metadata, 'provides'))); - $requires = preg_split('/\s+/', trim(idx($metadata, 'requires'))); - $provides = array_filter($provides); - $requires = array_filter($requires); - + $provides = $this->parseResourceSymbolList(idx($metadata, 'provides')); + $requires = $this->parseResourceSymbolList(idx($metadata, 'requires')); if (!$provides) { // Tests and documentation-only JS is permitted to @provide no targets. return array(null, null); @@ -364,4 +361,37 @@ EOFILE; return $result; } + private function parseResourceSymbolList($list) { + if (!$list) { + return array(); + } + + // This is valid: + // + // @requires x y + // + // But so is this: + // + // @requires x + // @requires y + // + // Accept either form and produce a list of symbols. + + $list = (array)$list; + + // We can get `true` values if there was a bare `@requires` in the input. + foreach ($list as $key => $item) { + if ($item === true) { + unset($list[$key]); + } + } + + $list = implode(' ', $list); + $list = trim($list); + $list = preg_split('/\s+/', $list); + $list = array_filter($list); + + return $list; + } + } From d26cca27d70b69080c10e1df92a10419fffbfc7e Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Tue, 23 Aug 2016 02:41:20 -0500 Subject: [PATCH 04/27] Removing deprecated method calls Summary: Removed call to the deprecated buildStandardPageResponse method from XHProfProfileController Test Plan: Install, configure, and use XHProf. I'll need some guidance with this Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16432 --- .../PhabricatorXHProfController.php | 22 ------------------- .../PhabricatorXHProfProfileController.php | 15 ++++++++----- src/view/page/PhabricatorStandardPageView.php | 3 ++- 3 files changed, 11 insertions(+), 29 deletions(-) diff --git a/src/applications/xhprof/controller/PhabricatorXHProfController.php b/src/applications/xhprof/controller/PhabricatorXHProfController.php index 6dda92560c..eb0a8f050b 100644 --- a/src/applications/xhprof/controller/PhabricatorXHProfController.php +++ b/src/applications/xhprof/controller/PhabricatorXHProfController.php @@ -2,26 +2,4 @@ abstract class PhabricatorXHProfController extends PhabricatorController { - public function buildStandardPageResponse($view, array $data) { - $page = $this->buildStandardPageView(); - - $page->setApplicationName(pht('XHProf')); - $page->setBaseURI('/xhprof/'); - $page->setTitle(idx($data, 'title')); - $page->setGlyph("\xE2\x98\x84"); - $page->appendChild($view); - $page->setDeviceReady(true); - - $response = new AphrontWebpageResponse(); - - if (isset($data['frame'])) { - $response->setFrameable(true); - $page->setFrameable(true); - $page->setShowChrome(false); - $page->setDisableConsole(true); - } - - return $response->setContent($page->render()); - } - } diff --git a/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php b/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php index 85815d8589..16744a1ca0 100644 --- a/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php +++ b/src/applications/xhprof/controller/PhabricatorXHProfProfileController.php @@ -47,11 +47,14 @@ final class PhabricatorXHProfProfileController $crumbs = $this->buildApplicationCrumbs(); $crumbs->addTextCrumb(pht('%s Profile', $symbol)); - return $this->buildStandardPageResponse( - array($crumbs, $view), - array( - 'title' => pht('Profile'), - 'frame' => $is_framed, - )); + $title = pht('Profile'); + + return $this->newPage() + ->setTitle($title) + ->setCrumbs($crumbs) + ->setFrameable(true) + ->setShowChrome(false) + ->setDisableConsole(true) + ->appendChild($view); } } diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index 866268c710..b668a4883b 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -890,7 +890,8 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView } else { $content = $this->render(); $response = id(new AphrontWebpageResponse()) - ->setContent($content); + ->setContent($content) + ->setFrameable($this->getFrameable()); } return $response; From 3c62be695653772c3d47a7abc385f03834883aa9 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Tue, 23 Aug 2016 03:40:45 -0500 Subject: [PATCH 05/27] Add patch to remove conduit_connectionlog table (Fixes T9982) Summary: Adds a schema patch that removes conduit_connectionlog. This table hasn't been used in 8ish months so it's probably safe to get rid of. Test Plan: Apply the patch locally and confirm that the table does indeed get dropped. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Differential Revision: https://secure.phabricator.com/D16438 --- .../autopatches/20160824.connectionlog.sql | 1 + src/__phutil_library_map__.php | 2 -- .../PhabricatorConduitConnectionLog.php | 26 ------------------- 3 files changed, 1 insertion(+), 28 deletions(-) create mode 100644 resources/sql/autopatches/20160824.connectionlog.sql delete mode 100644 src/applications/conduit/storage/PhabricatorConduitConnectionLog.php diff --git a/resources/sql/autopatches/20160824.connectionlog.sql b/resources/sql/autopatches/20160824.connectionlog.sql new file mode 100644 index 0000000000..499fc71845 --- /dev/null +++ b/resources/sql/autopatches/20160824.connectionlog.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_conduit.conduit_connectionlog; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index a6ab763246..e32c3d2c58 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -2114,7 +2114,6 @@ phutil_register_library_map(array( 'PhabricatorConduitAPIController' => 'applications/conduit/controller/PhabricatorConduitAPIController.php', 'PhabricatorConduitApplication' => 'applications/conduit/application/PhabricatorConduitApplication.php', 'PhabricatorConduitCertificateToken' => 'applications/conduit/storage/PhabricatorConduitCertificateToken.php', - 'PhabricatorConduitConnectionLog' => 'applications/conduit/storage/PhabricatorConduitConnectionLog.php', 'PhabricatorConduitConsoleController' => 'applications/conduit/controller/PhabricatorConduitConsoleController.php', 'PhabricatorConduitContentSource' => 'infrastructure/contentsource/PhabricatorConduitContentSource.php', 'PhabricatorConduitController' => 'applications/conduit/controller/PhabricatorConduitController.php', @@ -6846,7 +6845,6 @@ phutil_register_library_map(array( 'PhabricatorConduitAPIController' => 'PhabricatorConduitController', 'PhabricatorConduitApplication' => 'PhabricatorApplication', 'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO', - 'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO', 'PhabricatorConduitConsoleController' => 'PhabricatorConduitController', 'PhabricatorConduitContentSource' => 'PhabricatorContentSource', 'PhabricatorConduitController' => 'PhabricatorController', diff --git a/src/applications/conduit/storage/PhabricatorConduitConnectionLog.php b/src/applications/conduit/storage/PhabricatorConduitConnectionLog.php deleted file mode 100644 index 498bb6ecfa..0000000000 --- a/src/applications/conduit/storage/PhabricatorConduitConnectionLog.php +++ /dev/null @@ -1,26 +0,0 @@ - array( - 'client' => 'text255?', - 'clientVersion' => 'text255?', - 'clientDescription' => 'text255?', - 'username' => 'text255?', - ), - self::CONFIG_KEY_SCHEMA => array( - 'key_created' => array( - 'columns' => array('dateCreated'), - ), - ), - ) + parent::getConfiguration(); - } - -} From 2201c65eb73fb99b8625bea45c273d262f2c289f Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Tue, 23 Aug 2016 03:52:57 -0500 Subject: [PATCH 06/27] Removed unused buildApplicationPage method from PhabricatorController Summary: Getting rid of some code! This method has no callsites so it should be safe to remove completely. Ref T9690 Test Plan: Removed method and clicked around to make sure nothing broke. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: yelirekim, epriestley Maniphest Tasks: T9690 Differential Revision: https://secure.phabricator.com/D16439 --- .../base/controller/PhabricatorController.php | 40 ------------------- 1 file changed, 40 deletions(-) diff --git a/src/applications/base/controller/PhabricatorController.php b/src/applications/base/controller/PhabricatorController.php index 0cb8182776..f2b267b67f 100644 --- a/src/applications/base/controller/PhabricatorController.php +++ b/src/applications/base/controller/PhabricatorController.php @@ -575,44 +575,4 @@ abstract class PhabricatorController extends AphrontController { return $page->produceAphrontResponse(); } - - /** - * DEPRECATED. Use @{method:newPage}. - */ - public function buildApplicationPage($view, array $options) { - $page = $this->newPage(); - - $title = PhabricatorEnv::getEnvConfig('phabricator.serious-business') ? - 'Phabricator' : - pht('Bacon Ice Cream for Breakfast'); - - $page->setTitle(idx($options, 'title', $title)); - - if (idx($options, 'class')) { - $page->addClass($options['class']); - } - - if (!($view instanceof AphrontSideNavFilterView)) { - $nav = new AphrontSideNavFilterView(); - $nav->appendChild($view); - $view = $nav; - } - - $page->appendChild($view); - - $object_phids = idx($options, 'pageObjects', array()); - if ($object_phids) { - $page->setPageObjectPHIDs($object_phids); - } - - if (!idx($options, 'device', true)) { - $page->setDeviceReady(false); - } - - $page->setShowFooter(idx($options, 'showFooter', true)); - $page->setShowChrome(idx($options, 'chrome', true)); - - return $page->produceAphrontResponse(); - } - } From 8a4fbcd8c01522e49639d24114d42342dec00b6d Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Aug 2016 04:58:22 -0700 Subject: [PATCH 07/27] Provide a new "hint" table for weird commits (rewritten, unreadable) Summary: Ref T11522. This provides storage for tracking rewritten commits (new feature) and unreadable commits (existing feature, but really hacky). This doesn't do anything yet, just adds a table and a CLI tool for updating it. I'll document the tool once it works. You just pipe in some JSON, but I need to document the format. Test Plan: - Piped JSON for "none", "rewritten" and "unreadable" hints into `bin/repository hint`. - Examined the database to see that the table was written properly. - Tried to pipe bad JSON in, invalid hint types, etc. Got reasonable human-readable error messages. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11522 Differential Revision: https://secure.phabricator.com/D16434 --- .../autopatches/20160824.repohint.01.hint.sql | 8 ++ src/__phutil_library_map__.php | 9 ++ .../query/DiffusionCommitHintQuery.php | 64 +++++++++ ...icatorRepositoryManagementHintWorkflow.php | 97 +++++++++++++ .../PhabricatorRepositoryCommitHint.php | 128 ++++++++++++++++++ 5 files changed, 306 insertions(+) create mode 100644 resources/sql/autopatches/20160824.repohint.01.hint.sql create mode 100644 src/applications/diffusion/query/DiffusionCommitHintQuery.php create mode 100644 src/applications/repository/management/PhabricatorRepositoryManagementHintWorkflow.php create mode 100644 src/applications/repository/storage/PhabricatorRepositoryCommitHint.php diff --git a/resources/sql/autopatches/20160824.repohint.01.hint.sql b/resources/sql/autopatches/20160824.repohint.01.hint.sql new file mode 100644 index 0000000000..f29f2d1c5d --- /dev/null +++ b/resources/sql/autopatches/20160824.repohint.01.hint.sql @@ -0,0 +1,8 @@ +CREATE TABLE {$NAMESPACE}_repository.repository_commithint ( + id INT UNSIGNED NOT NULL AUTO_INCREMENT PRIMARY KEY, + repositoryPHID VARBINARY(64) NOT NULL, + oldCommitIdentifier VARCHAR(40) NOT NULL COLLATE {$COLLATE_TEXT}, + newCommitIdentifier VARCHAR(40) COLLATE {$COLLATE_TEXT}, + hintType VARCHAR(32) NOT NULL COLLATE {$COLLATE_TEXT}, + UNIQUE KEY `key_old` (repositoryPHID, oldCommitIdentifier) +) ENGINE=InnoDB, COLLATE {$COLLATE_TEXT}; diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index e32c3d2c58..dbe5519547 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -612,6 +612,7 @@ phutil_register_library_map(array( 'DiffusionCommitHash' => 'applications/diffusion/data/DiffusionCommitHash.php', 'DiffusionCommitHeraldField' => 'applications/diffusion/herald/DiffusionCommitHeraldField.php', 'DiffusionCommitHeraldFieldGroup' => 'applications/diffusion/herald/DiffusionCommitHeraldFieldGroup.php', + 'DiffusionCommitHintQuery' => 'applications/diffusion/query/DiffusionCommitHintQuery.php', 'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php', 'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php', 'DiffusionCommitMergeHeraldField' => 'applications/diffusion/herald/DiffusionCommitMergeHeraldField.php', @@ -3383,6 +3384,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryCommitChangeParserWorker' => 'applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php', 'PhabricatorRepositoryCommitData' => 'applications/repository/storage/PhabricatorRepositoryCommitData.php', 'PhabricatorRepositoryCommitHeraldWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitHeraldWorker.php', + 'PhabricatorRepositoryCommitHint' => 'applications/repository/storage/PhabricatorRepositoryCommitHint.php', 'PhabricatorRepositoryCommitMessageParserWorker' => 'applications/repository/worker/commitmessageparser/PhabricatorRepositoryCommitMessageParserWorker.php', 'PhabricatorRepositoryCommitOwnersWorker' => 'applications/repository/worker/PhabricatorRepositoryCommitOwnersWorker.php', 'PhabricatorRepositoryCommitPHIDType' => 'applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php', @@ -3403,6 +3405,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryManagementCacheWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementCacheWorkflow.php', 'PhabricatorRepositoryManagementClusterizeWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementClusterizeWorkflow.php', 'PhabricatorRepositoryManagementDiscoverWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementDiscoverWorkflow.php', + 'PhabricatorRepositoryManagementHintWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementHintWorkflow.php', 'PhabricatorRepositoryManagementImportingWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementImportingWorkflow.php', 'PhabricatorRepositoryManagementListPathsWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListPathsWorkflow.php', 'PhabricatorRepositoryManagementListWorkflow' => 'applications/repository/management/PhabricatorRepositoryManagementListWorkflow.php', @@ -5104,6 +5107,7 @@ phutil_register_library_map(array( 'DiffusionCommitHash' => 'Phobject', 'DiffusionCommitHeraldField' => 'HeraldField', 'DiffusionCommitHeraldFieldGroup' => 'HeraldFieldGroup', + 'DiffusionCommitHintQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DiffusionCommitHookEngine' => 'Phobject', 'DiffusionCommitHookRejectException' => 'Exception', 'DiffusionCommitMergeHeraldField' => 'DiffusionCommitHeraldField', @@ -8348,6 +8352,10 @@ phutil_register_library_map(array( 'PhabricatorRepositoryCommitChangeParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitData' => 'PhabricatorRepositoryDAO', 'PhabricatorRepositoryCommitHeraldWorker' => 'PhabricatorRepositoryCommitParserWorker', + 'PhabricatorRepositoryCommitHint' => array( + 'PhabricatorRepositoryDAO', + 'PhabricatorPolicyInterface', + ), 'PhabricatorRepositoryCommitMessageParserWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitOwnersWorker' => 'PhabricatorRepositoryCommitParserWorker', 'PhabricatorRepositoryCommitPHIDType' => 'PhabricatorPHIDType', @@ -8372,6 +8380,7 @@ phutil_register_library_map(array( 'PhabricatorRepositoryManagementCacheWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementClusterizeWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementDiscoverWorkflow' => 'PhabricatorRepositoryManagementWorkflow', + 'PhabricatorRepositoryManagementHintWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementImportingWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementListPathsWorkflow' => 'PhabricatorRepositoryManagementWorkflow', 'PhabricatorRepositoryManagementListWorkflow' => 'PhabricatorRepositoryManagementWorkflow', diff --git a/src/applications/diffusion/query/DiffusionCommitHintQuery.php b/src/applications/diffusion/query/DiffusionCommitHintQuery.php new file mode 100644 index 0000000000..34d7bb4e6c --- /dev/null +++ b/src/applications/diffusion/query/DiffusionCommitHintQuery.php @@ -0,0 +1,64 @@ +ids = $ids; + return $this; + } + + public function withRepositoryPHIDs(array $phids) { + $this->repositoryPHIDs = $phids; + return $this; + } + + public function withOldCommitIdentifiers(array $identifiers) { + $this->oldCommitIdentifiers = $identifiers; + return $this; + } + + public function newResultObject() { + return new PhabricatorRepositoryCommitHint(); + } + + protected function loadPage() { + return $this->loadStandardPage($this->newResultObject()); + } + + protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) { + $where = parent::buildWhereClauseParts($conn); + + if ($this->ids !== null) { + $where[] = qsprintf( + $conn, + 'id IN (%Ld)', + $this->ids); + } + + if ($this->repositoryPHIDs !== null) { + $where[] = qsprintf( + $conn, + 'reositoryPHID IN (%Ls)', + $this->repositoryPHIDs); + } + + if ($this->oldCommitIdentifiers !== null) { + $where[] = qsprintf( + $conn, + 'oldCommitIdentifier IN (%Ls)', + $this->oldCommitIdentifiers); + } + + return $where; + } + + public function getQueryApplicationClass() { + return 'PhabricatorDiffusionApplication'; + } + +} diff --git a/src/applications/repository/management/PhabricatorRepositoryManagementHintWorkflow.php b/src/applications/repository/management/PhabricatorRepositoryManagementHintWorkflow.php new file mode 100644 index 0000000000..a56d9a7f4e --- /dev/null +++ b/src/applications/repository/management/PhabricatorRepositoryManagementHintWorkflow.php @@ -0,0 +1,97 @@ +setName('hint') + ->setExamples('**hint** [options] ...') + ->setSynopsis( + pht( + 'Write hints about unusual (rewritten or unreadable) commits.')) + ->setArguments(array()); + } + + public function execute(PhutilArgumentParser $args) { + $viewer = $this->getViewer(); + + echo tsprintf( + "%s\n", + pht('Reading list of hints from stdin...')); + + $hints = file_get_contents('php://stdin'); + if ($hints === false) { + throw new PhutilArgumentUsageException(pht('Failed to read stdin.')); + } + + try { + $hints = phutil_json_decode($hints); + } catch (Exception $ex) { + throw new PhutilArgumentUsageException( + pht( + 'Expected a list of hints in JSON format: %s', + $ex->getMessage())); + } + + $repositories = array(); + foreach ($hints as $idx => $hint) { + if (!is_array($hint)) { + throw new PhutilArgumentUsageException( + pht( + 'Each item in the list of hints should be a JSON object, but '. + 'the item at index "%s" is not.', + $idx)); + } + + try { + PhutilTypeSpec::checkMap( + $hint, + array( + 'repository' => 'string|int', + 'old' => 'string', + 'new' => 'optional string|null', + 'hint' => 'string', + )); + } catch (Exception $ex) { + throw new PhutilArgumentUsageException( + pht( + 'Unexpected hint format at index "%s": %s', + $idx, + $ex->getMessage())); + } + + $repository_identifier = $hint['repository']; + $repository = idx($repositories, $repository_identifier); + if (!$repository) { + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->withIdentifiers(array($repository_identifier)) + ->executeOne(); + if (!$repository) { + throw new PhutilArgumentUsageException( + pht( + 'Repository identifier "%s" (in hint at index "%s") does not '. + 'identify a valid repository.', + $repository_identifier, + $idx)); + } + + $repositories[$repository_identifier] = $repository; + } + + PhabricatorRepositoryCommitHint::updateHint( + $repository->getPHID(), + $hint['old'], + idx($hint, 'new'), + $hint['hint']); + + echo tsprintf( + "%s\n", + pht( + 'Updated hint for "%s".', + $hint['old'])); + } + } + +} diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php new file mode 100644 index 0000000000..4523617038 --- /dev/null +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php @@ -0,0 +1,128 @@ + false, + self::CONFIG_COLUMN_SCHEMA => array( + 'oldCommitIdentifier' => 'text40', + 'newCommitIdentifier' => 'text40?', + 'hintType' => 'text32', + ), + self::CONFIG_KEY_SCHEMA => array( + 'key_old' => array( + 'columns' => array('repositoryPHID', 'oldCommitIdentifier'), + 'unique' => true, + ), + ), + ) + parent::getConfiguration(); + } + + public static function getAllHintTypes() { + return array( + self::HINT_NONE, + self::HINT_REWRITTEN, + self::HINT_UNREADABLE, + ); + } + + public static function updateHint($repository_phid, $old, $new, $type) { + switch ($type) { + case self::HINT_NONE: + break; + case self::HINT_REWRITTEN: + if (!$new) { + throw new Exception( + pht( + 'When hinting a commit ("%s") as rewritten, you must provide '. + 'the commit it was rewritten into.', + $old)); + } + break; + case self::HINT_UNREADABLE: + if ($new) { + throw new Exception( + pht( + 'When hinting a commit ("%s") as unreadable, you must not '. + 'provide a new commit ("%s").', + $old, + $new)); + } + break; + default: + $all_types = self::getAllHintTypes(); + throw new Exception( + pht( + 'Hint type ("%s") for commit ("%s") is not valid. Valid hints '. + 'are: %s.', + $type, + $old, + implode(', ', $all_types))); + } + + $table = new self(); + $table_name = $table->getTableName(); + $conn = $table->establishConnection('w'); + + if ($type == self::HINT_NONE) { + queryfx( + $conn, + 'DELETE FROM %T WHERE repositoryPHID = %s AND oldCommitIdentifier = %s', + $table_name, + $repository_phid, + $old); + } else { + queryfx( + $conn, + 'INSERT INTO %T + (repositoryPHID, oldCommitIdentifier, newCommitIdentifier, hintType) + VALUES (%s, %s, %ns, %s) + ON DUPLICATE KEY UPDATE + newCommitIdentifier = VALUES(newCommitIdentifier), + hintType = VALUES(hintType)', + $table_name, + $repository_phid, + $old, + $new, + $type); + } + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + switch ($capability) { + case PhabricatorPolicyCapability::CAN_VIEW: + return PhabricatorPolicies::getMostOpenPolicy(); + } + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + + public function describeAutomaticCapability($capability) { + return null; + } + +} From e4c4724afd6b8b1c261f429616b049c842a10ab5 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Aug 2016 05:24:07 -0700 Subject: [PATCH 08/27] Migrate the "badcommit" table to use the less-hacky "hint" mechanism Summary: Ref T11522. This migrates any "badcommit" data (which probably only exists at Facebook and on 1-2 other installs in the wild) to the new "hint" table. Test Plan: - Wrote some bad commit annotations to the badcommit table. - Viewed them in the web UI and used `bin/repository reparse --change ...` to reparse them. Saw "this is bad" messages. - Ran migration, verified that valid "badcommit" rows were successfully migrated to become "hint" rows. - Viewed the new web UI and re-parsed the change, saw "unreadable commit" messages. - Viewed a good commit; reparsed a good commit. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11522 Differential Revision: https://secure.phabricator.com/D16435 --- .../20160824.repohint.02.movebad.php | 39 +++++++++++++++++++ .../20160824.repohint.03.nukebad.sql | 1 + .../controller/DiffusionCommitController.php | 26 ++++++++----- .../query/DiffusionCommitHintQuery.php | 2 +- .../storage/PhabricatorRepository.php | 1 - .../PhabricatorRepositoryCommitHint.php | 5 +++ .../PhabricatorRepositorySchemaSpec.php | 14 ------- ...habricatorRepositoryCommitParserWorker.php | 16 ++++---- ...atorRepositoryCommitChangeParserWorker.php | 9 ++++- 9 files changed, 77 insertions(+), 36 deletions(-) create mode 100644 resources/sql/autopatches/20160824.repohint.02.movebad.php create mode 100644 resources/sql/autopatches/20160824.repohint.03.nukebad.sql diff --git a/resources/sql/autopatches/20160824.repohint.02.movebad.php b/resources/sql/autopatches/20160824.repohint.02.movebad.php new file mode 100644 index 0000000000..4127892f73 --- /dev/null +++ b/resources/sql/autopatches/20160824.repohint.02.movebad.php @@ -0,0 +1,39 @@ +establishConnection('w'); + +$rows = queryfx_all( + $conn, + 'SELECT fullCommitName FROM repository_badcommit'); + +$viewer = PhabricatorUser::getOmnipotentUser(); + +foreach ($rows as $row) { + $identifier = $row['fullCommitName']; + + $commit = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withIdentifiers(array($identifier)) + ->executeOne(); + + if (!$commit) { + echo tsprintf( + "%s\n", + pht( + 'Skipped hint for "%s", this is not a valid commit.', + $identifier)); + } else { + PhabricatorRepositoryCommitHint::updateHint( + $commit->getRepository()->getPHID(), + $commit->getCommitIdentifier(), + null, + PhabricatorRepositoryCommitHint::HINT_UNREADABLE); + + echo tsprintf( + "%s\n", + pht( + 'Updated commit hint for "%s".', + $identifier)); + } +} diff --git a/resources/sql/autopatches/20160824.repohint.03.nukebad.sql b/resources/sql/autopatches/20160824.repohint.03.nukebad.sql new file mode 100644 index 0000000000..88364aeef3 --- /dev/null +++ b/resources/sql/autopatches/20160824.repohint.03.nukebad.sql @@ -0,0 +1 @@ +DROP TABLE {$NAMESPACE}_repository.repository_badcommit; diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 1ee2ccedad..1346955f80 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -167,23 +167,29 @@ final class DiffusionCommitController extends DiffusionController { $count = count($changes); - $bad_commit = null; - if ($count == 0) { - $bad_commit = queryfx_one( - id(new PhabricatorRepository())->establishConnection('r'), - 'SELECT * FROM %T WHERE fullCommitName = %s', - PhabricatorRepository::TABLE_BADCOMMIT, - $commit->getMonogram()); + $is_unreadable = false; + if (!$count) { + $hint = id(new DiffusionCommitHintQuery()) + ->setViewer($viewer) + ->withRepositoryPHIDs(array($repository->getPHID())) + ->withOldCommitIdentifiers(array($commit->getCommitIdentifier())) + ->executeOne(); + if ($hint) { + $is_unreadable = $hint->isUnreadable(); + } } $show_changesets = false; $info_panel = null; $change_list = null; $change_table = null; - if ($bad_commit) { + if ($is_unreadable) { $info_panel = $this->renderStatusMessage( - pht('Bad Commit'), - $bad_commit['description']); + pht('Unreadable Commit'), + pht( + 'This commit has been marked as unreadable by an administrator. '. + 'It may have been corrupted or created improperly by an external '. + 'tool.')); } else if ($is_foreign) { // Don't render anything else. } else if (!$commit->isImported()) { diff --git a/src/applications/diffusion/query/DiffusionCommitHintQuery.php b/src/applications/diffusion/query/DiffusionCommitHintQuery.php index 34d7bb4e6c..28ae1ed709 100644 --- a/src/applications/diffusion/query/DiffusionCommitHintQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitHintQuery.php @@ -43,7 +43,7 @@ final class DiffusionCommitHintQuery if ($this->repositoryPHIDs !== null) { $where[] = qsprintf( $conn, - 'reositoryPHID IN (%Ls)', + 'repositoryPHID IN (%Ls)', $this->repositoryPHIDs); } diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 654ea42c3e..3e59c7c53d 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -35,7 +35,6 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO const TABLE_PATHCHANGE = 'repository_pathchange'; const TABLE_FILESYSTEM = 'repository_filesystem'; const TABLE_SUMMARY = 'repository_summary'; - const TABLE_BADCOMMIT = 'repository_badcommit'; const TABLE_LINTMESSAGE = 'repository_lintmessage'; const TABLE_PARENTS = 'repository_parents'; const TABLE_COVERAGE = 'repository_coverage'; diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php index 4523617038..9ab8d2d056 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php @@ -101,6 +101,11 @@ final class PhabricatorRepositoryCommitHint } + public function isUnreadable() { + return ($this->getHintType() == self::HINT_UNREADABLE); + } + + /* -( PhabricatorPolicyInterface )----------------------------------------- */ diff --git a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php index 8ff2ed0c9b..8f93e31c29 100644 --- a/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php +++ b/src/applications/repository/storage/PhabricatorRepositorySchemaSpec.php @@ -6,20 +6,6 @@ final class PhabricatorRepositorySchemaSpec public function buildSchemata() { $this->buildEdgeSchemata(new PhabricatorRepository()); - $this->buildRawSchema( - id(new PhabricatorRepository())->getApplicationName(), - PhabricatorRepository::TABLE_BADCOMMIT, - array( - 'fullCommitName' => 'text64', - 'description' => 'text', - ), - array( - 'PRIMARY' => array( - 'columns' => array('fullCommitName'), - 'unique' => true, - ), - )); - $this->buildRawSchema( id(new PhabricatorRepository())->getApplicationName(), PhabricatorRepository::TABLE_COVERAGE, diff --git a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php index b565d30c55..9d3b2793f8 100644 --- a/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php +++ b/src/applications/repository/worker/PhabricatorRepositoryCommitParserWorker.php @@ -95,16 +95,16 @@ abstract class PhabricatorRepositoryCommitParserWorker PhabricatorRepository $repository, PhabricatorRepositoryCommit $commit); - protected function isBadCommit(PhabricatorRepositoryCommit $commit) { - $repository = new PhabricatorRepository(); + protected function loadCommitHint(PhabricatorRepositoryCommit $commit) { + $viewer = PhabricatorUser::getOmnipotentUser(); - $bad_commit = queryfx_one( - $repository->establishConnection('w'), - 'SELECT * FROM %T WHERE fullCommitName = %s', - PhabricatorRepository::TABLE_BADCOMMIT, - $commit->getMonogram()); + $repository = $commit->getRepository(); - return (bool)$bad_commit; + return id(new DiffusionCommitHintQuery()) + ->setViewer($viewer) + ->withRepositoryPHIDs(array($repository->getPHID())) + ->withOldCommitIdentifiers(array($commit->getCommitIdentifier())) + ->executeOne(); } public function renderForDisplay(PhabricatorUser $viewer) { diff --git a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php index f58856d614..6d79742a32 100644 --- a/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php +++ b/src/applications/repository/worker/commitchangeparser/PhabricatorRepositoryCommitChangeParserWorker.php @@ -22,8 +22,13 @@ abstract class PhabricatorRepositoryCommitChangeParserWorker PhabricatorRepositoryCommit $commit) { $this->log("%s\n", pht('Parsing "%s"...', $commit->getMonogram())); - if ($this->isBadCommit($commit)) { - $this->log(pht('This commit is marked bad!')); + + $hint = $this->loadCommitHint($commit); + if ($hint && $hint->isUnreadable()) { + $this->log( + pht( + 'This commit is marked as unreadable, so changes will not be '. + 'parsed.')); return; } From 498fb331038cb286fceb32ab8010b398e423b818 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Aug 2016 05:55:47 -0700 Subject: [PATCH 09/27] When a commit has a "rewritten" hint, show it in the UI instead of the generic "deleted" message Summary: Ref T11522. When a commit is no longer reachable from any branch/tag, we currently show a "this has been deleted" message. Instead, go further: check if there is a "rewritten" hint pointing at a commit the current commit was rewritten into. If we find one, show a message about that instead. (This isn't super pretty, just getting it working for now. I expect to revisit this UI in T9713 if we don't get to it before that.) Test Plan: {F1780703} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11522 Differential Revision: https://secure.phabricator.com/D16436 --- .../controller/DiffusionCommitController.php | 100 ++++++++++++------ .../PhabricatorRepositoryCommitHint.php | 4 + 2 files changed, 71 insertions(+), 33 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionCommitController.php b/src/applications/diffusion/controller/DiffusionCommitController.php index 1346955f80..c27006350f 100644 --- a/src/applications/diffusion/controller/DiffusionCommitController.php +++ b/src/applications/diffusion/controller/DiffusionCommitController.php @@ -73,6 +73,38 @@ final class DiffusionCommitController extends DiffusionController { $commit_data = $commit->getCommitData(); $is_foreign = $commit_data->getCommitDetail('foreign-svn-stub'); $error_panel = null; + + $hard_limit = 1000; + + if ($commit->isImported()) { + $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( + $drequest); + $change_query->setLimit($hard_limit + 1); + $changes = $change_query->loadChanges(); + } else { + $changes = array(); + } + + $was_limited = (count($changes) > $hard_limit); + if ($was_limited) { + $changes = array_slice($changes, 0, $hard_limit); + } + + $count = count($changes); + + $is_unreadable = false; + $hint = null; + if (!$count || $commit->isUnreachable()) { + $hint = id(new DiffusionCommitHintQuery()) + ->setViewer($viewer) + ->withRepositoryPHIDs(array($repository->getPHID())) + ->withOldCommitIdentifiers(array($commit->getCommitIdentifier())) + ->executeOne(); + if ($hint) { + $is_unreadable = $hint->isUnreadable(); + } + } + if ($is_foreign) { $subpath = $commit_data->getCommitDetail('svn-subpath'); @@ -130,9 +162,41 @@ final class DiffusionCommitController extends DiffusionController { $message)); if ($commit->isUnreachable()) { - $this->commitErrors[] = pht( - 'This commit has been deleted in the repository: it is no longer '. - 'reachable from any branch, tag, or ref.'); + $did_rewrite = false; + if ($hint) { + if ($hint->isRewritten()) { + $rewritten = id(new DiffusionCommitQuery()) + ->setViewer($viewer) + ->withRepository($repository) + ->withIdentifiers(array($hint->getNewCommitIdentifier())) + ->executeOne(); + if ($rewritten) { + $did_rewrite = true; + $rewritten_uri = $rewritten->getURI(); + $rewritten_name = $rewritten->getLocalName(); + + $rewritten_link = phutil_tag( + 'a', + array( + 'href' => $rewritten_uri, + ), + $rewritten_name); + + $this->commitErrors[] = pht( + 'This commit was rewritten after it was published, which '. + 'changed the commit hash. This old version of the commit is '. + 'no longer reachable from any branch, tag or ref. The new '. + 'version of this commit is %s.', + $rewritten_link); + } + } + } + + if (!$did_rewrite) { + $this->commitErrors[] = pht( + 'This commit has been deleted in the repository: it is no longer '. + 'reachable from any branch, tag, or ref.'); + } } if ($this->getCommitErrors()) { @@ -143,42 +207,12 @@ final class DiffusionCommitController extends DiffusionController { } $timeline = $this->buildComments($commit); - $hard_limit = 1000; - - if ($commit->isImported()) { - $change_query = DiffusionPathChangeQuery::newFromDiffusionRequest( - $drequest); - $change_query->setLimit($hard_limit + 1); - $changes = $change_query->loadChanges(); - } else { - $changes = array(); - } - - $was_limited = (count($changes) > $hard_limit); - if ($was_limited) { - $changes = array_slice($changes, 0, $hard_limit); - } - $merge_table = $this->buildMergesTable($commit); $highlighted_audits = $commit->getAuthorityAudits( $viewer, $this->auditAuthorityPHIDs); - $count = count($changes); - - $is_unreadable = false; - if (!$count) { - $hint = id(new DiffusionCommitHintQuery()) - ->setViewer($viewer) - ->withRepositoryPHIDs(array($repository->getPHID())) - ->withOldCommitIdentifiers(array($commit->getCommitIdentifier())) - ->executeOne(); - if ($hint) { - $is_unreadable = $hint->isUnreadable(); - } - } - $show_changesets = false; $info_panel = null; $change_list = null; diff --git a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php index 9ab8d2d056..e8594a1496 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php +++ b/src/applications/repository/storage/PhabricatorRepositoryCommitHint.php @@ -105,6 +105,10 @@ final class PhabricatorRepositoryCommitHint return ($this->getHintType() == self::HINT_UNREADABLE); } + public function isRewritten() { + return ($this->getHintType() == self::HINT_REWRITTEN); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */ From be235301d0dc0d986f8e89766f8f46b046d105c3 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Aug 2016 06:24:05 -0700 Subject: [PATCH 10/27] When commits have a "rewritten" hint, try to show that in handles in other applications Summary: Ref T11522. This tries to reduce the cost of rewriting a repository by making handles smarter about rewritten commits. When a handle references an unreachable commit, try to load a rewrite hint for the commit. If we find one, change the handle name to "OldHash > NewHash" to provide a strong hint that the commit was rewritten and that copy/pasting the old hash (say, to the CLI) won't work. I think this notation isn't totally self-evident, but users can click it to see the big error message on the page, and it's at least obvious that something weird is going on, which I think is the important part. Some possible future work: - Not sure this ("Recycling Symbol") is the best symbol? Seems sort of reasonable but mabye there's a better one. - Putting this information directly on the hovercard could help explain what this means. Test Plan: {F1780719} Reviewers: chad Reviewed By: chad Maniphest Tasks: T11522 Differential Revision: https://secure.phabricator.com/D16437 --- .../query/DiffusionCommitHintQuery.php | 52 +++++++++++++++++++ .../remarkup/DiffusionCommitRemarkupRule.php | 19 +++++++ .../PhabricatorRepositoryCommitPHIDType.php | 39 ++++++++++++-- .../rule/PhabricatorObjectRemarkupRule.php | 11 +++- 4 files changed, 115 insertions(+), 6 deletions(-) diff --git a/src/applications/diffusion/query/DiffusionCommitHintQuery.php b/src/applications/diffusion/query/DiffusionCommitHintQuery.php index 28ae1ed709..bfd8045131 100644 --- a/src/applications/diffusion/query/DiffusionCommitHintQuery.php +++ b/src/applications/diffusion/query/DiffusionCommitHintQuery.php @@ -7,6 +7,9 @@ final class DiffusionCommitHintQuery private $repositoryPHIDs; private $oldCommitIdentifiers; + private $commits; + private $commitMap; + public function withIDs(array $ids) { $this->ids = $ids; return $this; @@ -22,10 +25,37 @@ final class DiffusionCommitHintQuery return $this; } + public function withCommits(array $commits) { + assert_instances_of($commits, 'PhabricatorRepositoryCommit'); + + $repository_phids = array(); + foreach ($commits as $commit) { + $repository_phids[] = $commit->getRepository()->getPHID(); + } + + $this->repositoryPHIDs = $repository_phids; + $this->oldCommitIdentifiers = mpull($commits, 'getCommitIdentifier'); + $this->commits = $commits; + + return $this; + } + + public function getCommitMap() { + if ($this->commitMap === null) { + throw new PhutilInvalidStateException('execute'); + } + + return $this->commitMap; + } + public function newResultObject() { return new PhabricatorRepositoryCommitHint(); } + protected function willExecute() { + $this->commitMap = array(); + } + protected function loadPage() { return $this->loadStandardPage($this->newResultObject()); } @@ -57,6 +87,28 @@ final class DiffusionCommitHintQuery return $where; } + protected function didFilterPage(array $hints) { + if ($this->commits) { + $map = array(); + foreach ($this->commits as $commit) { + $repository_phid = $commit->getRepository()->getPHID(); + $identifier = $commit->getCommitIdentifier(); + $map[$repository_phid][$identifier] = $commit->getPHID(); + } + + foreach ($hints as $hint) { + $repository_phid = $hint->getRepositoryPHID(); + $identifier = $hint->getOldCommitIdentifier(); + if (isset($map[$repository_phid][$identifier])) { + $commit_phid = $map[$repository_phid][$identifier]; + $this->commitMap[$commit_phid] = $hint; + } + } + } + + return $hints; + } + public function getQueryApplicationClass() { return 'PhabricatorDiffusionApplication'; } diff --git a/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php b/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php index 2722e9d819..4d2d130c32 100644 --- a/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php +++ b/src/applications/diffusion/remarkup/DiffusionCommitRemarkupRule.php @@ -14,6 +14,25 @@ final class DiffusionCommitRemarkupRule extends PhabricatorObjectRemarkupRule { return PhabricatorRepositoryCommitPHIDType::getCommitObjectNamePattern(); } + protected function getObjectNameText( + $object, + PhabricatorObjectHandle $handle, + $id) { + + // If this commit is unreachable, return the handle name instead of the + // normal text because it may be able to tell the user that the commit + // was rewritten and where to find the new one. + + // By default, we try to preserve what the user actually typed as + // faithfully as possible, but if they're referencing a deleted commit + // it's more valuable to try to pick up any rewrite. See T11522. + if ($object->isUnreachable()) { + return $handle->getName(); + } + + return parent::getObjectNameText($object, $handle, $id); + } + protected function loadObjects(array $ids) { $viewer = $this->getEngine()->getConfig('viewer'); diff --git a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php index 5d3ce3c961..df84f2dcfd 100644 --- a/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php +++ b/src/applications/repository/phid/PhabricatorRepositoryCommitPHIDType.php @@ -29,12 +29,47 @@ final class PhabricatorRepositoryCommitPHIDType extends PhabricatorPHIDType { array $handles, array $objects) { + $unreachable = array(); + foreach ($handles as $phid => $handle) { + $commit = $objects[$phid]; + if ($commit->isUnreachable()) { + $unreachable[$phid] = $commit; + } + } + + if ($unreachable) { + $query = id(new DiffusionCommitHintQuery()) + ->setViewer($query->getViewer()) + ->withCommits($unreachable); + + $query->execute(); + + $hints = $query->getCommitMap(); + } else { + $hints = array(); + } + foreach ($handles as $phid => $handle) { $commit = $objects[$phid]; $repository = $commit->getRepository(); $commit_identifier = $commit->getCommitIdentifier(); $name = $repository->formatCommitName($commit_identifier); + + if ($commit->isUnreachable()) { + $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); + + // If we have a hint about this commit being rewritten, add the + // rewrite target to the handle name. This reduces the chance users + // will be caught offguard by the rewrite. + $hint = idx($hints, $phid); + if ($hint && $hint->isRewritten()) { + $new_name = $hint->getNewCommitIdentifier(); + $new_name = $repository->formatCommitName($new_name); + $name = pht("%s \xE2\x99\xBB %s", $name, $new_name); + } + } + $summary = $commit->getSummary(); if (strlen($summary)) { $full_name = $name.': '.$summary; @@ -46,10 +81,6 @@ final class PhabricatorRepositoryCommitPHIDType extends PhabricatorPHIDType { $handle->setFullName($full_name); $handle->setURI($commit->getURI()); $handle->setTimestamp($commit->getEpoch()); - - if ($commit->isUnreachable()) { - $handle->setStatus(PhabricatorObjectHandle::STATUS_CLOSED); - } } } diff --git a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php index d5addbc556..35c0ecfad0 100644 --- a/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php +++ b/src/infrastructure/markup/rule/PhabricatorObjectRemarkupRule.php @@ -25,6 +25,13 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { return true; } + protected function getObjectNameText( + $object, + PhabricatorObjectHandle $handle, + $id) { + return $this->getObjectNamePrefix().$id; + } + protected function loadHandles(array $objects) { $phids = mpull($objects, 'getPHID'); @@ -60,7 +67,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { $id) { $href = $this->getObjectHref($object, $handle, $id); - $text = $this->getObjectNamePrefix().$id; + $text = $this->getObjectNameText($object, $handle, $id); if ($anchor) { $href = $href.'#'.$anchor; @@ -85,7 +92,7 @@ abstract class PhabricatorObjectRemarkupRule extends PhutilRemarkupRule { $id) { $href = $this->getObjectHref($object, $handle, $id); - $text = $this->getObjectNamePrefix().$id; + $text = $this->getObjectNameText($object, $handle, $id); $status_closed = PhabricatorObjectHandle::STATUS_CLOSED; if ($anchor) { From ae0cf00a232f6658f77670dbd0538e9bd498344e Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 24 Aug 2016 06:45:12 -0700 Subject: [PATCH 11/27] Document the use of repository commit hints Summary: Ref T11522. This explains how to actually use `bin/repository hint`. Test Plan: Read the document. Used `bin/repository hint` as directed. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11522 Differential Revision: https://secure.phabricator.com/D16441 --- src/docs/user/field/repository_hints.diviner | 134 +++++++++++++++++++ 1 file changed, 134 insertions(+) create mode 100644 src/docs/user/field/repository_hints.diviner diff --git a/src/docs/user/field/repository_hints.diviner b/src/docs/user/field/repository_hints.diviner new file mode 100644 index 0000000000..d618fac4aa --- /dev/null +++ b/src/docs/user/field/repository_hints.diviner @@ -0,0 +1,134 @@ +@title Repository Hints and Rewriting Commits +@group fieldmanual + +Dealing with rewrites of published repositories and other unusual problems. + +Overview +======== + +Some repositories have unusual commits. You can provide "hints" to Phabricator +about these commits to improve behavior. + +Supported hints are: + + - **Rewritten Commits**: If you have rewritten the history of a published + repository, you can provide hints about the mapping from old commits to + new commits so it can redirect users who visit old pages to the proper + new pages. + - **Unreadable Commits**: If some commits are not readable (which is rare, + but can happen in some cases if they are generated with an external tool) + you can provide hints so that Phabricator doesn't try to read them. + +The remainder of this document explains how to create and remove hints, and how +to specify each type of hint. + +Creating Hints +============== + +To create hints, pipe a JSON list of hints to `bin/repository hint`: + +``` +phabricator/ $ cat hints.json | ./bin/repository hint +``` + +The hints should be a list of objects like this: + +```lang=json +[ + ... + { + "repository": "XYZ", + "hint": "...", + "old": "abcdef1234abcdef1234abcdef1234abcdef1234", + "new": "..." + } + ... +] +``` + +Each hint may have these keys: + + - `repository`: A repository identifier (ID, PHID, callsign or short name). + - `hint`: The hint type, see below. + - `old`: The full identifier or commit hash of the commit you want to + provide a hint for. + - `new`: For hints which specify a new commit, the full identifier or commit + hash of the new commit. + +See below for exactly how to specify each type of hint. + + +Removing Hints +============== + +To remove a hint, create a hint of type `"none"`. This will remove any existing +hint. + +For example, use a hint specification like this: + +```lang=json +[ + { + "repository": "XYZ", + "hint": "none", + "old": "abcdef1234abcdef1234abcdef1234abcdef1234" + } +] +``` + +Phabricator won't treat commits without any hint specially. + + +Hint: Rewritten Commits +======================= + +The `"rewritten"` hint allows you to redirect old commits to new commits after +a rewrite of published history. You should normally avoid rewriting published +commits, but sometimes this is necessary: for example, if a repository has +become unwieldy because it contains large binaries, you may strip them from +history. + +To provide this kind of hint, pass the `"old"` commit hash (from before the +rewrite) and the `"new"` commit hash (from after the rewrite). + +For example, a hint might look like this: + +```lang=json +[ + { + "repository": "XYZ", + "hint": "rewritten", + "old": "abcdef1234abcdef1234abcdef1234abcdef1234", + "new": "098765ffaabbccdd4680098765ffaabbccdd4680" + } +] +``` + +Phabricator will show users that the commit was rewritten in the web UI. + + +Hint: Unreadable Commits +======================== + +The `"unreadable"` hint allows you to tell Phabricator that it should not +bother trying to read the changes associated with a particular commit. In +some rare cases, repositories can contain commits which aren't readable +(for example, if they were created by external tools during an import or +merge process). + +To provide this kind of hint, pass the `"old"` commit which is affected. + +For example, a hint might look like this: + +```lang=json +[ + { + "repository": "XYZ", + "hint": "unreadable", + "old": "abcdef1234abcdef1234abcdef1234abcdef1234" + } +] +``` + +Phabricator won't try to read, parse, import, or display the changes associated +with this commit. From 605210bc95af98ec173501a248527c0a3f501fa0 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Tue, 23 Aug 2016 07:09:51 -0500 Subject: [PATCH 12/27] Make the chatbot obey the object name blacklist Summary: Fixes T11508. The config entry `remarkup.ignored-object-names` already contains a blacklist of object names that should be ignored in the web UI. This change makes that blacklist also apply to the chatbot. This makes it possible to have a chatbot ignore things like V1, V2, Q1 and any other phrases the user may not want to generate links to objects. Test Plan: Create objects (tasks, slowvotes, etc.) then mention the object names in chat (with the bot running). The bot should respond with helpful links to the given objects. Then add the object names to the blacklist through the config web UI. This apparently triggers the bot to restart itself. Then mention the object names in chat again. The bot should no longer respond with links because those object names have been added to the blacklist regex. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: epriestley Maniphest Tasks: T11508 Differential Revision: https://secure.phabricator.com/D16442 --- .../daemon/bot/handler/PhabricatorBotObjectNameHandler.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php index 64f7665827..11c92af2cf 100644 --- a/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php +++ b/src/infrastructure/daemon/bot/handler/PhabricatorBotObjectNameHandler.php @@ -50,8 +50,14 @@ final class PhabricatorBotObjectNameHandler extends PhabricatorBotHandler { '(?:\b|$)'. '@'; + $regex = trim( + PhabricatorEnv::getEnvConfig('remarkup.ignored-object-names')); + if (preg_match_all($pattern, $message, $matches, PREG_SET_ORDER)) { foreach ($matches as $match) { + if ($regex && preg_match($regex, $match[0])) { + continue; + } switch ($match[1]) { case 'P': $paste_ids[] = $match[2]; From 2c9a93eda7ddd88c26fd9fdc8b6174f1837a2716 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 25 Aug 2016 03:06:46 +0000 Subject: [PATCH 13/27] Fix app icons in homepage settings Summary: These were blank, from last week's shenanigans. Test Plan: View homepage settings, see icons. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16447 --- .../PhabricatorHomePreferencesSettingsPanel.php | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php index 833a56deb7..fafe62ee4e 100644 --- a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php @@ -152,20 +152,12 @@ final class PhabricatorHomePreferencesSettingsPanel $icon = $application->getIcon(); if (!$icon) { - $icon = 'application'; + $icon = 'fa-globe'; } - $icon_view = javelin_tag( - 'span', - array( - 'class' => 'phui-icon-view phui-font-fa '.$icon, - 'aural' => false, - ), - ''); - $item = id(new PHUIObjectItemView()) ->setHeader($application->getName()) - ->setImageIcon($icon_view) + ->setImageIcon($icon) ->addAttribute($application->getShortDescription()) ->setGrippable(true); From 8cdf1a890a01c1dbf569347c21dab7047f1dafc1 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Wed, 24 Aug 2016 19:00:03 -0400 Subject: [PATCH 14/27] Updated the docs so chatbots can use the Conduit API Summary: Previously, the chatbot docs instructed users to get certificates for the conduit API and put the cert in a `conduit.cert` config key. In order to get the chatbot to work, I needed to instead get an API key and put it in the `conduit.token` config entry. Test Plan: Doc fix. Tried the new documented way and it worked. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Differential Revision: https://secure.phabricator.com/D16443 --- resources/chatbot/example_config.json | 3 +-- src/docs/tech/chatbot.diviner | 8 +++----- src/infrastructure/daemon/bot/PhabricatorBot.php | 7 +++---- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/resources/chatbot/example_config.json b/resources/chatbot/example_config.json index b688d88c82..7d150e65ae 100644 --- a/resources/chatbot/example_config.json +++ b/resources/chatbot/example_config.json @@ -15,8 +15,7 @@ ], "conduit.uri" : null, - "conduit.user" : null, - "conduit.cert" : null, + "conduit.token" : null, "macro.size" : 48, "macro.aspect" : 0.66, diff --git a/src/docs/tech/chatbot.diviner b/src/docs/tech/chatbot.diviner index 9bc1ce4796..6f79e2e465 100644 --- a/src/docs/tech/chatbot.diviner +++ b/src/docs/tech/chatbot.diviner @@ -32,7 +32,7 @@ These are the configuration values it reads: - `join` Array, list of channels to join. - `handlers` Array, list of handlers to run. These are like plugins for the bot. - - `conduit.uri`, `conduit.user`, `conduit.cert` Conduit configuration, + - `conduit.uri`, `conduit.token` Conduit configuration, see below. - `notification.channels` Notification configuration, see below. @@ -72,10 +72,8 @@ with. To do this, login to Phabricator as an administrator and go to - `conduit.uri` The URI for your Phabricator install, like `http://phabricator.example.com/` - - `conduit.user` The username your bot should login to Phabricator with -- - whatever you selected above, like `phabot`. - - `conduit.cert` The user's certificate, from the "Conduit Certificate" tab - in the user's administrative view. + - `conduit.token` The user's conduit API token, from the "Conduit API Tokens" + tab in the user's administrative view. Now the bot should be able to connect to Phabricator via Conduit. diff --git a/src/infrastructure/daemon/bot/PhabricatorBot.php b/src/infrastructure/daemon/bot/PhabricatorBot.php index 6c4bfafc8a..b6efdb8b6d 100644 --- a/src/infrastructure/daemon/bot/PhabricatorBot.php +++ b/src/infrastructure/daemon/bot/PhabricatorBot.php @@ -159,11 +159,10 @@ final class PhabricatorBot extends PhabricatorDaemon { if (empty($this->conduit)) { throw new Exception( pht( - "This bot is not configured with a Conduit uplink. Set '%s', ". - "'%s' and '%s' in the configuration to connect.", + "This bot is not configured with a Conduit uplink. Set '%s' and ". + "'%s' in the configuration to connect.", 'conduit.uri', - 'conduit.user', - 'conduit.cert')); + 'conduit.token')); } return $this->conduit; } From a1f25fdb3e630df31e9214b9b9169dcf918917e9 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Wed, 24 Aug 2016 17:03:21 -0400 Subject: [PATCH 15/27] Added high security requirement to add/delete email addresses Summary: Fixes T10999. Now MFA will be required for all email address related operations. Test Plan: Ensure that adding and removing email addresses now requires you to enter high security mode. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Maniphest Tasks: T10999 Differential Revision: https://secure.phabricator.com/D16444 --- .../panel/PhabricatorEmailAddressesSettingsPanel.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php index b5d8ea8617..31249985e4 100644 --- a/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorEmailAddressesSettingsPanel.php @@ -165,6 +165,11 @@ final class PhabricatorEmailAddressesSettingsPanel $user = $this->getUser(); $viewer = $this->getViewer(); + $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( + $viewer, + $request, + $this->getPanelURI()); + $e_email = true; $email = null; $errors = array(); @@ -276,6 +281,11 @@ final class PhabricatorEmailAddressesSettingsPanel $user = $this->getUser(); $viewer = $this->getViewer(); + $token = id(new PhabricatorAuthSessionEngine())->requireHighSecuritySession( + $viewer, + $request, + $this->getPanelURI()); + // NOTE: You can only delete your own email addresses, and you can not // delete your primary address. $email = id(new PhabricatorUserEmail())->loadOneWhere( From 7f7c3acfac92f91182b5abb0c9610a752c5cf1cf Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Wed, 24 Aug 2016 19:10:04 -0400 Subject: [PATCH 16/27] Remove unused apps from the DocumentType typeahead Summary: Ref T10951. This diff removes uninstalled applications from the result set for DocumentType restults Test Plan: Uninstall an application (diviner for example), then go to the document type search menu and ensure that the uninstalled application doesn't show up. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley Maniphest Tasks: T10951 Differential Revision: https://secure.phabricator.com/D16445 --- .../typeahead/PhabricatorSearchDocumentTypeDatasource.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php b/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php index 407ab1cad4..860620023c 100644 --- a/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php +++ b/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php @@ -25,8 +25,10 @@ final class PhabricatorSearchDocumentTypeDatasource } private function buildResults() { + $viewer = $this->getViewer(); $types = - PhabricatorSearchApplicationSearchEngine::getIndexableDocumentTypes(); + PhabricatorSearchApplicationSearchEngine::getIndexableDocumentTypes( + $viewer); $icons = mpull( PhabricatorPHIDType::getAllTypes(), From d135b3f2d56f0ecc4c3c184ed18fdb13cc063e13 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Wed, 24 Aug 2016 18:11:15 -0400 Subject: [PATCH 17/27] Added application name to the typeahead results for doc type search Summary: Ref T10951. This adds the application name as an attribute below the document type in the UI for doc type search. Test Plan: Verify that the application name appears as an attribute on the document type results. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, epriestley Maniphest Tasks: T10951 Differential Revision: https://secure.phabricator.com/D16446 --- .../PhabricatorSearchDocumentTypeDatasource.php | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php b/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php index 860620023c..c99cc4ede3 100644 --- a/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php +++ b/src/applications/search/typeahead/PhabricatorSearchDocumentTypeDatasource.php @@ -30,17 +30,25 @@ final class PhabricatorSearchDocumentTypeDatasource PhabricatorSearchApplicationSearchEngine::getIndexableDocumentTypes( $viewer); - $icons = mpull( - PhabricatorPHIDType::getAllTypes(), - 'getTypeIcon', + $phid_types = mpull(PhabricatorPHIDType::getAllTypes(), + null, 'getTypeConstant'); $results = array(); foreach ($types as $type => $name) { + $type_object = idx($phid_types, $type); + if (!$type_object) { + continue; + } + $application_class = $type_object->getPHIDTypeApplicationClass(); + $application = PhabricatorApplication::getByClass($application_class); + $application_name = $application->getName(); + $results[$type] = id(new PhabricatorTypeaheadResult()) ->setPHID($type) ->setName($name) - ->setIcon(idx($icons, $type)); + ->addAttribute($application_name) + ->setIcon($type_object->getTypeIcon()); } return $results; From a88dc2afc2a9854dab25df28351109f5aec6607d Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Thu, 25 Aug 2016 12:54:10 -0400 Subject: [PATCH 18/27] Added a setup check for empty REMOTE_ADDR Summary: Fixes T8850. Previously, if a user's preamble script mangled `$_SERVER['REMOTE_ADDR']` or somehow set it to `null`, the user would get errors when performing certain actions. Now those errors shouldn't occur, and instead the user will be warned that there is a setup issue related to their preamble script. Test Plan: Create a preamble script that contains `$_SERVER['REMOTE_ADDR'] = null;` then navigate to /config/issue/. There should be a warning there about `REMOTE_ADDR` not being available. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: Korvin, yelirekim, epriestley Maniphest Tasks: T8850 Differential Revision: https://secure.phabricator.com/D16450 --- .../check/PhabricatorPHPConfigSetupCheck.php | 27 +++++++++++++++++++ .../people/storage/PhabricatorUserLog.php | 2 +- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php index 6a08ec68ae..b3fd03fd19 100644 --- a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php @@ -197,5 +197,32 @@ final class PhabricatorPHPConfigSetupCheck extends PhabricatorSetupCheck { ->setMessage($message); } } + + if (empty($_SERVER['REMOTE_ADDR'])) { + $doc_href = PhabricatorEnv::getDocLink('Configuring a Preamble Script'); + + $summary = pht( + 'You likely need to fix your preamble script so '. + 'REMOTE_ADDR is no longer empty.'); + + $message = pht( + 'No REMOTE_ADDR is available, so Phabricator cannot determine the '. + 'origin address for requests. This will prevent Phabricator from '. + 'performing important security checks. This most often means you '. + 'have a mistake in your preamble script. Consult the documentation '. + '(%s) and double-check that the script is written correctly.', + phutil_tag( + 'a', + array( + 'href' => $doc_href, + 'target' => '_blank', + ), + pht('Configuring a Preamble Script'))); + + $this->newIssue('php.remote_addr') + ->setName(pht('No REMOTE_ADDR available')) + ->setSummary($summary) + ->setMessage($message); + } } } diff --git a/src/applications/people/storage/PhabricatorUserLog.php b/src/applications/people/storage/PhabricatorUserLog.php index 5939778f25..9727c81c63 100644 --- a/src/applications/people/storage/PhabricatorUserLog.php +++ b/src/applications/people/storage/PhabricatorUserLog.php @@ -108,7 +108,7 @@ final class PhabricatorUserLog extends PhabricatorUserDAO $log->setUserPHID((string)$object_phid); $log->setAction($action); - $log->remoteAddr = idx($_SERVER, 'REMOTE_ADDR', ''); + $log->remoteAddr = (string)idx($_SERVER, 'REMOTE_ADDR', ''); return $log; } From a7dcbe598030512435a6caa769f8aec395ffcb26 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Thu, 25 Aug 2016 13:33:47 -0400 Subject: [PATCH 19/27] Update People for handleRequest Summary: Ref T8628. Updates people controllers for handleRequest Test Plan: Viewed the people list, viewed the activity logs, then went through the approval process for a new user account. Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, yelirekim Maniphest Tasks: T8628 Differential Revision: https://secure.phabricator.com/D16451 --- .../PhabricatorPeopleApproveController.php | 22 ++++++------------- .../PhabricatorPeopleListController.php | 10 ++------- .../PhabricatorPeopleLogsController.php | 12 +++------- 3 files changed, 12 insertions(+), 32 deletions(-) diff --git a/src/applications/people/controller/PhabricatorPeopleApproveController.php b/src/applications/people/controller/PhabricatorPeopleApproveController.php index a63682239f..58cd2e2119 100644 --- a/src/applications/people/controller/PhabricatorPeopleApproveController.php +++ b/src/applications/people/controller/PhabricatorPeopleApproveController.php @@ -3,20 +3,12 @@ final class PhabricatorPeopleApproveController extends PhabricatorPeopleController { - private $id; - - public function willProcessRequest(array $data) { - $this->id = idx($data, 'id'); - } - - public function processRequest() { - - $request = $this->getRequest(); - $admin = $request->getUser(); + public function handleRequest(AphrontRequest $request) { + $viewer = $request->getViewer(); $user = id(new PhabricatorPeopleQuery()) - ->setViewer($admin) - ->withIDs(array($this->id)) + ->setViewer($viewer) + ->withIDs(array($request->getURIData('id'))) ->executeOne(); if (!$user) { return new Aphront404Response(); @@ -26,7 +18,7 @@ final class PhabricatorPeopleApproveController if ($request->isFormPost()) { id(new PhabricatorUserEditor()) - ->setActor($admin) + ->setActor($viewer) ->approveUser($user, true); $title = pht( @@ -39,12 +31,12 @@ final class PhabricatorPeopleApproveController 'Your Phabricator account (%s) has been approved by %s. You can '. 'login here:', $user->getUsername(), - $admin->getUsername()), + $viewer->getUsername()), PhabricatorEnv::getProductionURI('/')); $mail = id(new PhabricatorMetaMTAMail()) ->addTos(array($user->getPHID())) - ->addCCs(array($admin->getPHID())) + ->addCCs(array($viewer->getPHID())) ->setSubject('[Phabricator] '.$title) ->setForceDelivery(true) ->setBody($body) diff --git a/src/applications/people/controller/PhabricatorPeopleListController.php b/src/applications/people/controller/PhabricatorPeopleListController.php index d4ba9aa217..edcfc7ba0f 100644 --- a/src/applications/people/controller/PhabricatorPeopleListController.php +++ b/src/applications/people/controller/PhabricatorPeopleListController.php @@ -3,8 +3,6 @@ final class PhabricatorPeopleListController extends PhabricatorPeopleController { - private $key; - public function shouldAllowPublic() { return true; } @@ -13,16 +11,12 @@ final class PhabricatorPeopleListController return false; } - public function willProcessRequest(array $data) { - $this->key = idx($data, 'key'); - } - - public function processRequest() { + public function handleRequest(AphrontRequest $request) { $this->requireApplicationCapability( PeopleBrowseUserDirectoryCapability::CAPABILITY); $controller = id(new PhabricatorApplicationSearchController()) - ->setQueryKey($this->key) + ->setQueryKey($request->getURIData('key')) ->setSearchEngine(new PhabricatorPeopleSearchEngine()) ->setNavigation($this->buildSideNavView()); diff --git a/src/applications/people/controller/PhabricatorPeopleLogsController.php b/src/applications/people/controller/PhabricatorPeopleLogsController.php index 7ebf29ab65..e6054398d8 100644 --- a/src/applications/people/controller/PhabricatorPeopleLogsController.php +++ b/src/applications/people/controller/PhabricatorPeopleLogsController.php @@ -3,15 +3,9 @@ final class PhabricatorPeopleLogsController extends PhabricatorPeopleController { - private $queryKey; - - public function willProcessRequest(array $data) { - $this->queryKey = idx($data, 'queryKey'); - } - - public function processRequest() { - $controller = id(new PhabricatorApplicationSearchController()) - ->setQueryKey($this->queryKey) + public function handleRequest(AphrontRequest $request) { + $controller = id(new PhabricatorApplicationSearchController()) + ->setQueryKey($request->getURIData('queryKey')) ->setSearchEngine(new PhabricatorPeopleLogSearchEngine()) ->setNavigation($this->buildSideNavView()); From d5327fdba0764dd350424478561e7c9bb723e9fa Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 25 Aug 2016 11:16:34 -0700 Subject: [PATCH 20/27] New 'default' homepage Summary: Ref T11132. This is a new default default (no dashboard) homepage. It offers (Diffs) (Tasks) (Repositories) in the main column and (Feed) in the side column. No NUX stuff, No logged out public view (upcoming diff). This should be complete, but unclear how to bucketize Differential. Test Plan: Test new account's default homepage. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T11132 Differential Revision: https://secure.phabricator.com/D16449 --- .../view/DifferentialRevisionListView.php | 6 - .../PhabricatorHomeMainController.php | 447 ++++++------------ .../view/ManiphestTaskResultListView.php | 3 +- 3 files changed, 135 insertions(+), 321 deletions(-) diff --git a/src/applications/differential/view/DifferentialRevisionListView.php b/src/applications/differential/view/DifferentialRevisionListView.php index 74e3f7633f..7ab0f2d183 100644 --- a/src/applications/differential/view/DifferentialRevisionListView.php +++ b/src/applications/differential/view/DifferentialRevisionListView.php @@ -7,7 +7,6 @@ final class DifferentialRevisionListView extends AphrontView { private $revisions; private $handles; - private $highlightAge; private $header; private $noDataString; private $noBox; @@ -39,11 +38,6 @@ final class DifferentialRevisionListView extends AphrontView { return $this; } - public function setHighlightAge($bool) { - $this->highlightAge = $bool; - return $this; - } - public function setNoBox($box) { $this->noBox = $box; return $this; diff --git a/src/applications/home/controller/PhabricatorHomeMainController.php b/src/applications/home/controller/PhabricatorHomeMainController.php index 7bc8b06573..592478c894 100644 --- a/src/applications/home/controller/PhabricatorHomeMainController.php +++ b/src/applications/home/controller/PhabricatorHomeMainController.php @@ -2,8 +2,6 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { - private $minipanels = array(); - public function shouldAllowPublic() { return true; } @@ -13,32 +11,27 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { } public function handleRequest(AphrontRequest $request) { - $user = $request->getUser(); + $viewer = $request->getViewer(); $dashboard = PhabricatorDashboardInstall::getDashboard( - $user, - $user->getPHID(), + $viewer, + $viewer->getPHID(), get_class($this->getCurrentApplication())); if (!$dashboard) { $dashboard = PhabricatorDashboardInstall::getDashboard( - $user, + $viewer, PhabricatorHomeApplication::DASHBOARD_DEFAULT, get_class($this->getCurrentApplication())); } if ($dashboard) { $content = id(new PhabricatorDashboardRenderingEngine()) - ->setViewer($user) + ->setViewer($viewer) ->setDashboard($dashboard) ->renderDashboard(); } else { - $project_query = new PhabricatorProjectQuery(); - $project_query->setViewer($user); - $project_query->withMemberPHIDs(array($user->getPHID())); - $projects = $project_query->execute(); - - $content = $this->buildMainResponse($projects); + $content = $this->buildMainResponse(); } if (!$request->getURIData('only')) { @@ -46,7 +39,7 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { $nav->appendChild( array( $content, - id(new PhabricatorGlobalUploadTargetView())->setUser($user), + id(new PhabricatorGlobalUploadTargetView())->setUser($viewer), )); $content = $nav; } @@ -58,354 +51,180 @@ final class PhabricatorHomeMainController extends PhabricatorHomeController { } - private function buildMainResponse(array $projects) { - assert_instances_of($projects, 'PhabricatorProject'); - $viewer = $this->getRequest()->getUser(); + private function buildMainResponse() { + require_celerity_resource('phabricator-dashboard-css'); + $viewer = $this->getViewer(); $has_maniphest = PhabricatorApplication::isClassInstalledForViewer( 'PhabricatorManiphestApplication', $viewer); - $has_audit = PhabricatorApplication::isClassInstalledForViewer( - 'PhabricatorAuditApplication', + $has_diffusion = PhabricatorApplication::isClassInstalledForViewer( + 'PhabricatorDiffusionApplication', $viewer); $has_differential = PhabricatorApplication::isClassInstalledForViewer( 'PhabricatorDifferentialApplication', $viewer); - if ($has_maniphest) { - $unbreak_panel = $this->buildUnbreakNowPanel(); - $triage_panel = $this->buildNeedsTriagePanel($projects); - $tasks_panel = $this->buildTasksPanel(); - } else { - $unbreak_panel = null; - $triage_panel = null; - $tasks_panel = null; - } - - if ($has_audit) { - $audit_panel = $this->buildAuditPanel(); - $commit_panel = $this->buildCommitPanel(); - } else { - $audit_panel = null; - $commit_panel = null; - } - - if (PhabricatorEnv::getEnvConfig('welcome.html') !== null) { - $welcome_panel = $this->buildWelcomePanel(); - } else { - $welcome_panel = null; - } - + $revision_panel = null; if ($has_differential) { $revision_panel = $this->buildRevisionPanel(); - } else { - $revision_panel = null; } - $home = phutil_tag( + $tasks_panel = null; + if ($has_maniphest) { + $tasks_panel = $this->buildTasksPanel(); + } + + $repository_panel = null; + if ($has_diffusion) { + $repository_panel = $this->buildRepositoryPanel(); + } + + $feed_panel = $this->buildFeedPanel(); + + $dashboard = id(new AphrontMultiColumnView()) + ->setFluidlayout(true) + ->setGutter(AphrontMultiColumnView::GUTTER_LARGE); + + $main_panel = phutil_tag( 'div', array( 'class' => 'homepage-panel', ), array( - $welcome_panel, - $unbreak_panel, - $triage_panel, $revision_panel, $tasks_panel, - $audit_panel, - $commit_panel, - $this->minipanels, + $repository_panel, )); - return $home; + $dashboard->addColumn($main_panel, 'thirds'); + + $side_panel = phutil_tag( + 'div', + array( + 'class' => 'homepage-side-panel', + ), + array( + $feed_panel, + )); + $dashboard->addColumn($side_panel, 'third'); + + $view = id(new PHUIBoxView()) + ->addClass('dashboard-view') + ->appendChild($dashboard); + + return $view; } - private function buildUnbreakNowPanel() { - $unbreak_now = PhabricatorEnv::getEnvConfig( - 'maniphest.priorities.unbreak-now'); - if (!$unbreak_now) { - return null; - } - - $user = $this->getRequest()->getUser(); - - $task_query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->withPriorities(array($unbreak_now)) - ->needProjectPHIDs(true) - ->setLimit(10); - - $tasks = $task_query->execute(); - - if (!$tasks) { - return $this->renderMiniPanel( - pht('No "Unbreak Now!" Tasks'), - pht('Nothing appears to be critically broken right now.')); - } - - $href = urisprintf( - '/maniphest/?statuses=open()&priorities=%s#R', - $unbreak_now); - $title = pht('Unbreak Now!'); - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - $panel->setObjectList($this->buildTaskListView($tasks)); - - return $panel; - } - - private function buildNeedsTriagePanel(array $projects) { - assert_instances_of($projects, 'PhabricatorProject'); - - $needs_triage = PhabricatorEnv::getEnvConfig( - 'maniphest.priorities.needs-triage'); - if (!$needs_triage) { - return null; - } - - $user = $this->getRequest()->getUser(); - if (!$user->isLoggedIn()) { - return null; - } - - if ($projects) { - $task_query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->withPriorities(array($needs_triage)) - ->withEdgeLogicPHIDs( - PhabricatorProjectObjectHasProjectEdgeType::EDGECONST, - PhabricatorQueryConstraint::OPERATOR_OR, - mpull($projects, 'getPHID')) - ->needProjectPHIDs(true) - ->setLimit(10); - $tasks = $task_query->execute(); - } else { - $tasks = array(); - } - - if (!$tasks) { - return $this->renderMiniPanel( - pht('No "Needs Triage" Tasks'), - pht('No tasks tagged with projects you are a member of need triage.')); - } - - $title = pht('Needs Triage'); - $href = urisprintf( - '/maniphest/?statuses=open()&priorities=%s&projects=projects(%s)#R', - $needs_triage, - $user->getPHID()); - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - $panel->setObjectList($this->buildTaskListView($tasks)); - - return $panel; - } - - private function buildRevisionPanel() { - $viewer = $this->getViewer(); - - $revisions = PhabricatorDifferentialApplication::loadNeedAttentionRevisions( - $viewer); - - if (!$revisions) { - return $this->renderMiniPanel( - pht('No Waiting Revisions'), - pht('No revisions are waiting on you.')); - } - - $title = pht('Revisions Waiting on You'); - $href = '/differential/'; - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - - $revision_view = id(new DifferentialRevisionListView()) - ->setHighlightAge(true) - ->setRevisions($revisions) - ->setUser($viewer); - - $phids = array_merge( - array($viewer->getPHID()), - $revision_view->getRequiredHandlePHIDs()); - $handles = $this->loadViewerHandles($phids); - - $revision_view->setHandles($handles); - - $list_view = $revision_view->render(); - - $panel->setObjectList($list_view); - - return $panel; - } - - private function buildWelcomePanel() { - $panel = new PHUIObjectBoxView(); - $panel->setHeaderText(pht('Welcome')); - $panel->appendChild( - phutil_safe_html( - PhabricatorEnv::getEnvConfig('welcome.html'))); - - return $panel; - } - - private function buildTasksPanel() { - $user = $this->getRequest()->getUser(); - $user_phid = $user->getPHID(); - - $task_query = id(new ManiphestTaskQuery()) - ->setViewer($user) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->setGroupBy(ManiphestTaskQuery::GROUP_PRIORITY) - ->withOwners(array($user_phid)) - ->needProjectPHIDs(true) - ->setLimit(10); - - $tasks = $task_query->execute(); - - - if (!$tasks) { - return $this->renderMiniPanel( - pht('No Assigned Tasks'), - pht('You have no assigned tasks.')); - } - - $title = pht('Assigned Tasks'); - $href = '/maniphest/query/assigned/'; - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - $panel->setObjectList($this->buildTaskListView($tasks)); - - return $panel; - } - - private function buildTaskListView(array $tasks) { - assert_instances_of($tasks, 'ManiphestTask'); - $user = $this->getRequest()->getUser(); - - $phids = array_merge( - array_filter(mpull($tasks, 'getOwnerPHID')), - array_mergev(mpull($tasks, 'getProjectPHIDs'))); - - $handles = $this->loadViewerHandles($phids); - - $view = new ManiphestTaskListView(); - $view->setTasks($tasks); - $view->setUser($user); - $view->setHandles($handles); - - return $view; - } - - private function renderSectionHeader($title, $href) { + private function buildHomepagePanel($title, $href, $view) { $title = phutil_tag( 'a', array( 'href' => $href, ), $title); + $icon = id(new PHUIIconView()) ->setIcon('fa-search') ->setHref($href); + $header = id(new PHUIHeaderView()) ->setHeader($title) ->addActionItem($icon); - return $header; - } - private function renderMiniPanel($title, $body) { - $panel = new PHUIInfoView(); - $panel->setSeverity(PHUIInfoView::SEVERITY_NODATA); - $panel->appendChild( - phutil_tag( - 'p', - array( - ), - array( - phutil_tag('strong', array(), $title.': '), - $body, - ))); - $this->minipanels[] = $panel; - } + $box = id(new PHUIObjectBoxView()) + ->setHeader($header); - public function buildAuditPanel() { - $request = $this->getRequest(); - $user = $request->getUser(); - - $phids = PhabricatorAuditCommentEditor::loadAuditPHIDsForUser($user); - - $query = id(new DiffusionCommitQuery()) - ->setViewer($user) - ->withNeedsAuditByPHIDs($phids) - ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_OPEN) - ->needAuditRequests(true) - ->needCommitData(true) - ->setLimit(10); - - $commits = $query->execute(); - - if (!$commits) { - return $this->renderMinipanel( - pht('No Audits'), - pht('No commits are waiting for you to audit them.')); + if ($view->getObjectList()) { + $box->setObjectList($view->getObjectList()); + } + if ($view->getContent()) { + $box->appendChild($view->getContent()); } - $view = id(new PhabricatorAuditListView()) - ->setCommits($commits) - ->setUser($user); - - $phids = $view->getRequiredHandlePHIDs(); - $handles = $this->loadViewerHandles($phids); - $view->setHandles($handles); - - $title = pht('Audits'); - $href = '/audit/'; - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - $panel->setObjectList($view); - - return $panel; + return $box; } - public function buildCommitPanel() { - $request = $this->getRequest(); - $user = $request->getUser(); - - $phids = array($user->getPHID()); - - $query = id(new DiffusionCommitQuery()) - ->setViewer($user) - ->withAuthorPHIDs($phids) - ->withAuditStatus(DiffusionCommitQuery::AUDIT_STATUS_CONCERN) - ->needCommitData(true) - ->needAuditRequests(true) - ->setLimit(10); - - $commits = $query->execute(); - - if (!$commits) { - return $this->renderMinipanel( - pht('No Problem Commits'), - pht('No one has raised concerns with your commits.')); + private function buildRevisionPanel() { + $viewer = $this->getViewer(); + if (!$viewer->isLoggedIn()) { + return null; } - $view = id(new PhabricatorAuditListView()) - ->setCommits($commits) - ->setUser($user); + $engine = new DifferentialRevisionSearchEngine(); + $engine->setViewer($viewer); + $saved = $engine->buildSavedQueryFromBuiltin('active'); + $query = $engine->buildQueryFromSavedQuery($saved); + $pager = $engine->newPagerForSavedQuery($saved); + $pager->setPageSize(15); + $results = $engine->executeQuery($query, $pager); + $view = $engine->renderResults($results, $saved); - $phids = $view->getRequiredHandlePHIDs(); - $handles = $this->loadViewerHandles($phids); - $view->setHandles($handles); + $title = pht('Active Revisions'); + $href = '/differential/query/active/'; - $title = pht('Problem Commits'); - $href = '/audit/'; - $panel = new PHUIObjectBoxView(); - $panel->setHeader($this->renderSectionHeader($title, $href)); - $panel->setObjectList($view); + return $this->buildHomepagePanel($title, $href, $view); + } - return $panel; + private function buildTasksPanel() { + $viewer = $this->getViewer(); + + $query = 'assigned'; + $title = pht('Assigned Tasks'); + $href = '/maniphest/query/assigned/'; + if (!$viewer->isLoggedIn()) { + $query = 'open'; + $title = pht('Open Tasks'); + $href = '/maniphest/query/open/'; + } + + $engine = new ManiphestTaskSearchEngine(); + $engine->setViewer($viewer); + $saved = $engine->buildSavedQueryFromBuiltin($query); + $query = $engine->buildQueryFromSavedQuery($saved); + $pager = $engine->newPagerForSavedQuery($saved); + $pager->setPageSize(15); + $results = $engine->executeQuery($query, $pager); + $view = $engine->renderResults($results, $saved); + + return $this->buildHomepagePanel($title, $href, $view); + } + + public function buildFeedPanel() { + $viewer = $this->getViewer(); + + $engine = new PhabricatorFeedSearchEngine(); + $engine->setViewer($viewer); + $saved = $engine->buildSavedQueryFromBuiltin('all'); + $query = $engine->buildQueryFromSavedQuery($saved); + $pager = $engine->newPagerForSavedQuery($saved); + $pager->setPageSize(40); + $results = $engine->executeQuery($query, $pager); + $view = $engine->renderResults($results, $saved); + + $title = pht('Recent Activity'); + $href = '/feed/'; + + return $this->buildHomepagePanel($title, $href, $view); + } + + public function buildRepositoryPanel() { + $viewer = $this->getViewer(); + + $engine = new PhabricatorRepositorySearchEngine(); + $engine->setViewer($viewer); + $saved = $engine->buildSavedQueryFromBuiltin('active'); + $query = $engine->buildQueryFromSavedQuery($saved); + $pager = $engine->newPagerForSavedQuery($saved); + $pager->setPageSize(5); + $results = $engine->executeQuery($query, $pager); + $view = $engine->renderResults($results, $saved); + + $title = pht('Active Repositories'); + $href = '/diffusion/'; + + return $this->buildHomepagePanel($title, $href, $view); } } diff --git a/src/applications/maniphest/view/ManiphestTaskResultListView.php b/src/applications/maniphest/view/ManiphestTaskResultListView.php index 52f4a3b2d6..9e28e89d8d 100644 --- a/src/applications/maniphest/view/ManiphestTaskResultListView.php +++ b/src/applications/maniphest/view/ManiphestTaskResultListView.php @@ -41,7 +41,8 @@ final class ManiphestTaskResultListView extends ManiphestView { // If we didn't match anything, just pick up the default empty state. if (!$tasks) { return id(new PHUIObjectItemListView()) - ->setUser($viewer); + ->setUser($viewer) + ->setNoDataString(pht('No tasks found.')); } $group_parameter = nonempty($query->getParameter('group'), 'priority'); From 9d9a47e9cf4934232800dc5c49372f0522e366f9 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 25 Aug 2016 12:06:03 -0700 Subject: [PATCH 21/27] Add setup checks for unused homepage options Summary: Ref T11533, Fixes T5315. Remove and add extra setup checks for removed homepage options. Test Plan: Review text. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Maniphest Tasks: T5315, T11533 Differential Revision: https://secure.phabricator.com/D16453 --- .../PhabricatorExtraConfigSetupCheck.php | 16 ++++++++++++++ .../option/PhabricatorCoreConfigOptions.php | 4 ---- .../PhabricatorManiphestConfigOptions.php | 22 ------------------- 3 files changed, 16 insertions(+), 26 deletions(-) diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php index 6547f9ca96..e32d7a1476 100644 --- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php @@ -143,6 +143,9 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'phabricator.auth-permanent', 'phabricator.application-id', 'phabricator.application-secret', + 'maniphest.priorities.unbreak-now', + 'maniphest.priorities.needs-triage', + 'welcome.html', ); $ancient_config = array_fill_keys($auth_config, $reason_auth); @@ -332,6 +335,19 @@ final class PhabricatorExtraConfigSetupCheck extends PhabricatorSetupCheck { 'ui.custom-header' => pht( 'This option has been replaced with `ui.logo`, which provides more '. 'flexible configuration options.'), + + 'welcome.html' => pht( + 'This option has been removed, you can use Dashboards to provide '. + 'homepage customization. See T11533 for more details.'), + + 'maniphest.priorities.unbreak-now' => pht( + 'This option has been removed, you can use Dashboards to provide '. + 'homepage customization. See T11533 for more details.'), + + 'maniphest.priorities.needs-triage' => pht( + 'This option has been removed, you can use Dashboards to provide '. + 'homepage customization. See T11533 for more details.'), + ); return $ancient_config; diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php index c486ce0db1..6efd27d9a3 100644 --- a/src/applications/config/option/PhabricatorCoreConfigOptions.php +++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php @@ -213,10 +213,6 @@ final class PhabricatorCoreConfigOptions ->setLocked(true) ->setDescription( pht('Customized settings for Phabricator applications.')), - $this->newOption('welcome.html', 'string', null) - ->setLocked(true) - ->setDescription( - pht('Custom HTML to show on the main Phabricator dashboard.')), $this->newOption('phabricator.cache-namespace', 'string', 'phabricator') ->setLocked(true) ->setDescription(pht('Cache namespace.')), diff --git a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php index dc140d380a..21b76d1f33 100644 --- a/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php +++ b/src/applications/maniphest/config/PhabricatorManiphestConfigOptions.php @@ -356,28 +356,6 @@ EOTEXT 'string', '[Maniphest]') ->setDescription(pht('Subject prefix for Maniphest mail.')), - $this->newOption( - 'maniphest.priorities.unbreak-now', - 'int', - 100) - ->setSummary(pht('Priority used to populate "Unbreak Now" on home.')) - ->setDescription( - pht( - 'Temporary setting. If set, this priority is used to populate the '. - '"Unbreak Now" panel on the home page. You should adjust this if '. - 'you adjust priorities using `%s`.', - 'maniphest.priorities')), - $this->newOption( - 'maniphest.priorities.needs-triage', - 'int', - 90) - ->setSummary(pht('Priority used to populate "Needs Triage" on home.')) - ->setDescription( - pht( - 'Temporary setting. If set, this priority is used to populate the '. - '"Needs Triage" panel on the home page. You should adjust this if '. - 'you adjust priorities using `%s`.', - 'maniphest.priorities')), $this->newOption('maniphest.points', $points_type, array()) ->setSummary(pht('Configure point values for tasks.')) ->setDescription($points_description) From 9a52492f1b2c0449410d8935d35fb71ed562e9a3 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Thu, 25 Aug 2016 14:48:00 -0400 Subject: [PATCH 22/27] Added autopatch to remove ponder vote data Summary: Fixes T9117. Adds a migration to remove ponder vote data. Test Plan: I added a bunch of lines to phabricator_user.edge with type 18 and they were successfully removed by this patch Reviewers: #blessed_reviewers, epriestley Reviewed By: #blessed_reviewers, epriestley Subscribers: epriestley, yelirekim Maniphest Tasks: T9117 Differential Revision: https://secure.phabricator.com/D16452 --- resources/sql/autopatches/20160825.ponder.sql | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 resources/sql/autopatches/20160825.ponder.sql diff --git a/resources/sql/autopatches/20160825.ponder.sql b/resources/sql/autopatches/20160825.ponder.sql new file mode 100644 index 0000000000..73be3a781e --- /dev/null +++ b/resources/sql/autopatches/20160825.ponder.sql @@ -0,0 +1,7 @@ +/* Removes Ponder vote data. */ + +DELETE FROM {$NAMESPACE}_ponder.edge + WHERE type IN (17, 18, 19, 20); + +DELETE FROM {$NAMESPACE}_user.edge + WHERE type IN (17, 18, 19, 20); From 2b185daf7e870dd2325b3b461b3c36892e087c5a Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 25 Aug 2016 21:34:24 -0700 Subject: [PATCH 23/27] Wrap long text in setup issues Summary: Really long text strings break this ui when first setting up Phabricator. Test Plan: MySQL sql_mode setup issue. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D16455 --- resources/celerity/map.php | 4 ++-- webroot/rsrc/css/application/config/setup-issue.css | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index bec64fab6e..9fd15abad3 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -42,7 +42,7 @@ return array( 'rsrc/css/application/config/config-options.css' => '0ede4c9b', 'rsrc/css/application/config/config-template.css' => '8e6c6fcd', 'rsrc/css/application/config/config-welcome.css' => '035aa483', - 'rsrc/css/application/config/setup-issue.css' => 'db7e9c40', + 'rsrc/css/application/config/setup-issue.css' => 'f794cfc3', 'rsrc/css/application/config/unhandled-exception.css' => '4c96257a', 'rsrc/css/application/conpherence/durable-column.css' => '86396117', 'rsrc/css/application/conpherence/menu.css' => 'f99fee4c', @@ -891,7 +891,7 @@ return array( 'releeph-preview-branch' => 'b7a6f4a5', 'releeph-request-differential-create-dialog' => '8d8b92cd', 'releeph-request-typeahead-css' => '667a48ae', - 'setup-issue-css' => 'db7e9c40', + 'setup-issue-css' => 'f794cfc3', 'sprite-login-css' => '60e8560e', 'sprite-tokens-css' => '9cdfd599', 'syntax-default-css' => '9923583c', diff --git a/webroot/rsrc/css/application/config/setup-issue.css b/webroot/rsrc/css/application/config/setup-issue.css index 0d66e616ff..ae54394b4c 100644 --- a/webroot/rsrc/css/application/config/setup-issue.css +++ b/webroot/rsrc/css/application/config/setup-issue.css @@ -38,6 +38,7 @@ .setup-issue table td { border: 1px solid #BFCFDA; padding: 8px; + word-break: break-word; } .setup-issue pre { From 72a03dc03e7dd8681e21d643a5b584df85ae6dfe Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 25 Aug 2016 15:45:09 -0700 Subject: [PATCH 24/27] Add a setup warning for "always_populate_raw_post_data" Summary: Fixes T9235. When the stars align, PHP 5.6 or newer emits a deprecation warning on startup about "always_populate_raw_post_data" which occurs too early for us to intercept and can break responses by adding garbage to the output. These settings appear to be sufficient: ``` always_populate_raw_post_data = 1 display_errors = 1 display_startup_errors = 1 error_reporting = -1 ``` Then make a request with an unusual content type: ``` $ curl -X POST -H "Content-Type: application/json" -d "{foo: bar}" http://phabricator.example.com/ ``` This triggers the warning: ```
Deprecated: Automatically populating $HTTP_RAW_POST_DATA is deprecated and will be removed in a future version. To avoid this warning set 'always_populate_raw_post_data' to '-1' in php.ini and use the php://input stream instead. in Unknown on line 0

... ``` To avoid this, just instruct administrators to set this value to "-1", which completely disables the feature and silences the warning. Test Plan: - Reproduced this issue by following the instructions above. - Triggered the setup issue locally and read all the captivating prose: {F1786911} - Made the configuration change it directed me to, saw the setup issue resolve. Reviewers: jcox Reviewed By: jcox Maniphest Tasks: T9235 Differential Revision: https://secure.phabricator.com/D16454 --- .../check/PhabricatorPHPConfigSetupCheck.php | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php index b3fd03fd19..ec61b4764d 100644 --- a/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php +++ b/src/applications/config/check/PhabricatorPHPConfigSetupCheck.php @@ -224,5 +224,26 @@ final class PhabricatorPHPConfigSetupCheck extends PhabricatorSetupCheck { ->setSummary($summary) ->setMessage($message); } + + $raw_post_data = (int)ini_get('always_populate_raw_post_data'); + if ($raw_post_data != -1) { + $summary = pht( + 'PHP setting "%s" should be set to "-1" to avoid deprecation '. + 'warnings.', + 'always_populate_raw_post_data'); + + $message = pht( + 'The "%s" key is set to some value other than "-1" in your PHP '. + 'configuration. This can cause PHP to raise deprecation warnings '. + 'during process startup. Set this option to "-1" to prevent these '. + 'warnings from appearing.', + 'always_populate_raw_post_data'); + + $this->newIssue('php.always_populate_raw_post_data') + ->setName(pht('Disable PHP %s', 'always_populate_raw_post_data')) + ->setSummary($summary) + ->setMessage($message) + ->addPHPConfig('always_populate_raw_post_data'); + } } } From d952dd591266e3e44d50f45ec9646effc2b6d72e Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 26 Aug 2016 05:21:06 -0700 Subject: [PATCH 25/27] When importing Git repositories, treat out-of-range timestamps as the current time Summary: Fixes T11537. See that task for discussion. Although we could accommodate these faithfully, it requires a huge migration and affects one repository on one install which was written with buggy tools. At least for now, just replace out-of-32-bit-range epoch values with the current time, which is often somewhat close to the real value. Test Plan: - Following the instructions in T11537, created commits in 40,000 AD. - Tried to import them, reproducing the "epoch" database issue. - Applied the patch. - Successfully imported future-commits, with some liberties around commit dates. Note that author date (not stored in an `epoch` column) is still shown faithfully: {F1789302} Reviewers: chad, avivey Reviewed By: avivey Maniphest Tasks: T11537 Differential Revision: https://secure.phabricator.com/D16456 --- .../engine/PhabricatorRepositoryDiscoveryEngine.php | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php index a4f697da0c..9ccc72b8de 100644 --- a/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php +++ b/src/applications/repository/engine/PhabricatorRepositoryDiscoveryEngine.php @@ -406,9 +406,17 @@ final class PhabricatorRepositoryDiscoveryEngine $refs = array(); foreach ($commits as $commit) { + $epoch = $stream->getCommitDate($commit); + + // If the epoch doesn't fit into a uint32, treat it as though it stores + // the current time. For discussion, see T11537. + if ($epoch > 0xFFFFFFFF) { + $epoch = PhabricatorTime::getNow(); + } + $refs[] = id(new PhabricatorRepositoryCommitRef()) ->setIdentifier($commit) - ->setEpoch($stream->getCommitDate($commit)) + ->setEpoch($epoch) ->setCanCloseImmediately($close_immediately) ->setParents($stream->getParents($commit)); } From 90294713e8508a4df40918e9bf47956b3e7dd167 Mon Sep 17 00:00:00 2001 From: epriestley Date: Fri, 26 Aug 2016 05:34:22 -0700 Subject: [PATCH 26/27] Use phutil_json_encode() in AphrontResponse to raise better errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: Ref T11524. This problem was more difficult to diagnose than necessary because we swallow errors silently in `AphontResponse` when emitting JSON responses. Instead of using `json_encode()`, use `phutil_json_encode()` which throws on failure. Test Plan: Old behavior was HTTP 200 with no body. New behavior is HTTP 500 with this message: ``` [2016-08-26 07:33:59] EXCEPTION: (HTTPFutureHTTPResponseStatus) [HTTP/500] Internal Server Error Exception: Failed to JSON encode value (#5: Malformed UTF-8 characters, possibly incorrectly encoded): Dictionary value at key "result" is not valid UTF8, and cannot be JSON encoded: diff --git a/latin1.txt b/latin1.txt new file mode 100644 index 0000000..ce6c927 --- /dev/null +++ b/latin1.txt @@ -0,0 +1 @@ +<�> . at [/src/future/http/BaseHTTPFuture.php:339] ``` Reviewers: chad, avivey Reviewed By: avivey Maniphest Tasks: T11524 Differential Revision: https://secure.phabricator.com/D16457 --- src/aphront/response/AphrontResponse.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/aphront/response/AphrontResponse.php b/src/aphront/response/AphrontResponse.php index dbd60d473d..b213244f1c 100644 --- a/src/aphront/response/AphrontResponse.php +++ b/src/aphront/response/AphrontResponse.php @@ -162,7 +162,7 @@ abstract class AphrontResponse extends Phobject { $object, array(__CLASS__, 'processValueForJSONEncoding')); - $response = json_encode($object); + $response = phutil_json_encode($object); // Prevent content sniffing attacks by encoding "<" and ">", so browsers // won't try to execute the document as HTML even if they ignore From 067d12d7168a580bce695cb26331df785f86c2e9 Mon Sep 17 00:00:00 2001 From: Josh Cox Date: Thu, 25 Aug 2016 20:53:08 -0400 Subject: [PATCH 27/27] Converted the pinned applications selector to a typeahead. Summary: Fixes T11513. Previously the selector was just a giant dropdown which was just... just too much. Now there's a handy typeahead. Test Plan: Happy Path: Go to `Settings -> Home Page -> Pin Application`, start typing in the form then select one of the options. Click on "Pin Application". The application should now be in the list. Other paths: - Type nothing into the box and submit, nothing should happen. - Choose an application that is already pinned. The list should stay the same. - Type nonsense into the box and submit, nothing should happen. Reviewers: epriestley, #blessed_reviewers Reviewed By: epriestley, #blessed_reviewers Subscribers: chad, Korvin, epriestley, yelirekim Maniphest Tasks: T11513 Differential Revision: https://secure.phabricator.com/D16459 --- .../PhabricatorApplicationDatasource.php | 4 +++- ...habricatorHomePreferencesSettingsPanel.php | 22 ++++++++++++++----- 2 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/applications/meta/typeahead/PhabricatorApplicationDatasource.php b/src/applications/meta/typeahead/PhabricatorApplicationDatasource.php index 176c3554cf..7dddeda53b 100644 --- a/src/applications/meta/typeahead/PhabricatorApplicationDatasource.php +++ b/src/applications/meta/typeahead/PhabricatorApplicationDatasource.php @@ -38,7 +38,9 @@ final class PhabricatorApplicationDatasource ->setDisplayType($application->getShortDescription()) ->setImageuRI($application->getIconURI()) ->setPriorityType('apps') - ->setImageSprite('phabricator-search-icon '.$img); + ->setImageSprite('phabricator-search-icon '.$img) + ->setIcon($application->getIcon()) + ->addAttribute($application->getShortDescription()); } return $this->filterResultsAgainstTokens($results); diff --git a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php index fafe62ee4e..d12db30f83 100644 --- a/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php +++ b/src/applications/settings/panel/PhabricatorHomePreferencesSettingsPanel.php @@ -67,7 +67,19 @@ final class PhabricatorHomePreferencesSettingsPanel unset($options['PhabricatorApplicationsApplication']); if ($request->isFormPost()) { - $pin = $request->getStr('pin'); + $pins = $request->getArr('pin'); + $phid = head($pins); + $app = id(new PhabricatorApplicationQuery()) + ->setViewer($viewer) + ->withPHIDs(array($phid)) + ->executeOne(); + if ($app) { + $pin = get_class($app); + } else { + // This likely means the user submitted an empty form + // which will cause nothing to happen. + $pin = ''; + } if (isset($options[$pin]) && !in_array($pin, $pinned)) { $pinned[] = $pin; @@ -78,18 +90,18 @@ final class PhabricatorHomePreferencesSettingsPanel } } - $options_control = id(new AphrontFormSelectControl()) + $options_control = id(new AphrontFormTokenizerControl()) ->setName('pin') ->setLabel(pht('Application')) - ->setOptions($options) - ->setDisabledOptions(array_keys($app_list)); + ->setDatasource(new PhabricatorApplicationDatasource()) + ->setLimit(1); $form = id(new AphrontFormView()) ->setViewer($viewer) ->addHiddenInput('add', 'true') ->appendRemarkupInstructions( pht('Choose an application to pin to your home page.')) - ->appendChild($options_control); + ->appendControl($options_control); return $this->newDialog() ->setWidth(AphrontDialogView::WIDTH_FORM)