From 97ff7fe259d4cc2abdc7988bbf11ca5c55f1c442 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Sat, 6 Apr 2013 09:25:13 -0700
Subject: [PATCH] Make "isInstalled()" respect beta apps

Summary:
Currently, `isInstalled()` and `getAllInstalledApplications()` are inconsistent:

  - `isInstalled()` returns true for beta apps, even if `phabricator.show-beta-applications` is false.
  - `getAllInstalledApplications()` omits beta apps if `phabricator.show-beta-applications` is false.

Making the beta config control installs (not just homepage visibility) makes far more sense as we roll out more thorough application integrations.

Make `isInstalled()` respect beta, and clean up some callsites.

D5602 builds on this.

Test Plan: Installed/uninstalled beta apps, verified Conpherence menu/panel and other application integrations dropped out of the UI.

Reviewers: vrana, btrahan

Reviewed By: vrana

CC: aran

Differential Revision: https://secure.phabricator.com/D5603
---
 .../base/PhabricatorApplication.php            | 18 ++++++++----------
 .../option/PhabricatorCoreConfigOptions.php    | 16 +++++++++++++---
 ...atorSettingsPanelConpherencePreferences.php |  9 ++-------
 src/view/page/menu/PhabricatorMainMenuView.php |  9 ++++-----
 4 files changed, 27 insertions(+), 25 deletions(-)

diff --git a/src/applications/base/PhabricatorApplication.php b/src/applications/base/PhabricatorApplication.php
index 54610001cd..707f0476cc 100644
--- a/src/applications/base/PhabricatorApplication.php
+++ b/src/applications/base/PhabricatorApplication.php
@@ -63,13 +63,18 @@ abstract class PhabricatorApplication {
   }
 
   public function isInstalled() {
-    $uninstalled = PhabricatorEnv::getEnvConfig(
-      'phabricator.uninstalled-applications');
-
     if (!$this->canUninstall()) {
       return true;
     }
 
+    $beta = PhabricatorEnv::getEnvConfig('phabricator.show-beta-applications');
+    if (!$beta && $this->isBeta()) {
+      return false;
+    }
+
+    $uninstalled = PhabricatorEnv::getEnvConfig(
+      'phabricator.uninstalled-applications');
+
     return empty($uninstalled[get_class($this)]);
   }
 
@@ -270,9 +275,6 @@ abstract class PhabricatorApplication {
   public static function getAllInstalledApplications() {
     static $applications;
 
-    $show_beta = PhabricatorEnv::getEnvConfig(
-      'phabricator.show-beta-applications');
-
     if (empty($applications)) {
       $all_applications = self::getAllApplications();
       $apps = array();
@@ -285,10 +287,6 @@ abstract class PhabricatorApplication {
           continue;
         }
 
-        if (!$show_beta && $app->isBeta()) {
-          continue;
-        }
-
         $apps[] = $app;
       }
 
diff --git a/src/applications/config/option/PhabricatorCoreConfigOptions.php b/src/applications/config/option/PhabricatorCoreConfigOptions.php
index a5910c2441..604cedf476 100644
--- a/src/applications/config/option/PhabricatorCoreConfigOptions.php
+++ b/src/applications/config/option/PhabricatorCoreConfigOptions.php
@@ -54,9 +54,19 @@ final class PhabricatorCoreConfigOptions
       $this->newOption('phabricator.show-beta-applications', 'bool', false)
         ->setBoolOptions(
           array(
-            pht('Visible'),
-            pht('Invisible')
-          ))->setDescription(pht('Show beta applications on the home page.')),
+            pht('Install Beta Applications'),
+            pht('Uninstall Beta Applications')
+          ))
+        ->setDescription(
+          pht(
+            "Phabricator includes 'Beta' applications which are in an early ".
+            "stage of development. They range from very rough prototypes to ".
+            "relatively complete (but unpolished) applications.\n\n".
+            "By default, Beta applications are not installed. You can enable ".
+            "this option to install them if you're interested in previewing ".
+            "upcoming features.\n\n".
+            "After enabling Beta applications, you can selectively uninstall ".
+            "them (like normal applications).")),
       $this->newOption('phabricator.serious-business', 'bool', false)
         ->setBoolOptions(
           array(
diff --git a/src/applications/settings/panel/PhabricatorSettingsPanelConpherencePreferences.php b/src/applications/settings/panel/PhabricatorSettingsPanelConpherencePreferences.php
index 778f0c3888..838d88f113 100644
--- a/src/applications/settings/panel/PhabricatorSettingsPanelConpherencePreferences.php
+++ b/src/applications/settings/panel/PhabricatorSettingsPanelConpherencePreferences.php
@@ -4,14 +4,9 @@ final class PhabricatorSettingsPanelConpherencePreferences
   extends PhabricatorSettingsPanel {
 
   public function isEnabled() {
-    // TODO - epriestley - resolve isBeta and isInstalled for
-    // PhabricatorApplication
-    $app = PhabricatorApplication::getByClass(
+    $conpherence_app = PhabricatorApplication::getByClass(
       'PhabricatorApplicationConpherence');
-    $is_prod = !$app->isBeta();
-    $allow_beta =
-      PhabricatorEnv::getEnvConfig('phabricator.show-beta-applications');
-    return ($is_prod || $allow_beta) && $app->isInstalled();
+    return $conpherence_app->isInstalled();
   }
 
   public function getPanelKey() {
diff --git a/src/view/page/menu/PhabricatorMainMenuView.php b/src/view/page/menu/PhabricatorMainMenuView.php
index 91bbbdca82..2c56e7f50c 100644
--- a/src/view/page/menu/PhabricatorMainMenuView.php
+++ b/src/view/page/menu/PhabricatorMainMenuView.php
@@ -260,12 +260,11 @@ final class PhabricatorMainMenuView extends AphrontView {
       'alert-notifications',
     );
 
-    $conpherence = id(new PhabricatorApplicationConpherence())->isBeta();
-    $allow_beta =
-      PhabricatorEnv::getEnvConfig('phabricator.show-beta-applications');
-    $message_tag = '';
+    $conpherence_application = PhabricatorApplication::getByClass(
+      'PhabricatorApplicationConpherence');
 
-    if (!$conpherence || $allow_beta) {
+    $message_tag = '';
+    if ($conpherence_application->isInstalled()) {
       $message_id = celerity_generate_unique_node_id();
       $message_count_id = celerity_generate_unique_node_id();