Commit 8fe27800 authored by epriestley's avatar epriestley
Browse files

Don't show document types in search for uninstalled applications

Summary:
Fixes T4917. Currently, if a user doesn't have access to, e.g., Phriction, they still get a checkbox in the search results to search for Wiki Documents. Those results will be filtered anyway, so this is confusing at best.

Instead, bind PHID types to applications. This is a relatively tailored fix; some areas for potential future work:

  - Go through every PHID type and bind them all to applications. Vaguely nice to have, but doesn't get us anything for now.
  - If no searchable application is installed, we don't show you an error state. This isn't currently possible ("People" is always installed) but in the interest of generality we could throw an exception or something at least.
  - The elasticserach thing could probably constrain types to visible types, but we don't have a viewer there easily right now.

Test Plan: Uninstalled Phriction, saw the checkbox vanish.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4917

Differential Revision: https://secure.phabricator.com/D8904
parent 1d5731b1
......@@ -12,6 +12,10 @@ final class DifferentialPHIDTypeRevision extends PhabricatorPHIDType {
return pht('Revision');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationDifferential';
}
public function newObject() {
return new DifferentialRevision();
}
......
......@@ -12,6 +12,10 @@ final class ManiphestPHIDTypeTask extends PhabricatorPHIDType {
return pht('Task');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationManiphest';
}
public function newObject() {
return new ManiphestTask();
}
......
......@@ -12,6 +12,10 @@ final class PhabricatorPeoplePHIDTypeUser extends PhabricatorPHIDType {
return pht('User');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationPeople';
}
public function getTypeIcon() {
return 'policy-all';
}
......
......@@ -13,6 +13,20 @@ abstract class PhabricatorPHIDType {
return null;
}
/**
* Get the class name for the application this type belongs to.
*
* @return string|null Class name of the corresponding application, or null
* if the type is not bound to an application.
*/
public function getPHIDTypeApplicationClass() {
// TODO: Some day this should probably be abstract, but for now it only
// affects global search and there's no real burning need to go classify
// every PHID type.
return null;
}
/**
* Build a @{class:PhabricatorPolicyAwareQuery} to load objects of this type
* by PHID.
......@@ -103,6 +117,15 @@ abstract class PhabricatorPHIDType {
throw new Exception("Not implemented!");
}
/**
* Get all known PHID types.
*
* To get PHID types a given user has access to, see
* @{method:getAllInstalledTypes}.
*
* @return dict<string, PhabricatorPHIDType> Map of type constants to types.
*/
public static function getAllTypes() {
static $types;
if ($types === null) {
......@@ -133,4 +156,48 @@ abstract class PhabricatorPHIDType {
return $types;
}
/**
* Get all PHID types of applications installed for a given viewer.
*
* @param PhabricatorUser Viewing user.
* @return dict<string, PhabricatorPHIDType> Map of constants to installed
* types.
*/
public static function getAllInstalledTypes(PhabricatorUser $viewer) {
$all_types = self::getAllTypes();
$installed_types = array();
$app_classes = array();
foreach ($all_types as $key => $type) {
$app_class = $type->getPHIDTypeApplicationClass();
if ($app_class === null) {
// If the PHID type isn't bound to an application, include it as
// installed.
$installed_types[$key] = $type;
continue;
}
// Otherwise, we need to check if this application is installed before
// including the PHID type.
$app_classes[$app_class][$key] = $type;
}
if ($app_classes) {
$apps = id(new PhabricatorApplicationQuery())
->setViewer($viewer)
->withInstalled(true)
->withClasses(array_keys($app_classes))
->execute();
foreach ($apps as $app_class => $app) {
$installed_types += $app_classes[$app_class];
}
}
return $installed_types;
}
}
......@@ -12,6 +12,10 @@ final class PholioPHIDTypeMock extends PhabricatorPHIDType {
return pht('Mock');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationPholio';
}
public function newObject() {
return new PholioMock();
}
......
......@@ -12,6 +12,10 @@ final class PhrictionPHIDTypeDocument extends PhabricatorPHIDType {
return pht('Wiki Document');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationPhriction';
}
public function newObject() {
return new PhrictionDocument();
}
......
......@@ -12,6 +12,10 @@ final class PonderPHIDTypeQuestion extends PhabricatorPHIDType {
return pht('Question');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationPonder';
}
public function newObject() {
return new PonderQuestion();
}
......
......@@ -12,6 +12,10 @@ final class PhabricatorProjectPHIDTypeProject extends PhabricatorPHIDType {
return pht('Project');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationProject';
}
public function getTypeIcon() {
return 'policy-project';
}
......
......@@ -12,6 +12,10 @@ final class PhabricatorRepositoryPHIDTypeCommit extends PhabricatorPHIDType {
return pht('Commit');
}
public function getPHIDTypeApplicationClass() {
return 'PhabricatorApplicationDiffusion';
}
public function newObject() {
return new PhabricatorRepositoryCommit();
}
......
......@@ -96,7 +96,7 @@ final class PhabricatorSearchApplicationSearchEngine
$type_values = $saved->getParameter('types', array());
$type_values = array_fuse($type_values);
$types = self::getIndexableDocumentTypes();
$types = self::getIndexableDocumentTypes($this->requireViewer());
$types_control = id(new AphrontFormCheckboxControl())
->setLabel(pht('Document Types'));
......@@ -190,18 +190,21 @@ final class PhabricatorSearchApplicationSearchEngine
return parent::buildSavedQueryFromBuiltin($query_key);
}
public static function getIndexableDocumentTypes() {
public static function getIndexableDocumentTypes(
PhabricatorUser $viewer = null) {
// TODO: This is inelegant and not very efficient, but gets us reasonable
// results. It would be nice to do this more elegantly.
// TODO: We should hide types associated with applications the user can
// not access. There's no reasonable way to do this right now.
$indexers = id(new PhutilSymbolLoader())
->setAncestorClass('PhabricatorSearchDocumentIndexer')
->loadObjects();
$types = PhabricatorPHIDType::getAllTypes();
if ($viewer) {
$types = PhabricatorPHIDType::getAllInstalledTypes($viewer);
} else {
$types = PhabricatorPHIDType::getAllTypes();
}
$results = array();
foreach ($types as $type) {
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment