Remove "participationStatus" from ConpherenceParticipant

Summary:
Pathway to D17685. This column is a very complicated cache of: is participant.messageCount equal to thread.messageCount?

We can just ask this question with a JOIN instead and simplify things dramatically.

Test Plan:
  - Ran migration.
  - Browsed around.
  - Sent a message, saw unread count go up.
  - Read the message, saw unread count go down.

Reviewers: chad

Reviewed By: chad

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

Author: epriestley

resources/sql/autopatches/20170419.thread.02.status.sql

@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_conpherence.conpherence_participant
+  DROP participationStatus;

src/__phutil_library_map__.php

@@ -305,7 +305,6 @@
     'ConpherenceParticipantCountQuery' => 'applications/conpherence/query/ConpherenceParticipantCountQuery.php',
     'ConpherenceParticipantQuery' => 'applications/conpherence/query/ConpherenceParticipantQuery.php',
     'ConpherenceParticipantView' => 'applications/conpherence/view/ConpherenceParticipantView.php',
-    'ConpherenceParticipationStatus' => 'applications/conpherence/constants/ConpherenceParticipationStatus.php',
     'ConpherenceQueryThreadConduitAPIMethod' => 'applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php',
     'ConpherenceQueryTransactionConduitAPIMethod' => 'applications/conpherence/conduit/ConpherenceQueryTransactionConduitAPIMethod.php',
     'ConpherenceReplyHandler' => 'applications/conpherence/mail/ConpherenceReplyHandler.php',
@@ -5099,7 +5098,6 @@
     'ConpherenceParticipantCountQuery' => 'PhabricatorOffsetPagedQuery',
     'ConpherenceParticipantQuery' => 'PhabricatorOffsetPagedQuery',
     'ConpherenceParticipantView' => 'AphrontView',
-    'ConpherenceParticipationStatus' => 'ConpherenceConstants',
     'ConpherenceQueryThreadConduitAPIMethod' => 'ConpherenceConduitAPIMethod',
     'ConpherenceQueryTransactionConduitAPIMethod' => 'ConpherenceConduitAPIMethod',
     'ConpherenceReplyHandler' => 'PhabricatorMailReplyHandler',

src/applications/conpherence/constants/ConpherenceParticipationStatus.php

@@ -7,7 +7,6 @@ public function handleRequest(AphrontRequest $request) {
     $user = $request->getUser();
     $conpherences = array();
     require_celerity_resource('conpherence-notification-css');
-    $unread_status = ConpherenceParticipationStatus::BEHIND;
 
     $participant_data = id(new ConpherenceParticipantQuery())
       ->withParticipantPHIDs(array($user->getPHID()))
@@ -37,7 +36,7 @@ public function handleRequest(AphrontRequest $request) {
           'conpherence-notification',
         );
 
-        if ($p_data->getParticipationStatus() == $unread_status) {
+        if (!$p_data->isUpToDate($conpherence)) {
           $classes[] = 'phabricator-notification-unread';
         }
         $uri = $this->getApplicationURI($conpherence->getID().'/');
@@ -95,7 +94,7 @@ public function handleRequest(AphrontRequest $request) {
 
     $unread = id(new ConpherenceParticipantCountQuery())
       ->withParticipantPHIDs(array($user->getPHID()))
-      ->withParticipationStatus($unread_status)
+      ->withUnread(true)
       ->execute();
     $unread_count = idx($unread, $user->getPHID(), 0);
 

src/applications/conpherence/controller/ConpherenceNotificationPanelController.php

@@ -173,17 +173,14 @@ protected function applyInitialEffects(
           $phids = $this->getPHIDTransactionNewValue($xaction, array());
           foreach ($phids as $phid) {
             if ($phid == $this->getActor()->getPHID()) {
-              $status = ConpherenceParticipationStatus::UP_TO_DATE;
               $message_count = 1;
             } else {
-              $status = ConpherenceParticipationStatus::BEHIND;
               $message_count = 0;
             }
             $participants[$phid] =
               id(new ConpherenceParticipant())
               ->setConpherencePHID($object->getPHID())
               ->setParticipantPHID($phid)
-              ->setParticipationStatus($status)
               ->setDateTouched(time())
               ->setSeenMessageCount($message_count)
               ->save();
@@ -243,17 +240,14 @@ protected function applyCustomExternalTransaction(
         $add = array_keys(array_diff_key($new_map, $old_map));
         foreach ($add as $phid) {
           if ($phid == $this->getActor()->getPHID()) {
-            $status = ConpherenceParticipationStatus::UP_TO_DATE;
             $message_count = $object->getMessageCount();
           } else {
-            $status = ConpherenceParticipationStatus::BEHIND;
             $message_count = 0;
           }
           $participants[$phid] =
             id(new ConpherenceParticipant())
             ->setConpherencePHID($object->getPHID())
             ->setParticipantPHID($phid)
-            ->setParticipationStatus($status)
             ->setDateTouched(time())
             ->setSeenMessageCount($message_count)
             ->save();
@@ -279,22 +273,18 @@ protected function applyFinalEffects(
 
           // 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) {
+              if ($participant->isUpToDate($object)) {
                 $participant->setSeenMessageCount(
                   $object->getMessageCount() - $message_count);
               }
-              $participant->setParticipationStatus($behind);
               $participant->setDateTouched($time);
             } else {
               $participant->setSeenMessageCount($object->getMessageCount());
-              $participant->setParticipationStatus($up_to_date);
               $participant->setDateTouched($time);
             }
             $participant->save();

src/applications/conpherence/editor/ConpherenceEditor.php

@@ -1,73 +1,69 @@
 <?php
 
-/**
- * Query class that answers the question:
- *
- * - Q: How many unread conpherences am I participating in?
- * - A:
- *     id(new ConpherenceParticipantCountQuery())
- *     ->withParticipantPHIDs(array($my_phid))
- *     ->withParticipationStatus(ConpherenceParticipationStatus::BEHIND)
- *     ->execute();
- */
 final class ConpherenceParticipantCountQuery
   extends PhabricatorOffsetPagedQuery {
 
   private $participantPHIDs;
-  private $participationStatus;
+  private $unread;
 
   public function withParticipantPHIDs(array $phids) {
     $this->participantPHIDs = $phids;
     return $this;
   }
 
-  public function withParticipationStatus($participation_status) {
-    $this->participationStatus = $participation_status;
+  public function withUnread($unread) {
+    $this->unread = $unread;
     return $this;
   }
 
   public function execute() {
+    $thread = new ConpherenceThread();
     $table = new ConpherenceParticipant();
-    $conn_r = $table->establishConnection('r');
+    $conn = $table->establishConnection('r');
 
     $rows = queryfx_all(
-      $conn_r,
-      'SELECT COUNT(*) as count, participantPHID '.
-      'FROM %T participant %Q %Q %Q',
+      $conn,
+      'SELECT COUNT(*) as count, participantPHID
+        FROM %T participant JOIN %T thread
+        ON participant.conpherencePHID = thread.phid %Q %Q %Q',
       $table->getTableName(),
-      $this->buildWhereClause($conn_r),
-      $this->buildGroupByClause($conn_r),
-      $this->buildLimitClause($conn_r));
+      $thread->getTableName(),
+      $this->buildWhereClause($conn),
+      $this->buildGroupByClause($conn),
+      $this->buildLimitClause($conn));
 
     return ipull($rows, 'count', 'participantPHID');
   }
 
-  protected function buildWhereClause(AphrontDatabaseConnection $conn_r) {
+  protected function buildWhereClause(AphrontDatabaseConnection $conn) {
     $where = array();
 
-    if ($this->participantPHIDs) {
+    if ($this->participantPHIDs !== null) {
       $where[] = qsprintf(
-        $conn_r,
-        'participantPHID IN (%Ls)',
+        $conn,
+        'participant.participantPHID IN (%Ls)',
         $this->participantPHIDs);
     }
 
-    if ($this->participationStatus !== null) {
-      $where[] = qsprintf(
-        $conn_r,
-        'participationStatus = %d',
-        $this->participationStatus);
+    if ($this->unread !== null) {
+      if ($this->unread) {
+        $where[] = qsprintf(
+          $conn,
+          'participant.seenMessageCount < thread.messageCount');
+      } else {
+        $where[] = qsprintf(
+          $conn,
+          'participant.seenMessageCount >= thread.messageCount');
+      }
     }
 
     return $this->formatWhereClause($where);
   }
 
-  private function buildGroupByClause(AphrontDatabaseConnection $conn_r) {
-    $group_by = qsprintf(
-      $conn_r,
+  private function buildGroupByClause(AphrontDatabaseConnection $conn) {
+    return qsprintf(
+      $conn,
       'GROUP BY participantPHID');
-
-    return $group_by;
   }
 
 }

src/applications/conpherence/query/ConpherenceParticipantCountQuery.php

@@ -4,7 +4,6 @@ final class ConpherenceParticipant extends ConpherenceDAO {
 
   protected $participantPHID;
   protected $conpherencePHID;
-  protected $participationStatus;
   protected $seenMessageCount;
   protected $dateTouched;
   protected $settings = array();
@@ -15,7 +14,6 @@ protected function getConfiguration() {
         'settings' => self::SERIALIZATION_JSON,
       ),
       self::CONFIG_COLUMN_SCHEMA => array(
-        'participationStatus' => 'uint32',
         'dateTouched' => 'epoch',
         'seenMessageCount' => 'uint64',
       ),
@@ -24,12 +22,12 @@ protected function getConfiguration() {
           'columns' => array('conpherencePHID', 'participantPHID'),
           'unique' => true,
         ),
-        'unreadCount' => array(
-          'columns' => array('participantPHID', 'participationStatus'),
-        ),
         'participationIndex' => array(
           'columns' => array('participantPHID', 'dateTouched', 'id'),
         ),
+        'key_thread' => array(
+          'columns' => array('participantPHID', 'conpherencePHID'),
+        ),
       ),
     ) + parent::getConfiguration();
   }
@@ -41,8 +39,8 @@ public function getSettings() {
   public function markUpToDate(
     ConpherenceThread $conpherence,
     ConpherenceTransaction $xaction) {
+
     if (!$this->isUpToDate($conpherence)) {
-      $this->setParticipationStatus(ConpherenceParticipationStatus::UP_TO_DATE);
       $this->setSeenMessageCount($conpherence->getMessageCount());
       $this->save();
 
@@ -55,11 +53,7 @@ public function markUpToDate(
   }
 
   public function isUpToDate(ConpherenceThread $conpherence) {
-    return
-      ($this->getSeenMessageCount() == $conpherence->getMessageCount())
-        &&
-      ($this->getParticipationStatus() ==
-       ConpherenceParticipationStatus::UP_TO_DATE);
+    return ($this->getSeenMessageCount() == $conpherence->getMessageCount());
   }
 
 }

src/applications/conpherence/storage/ConpherenceParticipant.php

@@ -28,10 +28,9 @@ public function newValueForUsers($key, array $users) {
 
     $user_phids = mpull($users, 'getPHID');
 
-    $unread_status = ConpherenceParticipationStatus::BEHIND;
     $unread = id(new ConpherenceParticipantCountQuery())
       ->withParticipantPHIDs($user_phids)
-      ->withParticipationStatus($unread_status)
+      ->withUnread(true)
       ->execute();
 
     $empty = array_fill_keys($user_phids, 0);