From 6a8ac91599271f16852cd8364284e433c095bb84 Mon Sep 17 00:00:00 2001 From: epriestley Date: Sat, 2 Jun 2012 14:00:08 -0700 Subject: [PATCH] Make chatlog a bit less awful Summary: - Default to showing the newest page of chat. - Reformat for greater readability. - Add permalinks to specific lines. - Enable jump-to-date. Test Plan: {F12200} Reviewers: Koolvin, vrana, btrahan Reviewed By: btrahan CC: kdeggelman, aran Maniphest Tasks: T837, T1065 Differential Revision: https://secure.phabricator.com/D2641 --- src/__celerity_resource_map__.php | 2 +- src/__phutil_library_map__.php | 8 +- .../chatlog/PhabricatorChatLogQuery.php | 32 ++- ...PhabricatorChatLogChannelLogController.php | 223 +++++++++++++++--- .../storage/PhabricatorChatLogEvent.php | 21 +- .../rsrc/css/application/chatlog/chatlog.css | 58 +++-- 6 files changed, 277 insertions(+), 67 deletions(-) diff --git a/src/__celerity_resource_map__.php b/src/__celerity_resource_map__.php index c1acd44905..cd4a8d4e6e 100644 --- a/src/__celerity_resource_map__.php +++ b/src/__celerity_resource_map__.php @@ -1986,7 +1986,7 @@ celerity_register_resource_map(array( ), 'phabricator-chatlog-css' => array( - 'uri' => '/res/f674f526/rsrc/css/application/chatlog/chatlog.css', + 'uri' => '/res/f6631adc/rsrc/css/application/chatlog/chatlog.css', 'type' => 'css', 'requires' => array( diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index ce28b6e9d0..1fcf4c109b 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -1542,9 +1542,13 @@ phutil_register_library_map(array( 'PhabricatorChatLogChannelLogController' => 'PhabricatorChatLogController', 'PhabricatorChatLogController' => 'PhabricatorController', 'PhabricatorChatLogDAO' => 'PhabricatorLiskDAO', - 'PhabricatorChatLogEvent' => 'PhabricatorChatLogDAO', + 'PhabricatorChatLogEvent' => + array( + 0 => 'PhabricatorChatLogDAO', + 1 => 'PhabricatorPolicyInterface', + ), 'PhabricatorChatLogEventType' => 'PhabricatorChatLogConstants', - 'PhabricatorChatLogQuery' => 'PhabricatorOffsetPagedQuery', + 'PhabricatorChatLogQuery' => 'PhabricatorIDPagedPolicyQuery', 'PhabricatorConduitAPIController' => 'PhabricatorConduitController', 'PhabricatorConduitCertificateToken' => 'PhabricatorConduitDAO', 'PhabricatorConduitConnectionLog' => 'PhabricatorConduitDAO', diff --git a/src/applications/chatlog/PhabricatorChatLogQuery.php b/src/applications/chatlog/PhabricatorChatLogQuery.php index 2ebba619f6..a5dec050c6 100644 --- a/src/applications/chatlog/PhabricatorChatLogQuery.php +++ b/src/applications/chatlog/PhabricatorChatLogQuery.php @@ -16,36 +16,50 @@ * limitations under the License. */ -final class PhabricatorChatLogQuery extends PhabricatorOffsetPagedQuery { +final class PhabricatorChatLogQuery extends PhabricatorIDPagedPolicyQuery { + private $channels; + private $maximumEpoch; public function withChannels(array $channels) { $this->channels = $channels; return $this; } - public function execute() { + public function withMaximumEpoch($epoch) { + $this->maximumEpoch = $epoch; + return $this; + } + + public function loadPage() { $table = new PhabricatorChatLogEvent(); $conn_r = $table->establishConnection('r'); - $where_clause = $this->buildWhereClause($conn_r); - $limit_clause = $this->buildLimitClause($conn_r); - $data = queryfx_all( $conn_r, - 'SELECT * FROM %T e %Q ORDER BY epoch ASC %Q', + 'SELECT * FROM %T e %Q %Q %Q', $table->getTableName(), - $where_clause, - $limit_clause); + $this->buildWhereClause($conn_r), + $this->buildOrderClause($conn_r), + $this->buildLimitClause($conn_r)); $logs = $table->loadAllFromArray($data); - return $logs; + return $this->processResults($logs); } private function buildWhereClause($conn_r) { $where = array(); + $where[] = $this->buildPagingClause($conn_r); + + if ($this->maximumEpoch) { + $where[] = qsprintf( + $conn_r, + 'epoch <= %d', + $this->maximumEpoch); + } + if ($this->channels) { $where[] = qsprintf( $conn_r, diff --git a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php index 2f89b40a49..15af54aec4 100644 --- a/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php +++ b/src/applications/chatlog/controller/PhabricatorChatLogChannelLogController.php @@ -26,70 +26,217 @@ final class PhabricatorChatLogChannelLogController } public function processRequest() { + $request = $this->getRequest(); + $user = $request->getUser(); - $request = $this->getRequest(); - $user = $request->getUser(); - $offset = $request->getInt('offset', 0); - $page_size = 1000; - $pager = new AphrontPagerView(); - $request_uri = $request->getRequestURI(); - $pager->setURI($request_uri, 'offset'); - $pager->setPageSize($page_size); - $pager->setOffset($offset); + $uri = clone $request->getRequestURI(); + $uri->setQueryParams(array()); + + $pager = new AphrontIDPagerView(); + $pager->setURI($uri); + $pager->setPageSize(250); + + $query = id(new PhabricatorChatLogQuery()) + ->setViewer($user) + ->withChannels(array($this->channel)); + + + list($after, $before, $map) = $this->getPagingParameters($request, $query); + + $pager->setAfterID($after); + $pager->setBeforeID($before); - $query = new PhabricatorChatLogQuery(); - $query->withChannels(array($this->channel)); $logs = $query->executeWithPager($pager); - require_celerity_resource('phabricator-chatlog-css'); + // Show chat logs oldest-first. + $logs = array_reverse($logs); + + + // Divide all the logs into blocks, where a block is the same author saying + // several things in a row. A block ends when another user speaks, or when + // two minutes pass without the author speaking. + + $blocks = array(); + $block = null; $last_author = null; $last_epoch = null; - - $row_idx = 0; - $row_colors = array( - 'normal', - 'alternate', - ); - - $out = array(); - $out[] = ''; foreach ($logs as $log) { $this_author = $log->getAuthor(); $this_epoch = $log->getEpoch(); - if (($this_author !== $last_author) || - ($this_epoch - (60 * 5) > $last_epoch)) { - ++$row_idx; - $out[] = ''; - $out[] = ''; + // Decide whether we should start a new block or not. + $new_block = ($this_author !== $last_author) || + ($this_epoch - (60 * 2) > $last_epoch); - $author = $log->getAuthor(); - $author = phutil_utf8_shorten($author, 18); - $out[] = ''; + if ($new_block) { + if ($block) { + $blocks[] = $block; + } + $block = array( + 'id' => $log->getID(), + 'epoch' => $this_epoch, + 'author' => $this_author, + 'logs' => array($log), + ); } else { - $out[] = ''; - $out[] = ''; + $block['logs'][] = $log; } - $out[] = ''; - $out[] = ''; $last_author = $this_author; - $last_epoch = $this_epoch; + $last_epoch = $this_epoch; + } + if ($block) { + $blocks[] = $block; + } + + // Figure out CSS classes for the blocks. We alternate colors between + // lines, and highlight the entire block which contains the target ID or + // date, if applicable. + + foreach ($blocks as $key => $block) { + $classes = array(); + if ($key % 2) { + $classes[] = 'alternate'; + } + $ids = mpull($block['logs'], 'getID', 'getID'); + if (array_intersect_key($ids, $map)) { + $classes[] = 'highlight'; + } + $blocks[$key]['class'] = $classes ? implode(' ', $classes) : null; + } + + + require_celerity_resource('phabricator-chatlog-css'); + + $out = array(); + $out[] = '
'. - phabricator_datetime($log->getEpoch(), $user).''. - phutil_escape_html($author).'
'. - phutil_escape_html($log->getMessage()).'
'; + foreach ($blocks as $block) { + $author = $block['author']; + $author = phutil_utf8_shorten($author, 18); + $author = phutil_escape_html($author); + $author = phutil_render_tag('td', array('class' => 'author'), $author); + + $message = mpull($block['logs'], 'getMessage'); + $message = implode("\n", $message); + $message = phutil_escape_html($message); + $message = phutil_render_tag('td', array('class' => 'message'), $message); + + $href = $uri->alter('at', $block['id']); + $timestamp = $block['epoch']; + $timestamp = phabricator_datetime($timestamp, $user); + $timestamp = phutil_render_tag('a', array('href' => $href), $timestamp); + $timestamp = phutil_render_tag( + 'td', + array( + 'class' => 'timestamp', + ), + $timestamp); + + $out[] = phutil_render_tag( + 'tr', + array( + 'class' => $block['class'], + ), + $author.$message.$timestamp); } $out[] = '
'; + $form = id(new AphrontFormView()) + ->setUser($user) + ->setMethod('GET') + ->setAction($uri) + ->appendChild( + id(new AphrontFormTextControl()) + ->setLabel('Date') + ->setName('date') + ->setValue($request->getStr('date'))) + ->appendChild( + id(new AphrontFormSubmitControl()) + ->setValue('Jump')); + return $this->buildStandardPageResponse( array( + '
', + $form, + '
', implode("\n", $out), - $pager + $pager, + '
', ), array( 'title' => 'Channel Log', )); } + + /** + * From request parameters, figure out where we should jump to in the log. + * We jump to either a date or log ID, but load a few lines of context before + * it so the user can see the nearby conversation. + */ + private function getPagingParameters( + AphrontRequest $request, + PhabricatorChatLogQuery $query) { + + $user = $request->getUser(); + + $at_id = $request->getInt('at'); + $at_date = $request->getStr('date'); + + $context_log = null; + $map = array(); + + $query = clone $query; + $query->setLimit(8); + + if ($at_id) { + // Jump to the log in question, and load a few lines of context before + // it. + $context_logs = $query + ->setAfterID($at_id) + ->execute(); + + $context_log = last($context_logs); + + $map = array( + $at_id => true, + ); + + } else if ($at_date) { + $timezone = new DateTimeZone($user->getTimezoneIdentifier()); + try { + $date = new DateTime($at_date, $timezone); + $timestamp = $date->format('U'); + } catch (Exception $e) { + $timestamp = null; + } + + if ($timestamp) { + $context_logs = $query + ->withMaximumEpoch($timestamp) + ->execute(); + + $context_log = last($context_logs); + + $target_log = head($context_logs); + if ($target_log) { + $map = array( + $target_log->getID() => true, + ); + } + } + } + + if ($context_log) { + $after = null; + $before = $context_log->getID() - 1; + } else { + $after = $request->getInt('after'); + $before = $request->getInt('before'); + } + + return array($after, $before, $map); + } + } diff --git a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php index 9a5af593ef..6ac1b33e2c 100644 --- a/src/applications/chatlog/storage/PhabricatorChatLogEvent.php +++ b/src/applications/chatlog/storage/PhabricatorChatLogEvent.php @@ -16,7 +16,9 @@ * limitations under the License. */ -final class PhabricatorChatLogEvent extends PhabricatorChatLogDAO { +final class PhabricatorChatLogEvent + extends PhabricatorChatLogDAO + implements PhabricatorPolicyInterface { protected $channel; protected $epoch; @@ -25,6 +27,23 @@ final class PhabricatorChatLogEvent extends PhabricatorChatLogDAO { protected $message; protected $loggedByPHID; + public function getCapabilities() { + return array( + PhabricatorPolicyCapability::CAN_VIEW, + ); + } + + public function getPolicy($capability) { + // TODO: This is sort of silly and mostly just so that we can use + // IDPagedPolicyQuery; once we implement Channel objects we should + // just delegate policy to them. + return PhabricatorPolicies::POLICY_PUBLIC; + } + + public function hasAutomaticCapability($capability, PhabricatorUser $viewer) { + return false; + } + public function getConfiguration() { return array( self::CONFIG_TIMESTAMPS => false, diff --git a/webroot/rsrc/css/application/chatlog/chatlog.css b/webroot/rsrc/css/application/chatlog/chatlog.css index 5ae98f56d4..c9a94e20d4 100644 --- a/webroot/rsrc/css/application/chatlog/chatlog.css +++ b/webroot/rsrc/css/application/chatlog/chatlog.css @@ -2,38 +2,64 @@ * @provides phabricator-chatlog-css */ +.phabricator-chat-log-panel { + margin: 1em auto; + width: 80em; +} + .phabricator-chat-log { - margin: 1em 2em; font-size: 12px; -} - -.phabricator-chat-log tr.initial { - border-top: 4px solid white; -} - -.phabricator-chat-log tr.normal { - background: #e9e9e9; -} - -.phabricator-chat-log tr.alternate { - background: #f6f6f6; + width: 100%; + border: 1px solid #bbbbbb; } .phabricator-chat-log td { - padding: 2px 4px; + padding: 4px 8px; +} + +.phabricator-chat-log tr { + background: #fafafa; +} + +.phabricator-chat-log tr td.author { + background: #dfdfdf; +} + +.phabricator-chat-log tr.alternate { + background: #e9e9e9; +} + +.phabricator-chat-log tr.alternate td.author { + background: #d9d9d9; +} + +.phabricator-chat-log tr.highlight td { + background: #ffff88; +} + +.phabricator-chat-log tr.highlight td.author { + background: #eeee88; } .phabricator-chat-log td.timestamp { white-space: nowrap; - color: #666666; + text-align: right; + width: 12em; +} + +.phabricator-chat-log td.timestamp a { + color: #555555; + font-size: 11px; } .phabricator-chat-log td.author { white-space: nowrap; text-align: right; font-weight: bold; + width: 12em; + color: #555555; } .phabricator-chat-log td.message { - width: 100%; + white-space: pre-wrap; }