From 68d1441b4228a8abb79605f0bf552a7cc0a059c5 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Thu, 18 Apr 2013 16:54:06 -0700 Subject: [PATCH] Conpherence -- finish off the "show older" functionality Summary: Fixes T2956. Ref T2399. Test Plan: set message limit to 2 and verified "show older" showed up, and that clicking it again and again and again showed the right stuff, ultimately not showing a "show older" UI anymore. Reviewers: epriestley, chad Reviewed By: epriestley CC: aran, Korvin Maniphest Tasks: T2399, T2956 Differential Revision: https://secure.phabricator.com/D5721 --- .../controller/ConpherenceController.php | 13 ++- .../ConpherenceUpdateController.php | 2 +- .../controller/ConpherenceViewController.php | 87 ++++++++++++------- .../query/ConpherenceThreadQuery.php | 43 +++++++-- .../query/ConpherenceTransactionQuery.php | 5 ++ ...PhabricatorCursorPagedPolicyAwareQuery.php | 8 ++ .../application/conpherence/behavior-menu.js | 12 +-- 7 files changed, 124 insertions(+), 46 deletions(-) diff --git a/src/applications/conpherence/controller/ConpherenceController.php b/src/applications/conpherence/controller/ConpherenceController.php index ad7f45d58d..764797b26f 100644 --- a/src/applications/conpherence/controller/ConpherenceController.php +++ b/src/applications/conpherence/controller/ConpherenceController.php @@ -141,6 +141,15 @@ abstract class ConpherenceController extends PhabricatorController { $user = $this->getRequest()->getUser(); $transactions = $conpherence->getTransactions(); + $oldest_transaction_id = 0; + $too_many = ConpherenceThreadQuery::TRANSACTION_LIMIT + 1; + if (count($transactions) == $too_many) { + $last_transaction = end($transactions); + unset($transactions[$last_transaction->getID()]); + } + $transactions = array_reverse($transactions); + $oldest_transaction = reset($transactions); + $oldest_transaction_id = $oldest_transaction->getID(); $handles = $conpherence->getHandles(); $rendered_transactions = array(); $engine = id(new PhabricatorMarkupEngine()) @@ -166,11 +175,11 @@ abstract class ConpherenceController extends PhabricatorController { ->render(); } $latest_transaction_id = $transaction->getID(); - $rendered_transactions = phutil_implode_html(' ', $rendered_transactions); return array( 'transactions' => $rendered_transactions, - 'latest_transaction_id' => $latest_transaction_id + 'latest_transaction_id' => $latest_transaction_id, + 'oldest_transaction_id' => $oldest_transaction_id ); } diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 30c8b25cac..4fb76990e0 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -345,7 +345,7 @@ final class ConpherenceUpdateController $user = $this->getRequest()->getUser(); $conpherence = id(new ConpherenceThreadQuery()) ->setViewer($user) - ->setAfterMessageID($latest_transaction_id) + ->setAfterTransactionID($latest_transaction_id) ->needHeaderPics($need_header_pics) ->needWidgetData($need_widget_data) ->needTransactions($need_transactions) diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index 13691e400f..0f5857bcb4 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -37,12 +37,19 @@ final class ConpherenceViewController extends if (!$conpherence_id) { return new Aphront404Response(); } - $conpherence = id(new ConpherenceThreadQuery()) + $query = id(new ConpherenceThreadQuery()) ->setViewer($user) ->withIDs(array($conpherence_id)) ->needHeaderPics(true) + ->needParticipantCache(true) ->needTransactions(true) - ->executeOne(); + ->setTransactionLimit(ConpherenceThreadQuery::TRANSACTION_LIMIT); + $before_transaction_id = $request->getInt('oldest_transaction_id'); + if ($before_transaction_id) { + $query + ->setBeforeTransactionID($before_transaction_id); + } + $conpherence = $query->executeOne(); $this->setConpherence($conpherence); $participant = $conpherence->getParticipant($user->getPHID()); @@ -52,9 +59,23 @@ final class ConpherenceViewController extends $participant->markUpToDate($conpherence, $latest_transaction); unset($write_guard); - $header = $this->renderHeaderPaneContent(); - $messages = $this->renderMessagePaneContent(); - $content = $header + $messages; + $data = $this->renderConpherenceTransactions($conpherence); + $messages = $this->renderMessagePaneContent( + $data['transactions'], + $data['oldest_transaction_id']); + if ($before_transaction_id) { + $header = null; + $form = null; + $content = array('messages' => $messages); + } else { + $header = $this->renderHeaderPaneContent(); + $form = $this->renderFormContent($data['latest_transaction_id']); + $content = array( + 'header' => $header, + 'messages' => $messages, + 'form' => $form + ); + } if ($request->isAjax()) { return id(new AphrontAjaxResponse())->setContent($content); @@ -64,8 +85,8 @@ final class ConpherenceViewController extends ->setBaseURI($this->getApplicationURI()) ->setThread($conpherence) ->setHeader($header) - ->setMessages($messages['messages']) - ->setReplyForm($messages['form']) + ->setMessages($messages) + ->setReplyForm($form) ->setRole('thread'); return $this->buildApplicationPage( @@ -80,19 +101,39 @@ final class ConpherenceViewController extends require_celerity_resource('conpherence-header-pane-css'); $conpherence = $this->getConpherence(); $header = $this->buildHeaderPaneContent($conpherence); - return array('header' => hsprintf('%s', $header)); + return hsprintf('%s', $header); } - private function renderMessagePaneContent() { + private function renderMessagePaneContent( + array $transactions, + $oldest_transaction_id) { + require_celerity_resource('conpherence-message-pane-css'); - $user = $this->getRequest()->getUser(); + + $scrollbutton = ''; + if ($oldest_transaction_id) { + $scrollbutton = javelin_tag( + 'a', + array( + 'href' => '#', + 'mustcapture' => true, + 'sigil' => 'show-older-messages', + 'class' => 'conpherence-show-older-messages', + 'meta' => array( + 'oldest_transaction_id' => $oldest_transaction_id + ) + ), + pht('Show Older Messages')); + } + + return hsprintf('%s%s', $scrollbutton, $transactions); + } + + private function renderFormContent($latest_transaction_id) { + $conpherence = $this->getConpherence(); - - $data = $this->renderConpherenceTransactions($conpherence); - $latest_transaction_id = $data['latest_transaction_id']; - $transactions = $data['transactions']; - + $user = $this->getRequest()->getUser(); $update_uri = $this->getApplicationURI('update/'.$conpherence->getID().'/'); Javelin::initBehavior('conpherence-pontificate'); @@ -127,21 +168,7 @@ final class ConpherenceViewController extends '')) ->render(); - $scrollbutton = javelin_tag( - 'a', - array( - 'href' => '#', - 'mustcapture' => true, - 'sigil' => 'show-older-messages', - 'class' => 'conpherence-show-older-messages', - ), - pht('Show Older Messages')); - - return array( - 'messages' => hsprintf('%s%s', $scrollbutton, $transactions), - 'form' => $form - ); - + return $form; } } diff --git a/src/applications/conpherence/query/ConpherenceThreadQuery.php b/src/applications/conpherence/query/ConpherenceThreadQuery.php index b92139760f..12402e6bc0 100644 --- a/src/applications/conpherence/query/ConpherenceThreadQuery.php +++ b/src/applications/conpherence/query/ConpherenceThreadQuery.php @@ -6,6 +6,8 @@ final class ConpherenceThreadQuery extends PhabricatorCursorPagedPolicyAwareQuery { + const TRANSACTION_LIMIT = 2; + private $phids; private $ids; private $needWidgetData; @@ -14,7 +16,9 @@ final class ConpherenceThreadQuery private $needTransactions; private $needParticipantCache; private $needFilePHIDs; - private $afterMessageID; + private $afterTransactionID; + private $beforeTransactionID; + private $transactionLimit; public function needFilePHIDs($need_file_phids) { $this->needFilePHIDs = $need_file_phids; @@ -56,12 +60,25 @@ final class ConpherenceThreadQuery return $this; } - // TODO: This is pretty hacky!!!!~~ - public function setAfterMessageID($id) { - $this->afterMessageID = $id; + public function setAfterTransactionID($id) { + $this->afterTransactionID = $id; return $this; } + public function setBeforeTransactionID($id) { + $this->beforeTransactionID = $id; + return $this; + } + + public function setTransactionLimit($transaction_limit) { + $this->transactionLimit = $transaction_limit; + return $this; + } + + public function getTransactionLimit() { + return $this->transactionLimit; + } + protected function loadPage() { $table = new ConpherenceThread(); $conn_r = $table->establishConnection('r'); @@ -164,13 +181,23 @@ final class ConpherenceThreadQuery } private function loadTransactionsAndHandles(array $conpherences) { - $transactions = id(new ConpherenceTransactionQuery()) + $query = id(new ConpherenceTransactionQuery()) ->setViewer($this->getViewer()) ->withObjectPHIDs(array_keys($conpherences)) - ->needHandles(true) - ->setAfterID($this->afterMessageID) - ->execute(); + ->needHandles(true); + // We have to flip these for the underyling query class. The semantics of + // paging are tricky business. + if ($this->afterTransactionID) { + $query->setBeforeID($this->afterTransactionID); + } else if ($this->beforeTransactionID) { + $query->setAfterID($this->beforeTransactionID); + } + if ($this->getTransactionLimit()) { + // fetch an extra for "show older" scenarios + $query->setLimit($this->getTransactionLimit() + 1); + } + $transactions = $query->execute(); $transactions = mgroup($transactions, 'getObjectPHID'); foreach ($conpherences as $phid => $conpherence) { $current_transactions = $transactions[$phid]; diff --git a/src/applications/conpherence/query/ConpherenceTransactionQuery.php b/src/applications/conpherence/query/ConpherenceTransactionQuery.php index dc65c3c70b..c3feefa3d1 100644 --- a/src/applications/conpherence/query/ConpherenceTransactionQuery.php +++ b/src/applications/conpherence/query/ConpherenceTransactionQuery.php @@ -10,4 +10,9 @@ final class ConpherenceTransactionQuery return new ConpherenceTransaction(); } + protected function getReversePaging() { + return false; + } + + } diff --git a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php index 9dc13e6566..3717cfa252 100644 --- a/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php +++ b/src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php @@ -35,11 +35,19 @@ abstract class PhabricatorCursorPagedPolicyAwareQuery return $this; } + final protected function getAfterID() { + return $this->afterID; + } + final public function setBeforeID($object_id) { $this->beforeID = $object_id; return $this; } + final protected function getBeforeID() { + return $this->beforeID; + } + final protected function buildLimitClause(AphrontDatabaseConnection $conn_r) { if ($this->getRawResultLimit()) { return qsprintf($conn_r, 'LIMIT %d', $this->getRawResultLimit()); diff --git a/webroot/rsrc/js/application/conpherence/behavior-menu.js b/webroot/rsrc/js/application/conpherence/behavior-menu.js index 2a62235d2a..24a5b7f7ac 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-menu.js +++ b/webroot/rsrc/js/application/conpherence/behavior-menu.js @@ -235,16 +235,18 @@ JX.behavior('conpherence-menu', function(config) { JX.Stratcom.listen('click', 'show-older-messages', function(e) { e.kill(); - var last_offset = e.getNodeData('show-older-messages').offset; - var conf_id = e.getNodeData('show-older-messages').ID; + var data = e.getNodeData('show-older-messages'); + var oldest_transaction_id = data.oldest_transaction_id; + var conf_id = thread.selected; JX.DOM.remove(e.getNode('show-older-messages')); var root = JX.DOM.find(document, 'div', 'conpherence-layout'); var messages_root = JX.DOM.find(root, 'div', 'conpherence-messages'); new JX.Request(config.base_uri + conf_id + '/', function(r) { var messages = JX.$H(r.messages); - JX.DOM.prependContent(messages_root, - JX.$H(messages)); - }).setData({ offset: last_offset+1 }).send(); + JX.DOM.prependContent( + messages_root, + JX.$H(messages)); + }).setData({ oldest_transaction_id : oldest_transaction_id }).send(); });