Allow more granularity on real-time notifications

Summary: Fixes T12792. Expands the Notifications to "web, desktop, both, or none" for real-time notifications in settings. Test Plan: Test with "test notifications" button, and while logged into two accounts with each of the 4 settings. Reviewers: epriestley Reviewed By: epriestley Spies: Korvin Maniphest Tasks: T12792 Differential Revision: https://secure.phabricator.com/D18457
2024-11-22 06:42:42 +01:00 · 2017-08-23 14:35:02 -07:00 · 2017-08-23 14:35:02 -07:00 · 63bd1784b0
commit 63bd1784b0
parent 8c4f5aba33
10 changed files with 140 additions and 79 deletions
--- a/resources/celerity/map.php
+++ b/resources/celerity/map.php
@ -10,7 +10,7 @@ return array(
    'conpherence.pkg.css' => 'e68cf1fa',
    'conpherence.pkg.js' => 'b5b51108',
    'core.pkg.css' => 'fe4effd6',
-    'core.pkg.js' => '5d80e0db',
+    'core.pkg.js' => '396dee49',
    'darkconsole.pkg.js' => '1f9a31bc',
    'differential.pkg.css' => '45951e9e',
    'differential.pkg.js' => 'b71b8c5d',
@ -375,9 +375,9 @@ return array(
    'rsrc/image/texture/table_header_tall.png' => 'd56b434f',
    'rsrc/js/application/aphlict/Aphlict.js' => 'e1d4b11a',
    'rsrc/js/application/aphlict/behavior-aphlict-dropdown.js' => 'caade6f2',
-    'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => '3c547a81',
+    'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => 'a14cbdfc',
    'rsrc/js/application/aphlict/behavior-aphlict-status.js' => '5e2634b9',
-    'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => 'd5a2d665',
+    'rsrc/js/application/aphlict/behavior-desktop-notifications-control.js' => '27ca6289',
    'rsrc/js/application/calendar/behavior-day-view.js' => '4b3c4443',
    'rsrc/js/application/calendar/behavior-event-all-day.js' => 'b41537c9',
    'rsrc/js/application/calendar/behavior-month-view.js' => 'fe33e256',
@ -468,7 +468,7 @@ return array(
    'rsrc/js/core/KeyboardShortcut.js' => '1ae869f2',
    'rsrc/js/core/KeyboardShortcutManager.js' => 'c19dd9b9',
    'rsrc/js/core/MultirowRowManager.js' => 'b5d57730',
-    'rsrc/js/core/Notification.js' => 'ccf1cbf8',
+    'rsrc/js/core/Notification.js' => '5c3349b2',
    'rsrc/js/core/Prefab.js' => 'c5af80a2',
    'rsrc/js/core/ShapedRequest.js' => '7cbe244b',
    'rsrc/js/core/TextAreaUtils.js' => '320810c8',
@ -587,7 +587,7 @@ return array(
    'javelin-aphlict' => 'e1d4b11a',
    'javelin-behavior' => '61cbc29a',
    'javelin-behavior-aphlict-dropdown' => 'caade6f2',
-    'javelin-behavior-aphlict-listen' => '3c547a81',
+    'javelin-behavior-aphlict-listen' => 'a14cbdfc',
    'javelin-behavior-aphlict-status' => '5e2634b9',
    'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884',
    'javelin-behavior-aphront-drag-and-drop-textarea' => '484a6e22',
@ -612,7 +612,7 @@ return array(
    'javelin-behavior-dashboard-query-panel-select' => '453c5375',
    'javelin-behavior-dashboard-tab-panel' => 'd4eecc63',
    'javelin-behavior-day-view' => '4b3c4443',
-    'javelin-behavior-desktop-notifications-control' => 'd5a2d665',
+    'javelin-behavior-desktop-notifications-control' => '27ca6289',
    'javelin-behavior-detect-timezone' => '4c193c96',
    'javelin-behavior-device' => 'bb1dd507',
    'javelin-behavior-diff-preview-link' => '051c7832',
@ -791,7 +791,7 @@ return array(
    'phabricator-keyboard-shortcut-manager' => 'c19dd9b9',
    'phabricator-main-menu-view' => '16053029',
    'phabricator-nav-view-css' => 'faf6a6fc',
-    'phabricator-notification' => 'ccf1cbf8',
+    'phabricator-notification' => '5c3349b2',
    'phabricator-notification-css' => '3f6c89c9',
    'phabricator-notification-menu-css' => '73fefdfa',
    'phabricator-object-selector-css' => '85ee8ce6',
@ -1058,6 +1058,13 @@ return array(
      'phabricator-drag-and-drop-file-upload',
      'javelin-workboard-board',
    ),
+    '27ca6289' => array(
+      'javelin-behavior',
+      'javelin-stratcom',
+      'javelin-dom',
+      'javelin-uri',
+      'phabricator-notification',
+    ),
    '2926fff2' => array(
      'javelin-behavior',
      'javelin-dom',
@ -1115,20 +1122,6 @@ return array(
      'javelin-dom',
      'javelin-magical-init',
    ),
-    '3c547a81' => array(
-      'javelin-behavior',
-      'javelin-aphlict',
-      'javelin-stratcom',
-      'javelin-request',
-      'javelin-uri',
-      'javelin-dom',
-      'javelin-json',
-      'javelin-router',
-      'javelin-util',
-      'javelin-leader',
-      'javelin-sound',
-      'phabricator-notification',
-    ),
    '3cb0b2fc' => array(
      'javelin-behavior',
      'javelin-dom',
@ -1338,6 +1331,13 @@ return array(
      'javelin-vector',
      'javelin-dom',
    ),
+    '5c3349b2' => array(
+      'javelin-install',
+      'javelin-dom',
+      'javelin-stratcom',
+      'javelin-util',
+      'phabricator-notification-css',
+    ),
    '5c54cbf3' => array(
      'javelin-behavior',
      'javelin-stratcom',
@ -1676,6 +1676,20 @@ return array(
      'javelin-util',
      'phabricator-keyboard-shortcut',
    ),
+    'a14cbdfc' => array(
+      'javelin-behavior',
+      'javelin-aphlict',
+      'javelin-stratcom',
+      'javelin-request',
+      'javelin-uri',
+      'javelin-dom',
+      'javelin-json',
+      'javelin-router',
+      'javelin-util',
+      'javelin-leader',
+      'javelin-sound',
+      'phabricator-notification',
+    ),
    'a37126bd' => array(
      'javelin-install',
      'javelin-dom',
@ -1951,13 +1965,6 @@ return array(
    'cae95e89' => array(
      'syntax-default-css',
    ),
-    'ccf1cbf8' => array(
-      'javelin-install',
-      'javelin-dom',
-      'javelin-stratcom',
-      'javelin-util',
-      'phabricator-notification-css',
-    ),
    'cd2b9b77' => array(
      'phui-oi-list-view-css',
    ),
@ -1996,13 +2003,6 @@ return array(
      'javelin-dom',
      'javelin-stratcom',
    ),
-    'd5a2d665' => array(
-      'javelin-behavior',
-      'javelin-stratcom',
-      'javelin-dom',
-      'javelin-uri',
-      'phabricator-notification',
-    ),
    'd6a7e717' => array(
      'multirow-row-manager',
      'javelin-install',
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@ -2647,8 +2647,6 @@ phutil_register_library_map(array(
    'PhabricatorDebugController' => 'applications/system/controller/PhabricatorDebugController.php',
    'PhabricatorDefaultRequestExceptionHandler' => 'aphront/handler/PhabricatorDefaultRequestExceptionHandler.php',
    'PhabricatorDefaultSyntaxStyle' => 'infrastructure/syntax/PhabricatorDefaultSyntaxStyle.php',
-    'PhabricatorDesktopNotificationsSetting' => 'applications/settings/setting/PhabricatorDesktopNotificationsSetting.php',
-    'PhabricatorDesktopNotificationsSettingsPanel' => 'applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php',
    'PhabricatorDestructibleCodex' => 'applications/system/codex/PhabricatorDestructibleCodex.php',
    'PhabricatorDestructibleCodexInterface' => 'applications/system/interface/PhabricatorDestructibleCodexInterface.php',
    'PhabricatorDestructibleInterface' => 'applications/system/interface/PhabricatorDestructibleInterface.php',
@ -3214,6 +3212,8 @@ phutil_register_library_map(array(
    'PhabricatorNotificationTestFeedStory' => 'applications/notification/feed/PhabricatorNotificationTestFeedStory.php',
    'PhabricatorNotificationUIExample' => 'applications/uiexample/examples/PhabricatorNotificationUIExample.php',
    'PhabricatorNotificationsApplication' => 'applications/notification/application/PhabricatorNotificationsApplication.php',
+    'PhabricatorNotificationsSetting' => 'applications/settings/setting/PhabricatorNotificationsSetting.php',
+    'PhabricatorNotificationsSettingsPanel' => 'applications/settings/panel/PhabricatorNotificationsSettingsPanel.php',
    'PhabricatorNuanceApplication' => 'applications/nuance/application/PhabricatorNuanceApplication.php',
    'PhabricatorOAuth1AuthProvider' => 'applications/auth/provider/PhabricatorOAuth1AuthProvider.php',
    'PhabricatorOAuth1SecretTemporaryTokenType' => 'applications/auth/provider/PhabricatorOAuth1SecretTemporaryTokenType.php',
@ -7952,8 +7952,6 @@ phutil_register_library_map(array(
    'PhabricatorDebugController' => 'PhabricatorController',
    'PhabricatorDefaultRequestExceptionHandler' => 'PhabricatorRequestExceptionHandler',
    'PhabricatorDefaultSyntaxStyle' => 'PhabricatorSyntaxStyle',
-    'PhabricatorDesktopNotificationsSetting' => 'PhabricatorInternalSetting',
-    'PhabricatorDesktopNotificationsSettingsPanel' => 'PhabricatorSettingsPanel',
    'PhabricatorDestructibleCodex' => 'Phobject',
    'PhabricatorDestructionEngine' => 'Phobject',
    'PhabricatorDestructionEngineExtension' => 'Phobject',
@ -8577,6 +8575,8 @@ phutil_register_library_map(array(
    'PhabricatorNotificationTestFeedStory' => 'PhabricatorFeedStory',
    'PhabricatorNotificationUIExample' => 'PhabricatorUIExample',
    'PhabricatorNotificationsApplication' => 'PhabricatorApplication',
+    'PhabricatorNotificationsSetting' => 'PhabricatorInternalSetting',
+    'PhabricatorNotificationsSettingsPanel' => 'PhabricatorSettingsPanel',
    'PhabricatorNuanceApplication' => 'PhabricatorApplication',
    'PhabricatorOAuth1AuthProvider' => 'PhabricatorOAuthAuthProvider',
    'PhabricatorOAuth1SecretTemporaryTokenType' => 'PhabricatorAuthTemporaryTokenType',
--- a/src/applications/notification/builder/PhabricatorNotificationBuilder.php
+++ b/src/applications/notification/builder/PhabricatorNotificationBuilder.php
@ -145,13 +145,16 @@ final class PhabricatorNotificationBuilder extends Phobject {
    $dict = array();

    $viewer = $this->user;
-    $desktop_key = PhabricatorDesktopNotificationsSetting::SETTINGKEY;
-    $desktop_enabled = $viewer->getUserSetting($desktop_key);
+    $key = PhabricatorNotificationsSetting::SETTINGKEY;
+    $setting = $viewer->getUserSetting($key);
+    $desktop_ready = PhabricatorNotificationsSetting::desktopReady($setting);
+    $web_ready = PhabricatorNotificationsSetting::webReady($setting);

    foreach ($stories as $story) {
      if ($story instanceof PhabricatorApplicationTransactionFeedStory) {
        $dict[] = array(
-          'desktopReady' => $desktop_enabled,
+          'desktopReady' => $desktop_ready,
+          'webReady'     => $web_ready,
          'title'        => $story->renderText(),
          'body'         => $story->renderTextBody(),
          'href'         => $story->getURI(),
@ -159,7 +162,8 @@ final class PhabricatorNotificationBuilder extends Phobject {
        );
      } else if ($story instanceof PhabricatorNotificationTestFeedStory) {
        $dict[] = array(
-          'desktopReady' => $desktop_enabled,
+          'desktopReady' => $desktop_ready,
+          'webReady'     => $web_ready,
          'title'        => pht('Test Notification'),
          'body'         => $story->renderText(),
          'href'         => null,
@ -168,6 +172,7 @@ final class PhabricatorNotificationBuilder extends Phobject {
      } else {
        $dict[] = array(
          'desktopReady' => false,
+          'webReady'     => false,
          'title'        => null,
          'body'         => null,
          'href'         => null,
--- a/src/applications/notification/controller/PhabricatorNotificationIndividualController.php
+++ b/src/applications/notification/controller/PhabricatorNotificationIndividualController.php
@ -42,6 +42,7 @@ final class PhabricatorNotificationIndividualController
      'pertinent'         => true,
      'primaryObjectPHID' => $story->getPrimaryObjectPHID(),
      'desktopReady'      => $data['desktopReady'],
+      'webReady'          => $data['webReady'],
      'href'              => $data['href'],
      'icon'              => $data['icon'],
      'title'             => $data['title'],
--- a/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php
+++ b/src/applications/settings/panel/PhabricatorDesktopNotificationsSettingsPanel.php
@ -1,6 +1,6 @@
 <?php

-final class PhabricatorDesktopNotificationsSettingsPanel
+final class PhabricatorNotificationsSettingsPanel
  extends PhabricatorSettingsPanel {

  public function isEnabled() {
@ -14,11 +14,11 @@ final class PhabricatorDesktopNotificationsSettingsPanel
  }

  public function getPanelKey() {
-    return 'desktopnotifications';
+    return 'notifications';
  }

  public function getPanelName() {
-    return pht('Desktop Notifications');
+    return pht('Notifications');
  }

  public function getPanelGroupKey() {
@ -29,7 +29,7 @@ final class PhabricatorDesktopNotificationsSettingsPanel
    $viewer = $this->getViewer();
    $preferences = $this->getPreferences();

-    $notifications_key = PhabricatorDesktopNotificationsSetting::SETTINGKEY;
+    $notifications_key = PhabricatorNotificationsSetting::SETTINGKEY;
    $notifications_value = $preferences->getSettingValue($notifications_key);

    if ($request->isFormPost()) {
@ -43,7 +43,7 @@ final class PhabricatorDesktopNotificationsSettingsPanel
        ->setURI($this->getPanelURI('?saved=true'));
    }

-    $title = pht('Desktop Notifications');
+    $title = pht('Notifications');
    $control_id = celerity_generate_unique_node_id();
    $status_id = celerity_generate_unique_node_id();
    $browser_status_id = celerity_generate_unique_node_id();
@ -97,19 +97,31 @@ final class PhabricatorDesktopNotificationsSettingsPanel
        'id' => $message_id,
      ));

+    $saved_box = null;
+    if ($request->getBool('saved')) {
+      $saved_box = id(new PHUIInfoView())
+        ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
+        ->appendChild(pht('Changes saved.'));
+    }
+
    $status_box = id(new PHUIInfoView())
      ->setSeverity(PHUIInfoView::SEVERITY_NOTICE)
      ->setID($status_id)
      ->setIsHidden(true)
      ->appendChild($message_container);

+    $status_box = id(new PHUIBoxView())
+      ->addClass('mll mlr')
+      ->appendChild($status_box);
+
    $control_config = array(
       'controlID' => $control_id,
       'statusID' => $status_id,
       'messageID' => $message_id,
       'browserStatusID' => $browser_status_id,
       'defaultMode' => 0,
-       'desktopMode' => 1,
+       'desktop' => 1,
+       'desktopOnly' => 2,
       'cancelAsk' => $cancel_ask,
       'grantedAsk' => $accept_ask,
       'deniedAsk' => $reject_ask,
@ -127,16 +139,12 @@ final class PhabricatorDesktopNotificationsSettingsPanel
        ->setControlID($control_id)
        ->setName($notifications_key)
        ->setValue($notifications_value)
-        ->setOptions(
-          array(
-            1 => pht('Send Desktop Notifications Too'),
-            0 => pht('Send Application Notifications Only'),
-          ))
+        ->setOptions(PhabricatorNotificationsSetting::getOptionsMap())
        ->setCaption(
          pht(
-            'Should Phabricator send desktop notifications? These are sent '.
-            'in addition to the notifications within the Phabricator '.
-            'application.'))
+            'Phabricator can send real-time notifications to your web browser '.
+            'or to your desktop. Select where you\'d want to receive these '.
+            'real-time updates.'))
        ->initBehavior(
          'desktop-notifications-control',
          $control_config))
@ -154,12 +162,14 @@ final class PhabricatorDesktopNotificationsSettingsPanel
    $form_box = id(new PHUIObjectBoxView())
      ->setHeader(
        id(new PHUIHeaderView())
-        ->setHeader(pht('Desktop Notifications'))
+        ->setHeader(pht('Notifications'))
        ->addActionLink($test_button))
-      ->setForm($form)
      ->setBackground(PHUIObjectBoxView::BLUE_PROPERTY)
-      ->setInfoView($status_box)
-      ->setFormSaved($request->getBool('saved'));
+      ->appendChild(array(
+        $saved_box,
+        $status_box,
+        $form,
+      ));

    $browser_status_box = id(new PHUIInfoView())
      ->setID($browser_status_id)
--- a/src/applications/settings/setting/PhabricatorDesktopNotificationsSetting.php
+++ b/src/applications/settings/setting/PhabricatorDesktopNotificationsSetting.php
@ -1,12 +0,0 @@
-<?php
-
-final class PhabricatorDesktopNotificationsSetting
-  extends PhabricatorInternalSetting {
-
-  const SETTINGKEY = 'desktop-notifications';
-
-  public function getSettingName() {
-    return pht('Desktop Notifications');
-  }
-
-}
--- a/src/applications/settings/setting/PhabricatorNotificationsSetting.php
+++ b/src/applications/settings/setting/PhabricatorNotificationsSetting.php
@ -0,0 +1,44 @@
+<?php
+
+final class PhabricatorNotificationsSetting
+  extends PhabricatorInternalSetting {
+
+  const SETTINGKEY = 'desktop-notifications';
+
+  const WEB_ONLY = 0;
+  const WEB_AND_DESKTOP = 1;
+  const DESKTOP_ONLY = 2;
+  const NONE = 3;
+
+  public function getSettingName() {
+    return pht('Notifications');
+  }
+
+  public static function getOptionsMap() {
+    return array(
+      self::WEB_ONLY => pht('Web Only'),
+      self::WEB_AND_DESKTOP => pht('Web and Desktop'),
+      self::DESKTOP_ONLY => pht('Desktop Only'),
+      self::NONE => pht('No Notifications'),
+    );
+  }
+
+  public static function desktopReady($option) {
+    switch ($option) {
+      case self::WEB_AND_DESKTOP:
+      case self::DESKTOP_ONLY:
+        return true;
+    }
+    return false;
+  }
+
+  public static function webReady($option) {
+    switch ($option) {
+      case self::WEB_AND_DESKTOP:
+      case self::WEB_ONLY:
+        return true;
+    }
+    return false;
+  }
+
+}
--- a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js
+++ b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js
@ -82,6 +82,7 @@ JX.behavior('aphlict-listen', function(config) {
    new JX.Notification()
      .setContent(JX.$H(response.content))
      .setDesktopReady(response.desktopReady)
+      .setWebReady(response.webReady)
      .setKey(response.primaryObjectPHID)
      .setTitle(response.title)
      .setBody(response.body)
--- a/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js
+++ b/webroot/rsrc/js/application/aphlict/behavior-desktop-notifications-control.js
@ -84,12 +84,12 @@ JX.behavior('desktop-notifications-control', function(config, statics) {
          return;
        }
        var value = e.getTarget().value;
-        if (value == config.desktopMode) {
-          window.Notification.requestPermission(
-            function (permission) {
-              updateFormStatus(permission);
-              updateBrowserStatus(permission);
-            });
+        if ((value == config.desktop) || (value == config.desktopOnly)) {
+            window.Notification.requestPermission(
+              function (permission) {
+                updateFormStatus(permission);
+                updateBrowserStatus(permission);
+              });
        } else {
          var statusEl = JX.$(config.statusID);
          JX.DOM.hide(statusEl);
--- a/webroot/rsrc/js/core/Notification.js
+++ b/webroot/rsrc/js/core/Notification.js
@ -27,6 +27,7 @@ JX.install('Notification', {
    _hideTimer : null,
    _duration : 12000,
    _desktopReady : false,
+    _webReady : false,
    _key : null,
    _title : null,
    _body : null,
@ -35,6 +36,12 @@ JX.install('Notification', {

    show : function() {
      var self = JX.Notification;
+
+      // This person doesn't like any real-time notification
+      if (!this._desktopReady && !this._webReady) {
+        return;
+      }
+
      if (!this._visible) {
        this._visible = true;

@ -92,6 +99,11 @@ JX.install('Notification', {
      return this;
    },

+    setWebReady : function(ready) {
+      this._webReady = ready;
+      return this;
+    },
+
    setTitle : function(title) {
      this._title = title;
      return this;