Remove recentParticipants from ConpherenceThread

Chad Little · Chad Little · commit 5587abf04c8b · 2017-04-13T13:55:08.000-07:00
Summary: We no longer display this any more in the UI, so go ahead and remove the callsites and db column. Test Plan: New Room, with and without participants. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin Differential Revision: https://secure.phabricator.com/D17683
diff --git a/resources/sql/autopatches/20170413.conpherence.01.recentparty.sql b/resources/sql/autopatches/20170413.conpherence.01.recentparty.sql
@@ -0,0 +1,2 @@
+ALTER TABLE {$NAMESPACE}_conpherence.conpherence_thread
+  DROP COLUMN recentParticipantPHIDs;
diff --git a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php
@@ -16,9 +16,6 @@ public function testOneUserRoomCreate() {
 
     $this->assertTrue((bool)$conpherence->getID());
     $this->assertEqual(1, count($conpherence->getParticipants()));
-    $this->assertEqual(
-      $participant_phids,
-      $conpherence->getRecentParticipantPHIDs());
   }
 
   public function testNUserRoomCreate() {
@@ -38,9 +35,6 @@ public function testNUserRoomCreate() {
 
     $this->assertTrue((bool)$conpherence->getID());
     $this->assertEqual(4, count($conpherence->getParticipants()));
-    $this->assertEqual(
-      $participant_phids,
-      $conpherence->getRecentParticipantPHIDs());
   }
 
   public function testRoomParticipantAddition() {
@@ -58,16 +52,11 @@ public function testRoomParticipantAddition() {
 
     $this->assertTrue((bool)$conpherence->getID());
     $this->assertEqual(2, count($conpherence->getParticipants()));
-    $this->assertEqual(
-      $participant_phids,
-      $conpherence->getRecentParticipantPHIDs());
 
     // test add by creator
     $participant_phids[] = $friend_2->getPHID();
     $this->addParticipants($creator, $conpherence, array($friend_2->getPHID()));
-    $this->assertEqual(
-      $participant_phids,
-      $conpherence->getRecentParticipantPHIDs());
+    $this->assertEqual(3, count($conpherence->getParticipants()));
 
     // test add by other participant, so recent participation should
     // meaningfully change
@@ -81,9 +70,7 @@ public function testRoomParticipantAddition() {
       $friend_2,
       $conpherence,
       array($friend_3->getPHID()));
-    $this->assertEqual(
-      $participant_phids,
-      $conpherence->getRecentParticipantPHIDs());
+    $this->assertEqual(4, count($conpherence->getParticipants()));
   }
 
   public function testRoomParticipantDeletion() {
diff --git a/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php b/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php
@@ -71,7 +71,6 @@ protected function execute(ConduitAPIRequest $request) {
         'conpherencePHID' => $conpherence->getPHID(),
         'conpherenceTitle' => $conpherence->getTitle(),
         'messageCount' => $conpherence->getMessageCount(),
-        'recentParticipantPHIDs' => $conpherence->getRecentParticipantPHIDs(),
         'conpherenceURI' => $this->getConpherenceURI($conpherence),
       );
     }
diff --git a/src/applications/conpherence/controller/ConpherenceColumnViewController.php b/src/applications/conpherence/controller/ConpherenceColumnViewController.php
@@ -17,7 +17,6 @@ public function handleRequest(AphrontRequest $request) {
         ->setViewer($user)
         ->withPHIDs($conpherence_phids)
         ->needProfileImage(true)
-        ->needParticipantCache(true)
         ->execute();
       $latest_conpherences = mpull($latest_conpherences, null, 'getPHID');
       $latest_conpherences = array_select_keys(
diff --git a/src/applications/conpherence/controller/ConpherenceListController.php b/src/applications/conpherence/controller/ConpherenceListController.php
@@ -159,7 +159,6 @@ private function loadConpherenceThreadData($participation) {
         ->setViewer($user)
         ->withPHIDs($conpherence_phids)
         ->needProfileImage(true)
-        ->needParticipantCache(true)
         ->execute();
 
       // this will re-sort by participation data
diff --git a/src/applications/conpherence/controller/ConpherenceNotificationPanelController.php b/src/applications/conpherence/controller/ConpherenceNotificationPanelController.php
@@ -21,7 +21,6 @@ public function handleRequest(AphrontRequest $request) {
         ->needProfileImage(true)
         ->needTransactions(true)
         ->setTransactionLimit(100)
-        ->needParticipantCache(true)
         ->execute();
     }
 
diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php
@@ -470,7 +470,6 @@ private function loadAndRenderUpdates(
     $latest_transaction_id) {
 
     $need_transactions = false;
-    $need_participant_cache = true;
     switch ($action) {
       case ConpherenceUpdateActions::METADATA:
       case ConpherenceUpdateActions::LOAD:
@@ -491,7 +490,6 @@ private function loadAndRenderUpdates(
       ->setViewer($user)
       ->setAfterTransactionID($latest_transaction_id)
       ->needProfileImage(true)
-      ->needParticipantCache($need_participant_cache)
       ->needParticipants(true)
       ->needTransactions($need_transactions)
       ->withIDs(array($conpherence_id))
diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php
@@ -20,7 +20,6 @@ public function handleRequest(AphrontRequest $request) {
       ->setViewer($user)
       ->withIDs(array($conpherence_id))
       ->needProfileImage(true)
-      ->needParticipantCache(true)
       ->needTransactions(true)
       ->setTransactionLimit($this->getMainQueryLimit());
 
diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php
@@ -190,7 +190,6 @@ protected function applyInitialEffects(
               ->setSeenMessageCount($message_count)
               ->save();
             $object->attachParticipants($participants);
-            $object->setRecentParticipantPHIDs(array_keys($participants));
           }
           break;
       }
@@ -201,39 +200,12 @@ protected function applyCustomInternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
 
-    $make_author_recent_participant = true;
     switch ($xaction->getTransactionType()) {
       case ConpherenceTransaction::TYPE_PARTICIPANTS:
-        if (!$this->getIsNewObject()) {
-          $old_map = array_fuse($xaction->getOldValue());
-          $new_map = array_fuse($xaction->getNewValue());
-          // if we added people, add them to the end of "recent" participants
-          $add = array_keys(array_diff_key($new_map, $old_map));
-          // if we remove people, then definintely remove them from "recent"
-          // participants
-          $del = array_keys(array_diff_key($old_map, $new_map));
-          if ($add || $del) {
-            $participants = $object->getRecentParticipantPHIDs();
-            if ($add) {
-              $participants = array_merge($participants, $add);
-            }
-            if ($del) {
-              $participants = array_diff($participants, $del);
-              $actor = $this->requireActor();
-              if (in_array($actor->getPHID(), $del)) {
-                $make_author_recent_participant = false;
-              }
-            }
-            $participants = array_slice(array_unique($participants), 0, 10);
-            $object->setRecentParticipantPHIDs($participants);
-          }
-        }
+        if (!$this->getIsNewObject()) {}
         break;
     }
 
-    if ($make_author_recent_participant) {
-      $this->makeAuthorMostRecentParticipant($object, $xaction);
-    }
   }
 
   protected function applyBuiltinInternalTransaction(
@@ -249,17 +221,6 @@ protected function applyBuiltinInternalTransaction(
     return parent::applyBuiltinInternalTransaction($object, $xaction);
   }
 
-  private function makeAuthorMostRecentParticipant(
-    PhabricatorLiskDAO $object,
-    PhabricatorApplicationTransaction $xaction) {
-
-    $participants = $object->getRecentParticipantPHIDs();
-    array_unshift($participants, $xaction->getAuthorPHID());
-    $participants = array_slice(array_unique($participants), 0, 10);
-
-    $object->setRecentParticipantPHIDs($participants);
-  }
-
   protected function applyCustomExternalTransaction(
     PhabricatorLiskDAO $object,
     PhabricatorApplicationTransaction $xaction) {
diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php
@@ -10,18 +10,12 @@ final class ConpherenceThreadQuery
   private $participantPHIDs;
   private $needParticipants;
   private $needTransactions;
-  private $needParticipantCache;
   private $afterTransactionID;
   private $beforeTransactionID;
   private $transactionLimit;
   private $fulltext;
   private $needProfileImage;
 
-  public function needParticipantCache($participant_cache) {
-    $this->needParticipantCache = $participant_cache;
-    return $this;
-  }
-
   public function needParticipants($need) {
     $this->needParticipants = $need;
     return $this;
@@ -101,9 +95,6 @@ protected function loadPage() {
     if ($conpherences) {
       $conpherences = mpull($conpherences, null, 'getPHID');
       $this->loadParticipantsAndInitHandles($conpherences);
-      if ($this->needParticipantCache) {
-        $this->loadCoreHandles($conpherences, 'getRecentParticipantPHIDs');
-      }
       if ($this->needParticipants) {
         $this->loadCoreHandles($conpherences, 'getParticipantPHIDs');
       }
diff --git a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php
@@ -13,7 +13,6 @@ public function getApplicationClassName() {
 
   public function newQuery() {
     return id(new ConpherenceThreadQuery())
-      ->needParticipantCache(true)
       ->needProfileImage(true);
   }
 
@@ -92,14 +91,6 @@ public function buildSavedQueryFromBuiltin($query_key) {
     return parent::buildSavedQueryFromBuiltin($query_key);
   }
 
-  protected function getRequiredHandlePHIDsForResultList(
-    array $conpherences,
-    PhabricatorSavedQuery $query) {
-
-    $recent = mpull($conpherences, 'getRecentParticipantPHIDs');
-    return array_unique(array_mergev($recent));
-  }
-
   protected function renderResultList(
     array $conpherences,
     PhabricatorSavedQuery $query,
@@ -153,7 +144,7 @@ protected function renderResultList(
     $list->setUser($viewer);
     foreach ($conpherences as $conpherence_phid => $conpherence) {
       $created = phabricator_date($conpherence->getDateCreated(), $viewer);
-      $title = $conpherence->getDisplayTitle($viewer);
+      $title = $conpherence->getTitle();
       $monogram = $conpherence->getMonogram();
 
       $icon_name = $conpherence->getPolicyIconName($policy_objects);
diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php
@@ -12,7 +12,6 @@ final class ConpherenceThread extends ConpherenceDAO
   protected $topic;
   protected $profileImagePHID;
   protected $messageCount;
-  protected $recentParticipantPHIDs = array();
   protected $mailKey;
   protected $viewPolicy;
   protected $editPolicy;
@@ -39,9 +38,6 @@ public static function initializeNewRoom(PhabricatorUser $sender) {
   protected function getConfiguration() {
     return array(
       self::CONFIG_AUX_PHID => true,
-      self::CONFIG_SERIALIZATION => array(
-        'recentParticipantPHIDs' => self::SERIALIZATION_JSON,
-      ),
       self::CONFIG_COLUMN_SCHEMA => array(
         'title' => 'text255?',
         'topic' => 'text255',
@@ -165,72 +161,6 @@ public function getStaticTitle() {
     return pht('Private Room');
   }
 
-  /**
-   * Get the thread's display title for a user.
-   *
-   * If a thread doesn't have a title set, this will return a string describing
-   * recent participants.
-   *
-   * @param PhabricatorUser Viewer.
-   * @return string Thread title.
-   */
-  public function getDisplayTitle(PhabricatorUser $viewer) {
-    $title = $this->getTitle();
-    if (strlen($title)) {
-      return $title;
-    }
-
-    return $this->getRecentParticipantsString($viewer);
-  }
-
-
-  /**
-   * Get recent participants (other than the viewer) as a string.
-   *
-   * For example, this method might return "alincoln, htaft, gwashington...".
-   *
-   * @param PhabricatorUser Viewer.
-   * @return string Description of other participants.
-   */
-  private function getRecentParticipantsString(PhabricatorUser $viewer) {
-    $handles = $this->getHandles();
-    $phids = $this->getOtherRecentParticipantPHIDs($viewer);
-
-    if (count($phids) == 0) {
-      $phids[] = $viewer->getPHID();
-      $more = false;
-    } else {
-      $limit = 3;
-      $more = (count($phids) > $limit);
-      $phids = array_slice($phids, 0, $limit);
-    }
-
-    $names = array_select_keys($handles, $phids);
-    $names = mpull($names, 'getName');
-    $names = implode(', ', $names);
-
-    if ($more) {
-      $names = $names.'...';
-    }
-
-    return $names;
-  }
-
-
-  /**
-   * Get PHIDs for recent participants who are not the viewer.
-   *
-   * @param PhabricatorUser Viewer.
-   * @return list<phid> Participants who are not the viewer.
-   */
-  private function getOtherRecentParticipantPHIDs(PhabricatorUser $viewer) {
-    $phids = $this->getRecentParticipantPHIDs();
-    $phids = array_fuse($phids);
-    unset($phids[$viewer->getPHID()]);
-    return array_values($phids);
-  }
-
-
   public function getDisplayData(PhabricatorUser $viewer) {
     $handles = $this->getHandles();
 
@@ -277,7 +207,7 @@ public function getDisplayData(PhabricatorUser $viewer) {
     }
     $unread_count = $this->getMessageCount() - $user_seen_count;
 
-    $title = $this->getDisplayTitle($viewer);
+    $title = $this->getTitle();
     $topic = $this->getTopic();
 
     return array(
diff --git a/src/applications/search/menuitem/PhabricatorConpherenceProfileMenuItem.php b/src/applications/search/menuitem/PhabricatorConpherenceProfileMenuItem.php
@@ -45,7 +45,6 @@ public function willBuildNavigationItems(array $items) {
     $rooms = id(new ConpherenceThreadQuery())
       ->setViewer($viewer)
       ->withPHIDs($room_phids)
-      ->needParticipantCache(true)
       ->needProfileImage(true)
       ->execute();
 

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+ALTER TABLE {$NAMESPACE}_conpherence.conpherence_thread`
	`2`	`+ DROP COLUMN recentParticipantPHIDs;`
Original file line number	Diff line number	Diff line change
`@@ -71,7 +71,6 @@ protected function execute(ConduitAPIRequest $request) {`
`71`	`71`	`'conpherencePHID' => $conpherence->getPHID(),`
`72`	`72`	`'conpherenceTitle' => $conpherence->getTitle(),`
`73`	`73`	`'messageCount' => $conpherence->getMessageCount(),`
`74`		`- 'recentParticipantPHIDs' => $conpherence->getRecentParticipantPHIDs(),`
`75`	`74`	`'conpherenceURI' => $this->getConpherenceURI($conpherence),`
`76`	`75`	`);`
`77`	`76`	`}`
Original file line number	Diff line number	Diff line change
`@@ -21,7 +21,6 @@ public function handleRequest(AphrontRequest $request) {`
`21`	`21`	`->needProfileImage(true)`
`22`	`22`	`->needTransactions(true)`
`23`	`23`	`->setTransactionLimit(100)`
`24`		`- ->needParticipantCache(true)`
`25`	`24`	`->execute();`
`26`	`25`	`}`
`27`	`26`