Enable notifications by default

Summary: I think we've sorted out enough of the problems with these to turn them on for everyone. The real-time component remains configuration-dependent.

Test Plan: Turned off "notification.enabled", still saw notifications.

Reviewers: btrahan, chad

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D4120

conf/default.conf.php

@@ -191,7 +191,10 @@ return array(
 
 // -- Notifications --------------------------------------------------------- //
 
-  'notification.enabled' => false,
+  // Set this to true to enable real-time notifications. You must also run a
+  // notification server for this to work. Consult the documentation in
+  // "Notifications User Guide: Setup and Configuration" for instructions.
+  'notification.enabled'      => false,
 
   // Client port for the realtime server to listen on, and for realtime clients
   // to connect to. Use "localhost" if you are running the notification server

src/applications/feed/PhabricatorFeedStoryPublisher.php

@@ -99,8 +99,8 @@ final class PhabricatorFeedStoryPublisher {
         implode(', ', $sql));
     }
 
+    $this->insertNotifications($chrono_key);
     if (PhabricatorEnv::getEnvConfig('notification.enabled')) {
-      $this->insertNotifications($chrono_key);
       $this->sendNotification($chrono_key);
     }
 

src/applications/notification/storage/PhabricatorFeedStoryNotification.php

@@ -18,25 +18,23 @@ final class PhabricatorFeedStoryNotification extends PhabricatorFeedDAO {
     PhabricatorUser $user,
     $object_phid) {
 
-    if (PhabricatorEnv::getEnvConfig('notification.enabled')) {
-      $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
+    $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
 
-      $notification_table = new PhabricatorFeedStoryNotification();
-      $conn = $notification_table->establishConnection('w');
+    $notification_table = new PhabricatorFeedStoryNotification();
+    $conn = $notification_table->establishConnection('w');
 
-      queryfx(
-        $conn,
-        "UPDATE %T
-         SET hasViewed = 1
-         WHERE userPHID = %s
-           AND primaryObjectPHID = %s
-           AND hasViewed = 0",
-        $notification_table->getTableName(),
-        $user->getPHID(),
-        $object_phid);
+    queryfx(
+      $conn,
+      "UPDATE %T
+       SET hasViewed = 1
+       WHERE userPHID = %s
+         AND primaryObjectPHID = %s
+         AND hasViewed = 0",
+      $notification_table->getTableName(),
+      $user->getPHID(),
+      $object_phid);
 
-      unset($unguarded);
-    }
+    unset($unguarded);
   }
 
   public function countUnread(PhabricatorUser $user) {

src/docs/userguide/notifications.diviner

@@ -5,19 +5,20 @@ Guide to setting up notifications.
 
 = Overview =
 
-Phabricator can be configured to notify users when events happen in real time,
-so they'll get a message in their browser window if something has happened or
-the object they're looking at has been updated.
+By default, Phabricator delivers information about events (like users creating
+tasks or commenting on code reviews) through email and in-application
+notifications.
 
-NOTE: This feature is new and still needs some work.
+Phabricator can also be configured to deliver notifications in real time, by
+popping up by popping up a message in any open browser windows if something has
+happened or an object has been updated.
 
-= Enabling Notifications =
+To enable real-time notifications:
 
-To enable notifications, set `notification.enabled` to `true` in your
-configuration. This will enable the notification menu in the menu bar, and
-notifications will be published when users take actions.
+  - Set `notification.enabled` in your configuration to true.
+  - Run the notification server, as described below.
 
-NOTE: This setting will be turned on for everyone soon.
+This document describes the process in detail.
 
 = Running the Aphlict Server =
 
@@ -52,7 +53,7 @@ You may want to adjust these settings:
     aphlict is restarted.
 
 In most cases, the defaults are appropriate, except that you should set
-`notification.user` to some valid user so Aphlict isn't running as root.
+`notification.user` to some valid system user so Aphlict isn't running as root.
 
 == Verifying Server Status ==
 
@@ -62,13 +63,11 @@ server is working. If it isn't working, you should see an error.
 
 == Testing the Server ==
 
-The easiest way to test the server is to open a Maniphest Task or Differential
-Revision in two browser windows at the same time, then comment on it with one.
-You should get a notification in the other window.
+The easiest way to test the server is to have two users login and comment on
+the same Maniphest Task or Differential Revision. They should receive in-browser
+notifications about the other user's activity.
 
-NOTE: This will stop working soon, since we'll soon stop delivering
-notifications about your own actions. We'll provide an alternate way to test
-operation when this stops working.
+NOTE: This is cumbersome. There will be better testing tools at some point.
 
 == Debugging Server Problems ==
 
@@ -85,4 +84,4 @@ output in your browser.
 
 The server also generates a log, by default in `/var/log/aphlict.log`. You can
 change this location by changing `notification.log` in your configuration. The
-log may contain information useful in resolving issues.
+log may contain information useful in resolving issues.

src/view/page/PhabricatorStandardPageView.php

@@ -323,8 +323,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
     $user = $request->getUser();
 
     $container = null;
-    if (PhabricatorEnv::getEnvConfig('notification.enabled') &&
-        $user->isLoggedIn()) {
+    if ($user->isLoggedIn()) {
 
       $aphlict_object_id = celerity_generate_unique_node_id();
       $aphlict_container_id = celerity_generate_unique_node_id();

src/view/page/menu/PhabricatorMainMenuView.php

@@ -52,8 +52,7 @@ final class PhabricatorMainMenuView extends AphrontView {
     $menus = array();
     $alerts = array();
 
-    if (PhabricatorEnv::getEnvConfig('notification.enabled') &&
-        $user->isLoggedIn()) {
+    if ($user->isLoggedIn()) {
       list($menu, $dropdown) = $this->renderNotificationMenu();
       $alerts[] = $menu;
       $menus[] = $dropdown;