From d8ab5f594c7f9cd05e333f507f2ae26e55c6b411 Mon Sep 17 00:00:00 2001 From: Bob Trahan Date: Mon, 20 Apr 2015 16:43:32 -0700 Subject: [PATCH] Quicksand - make page object notifications work properly with quicksand Summary: Fixes T7680. Make it so the listen behavior can be initialized multiple times from the server by having the behavior only update a few static data variables on subsequent initializations. Test Plan: visited TX with user A and left a comment with user B and got the "reload" and "TX updated" bubbles. Reloaded and navigated to /maniphest/ with user A and had user B leave another comment on TX - no "reload" bubble and correct "TX updated" bubble. Navigated to TX again with user A and had user B leave a comment and got the "reload" and "TX updated" bubbles. visited TX with user A and left a comment with user B and got the "reload" and "TX updated" bubbles. navigated away with user A and the "reload" bubble was automagically closed. Reviewers: epriestley Reviewed By: epriestley Subscribers: Korvin, epriestley Maniphest Tasks: T7680 Differential Revision: https://secure.phabricator.com/D12448 --- resources/celerity/map.php | 52 +++++++++---------- src/view/page/PhabricatorStandardPageView.php | 22 ++++---- .../rsrc/js/application/aphlict/Aphlict.js | 14 +++++ .../aphlict/behavior-aphlict-listen.js | 41 +++++++++++++-- 4 files changed, 90 insertions(+), 39 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 30d6e9f734..4bf9aecbbf 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -8,7 +8,7 @@ return array( 'names' => array( 'core.pkg.css' => 'a2a90172', - 'core.pkg.js' => 'dfea788f', + 'core.pkg.js' => '328a9980', 'darkconsole.pkg.js' => '8ab24e01', 'differential.pkg.css' => '3500921f', 'differential.pkg.js' => 'c0506961', @@ -349,9 +349,9 @@ return array( 'rsrc/image/texture/table_header.png' => '5c433037', 'rsrc/image/texture/table_header_hover.png' => '038ec3b9', 'rsrc/image/texture/table_header_tall.png' => 'd56b434f', - 'rsrc/js/application/aphlict/Aphlict.js' => '2be71d56', + 'rsrc/js/application/aphlict/Aphlict.js' => '30a6303c', 'rsrc/js/application/aphlict/behavior-aphlict-dropdown.js' => '00def500', - 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => 'bdf2226d', + 'rsrc/js/application/aphlict/behavior-aphlict-listen.js' => 'b1a59974', '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', @@ -548,10 +548,10 @@ return array( 'herald-test-css' => '778b008e', 'homepage-panel-css' => 'e34bf140', 'inline-comment-summary-css' => 'eb5f8e8c', - 'javelin-aphlict' => '2be71d56', + 'javelin-aphlict' => '30a6303c', 'javelin-behavior' => '61cbc29a', 'javelin-behavior-aphlict-dropdown' => '00def500', - 'javelin-behavior-aphlict-listen' => 'bdf2226d', + 'javelin-behavior-aphlict-listen' => 'b1a59974', 'javelin-behavior-aphlict-status' => 'ea681761', 'javelin-behavior-aphront-basic-tokenizer' => 'b3a4b884', 'javelin-behavior-aphront-crop' => 'fa0f4fc2', @@ -1034,13 +1034,6 @@ return array( 'javelin-install', 'javelin-util', ), - '2be71d56' => array( - 'javelin-install', - 'javelin-util', - 'javelin-websocket', - 'javelin-leader', - 'javelin-json', - ), '2bfa2836' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1056,6 +1049,13 @@ return array( 'javelin-install', 'javelin-event', ), + '30a6303c' => array( + 'javelin-install', + 'javelin-util', + 'javelin-websocket', + 'javelin-leader', + 'javelin-json', + ), '316b8fa1' => array( 'javelin-install', 'javelin-typeahead-source', @@ -1689,6 +1689,20 @@ return array( 'javelin-util', 'phabricator-prefab', ), + 'b1a59974' => array( + 'javelin-behavior', + 'javelin-aphlict', + 'javelin-stratcom', + 'javelin-request', + 'javelin-uri', + 'javelin-dom', + 'javelin-json', + 'javelin-router', + 'javelin-util', + 'javelin-leader', + 'javelin-sound', + 'phabricator-notification', + ), 'b1f0ccee' => array( 'javelin-install', 'javelin-dom', @@ -1748,20 +1762,6 @@ return array( 'javelin-util', 'javelin-request', ), - 'bdf2226d' => array( - 'javelin-behavior', - 'javelin-aphlict', - 'javelin-stratcom', - 'javelin-request', - 'javelin-uri', - 'javelin-dom', - 'javelin-json', - 'javelin-router', - 'javelin-util', - 'javelin-leader', - 'javelin-sound', - 'phabricator-notification', - ), 'be807912' => array( 'javelin-behavior', 'javelin-dom', diff --git a/src/view/page/PhabricatorStandardPageView.php b/src/view/page/PhabricatorStandardPageView.php index 23f2377748..c081238712 100644 --- a/src/view/page/PhabricatorStandardPageView.php +++ b/src/view/page/PhabricatorStandardPageView.php @@ -461,11 +461,6 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { $client_uri->setDomain($this_host->getDomain()); } - $subscriptions = $this->pageObjects; - if ($user) { - $subscriptions[] = $user->getPHID(); - } - if ($request->isHTTPS()) { $client_uri->setProtocol('wss'); } else { @@ -476,9 +471,7 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { 'aphlict-listen', array( 'websocketURI' => (string)$client_uri, - 'pageObjects' => array_fill_keys($this->pageObjects, true), - 'subscriptions' => $subscriptions, - )); + ) + $this->buildAphlictListenConfigData()); } } @@ -590,7 +583,18 @@ final class PhabricatorStandardPageView extends PhabricatorBarePageView { } private function buildQuicksandConfig() { - return array(); + return $this->buildAphlictListenConfigData(); + } + + private function buildAphlictListenConfigData() { + $user = $this->getRequest()->getUser(); + $subscriptions = $this->pageObjects; + $subscriptions[] = $user->getPHID(); + + return array( + 'pageObjects' => array_fill_keys($this->pageObjects, true), + 'subscriptions' => $subscriptions, + ); } private function getQuicksandURIPatternBlacklist() { diff --git a/webroot/rsrc/js/application/aphlict/Aphlict.js b/webroot/rsrc/js/application/aphlict/Aphlict.js index b7bcaafa22..d4886a12a0 100644 --- a/webroot/rsrc/js/application/aphlict/Aphlict.js +++ b/webroot/rsrc/js/application/aphlict/Aphlict.js @@ -49,6 +49,20 @@ JX.install('Aphlict', { JX.Leader.call(JX.bind(this, this._begin)); }, + setSubscriptions: function(subscriptions) { + this._subscriptions = subscriptions; + JX.Leader.broadcast( + null, + {type: 'aphlict.subscribe', data: this._subscriptions}); + }, + + clearSubscriptions: function(subscriptions) { + this._subscriptions = null; + JX.Leader.broadcast( + null, + {type: 'aphlict.unsubscribe', data: subscriptions}); + }, + getStatus: function() { return this._status; }, diff --git a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js index 21b3f2966e..69ce78893d 100644 --- a/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js +++ b/webroot/rsrc/js/application/aphlict/behavior-aphlict-listen.js @@ -15,7 +15,8 @@ */ JX.behavior('aphlict-listen', function(config) { - var showing_reload = false; + var page_objects = config.pageObjects; + var reload_notification = null; JX.Stratcom.listen('aphlict-server-message', null, function(e) { var message = e.getData(); @@ -78,15 +79,28 @@ JX.behavior('aphlict-listen', function(config) { // If the notification affected an object on this page, show a // permanent reload notification if we aren't already. - if ((response.primaryObjectPHID in config.pageObjects) && !showing_reload) { + if ((response.primaryObjectPHID in page_objects) && + reload_notification === null) { + var reload = new JX.Notification() .setContent('Page updated, click to reload.') .alterClassName('jx-notification-alert', true) .setDuration(0); - reload.listen('activate', function() { JX.$U().go(); }); + reload.listen( + 'activate', + function() { + // double check we are still on the page where re-loading makes + // sense...! + if (response.primaryObjectPHID in page_objects) { + JX.$U().go(); + } + }); reload.show(); - showing_reload = true; + reload_notification = { + dialog: reload, + phid: response.primaryObjectPHID + }; } }); @@ -98,6 +112,25 @@ JX.behavior('aphlict-listen', function(config) { .setHandler(onAphlictMessage) .start(); + JX.Stratcom.listen( + 'quicksand-redraw', + null, + function (e) { + var old_data = e.getData().oldResponse; + var new_data = e.getData().newResponse; + client.clearSubscriptions(old_data.subscriptions); + client.setSubscriptions(new_data.subscriptions); + + page_objects = new_data.pageObjects; + if (reload_notification) { + if (reload_notification.phid in page_objects) { + return; + } + reload_notification.dialog.hide(); + reload_notification = null; + } + }); + JX.Leader.listen('onReceiveBroadcast', function(message, is_leader) { if (message.type !== 'sound') { return;