From fa673dde35ef2dce3772e692f7505c37cd536cee Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Wed, 1 Apr 2015 15:48:47 -0700 Subject: [PATCH] Conpherence - finish basic application search Summary: Fixes T7584. Adds the ability to specify rooms, messages, or both. Adds policy icon to rooms result view and envelope icon to messages result view. Fixes a missing group by clause in thread query. Enforces having participant phid if the query isn't looking at rooms and doesn't have other particpant phids. This last bit has a small UI quirk if the user searches for "messages" or "both" with no participant phids as we don't give them the feedback that they were included in the query. We could just slap the viewer in the particpants list in this case but it seemed like a buggier feeling experience to have the viewer appear up there? (Especially so in messages case, where we are basically being smart about policy filtering to come.) Test Plan: clicked around and got sensible results Reviewers: chad, epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7584 Differential Revision: https://secure.phabricator.com/D12232 --- .../query/ConpherenceThreadQuery.php | 25 +++++- .../query/ConpherenceThreadSearchEngine.php | 83 +++++++++++++------ .../conpherence/storage/ConpherenceThread.php | 24 ++++++ .../view/ConpherenceThreadListView.php | 21 ++--- 4 files changed, 110 insertions(+), 43 deletions(-) diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index 265e7f0327..a1a3516f50 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -82,10 +82,11 @@ final class ConpherenceThreadQuery $data = queryfx_all( $conn_r, - 'SELECT conpherence_thread.* FROM %T conpherence_thread %Q %Q %Q %Q', + 'SELECT conpherence_thread.* FROM %T conpherence_thread %Q %Q %Q %Q %Q', $table->getTableName(), $this->buildJoinClause($conn_r), $this->buildWhereClause($conn_r), + $this->buildGroupClause($conn_r), $this->buildOrderClause($conn_r), $this->buildLimitClause($conn_r)); @@ -131,6 +132,17 @@ final class ConpherenceThreadQuery id(new ConpherenceParticipant())->getTableName()); } + $viewer = $this->getViewer(); + if ($viewer->isLoggedIn()) { + $joins[] = qsprintf( + $conn_r, + 'LEFT JOIN %T v ON v.conpherencePHID = conpherence_thread.phid '. + 'AND v.participantPHID = %s', + id(new ConpherenceParticipant())->getTableName(), + $viewer->getPHID()); + } + + $joins[] = $this->buildApplicationSearchJoinClause($conn_r); return implode(' ', $joins); } @@ -168,6 +180,17 @@ final class ConpherenceThreadQuery (int)$this->isRoom); } + $viewer = $this->getViewer(); + if ($viewer->isLoggedIn()) { + $where[] = qsprintf( + $conn_r, + 'conpherence_thread.isRoom = 1 OR v.participantPHID IS NOT NULL'); + } else { + $where[] = qsprintf( + $conn_r, + 'conpherence_thread.isRoom = 1'); + } + return $this->formatWhereClause($where); } diff --git a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php index 822b41e440..de7d6624e4 100644 --- a/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php +++ b/src/applications/conpherence/query/ConpherenceThreadSearchEngine.php @@ -3,22 +3,8 @@ final class ConpherenceThreadSearchEngine extends PhabricatorApplicationSearchEngine { - // For now, we only search for rooms, but write this code so its easy to - // change that decision later - private $isRooms = true; - - public function setIsRooms($bool) { - $this->isRooms = $bool; - return $this; - } - public function getResultTypeDescription() { - if ($this->isRooms) { - $type = pht('Rooms'); - } else { - $type = pht('Threads'); - } - return $type; + return pht('Threads'); } public function getApplicationClassName() { @@ -32,12 +18,15 @@ final class ConpherenceThreadSearchEngine 'participantPHIDs', $this->readUsersFromRequest($request, 'participants')); + $saved->setParameter( + 'threadType', + $request->getStr('threadType')); + return $saved; } public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) { $query = id(new ConpherenceThreadQuery()) - ->withIsRoom($this->isRooms) ->needParticipantCache(true); $participant_phids = $saved->getParameter('participantPHIDs', array()); @@ -45,6 +34,21 @@ final class ConpherenceThreadSearchEngine $query->withParticipantPHIDs($participant_phids); } + $thread_type = $saved->getParameter('threadType'); + if (idx($this->getTypeOptions(), $thread_type)) { + switch ($thread_type) { + case 'rooms': + $query->withIsRoom(true); + break; + case 'messages': + $query->withIsRoom(false); + break; + case 'both': + $query->withIsRoom(null); + break; + } + } + return $query; } @@ -60,7 +64,13 @@ final class ConpherenceThreadSearchEngine ->setDatasource(new PhabricatorPeopleDatasource()) ->setName('participants') ->setLabel(pht('Participants')) - ->setValue($participant_phids)); + ->setValue($participant_phids)) + ->appendControl( + id(new AphrontFormSelectControl()) + ->setLabel(pht('Type')) + ->setName('threadType') + ->setOptions($this->getTypeOptions()) + ->setValue($saved->getParameter('threadType'))); } protected function getURI($path) { @@ -70,15 +80,13 @@ final class ConpherenceThreadSearchEngine protected function getBuiltinQueryNames() { $names = array(); - if ($this->isRooms) { - $names = array( - 'all' => pht('All Rooms'), - ); + $names = array( + 'all' => pht('All Rooms'), + ); - if ($this->requireViewer()->isLoggedIn()) { - $names['participant'] = pht('Joined Rooms'); - $names['messages'] = pht('All Messages'); - } + if ($this->requireViewer()->isLoggedIn()) { + $names['participant'] = pht('Joined Rooms'); + $names['messages'] = pht('All Messages'); } return $names; @@ -91,13 +99,15 @@ final class ConpherenceThreadSearchEngine switch ($query_key) { case 'all': + $query->setParameter('threadType', 'rooms'); return $query; case 'participant': + $query->setParameter('threadType', 'rooms'); return $query->setParameter( 'participantPHIDs', array($this->requireViewer()->getPHID())); case 'messages': - $this->setIsRooms(false); + $query->setParameter('threadType', 'messages'); return $query->setParameter( 'participantPHIDs', array($this->requireViewer()->getPHID())); @@ -122,6 +132,10 @@ final class ConpherenceThreadSearchEngine $viewer = $this->requireViewer(); + $policy_objects = ConpherenceThread::loadPolicyObjects( + $viewer, + $conpherences); + $list = new PHUIObjectItemListView(); $list->setUser($viewer); foreach ($conpherences as $conpherence) { @@ -129,6 +143,13 @@ final class ConpherenceThreadSearchEngine $data = $conpherence->getDisplayData($viewer); $title = $data['title']; + if ($conpherence->getIsRoom()) { + $icon_name = $conpherence->getPolicyIconName($policy_objects); + } else { + $icon_name = 'fa-envelope-o'; + } + $icon = id(new PHUIIconView()) + ->setIconFont($icon_name); $item = id(new PHUIObjectItemView()) ->setObjectName($conpherence->getMonogram()) ->setHeader($title) @@ -140,7 +161,7 @@ final class ConpherenceThreadSearchEngine pht('Messages: %d', $conpherence->getMessageCount())) ->addAttribute( array( - id(new PHUIIconView())->setIconFont('fa-envelope-o', 'green'), + $icon, ' ', pht( 'Last updated %s', @@ -152,4 +173,12 @@ final class ConpherenceThreadSearchEngine return $list; } + + private function getTypeOptions() { + return array( + 'rooms' => pht('Rooms'), + 'messages' => pht('Messages'), + 'both' => pht('Both'),); + } + } diff --git a/src/applications/conpherence/storage/ConpherenceThread.php b/src/applications/conpherence/storage/ConpherenceThread.php index 9de9130532..81f153038d 100644 --- a/src/applications/conpherence/storage/ConpherenceThread.php +++ b/src/applications/conpherence/storage/ConpherenceThread.php @@ -279,6 +279,30 @@ final class ConpherenceThread extends ConpherenceDAO } } + public static function loadPolicyObjects( + PhabricatorUser $viewer, + array $conpherences) { + + assert_instances_of($conpherences, 'ConpherenceThread'); + + $grouped = mgroup($conpherences, 'getIsRoom'); + $rooms = idx($grouped, 1, array()); + + $policies = array(); + foreach ($rooms as $room) { + $policies[] = $room->getViewPolicy(); + } + $policy_objects = array(); + if ($policies) { + $policy_objects = id(new PhabricatorPolicyQuery()) + ->setViewer($viewer) + ->withPHIDs($policies) + ->execute(); + } + + return $policy_objects; + } + public function getPolicyIconName(array $policy_objects) { assert_instances_of($policy_objects, 'PhabricatorPolicy'); diff --git a/src/applications/conpherence/view/ConpherenceThreadListView.php b/src/applications/conpherence/view/ConpherenceThreadListView.php index 3a3c04706d..5bd8016843 100644 --- a/src/applications/conpherence/view/ConpherenceThreadListView.php +++ b/src/applications/conpherence/view/ConpherenceThreadListView.php @@ -21,25 +21,16 @@ final class ConpherenceThreadListView extends AphrontView { public function render() { require_celerity_resource('conpherence-menu-css'); - $grouped = mgroup($this->threads, 'getIsRoom'); - $rooms = idx($grouped, 1, array()); - - $policies = array(); - foreach ($rooms as $room) { - $policies[] = $room->getViewPolicy(); - } - $policy_objects = array(); - if ($policies) { - $policy_objects = id(new PhabricatorPolicyQuery()) - ->setViewer($this->getUser()) - ->withPHIDs($policies) - ->execute(); - } - $menu = id(new PHUIListView()) ->addClass('conpherence-menu') ->setID('conpherence-menu'); + $policy_objects = ConpherenceThread::loadPolicyObjects( + $this->getUser(), + $this->threads); + + $grouped = mgroup($this->threads, 'getIsRoom'); + $rooms = idx($grouped, 1, array()); $this->addRoomsToMenu($menu, $rooms, $policy_objects); $messages = idx($grouped, 0, array()); $this->addMessagesToMenu($menu, $messages);