From 4bc561f17bc8656251381d58f7f3202596433e88 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 11 Jun 2014 13:52:15 -0700 Subject: [PATCH] Make Conpherence threads update in real time, very roughly Summary: Ref T4083. This needs some work (mostly in the Conpherence JS itself), but is sort of functional. In particular: - On thread pages, add the thread as a `pageObject`. - After updating a thread, send a new "message" event to the server. - Share a little more event posting code. - In the browser, use event dispatch to respond to events. - Add a listener for the new event type. - Update conpherence threads (this part is really yucky). Test Plan: With multiple browser windows / browsers open, posted a message to a thread, and saw it update everywhere. Reviewers: joshuaspence Reviewed By: joshuaspence Subscribers: chad, epriestley Maniphest Tasks: T4083 Differential Revision: https://secure.phabricator.com/D9486 --- resources/celerity/map.php | 50 +++++++++---------- .../constants/ConpherenceUpdateActions.php | 1 + .../ConpherenceUpdateController.php | 36 ++++++++----- .../controller/ConpherenceViewController.php | 7 ++- .../conpherence/editor/ConpherenceEditor.php | 11 ++++ .../feed/PhabricatorFeedStoryPublisher.php | 10 +--- .../client/PhabricatorNotificationClient.php | 15 +++++- .../aphlict/behavior-aphlict-listen.js | 37 +++++++++----- .../conpherence/behavior-pontificate.js | 46 +++++++++++++++++ 9 files changed, 152 insertions(+), 61 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 73bf677c9d..cd0e6e47d2 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'd82d2f53', - 'core.pkg.js' => '4af4aa9d', + 'core.pkg.js' => '0627d27e', 'darkconsole.pkg.js' => 'ca8671ce', 'differential.pkg.css' => '4a93db37', 'differential.pkg.js' => 'eca39a2c', @@ -336,11 +336,11 @@ return array( 'rsrc/image/texture/table_header_tall.png' => 'd56b434f', 'rsrc/js/application/aphlict/Aphlict.js' => '08be8878', 'rsrc/js/application/aphlict/behavior-aphlict-dropdown.js' => '2a2dba85', - 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => 'acda9f51', + 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => '1da67f34', 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 'rsrc/js/application/config/behavior-reorder-fields.js' => '938aed89', 'rsrc/js/application/conpherence/behavior-menu.js' => '7ee23816', - 'rsrc/js/application/conpherence/behavior-pontificate.js' => '53f6f2dd', + 'rsrc/js/application/conpherence/behavior-pontificate.js' => 'd83a949c', 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '40b1ff90', 'rsrc/js/application/countdown/timer.js' => '361e3ed3', 'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => '469c0d9e', @@ -528,7 +528,7 @@ return array( 'javelin-aphlict' => '08be8878', 'javelin-behavior' => '8a3ed18b', 'javelin-behavior-aphlict-dropdown' => '2a2dba85', - 'javelin-behavior-aphlict-listen' => 'acda9f51', + 'javelin-behavior-aphlict-listen' => '1da67f34', 'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884', 'javelin-behavior-aphront-crop' => 'b98fc918', 'javelin-behavior-aphront-drag-and-drop-textarea' => '4a11ea9c', @@ -540,7 +540,7 @@ return array( 'javelin-behavior-boards-filter' => '22f113af', 'javelin-behavior-config-reorder-fields' => '938aed89', 'javelin-behavior-conpherence-menu' => '7ee23816', - 'javelin-behavior-conpherence-pontificate' => '53f6f2dd', + 'javelin-behavior-conpherence-pontificate' => 'd83a949c', 'javelin-behavior-conpherence-widget-pane' => '40b1ff90', 'javelin-behavior-countdown-timer' => '361e3ed3', 'javelin-behavior-dark-console' => 'e9fdb5e5', @@ -956,6 +956,18 @@ return array( 1 => 'javelin-util', 2 => 'phabricator-keyboard-shortcut-manager', ), + '1da67f34' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-aphlict', + 2 => 'javelin-stratcom', + 3 => 'javelin-request', + 4 => 'javelin-uri', + 5 => 'javelin-dom', + 6 => 'javelin-json', + 7 => 'javelin-router', + 8 => 'phabricator-notification', + ), '1e1c8a59' => array( 0 => 'javelin-behavior', @@ -1192,14 +1204,6 @@ return array( 1 => 'javelin-dom', 2 => 'phabricator-prefab', ), - '53f6f2dd' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-util', - 3 => 'javelin-workflow', - 4 => 'javelin-stratcom', - ), '54b612ba' => array( 0 => 'javelin-color', @@ -1608,18 +1612,6 @@ return array( 1 => 'javelin-dom', 2 => 'javelin-stratcom', ), - 'acda9f51' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-aphlict', - 2 => 'javelin-stratcom', - 3 => 'javelin-request', - 4 => 'javelin-uri', - 5 => 'javelin-dom', - 6 => 'javelin-json', - 7 => 'javelin-router', - 8 => 'phabricator-notification', - ), 'ad7a69ca' => array( 0 => 'javelin-install', @@ -1877,6 +1869,14 @@ return array( 3 => 'javelin-dom', 4 => 'phabricator-keyboard-shortcut', ), + 'd83a949c' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-util', + 3 => 'javelin-workflow', + 4 => 'javelin-stratcom', + ), 'd8e135db' => array( 0 => 'javelin-behavior', diff --git a/src/applications/conpherence/constants/ConpherenceUpdateActions.php b/src/applications/conpherence/constants/ConpherenceUpdateActions.php index 7bbb719174..2fe1f6ba84 100644 --- a/src/applications/conpherence/constants/ConpherenceUpdateActions.php +++ b/src/applications/conpherence/constants/ConpherenceUpdateActions.php @@ -9,4 +9,5 @@ final class ConpherenceUpdateActions extends ConpherenceConstants { const REMOVE_PERSON = 'remove_person'; const NOTIFICATIONS = 'notifications'; const ADD_STATUS = 'add_status'; + const LOAD = 'load'; } diff --git a/src/applications/conpherence/controller/ConpherenceUpdateController.php b/src/applications/conpherence/controller/ConpherenceUpdateController.php index 1dda17da57..137d9f13ac 100644 --- a/src/applications/conpherence/controller/ConpherenceUpdateController.php +++ b/src/applications/conpherence/controller/ConpherenceUpdateController.php @@ -31,6 +31,7 @@ final class ConpherenceUpdateController ->executeOne(); $action = $request->getStr('action', ConpherenceUpdateActions::METADATA); + $latest_transaction_id = null; $response_mode = $request->isAjax() ? 'ajax' : 'redirect'; $error_view = null; @@ -38,7 +39,7 @@ final class ConpherenceUpdateController $errors = array(); $delete_draft = false; $xactions = array(); - if ($request->isFormPost()) { + if ($request->isFormPost() || ($action == ConpherenceUpdateActions::LOAD)) { $editor = id(new ConpherenceEditor()) ->setContinueOnNoEffect($request->isContinueRequest()) ->setContentSourceFromRequest($request) @@ -114,24 +115,32 @@ final class ConpherenceUpdateController 'That was a non-update. Try cancel.'); } break; + case ConpherenceUpdateActions::LOAD: + $updated = false; + $response_mode = 'ajax'; + break; default: throw new Exception('Unknown action: '.$action); break; } - if ($xactions) { - try { - $xactions = $editor->applyTransactions($conpherence, $xactions); - if ($delete_draft) { - $draft = PhabricatorDraft::newFromUserAndKey( - $user, - $conpherence->getPHID()); - $draft->delete(); + + if ($xactions || ($action == ConpherenceUpdateActions::LOAD)) { + if ($xactions) { + try { + $xactions = $editor->applyTransactions($conpherence, $xactions); + if ($delete_draft) { + $draft = PhabricatorDraft::newFromUserAndKey( + $user, + $conpherence->getPHID()); + $draft->delete(); + } + } catch (PhabricatorApplicationTransactionNoEffectException $ex) { + return id(new PhabricatorApplicationTransactionNoEffectResponse()) + ->setCancelURI($this->getApplicationURI($conpherence_id.'/')) + ->setException($ex); } - } catch (PhabricatorApplicationTransactionNoEffectException $ex) { - return id(new PhabricatorApplicationTransactionNoEffectResponse()) - ->setCancelURI($this->getApplicationURI($conpherence_id.'/')) - ->setException($ex); } + switch ($response_mode) { case 'ajax': $latest_transaction_id = $request->getInt('latest_transaction_id'); @@ -266,6 +275,7 @@ final class ConpherenceUpdateController $need_transactions = false; switch ($action) { case ConpherenceUpdateActions::METADATA: + case ConpherenceUpdateActions::LOAD: $need_transactions = true; break; case ConpherenceUpdateActions::MESSAGE: diff --git a/src/applications/conpherence/controller/ConpherenceViewController.php b/src/applications/conpherence/controller/ConpherenceViewController.php index cdd5d8823c..b4df3a88ba 100644 --- a/src/applications/conpherence/controller/ConpherenceViewController.php +++ b/src/applications/conpherence/controller/ConpherenceViewController.php @@ -97,6 +97,7 @@ final class ConpherenceViewController extends array( 'title' => $title, 'device' => true, + 'pageObjects' => array($conpherence->getPHID()), )); } @@ -156,7 +157,11 @@ final class ConpherenceViewController extends 'type' => 'hidden', 'name' => 'latest_transaction_id', 'value' => $latest_transaction_id, - 'sigil' => 'latest-transaction-id' + 'sigil' => 'latest-transaction-id', + 'meta' => array( + 'threadPHID' => $conpherence->getPHID(), + 'threadID' => $conpherence->getID(), + ), ), '')) ->render(); diff --git a/src/applications/conpherence/editor/ConpherenceEditor.php b/src/applications/conpherence/editor/ConpherenceEditor.php index 813fda858d..e1894c92ee 100644 --- a/src/applications/conpherence/editor/ConpherenceEditor.php +++ b/src/applications/conpherence/editor/ConpherenceEditor.php @@ -306,6 +306,17 @@ final class ConpherenceEditor extends PhabricatorApplicationTransactionEditor { $participant->save(); } + if ($xactions) { + $data = array( + 'type' => 'message', + 'threadPHID' => $object->getPHID(), + 'messageID' => last($xactions)->getID(), + 'subscribers' => array($object->getPHID()), + ); + + PhabricatorNotificationClient::tryToPostMessage($data); + } + return $xactions; } diff --git a/src/applications/feed/PhabricatorFeedStoryPublisher.php b/src/applications/feed/PhabricatorFeedStoryPublisher.php index 274f0d8df6..fbbeee48a0 100644 --- a/src/applications/feed/PhabricatorFeedStoryPublisher.php +++ b/src/applications/feed/PhabricatorFeedStoryPublisher.php @@ -112,9 +112,7 @@ final class PhabricatorFeedStoryPublisher { } $this->insertNotifications($chrono_key); - if (PhabricatorEnv::getEnvConfig('notification.enabled')) { - $this->sendNotification($chrono_key); - } + $this->sendNotification($chrono_key); PhabricatorWorker::scheduleTask( 'FeedPublisherWorker', @@ -181,11 +179,7 @@ final class PhabricatorFeedStoryPublisher { 'subscribers' => $this->subscribedPHIDs, ); - try { - PhabricatorNotificationClient::postMessage($data); - } catch (Exception $ex) { - // Ignore, these are not critical. - } + PhabricatorNotificationClient::tryToPostMessage($data); } /** diff --git a/src/applications/notification/client/PhabricatorNotificationClient.php b/src/applications/notification/client/PhabricatorNotificationClient.php index 4c433c9b4f..f7d45316b4 100644 --- a/src/applications/notification/client/PhabricatorNotificationClient.php +++ b/src/applications/notification/client/PhabricatorNotificationClient.php @@ -25,7 +25,20 @@ final class PhabricatorNotificationClient { return $status; } - public static function postMessage(array $data) { + public static function tryToPostMessage(array $data) { + if (!PhabricatorEnv::getEnvConfig('notification.enabled')) { + return; + } + + try { + self::postMessage($data); + } catch (Exception $ex) { + // Just ignore any issues here. + phlog($ex); + } + } + + private static function postMessage(array $data) { $server_uri = PhabricatorEnv::getEnvConfig('notification.server-uri'); id(new HTTPSFuture($server_uri, json_encode($data))) diff --git a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js index 8cb06bf44a..9568c306d9 100644 --- a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js +++ b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js @@ -27,6 +27,29 @@ JX.behavior('aphlict-listen', function(config) { .start(); } + JX.Stratcom.listen('aphlict-receive-message', null, function(e) { + var message = e.getData(); + + if (message.type != 'notification') { + return; + } + + var request = new JX.Request( + '/notification/individual/', + onnotification); + + var routable = request + .addData({key: message.key}) + .getRoutable(); + + routable + .setType('notification') + .setPriority(250); + + JX.Router.getInstance().queue(routable); + }); + + // Respond to a notification from the Aphlict notification server. We send // a request to Phabricator to get notification details. function onaphlictmessage(type, message) { @@ -40,19 +63,7 @@ JX.behavior('aphlict-listen', function(config) { break; case 'receive': - var request = new JX.Request( - '/notification/individual/', - onnotification); - - var routable = request - .addData({key: message.key}) - .getRoutable(); - - routable - .setType('notification') - .setPriority(250); - - JX.Router.getInstance().queue(routable); + JX.Stratcom.invoke('aphlict-receive-message', null, message); break; default: diff --git a/webroot/rsrc/js/application/conpherence/behavior-pontificate.js b/webroot/rsrc/js/application/conpherence/behavior-pontificate.js index 1690701b0e..d23df88a4d 100644 --- a/webroot/rsrc/js/application/conpherence/behavior-pontificate.js +++ b/webroot/rsrc/js/application/conpherence/behavior-pontificate.js @@ -8,6 +8,52 @@ */ JX.behavior('conpherence-pontificate', function(config) { + + JX.Stratcom.listen('aphlict-receive-message', null, function(e) { + var message = e.getData(); + + if (message.type != 'message') { + // Not a message event. + return; + } + + // TODO: This is really, really gross. + var infonode = JX.DOM.find(document, 'input', 'latest-transaction-id'); + var data = JX.Stratcom.getData(infonode); + + var latest_id = infonode.value; + var thread_phid = data.threadPHID; + var thread_id = data.threadID; + + if (message.threadPHID != thread_phid) { + // Message event for some thread other than the visible one. + return; + } + + if (message.messageID <= latest_id) { + // Message event for something we already know about. + return; + } + + var params = { + action: 'load', + latest_transaction_id: latest_id + }; + + new JX.Workflow('/conpherence/update/' + thread_id + '/') + .setData(params) + .setHandler(function(r) { + var messages = JX.DOM.find(document, 'div', 'conpherence-messages'); + JX.DOM.appendContent(messages, JX.$H(r.transactions)); + messages.scrollTop = messages.scrollHeight; + + // TODO: Continued grossness from above. + infonode.value = r.latest_transaction_id; + }) + .start(); + }); + + var onsubmit = function(e) { e.kill();