From 5587abf04c8bafce4d27f1f53aa0b77981fcc533 Mon Sep 17 00:00:00 2001 From: Chad Little Date: Thu, 13 Apr 2017 13:30:05 -0700 Subject: [PATCH] Remove recentParticipants from ConpherenceThread 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 --- .../20170413.conpherence.01.recentparty.sql | 2 + .../__tests__/ConpherenceRoomTestCase.php | 17 +---- ...ConpherenceQueryThreadConduitAPIMethod.php | 1 - .../ConpherenceColumnViewController.php | 1 - .../controller/ConpherenceListController.php | 1 - ...ConpherenceNotificationPanelController.php | 1 - .../ConpherenceUpdateController.php | 2 - .../controller/ConpherenceViewController.php | 1 - .../conpherence/editor/ConpherenceEditor.php | 41 +---------- .../query/ConpherenceThreadQuery.php | 9 --- .../query/ConpherenceThreadSearchEngine.php | 11 +-- .../conpherence/storage/ConpherenceThread.php | 72 +------------------ .../PhabricatorConpherenceProfileMenuItem.php | 1 - 13 files changed, 7 insertions(+), 153 deletions(-) create mode 100644 resources/sql/autopatches/20170413.conpherence.01.recentparty.sql diff --git a/resources/sql/autopatches/20170413.conpherence.01.recentparty.sql b/resources/sql/autopatches/20170413.conpherence.01.recentparty.sql new file mode 100644 index 0000000000..996a058c5b --- /dev/null +++ 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 index 1fb4dbabaa..85c4b9c1f1 100644 --- a/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php +++ b/src/applications/conpherence/__tests__/ConpherenceRoomTestCase.php @@ -16,9 +16,6 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { $this->assertTrue((bool)$conpherence->getID()); $this->assertEqual(1, count($conpherence->getParticipants())); - $this->assertEqual( - $participant_phids, - $conpherence->getRecentParticipantPHIDs()); } public function testNUserRoomCreate() { @@ -38,9 +35,6 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { $this->assertTrue((bool)$conpherence->getID()); $this->assertEqual(4, count($conpherence->getParticipants())); - $this->assertEqual( - $participant_phids, - $conpherence->getRecentParticipantPHIDs()); } public function testRoomParticipantAddition() { @@ -58,16 +52,11 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { $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 @@ final class ConpherenceRoomTestCase extends ConpherenceTestCase { $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 index c66a90f585..fd602058a3 100644 --- a/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php +++ b/src/applications/conpherence/conduit/ConpherenceQueryThreadConduitAPIMethod.php @@ -71,7 +71,6 @@ final class ConpherenceQueryThreadConduitAPIMethod '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 index de83379aab..aa4f94edfc 100644 --- a/src/applications/conpherence/controller/ConpherenceColumnViewController.php +++ b/src/applications/conpherence/controller/ConpherenceColumnViewController.php @@ -17,7 +17,6 @@ final class ConpherenceColumnViewController extends ->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 index 4e81526004..00968af576 100644 --- a/src/applications/conpherence/controller/ConpherenceListController.php +++ b/src/applications/conpherence/controller/ConpherenceListController.php @@ -159,7 +159,6 @@ final class ConpherenceListController extends ConpherenceController { ->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 index 402da2aac3..ed728c6ab7 100644 --- a/src/applications/conpherence/controller/ConpherenceNotificationPanelController.php +++ b/src/applications/conpherence/controller/ConpherenceNotificationPanelController.php @@ -21,7 +21,6 @@ final class ConpherenceNotificationPanelController ->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 index b8ebe5a826..9cc666109c 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -470,7 +470,6 @@ final class ConpherenceUpdateController $latest_transaction_id) { $need_transactions = false; - $need_participant_cache = true; switch ($action) { case ConpherenceUpdateActions::METADATA: case ConpherenceUpdateActions::LOAD: @@ -491,7 +490,6 @@ final class ConpherenceUpdateController ->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 index 57fe194845..6b953b9de4 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -20,7 +20,6 @@ final class ConpherenceViewController extends ->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 index e4728f3c00..e358700348 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -190,7 +190,6 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { ->setSeenMessageCount($message_count) ->save(); $object->attachParticipants($participants); - $object->setRecentParticipantPHIDs(array_keys($participants)); } break; } @@ -201,39 +200,12 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { 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 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { 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 index 48dc3323e0..1bb96537dc 100644 --- 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 @@ final class ConpherenceThreadQuery 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 index 4e0a89d266..cbaf43b0a9 100644 --- a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php +++ b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php @@ -13,7 +13,6 @@ final class ConpherenceThreadSearchEngine public function newQuery() { return id(new ConpherenceThreadQuery()) - ->needParticipantCache(true) ->needProfileImage(true); } @@ -92,14 +91,6 @@ final class ConpherenceThreadSearchEngine 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 @@ final class ConpherenceThreadSearchEngine $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 index db6d4a7e61..f0d0e60bb3 100644 --- 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 @@ final class ConpherenceThread extends ConpherenceDAO 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 @@ final class ConpherenceThread extends ConpherenceDAO 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 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 @@ final class ConpherenceThread extends ConpherenceDAO } $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 index cd41d41148..6a91188c8f 100644 --- a/src/applications/search/menuitem/PhabricatorConpherenceProfileMenuItem.php +++ b/src/applications/search/menuitem/PhabricatorConpherenceProfileMenuItem.php @@ -45,7 +45,6 @@ final class PhabricatorConpherenceProfileMenuItem $rooms = id(new ConpherenceThreadQuery()) ->setViewer($viewer) ->withPHIDs($room_phids) - ->needParticipantCache(true) ->needProfileImage(true) ->execute();