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

@@ -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(

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',

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,

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[] = '<table class="phabricator-chat-log">';
     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[] = '<tr class="initial '.$row_colors[$row_idx % 2].'">';
-        $out[] = '<td class="timestamp">'.
-          phabricator_datetime($log->getEpoch(), $user).'</td>';
+      // 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[] = '<td class="author">'.
-          phutil_escape_html($author).'</td>';
+      if ($new_block) {
+        if ($block) {
+          $blocks[] = $block;
+        }
+        $block = array(
+          'id'      => $log->getID(),
+          'epoch'   => $this_epoch,
+          'author'  => $this_author,
+          'logs'    => array($log),
+        );
       } else {
-        $out[] = '<tr class="'.$row_colors[$row_idx % 2].'">';
-        $out[] = '<td class="similar" colspan="2"></td>';
+        $block['logs'][] = $log;
       }
-      $out[] = '<td class="message">'.
-        phutil_escape_html($log->getMessage()).'</td>';
-      $out[] = '</tr>';
 
       $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[] = '<table class="phabricator-chat-log">';
+    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[] = '</table>';
 
+    $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(
+        '<div class="phabricator-chat-log-panel">',
+        $form,
+        '<br />',
         implode("\n", $out),
-        $pager
+        $pager,
+        '</div>',
       ),
       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);
+  }
+
 }

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,

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;
 }