Conpherence - add support for linking directly to messages regardless of age of message

Summary: Fixes T7757. Since anchor links can't be processed server side, we have to detect the message is old in javascript, then re-loaded the page. This opens up a new corner case where we have to paginate in newer messages, so this also adds support for that.

Test Plan:
- set main query limit to 8 and then visited ZXX#YYY. noted a second quick load of YYY, that YYY ended up highlighted and scrolled to.
- used "show newer messages" and "show older messages" successfully, taking care to make sure transaction ids were all correct with no off by one errors, etc.
- opened and closed durable column to make sure that still works too

Reviewers: chad, epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7757

Differential Revision: https://secure.phabricator.com/D12633

resources/celerity/map.php

@@ -46,7 +46,7 @@ return array(
     'rsrc/css/application/config/unhandled-exception.css' => '37d4f9a2',
     'rsrc/css/application/conpherence/durable-column.css' => '2e68a92f',
     'rsrc/css/application/conpherence/menu.css' => 'f389e048',
-    'rsrc/css/application/conpherence/message-pane.css' => 'e7c09fda',
+    'rsrc/css/application/conpherence/message-pane.css' => '3150e2a2',
     'rsrc/css/application/conpherence/notification.css' => 'd208f806',
     'rsrc/css/application/conpherence/transaction.css' => '25138b7f',
     'rsrc/css/application/conpherence/update.css' => '1099a660',
@@ -355,9 +355,9 @@ return array(
     'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761',
     'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18',
     'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de',
-    'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '0a5192c4',
+    'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '6709c934',
     'rsrc/js/application/conpherence/behavior-durable-column.js' => '657c2b50',
-    'rsrc/js/application/conpherence/behavior-menu.js' => '077a1dab',
+    'rsrc/js/application/conpherence/behavior-menu.js' => '804b0773',
     'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861',
     'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3',
     'rsrc/js/application/conpherence/behavior-widget-pane.js' => '93568464',
@@ -519,9 +519,9 @@ return array(
     'config-welcome-css' => '6abd79be',
     'conpherence-durable-column-view' => '2e68a92f',
     'conpherence-menu-css' => 'f389e048',
-    'conpherence-message-pane-css' => 'e7c09fda',
+    'conpherence-message-pane-css' => '3150e2a2',
     'conpherence-notification-css' => 'd208f806',
-    'conpherence-thread-manager' => '0a5192c4',
+    'conpherence-thread-manager' => '6709c934',
     'conpherence-transaction-css' => '25138b7f',
     'conpherence-update-css' => '1099a660',
     'conpherence-widget-pane-css' => '2af42ebe',
@@ -561,7 +561,7 @@ return array(
     'javelin-behavior-audit-preview' => 'd835b03a',
     'javelin-behavior-choose-control' => '6153c708',
     'javelin-behavior-config-reorder-fields' => '14a827de',
-    'javelin-behavior-conpherence-menu' => '077a1dab',
+    'javelin-behavior-conpherence-menu' => '804b0773',
     'javelin-behavior-conpherence-pontificate' => '21ba5861',
     'javelin-behavior-conpherence-widget-pane' => '93568464',
     'javelin-behavior-countdown-timer' => 'e4cc26b3',
@@ -873,20 +873,6 @@ return array(
       'javelin-stratcom',
       'javelin-workflow',
     ),
-    '077a1dab' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'javelin-behavior-device',
-      'javelin-history',
-      'javelin-vector',
-      'javelin-scrollbar',
-      'phabricator-title',
-      'phabricator-shaped-request',
-      'conpherence-thread-manager',
-    ),
     '07de8873' => array(
       'javelin-install',
       'javelin-util',
@@ -902,16 +888,6 @@ return array(
       'javelin-dom',
       'javelin-router',
     ),
-    '0a5192c4' => array(
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-install',
-      'javelin-workflow',
-      'javelin-router',
-      'javelin-behavior-device',
-      'javelin-vector',
-    ),
     '0c6946e7' => array(
       'javelin-install',
       'javelin-dom',
@@ -1311,6 +1287,16 @@ return array(
       'phabricator-keyboard-shortcut',
       'conpherence-thread-manager',
     ),
+    '6709c934' => array(
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-install',
+      'javelin-workflow',
+      'javelin-router',
+      'javelin-behavior-device',
+      'javelin-vector',
+    ),
     '6882e80a' => array(
       'javelin-dom',
     ),
@@ -1440,6 +1426,20 @@ return array(
       'javelin-behavior',
       'javelin-history',
     ),
+    '804b0773' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'javelin-behavior-device',
+      'javelin-history',
+      'javelin-vector',
+      'javelin-scrollbar',
+      'phabricator-title',
+      'phabricator-shaped-request',
+      'conpherence-thread-manager',
+    ),
     82439934 => array(
       'javelin-behavior',
       'javelin-dom',

src/applications/conpherence/ConpherenceTransactionRenderer.php

@@ -5,17 +5,40 @@ final class ConpherenceTransactionRenderer {
   public static function renderTransactions(
     PhabricatorUser $user,
     ConpherenceThread $conpherence,
-    $full_display = true) {
+    $full_display = true,
+    $marker_type = 'older') {
 
     $transactions = $conpherence->getTransactions();
+
     $oldest_transaction_id = 0;
+    $newest_transaction_id = 0;
     $too_many = ConpherenceThreadQuery::TRANSACTION_LIMIT + 1;
     if (count($transactions) == $too_many) {
-      $last_transaction = end($transactions);
-      unset($transactions[$last_transaction->getID()]);
-      $oldest_transaction = end($transactions);
-      $oldest_transaction_id = $oldest_transaction->getID();
+      if ($marker_type == 'olderandnewer') {
+        $last_transaction = end($transactions);
+        $first_transaction = reset($transactions);
+        unset($transactions[$last_transaction->getID()]);
+        unset($transactions[$first_transaction->getID()]);
+        $oldest_transaction_id = $last_transaction->getID();
+        $newest_transaction_id = $first_transaction->getID();
+      } else if ($marker_type == 'newer') {
+        $first_transaction = reset($transactions);
+        unset($transactions[$first_transaction->getID()]);
+        $newest_transaction_id = $first_transaction->getID();
+      } else if ($marker_type == 'older') {
+        $last_transaction = end($transactions);
+        unset($transactions[$last_transaction->getID()]);
+        $oldest_transaction = end($transactions);
+        $oldest_transaction_id = $oldest_transaction->getID();
+      }
+    // we need **at least** the newer marker in this mode even if
+    // we didn't get a full set of transactions
+    } else if ($marker_type == 'olderandnewer') {
+      $first_transaction = reset($transactions);
+      unset($transactions[$first_transaction->getID()]);
+      $newest_transaction_id = $first_transaction->getID();
     }
+
     $transactions = array_reverse($transactions);
     $handles = $conpherence->getHandles();
     $rendered_transactions = array();
@@ -98,22 +121,24 @@ final class ConpherenceTransactionRenderer {
       'latest_transaction' => $transaction,
       'latest_transaction_id' => $latest_transaction_id,
       'oldest_transaction_id' => $oldest_transaction_id,
+      'newest_transaction_id' => $newest_transaction_id,
     );
   }
 
   public static function renderMessagePaneContent(
     array $transactions,
-    $oldest_transaction_id) {
+    $oldest_transaction_id,
+    $newest_transaction_id) {
 
-    $scrollbutton = '';
+    $oldscrollbutton = '';
     if ($oldest_transaction_id) {
-      $scrollbutton = javelin_tag(
+      $oldscrollbutton = javelin_tag(
         'a',
         array(
           'href' => '#',
           'mustcapture' => true,
           'sigil' => 'show-older-messages',
-          'class' => 'conpherence-show-older-messages',
+          'class' => 'conpherence-show-more-messages',
           'meta' => array(
             'oldest_transaction_id' => $oldest_transaction_id,
           ),
@@ -121,7 +146,27 @@ final class ConpherenceTransactionRenderer {
         pht('Show Older Messages'));
     }
 
-    return hsprintf('%s%s', $scrollbutton, $transactions);
+    $newscrollbutton = '';
+    if ($newest_transaction_id) {
+      $newscrollbutton = javelin_tag(
+        'a',
+        array(
+          'href' => '#',
+          'mustcapture' => true,
+          'sigil' => 'show-newer-messages',
+          'class' => 'conpherence-show-more-messages',
+          'meta' => array(
+            'newest_transaction_id' => $newest_transaction_id,
+          ),
+        ),
+        pht('Show Newer Messages'));
+    }
+
+    return hsprintf(
+      '%s%s%s',
+      $oldscrollbutton,
+      $transactions,
+      $newscrollbutton);
   }
 
 }

src/applications/conpherence/application/PhabricatorConpherenceApplication.php

@@ -41,6 +41,8 @@ final class PhabricatorConpherenceApplication extends PhabricatorApplication {
         ''                         => 'ConpherenceListController',
         'thread/(?P<id>[1-9]\d*)/' => 'ConpherenceListController',
         '(?P<id>[1-9]\d*)/'        => 'ConpherenceViewController',
+        '(?P<id>[1-9]\d*)/(?P<messageID>[1-9]\d*)/'
+                                   => 'ConpherenceViewController',
         'columnview/'              => 'ConpherenceColumnViewController',
         'new/'                     => 'ConpherenceNewController',
         'room/new/'                => 'ConpherenceNewRoomController',

src/applications/conpherence/controller/ConpherenceViewController.php

@@ -3,6 +3,8 @@
 final class ConpherenceViewController extends
   ConpherenceController {
 
+  const OLDER_FETCH_LIMIT = 5;
+
   public function handleRequest(AphrontRequest $request) {
     $user = $request->getUser();
 
@@ -15,19 +17,43 @@ final class ConpherenceViewController extends
       ->withIDs(array($conpherence_id))
       ->needParticipantCache(true)
       ->needTransactions(true)
-      ->setTransactionLimit(ConpherenceThreadQuery::TRANSACTION_LIMIT);
+      ->setTransactionLimit($this->getMainQueryLimit());
+
     $before_transaction_id = $request->getInt('oldest_transaction_id');
+    $after_transaction_id = $request->getInt('newest_transaction_id');
+    $old_message_id = $request->getURIData('messageID');
+    if ($before_transaction_id && ($old_message_id || $after_transaction_id)) {
+      throw new Aphront400Response();
+    }
+    if ($old_message_id && $after_transaction_id) {
+      throw new Aphront400Response();
+    }
+
+    $marker_type = 'older';
     if ($before_transaction_id) {
       $query
         ->setBeforeTransactionID($before_transaction_id);
     }
+    if ($old_message_id) {
+      $marker_type = 'olderandnewer';
+      $query
+        ->setAfterTransactionID($old_message_id - 1);
+    }
+    if ($after_transaction_id) {
+      $marker_type = 'newer';
+      $query
+        ->setAfterTransactionID($after_transaction_id);
+    }
+
     $conpherence = $query->executeOne();
     if (!$conpherence) {
       return new Aphront404Response();
     }
     $this->setConpherence($conpherence);
 
-    $transactions = $conpherence->getTransactions();
+    $transactions = $this->getNeededTransactions(
+      $conpherence,
+      $old_message_id);
     $latest_transaction = head($transactions);
     $participant = $conpherence->getParticipantIfExists($user->getPHID());
     if ($participant) {
@@ -38,11 +64,14 @@ final class ConpherenceViewController extends
 
     $data = ConpherenceTransactionRenderer::renderTransactions(
       $user,
-      $conpherence);
+      $conpherence,
+      $full_display = true,
+      $marker_type);
     $messages = ConpherenceTransactionRenderer::renderMessagePaneContent(
       $data['transactions'],
-      $data['oldest_transaction_id']);
-    if ($before_transaction_id) {
+      $data['oldest_transaction_id'],
+      $data['newest_transaction_id']);
+    if ($before_transaction_id || $after_transaction_id) {
       $header = null;
       $form = null;
       $content = array('messages' => $messages);
@@ -138,5 +167,39 @@ final class ConpherenceViewController extends
     return $form;
   }
 
+  private function getNeededTransactions(
+    ConpherenceThread $conpherence,
+    $message_id) {
 
+    if ($message_id) {
+      $newer_transactions = $conpherence->getTransactions();
+      $query = id(new ConpherenceTransactionQuery())
+        ->setViewer($this->getRequest()->getUser())
+        ->withObjectPHIDs(array($conpherence->getPHID()))
+        ->setAfterID($message_id)
+        ->needHandles(true)
+        ->setLimit(self::OLDER_FETCH_LIMIT);
+      $older_transactions = $query->execute();
+      $handles = array();
+      foreach ($older_transactions as $transaction) {
+        $handles += $transaction->getHandles();
+      }
+      $conpherence->attachHandles($conpherence->getHandles() + $handles);
+      $transactions = array_merge($newer_transactions, $older_transactions);
+      $conpherence->attachTransactions($transactions);
+    } else {
+      $transactions = $conpherence->getTransactions();
+    }
+
+    return $transactions;
+  }
+
+  private function getMainQueryLimit() {
+    $request = $this->getRequest();
+    $base_limit = ConpherenceThreadQuery::TRANSACTION_LIMIT;
+    if ($request->getURIData('messageID')) {
+      $base_limit = $base_limit - self::OLDER_FETCH_LIMIT;
+    }
+    return $base_limit;
+  }
 }

src/applications/conpherence/view/ConpherenceDurableColumnView.php

@@ -463,7 +463,8 @@ final class ConpherenceDurableColumnView extends AphrontTagView {
       $full_display = false);
     $messages = ConpherenceTransactionRenderer::renderMessagePaneContent(
       $data['transactions'],
-      $data['oldest_transaction_id']);
+      $data['oldest_transaction_id'],
+      $data['newest_transaction_id']);
 
     return $messages;
   }

src/view/page/PhabricatorStandardPageView.php

@@ -97,15 +97,27 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       return false;
     }
 
+    if ($this->isQuicksandBlacklistURI()) {
+      return false;
+    }
+
+    return true;
+  }
+
+  private function isQuicksandBlacklistURI() {
+    $request = $this->getRequest();
+    if (!$request) {
+      return false;
+    }
+
     $patterns = $this->getQuicksandURIPatternBlacklist();
     $path = $request->getRequestURI()->getPath();
     foreach ($patterns as $pattern) {
       if (preg_match('(^'.$pattern.'$)', $path)) {
-        return false;
+        return true;
       }
     }
-
-    return true;
+    return false;
   }
 
   public function getDurableColumnVisible() {
@@ -365,12 +377,14 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView {
       }
     }
 
-    Javelin::initBehavior(
-      'scrollbar',
-      array(
-        'nodeID' => 'phabricator-standard-page',
-        'isMainContent' => true,
-      ));
+    if (!$this->isQuicksandBlacklistURI()) {
+      Javelin::initBehavior(
+        'scrollbar',
+        array(
+          'nodeID' => 'phabricator-standard-page',
+          'isMainContent' => true,
+        ));
+    }
 
     $main_page = phutil_tag(
       'div',

webroot/rsrc/css/application/conpherence/message-pane.css

@@ -37,7 +37,7 @@
   background: #EBECEE;
 }
 
-.conpherence-show-older-messages {
+.conpherence-show-more-messages {
   display: block;
   background: #e0e3ec;
   margin: 10px;
@@ -46,7 +46,7 @@
   color: {$bluetext};
 }
 
-.conpherence-show-older-messages-loading {
+.conpherence-show-more-messages-loading {
   font-style: italic;
 }
 

webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js

@@ -254,9 +254,10 @@ JX.install('ConpherenceThreadManager', {
       this.syncWorkflow(workflow, params.stage);
     },
 
-    loadThreadByID: function(thread_id) {
+    loadThreadByID: function(thread_id, force_reload) {
       if (this.isThreadLoaded() &&
-          this.isThreadIDLoaded(thread_id)) {
+          this.isThreadIDLoaded(thread_id) &&
+          !force_reload) {
         return;
       }
 
@@ -277,6 +278,10 @@ JX.install('ConpherenceThreadManager', {
         JX.Stratcom.invoke('notification-panel-update', null, {});
 
         this._didLoadThreadCallback(r);
+
+        if (force_reload) {
+          JX.Stratcom.invoke('hashchange');
+        }
       });
 
       // should this be sync'd too?

webroot/rsrc/js/application/conpherence/behavior-menu.js

@@ -119,6 +119,7 @@ JX.behavior('conpherence-menu', function(config) {
     var messages_root = JX.DOM.find(root, 'div', 'conpherence-message-pane');
     var messages = JX.DOM.find(messages_root, 'div', 'conpherence-messages');
     scrollbar = new JX.Scrollbar(messages);
+    scrollbar.setAsScrollFrame();
   }
   init();
 
@@ -317,12 +318,26 @@ JX.behavior('conpherence-menu', function(config) {
         buildDeviceWidgetSelector : build_device_widget_selector
       });
   }
+
   var _firstScroll = true;
   function _scrollMessageWindow() {
     if (_firstScroll) {
       _firstScroll = false;
-      // let the standard #anchor tech take over
+
+      // We want to let the standard #anchor tech take over after we make sure
+      // we don't have to present the user with a "load older message?" dialog
       if (window.location.hash) {
+        var hash = window.location.hash.replace(/^#/, '');
+        try {
+          JX.$('anchor-' + hash);
+        } catch (ex) {
+          var uri = '/conpherence/' +
+            _thread.selected + '/' + hash + '/';
+          threadManager.setLoadThreadURI(uri);
+          threadManager.loadThreadByID(_thread.selected, true);
+          _firstScroll = true;
+          return;
+        }
         return;
       }
     }
@@ -374,7 +389,7 @@ JX.behavior('conpherence-menu', function(config) {
     var form = JX.DOM.find(root, 'form', 'conpherence-pontificate');
     var data = e.getNodeData('conpherence-edit-metadata');
     var header = JX.DOM.find(root, 'div', 'conpherence-header-pane');
-    var messages = JX.DOM.find(root, 'div', 'conpherence-messages');
+    var messages = scrollbar.getContentNode();
 
     new JX.Workflow.newFromForm(form, data)
       .setHandler(JX.bind(this, function(r) {
@@ -400,21 +415,21 @@ JX.behavior('conpherence-menu', function(config) {
       .start();
   });
 
-  var _loadingTransactionID = null;
+  var _oldLoadingTransactionID = null;
   JX.Stratcom.listen('click', 'show-older-messages', function(e) {
     e.kill();
     var data = e.getNodeData('show-older-messages');
-    if (data.oldest_transaction_id == _loadingTransactionID) {
+    if (data.oldest_transaction_id == _oldLoadingTransactionID) {
       return;
     }
-    _loadingTransactionID = data.oldest_transaction_id;
+    _oldLoadingTransactionID = data.oldest_transaction_id;
+
     var node = e.getNode('show-older-messages');
     JX.DOM.setContent(node, 'Loading...');
-    JX.DOM.alterClass(node, 'conpherence-show-older-messages-loading', true);
+    JX.DOM.alterClass(node, 'conpherence-show-more-messages-loading', true);
 
     var conf_id = _thread.selected;
-    var root = JX.DOM.find(document, 'div', 'conpherence-layout');
-    var messages_root = JX.DOM.find(root, 'div', 'conpherence-messages');
+    var messages_root = scrollbar.getContentNode();
     new JX.Workflow(config.baseURI + conf_id + '/', data)
     .setHandler(function(r) {
       JX.DOM.remove(node);
@@ -425,6 +440,31 @@ JX.behavior('conpherence-menu', function(config) {
     }).start();
   });
 
+  var _newLoadingTransactionID = null;
+  JX.Stratcom.listen('click', 'show-newer-messages', function(e) {
+    e.kill();
+    var data = e.getNodeData('show-newer-messages');
+    if (data.newest_transaction_id == _newLoadingTransactionID) {
+      return;
+    }
+    _newLoadingTransactionID = data.newest_transaction_id;
+
+    var node = e.getNode('show-newer-messages');
+    JX.DOM.setContent(node, 'Loading...');
+    JX.DOM.alterClass(node, 'conpherence-show-more-messages-loading', true);
+
+    var conf_id = _thread.selected;
+    var messages_root = scrollbar.getContentNode();
+    new JX.Workflow(config.baseURI + conf_id + '/', data)
+    .setHandler(function(r) {
+      JX.DOM.remove(node);
+      var messages = JX.$H(r.messages);
+      JX.DOM.appendContent(
+        messages_root,
+        JX.$H(messages));
+    }).start();
+  });
+
   /**
    * On devices, we just show a thread list, so we don't want to automatically
    * select or load any threads. On desktop, we automatically select the first