From facd3ea8ef5a3633863afd9ed847b7ceb0cd558b Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Apr 2014 11:45:21 -0700 Subject: [PATCH] Modernize chatlog a bit Summary: Ref T4786. This doesn't fully fix the issue since there's no way to make channels public yet, but gets some of the infrastructure more up to date. - Allow public access to the list and log controllers. - Implement proper policy checks in the Events (this has no practical impact on the only controller that loads this stuff, it's just for general/future purposes). - Remove a old-style unused method for building page frames. Test Plan: Viewed log list and log details as logged-in and logged out users. Reviewers: btrahan Reviewed By: btrahan Subscribers: epriestley Maniphest Tasks: T4786 Differential Revision: https://secure.phabricator.com/D8746 --- src/__phutil_library_map__.php | 4 +-- ...habricatorChatLogChannelListController.php | 4 +++ ...PhabricatorChatLogChannelLogController.php | 4 +++ .../PhabricatorChatLogController.php | 13 ------- .../PhabricatorChatLogChannelQuery.php | 0 .../{ => query}/PhabricatorChatLogQuery.php | 22 ++++++++++++ .../storage/PhabricatorChatLogEvent.php | 34 +++++++++++++------ 7 files changed, 55 insertions(+), 26 deletions(-) rename src/applications/chatlog/{ => query}/PhabricatorChatLogChannelQuery.php (100%) rename src/applications/chatlog/{ => query}/PhabricatorChatLogQuery.php (70%) diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 413dcf82c2..943acb974b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1299,13 +1299,13 @@ phutil_register_library_map(array( 'PhabricatorChatLogChannel' => 'applications/chatlog/storage/PhabricatorChatLogChannel.php', 'PhabricatorChatLogChannelListController' => 'applications/chatlog/controller/PhabricatorChatLogChannelListController.php', 'PhabricatorChatLogChannelLogController' => 'applications/chatlog/controller/PhabricatorChatLogChannelLogController.php', - 'PhabricatorChatLogChannelQuery' => 'applications/chatlog/PhabricatorChatLogChannelQuery.php', + 'PhabricatorChatLogChannelQuery' => 'applications/chatlog/query/PhabricatorChatLogChannelQuery.php', 'PhabricatorChatLogConstants' => 'applications/chatlog/constants/PhabricatorChatLogConstants.php', 'PhabricatorChatLogController' => 'applications/chatlog/controller/PhabricatorChatLogController.php', 'PhabricatorChatLogDAO' => 'applications/chatlog/storage/PhabricatorChatLogDAO.php', 'PhabricatorChatLogEvent' => 'applications/chatlog/storage/PhabricatorChatLogEvent.php', 'PhabricatorChatLogEventType' => 'applications/chatlog/constants/PhabricatorChatLogEventType.php', - 'PhabricatorChatLogQuery' => 'applications/chatlog/PhabricatorChatLogQuery.php', + 'PhabricatorChatLogQuery' => 'applications/chatlog/query/PhabricatorChatLogQuery.php', 'PhabricatorCommitBranchesField' => 'applications/repository/customfield/PhabricatorCommitBranchesField.php', 'PhabricatorCommitCustomField' => 'applications/repository/customfield/PhabricatorCommitCustomField.php', 'PhabricatorCommitTagsField' => 'applications/repository/customfield/PhabricatorCommitTagsField.php', diff --git a/src/applications/chatlog/controller/PhabricatorChatLogChannelListController.php b/src/applications/chatlog/controller/PhabricatorChatLogChannelListController.php index 945909bbbb..d313aa132c 100644 --- a/src/applications/chatlog/controller/PhabricatorChatLogChannelListController.php +++ b/src/applications/chatlog/controller/PhabricatorChatLogChannelListController.php @@ -3,6 +3,10 @@ final class PhabricatorChatLogChannelListController extends PhabricatorChatLogController { + public function shouldAllowPublic() { + return true; + } + public function processRequest() { $request = $this->getRequest(); $user = $request->getUser(); diff --git a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php index b457b6f001..1bcdf1bc59 100644 --- a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php +++ b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php @@ -5,6 +5,10 @@ final class PhabricatorChatLogChannelLogController private $channelID; + public function shouldAllowPublic() { + return true; + } + public function willProcessRequest(array $data) { $this->channelID = $data['channelID']; } diff --git a/src/applications/chatlog/controller/PhabricatorChatLogController.php b/src/applications/chatlog/controller/PhabricatorChatLogController.php index b7f12ab97c..55d02940ab 100644 --- a/src/applications/chatlog/controller/PhabricatorChatLogController.php +++ b/src/applications/chatlog/controller/PhabricatorChatLogController.php @@ -2,17 +2,4 @@ abstract class PhabricatorChatLogController extends PhabricatorController { - public function buildStandardPageResponse($view, array $data) { - $page = $this->buildStandardPageView(); - - $page->setApplicationName(pht('Chat Log')); - $page->setBaseURI('/chatlog/'); - $page->setTitle(idx($data, 'title')); - $page->setGlyph('#'); - $page->appendChild($view); - - $response = new AphrontWebpageResponse(); - return $response->setContent($page->render()); - } - } diff --git a/src/applications/chatlog/PhabricatorChatLogChannelQuery.php b/src/applications/chatlog/query/PhabricatorChatLogChannelQuery.php similarity index 100% rename from src/applications/chatlog/PhabricatorChatLogChannelQuery.php rename to src/applications/chatlog/query/PhabricatorChatLogChannelQuery.php diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/query/PhabricatorChatLogQuery.php similarity index 70% rename from src/applications/chatlog/PhabricatorChatLogQuery.php rename to src/applications/chatlog/query/PhabricatorChatLogQuery.php index c5b1f8ac66..8cb4185a21 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/query/PhabricatorChatLogQuery.php @@ -33,6 +33,28 @@ final class PhabricatorChatLogQuery return $logs; } + public function willFilterPage(array $events) { + $channel_ids = mpull($events, 'getChannelID', 'getChannelID'); + + $channels = id(new PhabricatorChatLogChannelQuery()) + ->setViewer($this->getViewer()) + ->withIDs($channel_ids) + ->execute(); + $channels = mpull($channels, null, 'getID'); + + foreach ($events as $key => $event) { + $channel = idx($channels, $event->getChannelID()); + if (!$channel) { + unset($events[$key]); + continue; + } + + $event->attachChannel($channel); + } + + return $events; + } + private function buildWhereClause($conn_r) { $where = array(); diff --git a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php index c889fd19fe..1fe4e8bd1e 100644 --- a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php +++ b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php @@ -11,6 +11,27 @@ final class PhabricatorChatLogEvent protected $message; protected $loggedByPHID; + private $channel = self::ATTACHABLE; + + public function getConfiguration() { + return array( + self::CONFIG_TIMESTAMPS => false, + ) + parent::getConfiguration(); + } + + public function attachChannel(PhabricatorChatLogChannel $channel) { + $this->channel = $channel; + return $this; + } + + public function getChannel() { + return $this->assertAttached($this->channel); + } + + +/* -( PhabricatorPolicyInterface )----------------------------------------- */ + + public function getCapabilities() { return array( PhabricatorPolicyCapability::CAN_VIEW, @@ -18,24 +39,15 @@ final class PhabricatorChatLogEvent } public function getPolicy($capability) { - // TODO: This is sort of silly and mostly just so that we can use - // CursorPagedPolicyAwareQuery; once we implement Channel objects we should - // just delegate policy to them. - return PhabricatorPolicies::POLICY_PUBLIC; + return $this->getChannel()->getPolicy($capability); } public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { - return false; + return $this->getChannel()->hasAutomaticCapability($capability, $viewer); } public function describeAutomaticCapability($capability) { return null; } - public function getConfiguration() { - return array( - self::CONFIG_TIMESTAMPS => false, - ) + parent::getConfiguration(); - } - } -- 2.51.2