Clean up Conpherence Transactions and notifications

Summary: Does a few things. Turns off feed stories (again), removes "action" transactions from notificiations, and only updates message count on actual messages. This feels a bit cleaner and less spammy... I guess... I think @epriestley will really like it and do me a favor or something.

Test Plan: Pull up two windows. test a message, see message count on second screen. Edit a topic or title, get no notification. At all. Ever.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

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

Author: Chad Little

resources/celerity/map.php

@@ -7,7 +7,7 @@
  */
 return array(
   'names' => array(
-    'conpherence.pkg.css' => 'b5ee2073',
+    'conpherence.pkg.css' => 'f8390290',
     'conpherence.pkg.js' => '281b1a73',
     'core.pkg.css' => '30a64ed6',
     'core.pkg.js' => 'fbc1c380',
@@ -49,7 +49,7 @@
     'rsrc/css/application/conpherence/header-pane.css' => '4082233d',
     'rsrc/css/application/conpherence/menu.css' => '5abfb32d',
     'rsrc/css/application/conpherence/message-pane.css' => 'd1fc13e1',
-    'rsrc/css/application/conpherence/notification.css' => '965db05b',
+    'rsrc/css/application/conpherence/notification.css' => 'cef0a3fc',
     'rsrc/css/application/conpherence/participant-pane.css' => '604a8b02',
     'rsrc/css/application/conpherence/transaction.css' => '85129c68',
     'rsrc/css/application/contentsource/content-source-view.css' => '4b8b05d4',
@@ -556,7 +556,7 @@
     'conpherence-header-pane-css' => '4082233d',
     'conpherence-menu-css' => '5abfb32d',
     'conpherence-message-pane-css' => 'd1fc13e1',
-    'conpherence-notification-css' => '965db05b',
+    'conpherence-notification-css' => 'cef0a3fc',
     'conpherence-participant-pane-css' => '604a8b02',
     'conpherence-thread-manager' => 'c8b5ee6f',
     'conpherence-transaction-css' => '85129c68',

src/applications/conpherence/controller/ConpherenceNotificationPanelController.php

@@ -20,7 +20,7 @@ public function handleRequest(AphrontRequest $request) {
         ->withPHIDs(array_keys($participant_data))
         ->needProfileImage(true)
         ->needTransactions(true)
-        ->setTransactionLimit(50)
+        ->setTransactionLimit(100)
         ->needParticipantCache(true)
         ->execute();
     }

src/applications/conpherence/editor/ConpherenceEditor.php

@@ -87,9 +87,9 @@ public function generateTransactionsFromText(
   public function getTransactionTypes() {
     $types = parent::getTransactionTypes();
 
-    $types[] = PhabricatorTransactions::TYPE_COMMENT;
-
     $types[] = ConpherenceTransaction::TYPE_PARTICIPANTS;
+
+    $types[] = PhabricatorTransactions::TYPE_COMMENT;
     $types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
     $types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
     $types[] = PhabricatorTransactions::TYPE_JOIN_POLICY;
@@ -101,25 +101,6 @@ public function getCreateObjectTitle($author, $object) {
     return pht('%s created this room.', $author);
   }
 
-  protected function shouldPublishFeedStory(
-    PhabricatorLiskDAO $object,
-    array $xactions) {
-
-    foreach ($xactions as $xaction) {
-      switch ($xaction->getTransactionType()) {
-        case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE:
-        case ConpherenceThreadTopicTransaction::TRANSACTIONTYPE:
-        case ConpherenceThreadPictureTransaction::TRANSACTIONTYPE:
-          return true;
-        default:
-          return false;
-      }
-    }
-    return true;
-  }
-
-
-
   protected function getCustomTransactionOldValue(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
@@ -338,40 +319,40 @@ protected function applyFinalEffects(
       switch ($xaction->getTransactionType()) {
         case PhabricatorTransactions::TYPE_COMMENT:
           $message_count++;
-          break;
-      }
-    }
 
-    // update everyone's participation status on the last xaction -only-
-    $xaction = end($xactions);
-    $xaction_phid = $xaction->getPHID();
-    $behind = ConpherenceParticipationStatus::BEHIND;
-    $up_to_date = ConpherenceParticipationStatus::UP_TO_DATE;
-    $participants = $object->getParticipants();
-    $user = $this->getActor();
-    $time = time();
-    foreach ($participants as $phid => $participant) {
-      if ($phid != $user->getPHID()) {
-        if ($participant->getParticipationStatus() != $behind) {
-          $participant->setBehindTransactionPHID($xaction_phid);
-          $participant->setSeenMessageCount(
-            $object->getMessageCount() - $message_count);
-        }
-        $participant->setParticipationStatus($behind);
-        $participant->setDateTouched($time);
-      } else {
-        $participant->setSeenMessageCount($object->getMessageCount());
-        $participant->setBehindTransactionPHID($xaction_phid);
-        $participant->setParticipationStatus($up_to_date);
-        $participant->setDateTouched($time);
+          // update everyone's participation status on a message -only-
+          $xaction_phid = $xaction->getPHID();
+          $behind = ConpherenceParticipationStatus::BEHIND;
+          $up_to_date = ConpherenceParticipationStatus::UP_TO_DATE;
+          $participants = $object->getParticipants();
+          $user = $this->getActor();
+          $time = time();
+          foreach ($participants as $phid => $participant) {
+            if ($phid != $user->getPHID()) {
+              if ($participant->getParticipationStatus() != $behind) {
+                $participant->setBehindTransactionPHID($xaction_phid);
+                $participant->setSeenMessageCount(
+                  $object->getMessageCount() - $message_count);
+              }
+              $participant->setParticipationStatus($behind);
+              $participant->setDateTouched($time);
+            } else {
+              $participant->setSeenMessageCount($object->getMessageCount());
+              $participant->setBehindTransactionPHID($xaction_phid);
+              $participant->setParticipationStatus($up_to_date);
+              $participant->setDateTouched($time);
+            }
+            $participant->save();
+          }
+
+          PhabricatorUserCache::clearCaches(
+            PhabricatorUserMessageCountCacheType::KEY_COUNT,
+            array_keys($participants));
+
+          break;
       }
-      $participant->save();
     }
 
-    PhabricatorUserCache::clearCaches(
-      PhabricatorUserMessageCountCacheType::KEY_COUNT,
-      array_keys($participants));
-
     if ($xactions) {
       $data = array(
         'type'        => 'message',

src/applications/conpherence/storage/ConpherenceThread.php

@@ -240,71 +240,33 @@ public function getDisplayData(PhabricatorUser $viewer) {
       $transactions = array();
     }
 
-    if ($transactions) {
-      $subtitle_mode = 'message';
-    } else {
-      $subtitle_mode = 'recent';
-    }
-
-    $lucky_phid = head($this->getOtherRecentParticipantPHIDs($viewer));
-    if ($lucky_phid) {
-      $lucky_handle = $handles[$lucky_phid];
-    } else {
-      // This will be just the user talking to themselves. Weirdo.
-      $lucky_handle = reset($handles);
-    }
-
     $img_src = $this->getProfileImageURI();
 
-    $message_title = null;
-    if ($subtitle_mode == 'message') {
-      $message_transaction = null;
-      $action_transaction = null;
-      foreach ($transactions as $transaction) {
-        if ($message_transaction || $action_transaction) {
-          break;
-        }
-        switch ($transaction->getTransactionType()) {
-          case PhabricatorTransactions::TYPE_COMMENT:
-            $message_transaction = $transaction;
-            break;
-          case ConpherenceThreadTitleTransaction::TRANSACTIONTYPE:
-          case ConpherenceThreadTopicTransaction::TRANSACTIONTYPE:
-          case ConpherenceThreadPictureTransaction::TRANSACTIONTYPE:
-          case ConpherenceTransaction::TYPE_PARTICIPANTS:
-            $action_transaction = $transaction;
-            break;
-          default:
-            break;
-        }
-      }
+    $message_transaction = null;
+    foreach ($transactions as $transaction) {
       if ($message_transaction) {
-        $message_handle = $handles[$message_transaction->getAuthorPHID()];
-        $message_title = sprintf(
-          '%s: %s',
-          $message_handle->getName(),
-          id(new PhutilUTF8StringTruncator())
-            ->setMaximumGlyphs(60)
-            ->truncateString(
-              $message_transaction->getComment()->getContent()));
+        break;
       }
-      if ($action_transaction) {
-        $message_title = id(clone $action_transaction)
-          ->setRenderingTarget(PhabricatorApplicationTransaction::TARGET_TEXT)
-          ->getTitle();
+      switch ($transaction->getTransactionType()) {
+        case PhabricatorTransactions::TYPE_COMMENT:
+          $message_transaction = $transaction;
+          break;
+        default:
+          break;
       }
     }
-    switch ($subtitle_mode) {
-      case 'recent':
-        $subtitle = $this->getRecentParticipantsString($viewer);
-        break;
-      case 'message':
-        if ($message_title) {
-          $subtitle = $message_title;
-        } else {
-          $subtitle = $this->getRecentParticipantsString($viewer);
-        }
-        break;
+    if ($message_transaction) {
+      $message_handle = $handles[$message_transaction->getAuthorPHID()];
+      $subtitle = sprintf(
+        '%s: %s',
+        $message_handle->getName(),
+        id(new PhutilUTF8StringTruncator())
+          ->setMaximumGlyphs(60)
+          ->truncateString(
+            $message_transaction->getComment()->getContent()));
+    } else {
+      // Kinda lame, but maybe add last message to cache?
+      $subtitle = pht('No recent messages');
     }
 
     $user_participation = $this->getParticipantIfExists($viewer->getPHID());

webroot/rsrc/css/application/conpherence/notification.css

@@ -8,7 +8,7 @@
 
 .phabricator-notification .conpherence-menu-item-view {
   display: block;
-  height: 46px;
+  height: 48px;
   overflow: hidden;
   position: relative;
   text-decoration: none;
@@ -23,8 +23,8 @@
   left: 8px;
   display: block;
   position: absolute;
-  width: 30px;
-  height: 30px;
+  width: 32px;
+  height: 32px;
   background-size: 100%;
   border-radius: 3px;
 }
@@ -33,7 +33,7 @@
 .conpherence-menu-item-title {
   display: block;
   margin-top: 8px;
-  margin-left: 46px;
+  margin-left: 48px;
   text-align: left;
   font-weight: bold;
   font-size: {$normalfontsize};
@@ -49,8 +49,7 @@
   display: block;
   color: {$lightgreytext};
   font-size: {$smallestfontsize};
-  margin-top: 2px;
-  margin-left: 46px;
+  margin-left: 48px;
   width: 290px;
   text-overflow: ellipsis;
   white-space: nowrap;