@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.) hq.recaptime.dev/wiki/Phorge
phorge phabricator
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Conpherence - change message rendering logic to eradicate possibility of duplicates

Summary:
Fixes T6713. Before this diff, we would update the DOM when various requests came back, but the logic to erase race conditions proved too tricky for me to get right. Instead, change the algorithm up and keep a set of transaction ids around per thread. When its time to update the transactions, sort the list of ids and just render the whole darn set again.

To make this work, this ends up adding transacton ids to fake transactons like "show older" and date markers. This is able to work by using a float sort and giving these transactions ids that are .5 from being an integer and in the right place numerically.

Test Plan: for durable column, clicked show older and it worked. sent a message and it worked. for main view, clicked show older and it worked. sent a message and it worked.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T6713

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

+162 -68
+42 -42
resources/celerity/map.php
··· 8 8 return array( 9 9 'names' => array( 10 10 'core.pkg.css' => 'ed3d6355', 11 - 'core.pkg.js' => '616511ac', 11 + 'core.pkg.js' => 'ac41c400', 12 12 'darkconsole.pkg.js' => 'e7393ebb', 13 13 'differential.pkg.css' => 'bb338e4b', 14 14 'differential.pkg.js' => '895b8d62', ··· 348 348 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 349 349 'rsrc/js/application/calendar/event-all-day.js' => 'ca5fa62a', 350 350 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', 351 - 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'de397217', 351 + 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'b7342ddb', 352 352 'rsrc/js/application/conpherence/behavior-drag-and-drop-photo.js' => 'cf86d16a', 353 - 'rsrc/js/application/conpherence/behavior-durable-column.js' => '61252a27', 354 - 'rsrc/js/application/conpherence/behavior-menu.js' => 'eb61cb03', 353 + 'rsrc/js/application/conpherence/behavior-durable-column.js' => '16c695bf', 354 + 'rsrc/js/application/conpherence/behavior-menu.js' => '4351c4a0', 355 355 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', 356 356 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', 357 357 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '93568464', ··· 516 516 'conpherence-menu-css' => 'f389e048', 517 517 'conpherence-message-pane-css' => '0e75feef', 518 518 'conpherence-notification-css' => 'd208f806', 519 - 'conpherence-thread-manager' => 'de397217', 519 + 'conpherence-thread-manager' => 'b7342ddb', 520 520 'conpherence-transaction-css' => '42a457f6', 521 521 'conpherence-update-css' => '1099a660', 522 522 'conpherence-widget-pane-css' => '2af42ebe', ··· 557 557 'javelin-behavior-choose-control' => '6153c708', 558 558 'javelin-behavior-config-reorder-fields' => '14a827de', 559 559 'javelin-behavior-conpherence-drag-and-drop-photo' => 'cf86d16a', 560 - 'javelin-behavior-conpherence-menu' => 'eb61cb03', 560 + 'javelin-behavior-conpherence-menu' => '4351c4a0', 561 561 'javelin-behavior-conpherence-pontificate' => '21ba5861', 562 562 'javelin-behavior-conpherence-widget-pane' => '93568464', 563 563 'javelin-behavior-countdown-timer' => 'e4cc26b3', ··· 584 584 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 585 585 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', 586 586 'javelin-behavior-doorkeeper-tag' => 'e5822781', 587 - 'javelin-behavior-durable-column' => '61252a27', 587 + 'javelin-behavior-durable-column' => '16c695bf', 588 588 'javelin-behavior-error-log' => '6882e80a', 589 589 'javelin-behavior-event-all-day' => 'ca5fa62a', 590 590 'javelin-behavior-fancy-datepicker' => '5c0f680f', ··· 930 930 'javelin-request', 931 931 'javelin-util', 932 932 ), 933 + '16c695bf' => array( 934 + 'javelin-behavior', 935 + 'javelin-dom', 936 + 'javelin-stratcom', 937 + 'javelin-behavior-device', 938 + 'javelin-scrollbar', 939 + 'javelin-quicksand', 940 + 'phabricator-keyboard-shortcut', 941 + 'conpherence-thread-manager', 942 + ), 933 943 '1ad0a787' => array( 934 944 'javelin-install', 935 945 'javelin-reactor', ··· 1072 1082 'javelin-behavior', 1073 1083 'javelin-dom', 1074 1084 'javelin-request', 1085 + ), 1086 + '4351c4a0' => array( 1087 + 'javelin-behavior', 1088 + 'javelin-dom', 1089 + 'javelin-util', 1090 + 'javelin-stratcom', 1091 + 'javelin-workflow', 1092 + 'javelin-behavior-device', 1093 + 'javelin-history', 1094 + 'javelin-vector', 1095 + 'javelin-scrollbar', 1096 + 'phabricator-title', 1097 + 'phabricator-shaped-request', 1098 + 'conpherence-thread-manager', 1075 1099 ), 1076 1100 '44168bad' => array( 1077 1101 'javelin-behavior', ··· 1256 1280 'javelin-stratcom', 1257 1281 'javelin-dom', 1258 1282 ), 1259 - '61252a27' => array( 1260 - 'javelin-behavior', 1261 - 'javelin-dom', 1262 - 'javelin-stratcom', 1263 - 'javelin-behavior-device', 1264 - 'javelin-scrollbar', 1265 - 'javelin-quicksand', 1266 - 'phabricator-keyboard-shortcut', 1267 - 'conpherence-thread-manager', 1268 - ), 1269 1283 '6153c708' => array( 1270 1284 'javelin-behavior', 1271 1285 'javelin-stratcom', ··· 1705 1719 'javelin-dom', 1706 1720 'javelin-util', 1707 1721 ), 1722 + 'b7342ddb' => array( 1723 + 'javelin-dom', 1724 + 'javelin-util', 1725 + 'javelin-stratcom', 1726 + 'javelin-install', 1727 + 'javelin-aphlict', 1728 + 'javelin-workflow', 1729 + 'javelin-router', 1730 + 'javelin-behavior-device', 1731 + 'javelin-vector', 1732 + ), 1708 1733 'ba4fa35c' => array( 1709 1734 'javelin-behavior', 1710 1735 'javelin-dom', ··· 1832 1857 'javelin-typeahead-ondemand-source', 1833 1858 'javelin-dom', 1834 1859 ), 1835 - 'de397217' => array( 1836 - 'javelin-dom', 1837 - 'javelin-util', 1838 - 'javelin-stratcom', 1839 - 'javelin-install', 1840 - 'javelin-aphlict', 1841 - 'javelin-workflow', 1842 - 'javelin-router', 1843 - 'javelin-behavior-device', 1844 - 'javelin-vector', 1845 - ), 1846 1860 'df5e11d2' => array( 1847 1861 'javelin-install', 1848 1862 ), ··· 1919 1933 'javelin-aphlict', 1920 1934 'phabricator-phtize', 1921 1935 'javelin-dom', 1922 - ), 1923 - 'eb61cb03' => array( 1924 - 'javelin-behavior', 1925 - 'javelin-dom', 1926 - 'javelin-util', 1927 - 'javelin-stratcom', 1928 - 'javelin-workflow', 1929 - 'javelin-behavior-device', 1930 - 'javelin-history', 1931 - 'javelin-vector', 1932 - 'javelin-scrollbar', 1933 - 'phabricator-title', 1934 - 'phabricator-shaped-request', 1935 - 'conpherence-thread-manager', 1936 1936 ), 1937 1937 'efe49472' => array( 1938 1938 'javelin-install',
+19
src/applications/conpherence/ConpherenceTransactionRenderer.php
··· 90 90 if ($previous_day != $current_day) { 91 91 $date_marker_transaction->setDateCreated( 92 92 $transaction->getDateCreated()); 93 + $date_marker_transaction->setID($previous_transaction->getID()); 93 94 $rendered_transactions[] = $date_marker_transaction_view->render(); 94 95 } 95 96 } ··· 144 145 ), 145 146 ), 146 147 pht('Show Older Messages')); 148 + $oldscrollbutton = javelin_tag( 149 + 'div', 150 + array( 151 + 'sigil' => 'conpherence-transaction-view', 152 + 'meta' => array( 153 + 'id' => $oldest_transaction_id - 0.5, 154 + ), 155 + ), 156 + $oldscrollbutton); 147 157 } 148 158 149 159 $newscrollbutton = ''; ··· 160 170 ), 161 171 ), 162 172 pht('Show Newer Messages')); 173 + $newscrollbutton = javelin_tag( 174 + 'div', 175 + array( 176 + 'sigil' => 'conpherence-transaction-view', 177 + 'meta' => array( 178 + 'id' => $newest_transaction_id + 0.5, 179 + ), 180 + ), 181 + $newscrollbutton); 163 182 } 164 183 165 184 return hsprintf(
+2 -2
src/applications/conpherence/controller/ConpherenceViewController.php
··· 79 79 if ($before_transaction_id || $after_transaction_id) { 80 80 $header = null; 81 81 $form = null; 82 - $content = array('messages' => $messages); 82 + $content = array('transactions' => $messages); 83 83 } else { 84 84 $policy_objects = id(new PhabricatorPolicyQuery()) 85 85 ->setViewer($user) ··· 89 89 $form = $this->renderFormContent(); 90 90 $content = array( 91 91 'header' => $header, 92 - 'messages' => $messages, 92 + 'transactions' => $messages, 93 93 'form' => $form, 94 94 ); 95 95 }
+10 -2
src/applications/conpherence/view/ConpherenceTransactionView.php
··· 103 103 $transaction = $this->getConpherenceTransaction(); 104 104 switch ($transaction->getTransactionType()) { 105 105 case ConpherenceTransactionType::TYPE_DATE_MARKER: 106 - return phutil_tag( 106 + return javelin_tag( 107 107 'div', 108 108 array( 109 109 'class' => 'conpherence-transaction-view date-marker', 110 + 'sigil' => 'conpherence-transaction-view', 111 + 'meta' => array( 112 + 'id' => $transaction->getID() + 0.5, 113 + ), 110 114 ), 111 115 array( 112 116 phutil_tag( ··· 134 138 'conpherence-transaction-header grouped', 135 139 array($actions, $info)); 136 140 137 - return phutil_tag( 141 + return javelin_tag( 138 142 'div', 139 143 array( 140 144 'class' => 'conpherence-transaction-view '.$classes, 141 145 'id' => $transaction_id, 146 + 'sigil' => 'conpherence-transaction-view', 147 + 'meta' => array( 148 + 'id' => $transaction->getID(), 149 + ), 142 150 ), 143 151 array( 144 152 $image,
+86 -15
webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js
··· 27 27 _loadedThreadID: null, 28 28 _loadedThreadPHID: null, 29 29 _latestTransactionID: null, 30 + _transactionIDMap: null, 31 + _transactionCache: null, 30 32 _canEditLoadedThread: null, 31 33 _updating: null, 32 34 _minimalDisplay: false, ··· 83 85 return this; 84 86 }, 85 87 88 + _updateTransactionIDMap: function(transactions) { 89 + var loaded_id = this.getLoadedThreadID(); 90 + if (!this._transactionIDMap[loaded_id]) { 91 + this._transactionIDMap[this._loadedThreadID] = {}; 92 + } 93 + var loaded_transaction_ids = this._transactionIDMap[loaded_id]; 94 + var transaction; 95 + for (var ii = 0; ii < transactions.length; ii++) { 96 + transaction = transactions[ii]; 97 + loaded_transaction_ids[JX.Stratcom.getData(transaction).id] = 1; 98 + } 99 + this._transactionIDMap[this._loadedThreadID] = loaded_transaction_ids; 100 + return this; 101 + }, 102 + 103 + _updateTransactionCache: function(transactions) { 104 + var transaction; 105 + for (var ii = 0; ii < transactions.length; ii++) { 106 + transaction = transactions[ii]; 107 + this._transactionCache[JX.Stratcom.getData(transaction).id] = 108 + transaction; 109 + } 110 + return this; 111 + }, 112 + 113 + _getLoadedTransactions: function() { 114 + var loaded_id = this.getLoadedThreadID(); 115 + var loaded_tx_ids = JX.keys(this._transactionIDMap[loaded_id]); 116 + loaded_tx_ids.sort(function (a, b) { 117 + var x = parseFloat(a); 118 + var y = parseFloat(b); 119 + if (x > y) { 120 + return 1; 121 + } 122 + if (x < y) { 123 + return -1; 124 + } 125 + return 0; 126 + }); 127 + var transactions = []; 128 + for (var ii = 0; ii < loaded_tx_ids.length; ii++) { 129 + transactions.push(this._transactionCache[loaded_tx_ids[ii]]); 130 + } 131 + return transactions; 132 + }, 133 + 134 + _deleteTransactionCaches: function(id) { 135 + delete this._transactionCache[id]; 136 + delete this._transactionIDMap[this._loadedThreadID][id]; 137 + 138 + return this; 139 + }, 140 + 86 141 setCanEditLoadedThread: function(bool) { 87 142 this._canEditLoadedThread = bool; 88 143 return this; ··· 151 206 }, 152 207 153 208 start: function() { 209 + 210 + this._transactionIDMap = {}; 211 + this._transactionCache = {}; 212 + 154 213 JX.Stratcom.listen( 155 214 'aphlict-server-message', 156 215 null, ··· 206 265 207 266 new JX.Workflow(this._getMoreMessagesURI(), data) 208 267 .setHandler(JX.bind(this, function(r) { 268 + this._deleteTransactionCaches(JX.Stratcom.getData(node).id); 209 269 JX.DOM.remove(node); 210 - var messages = JX.$H(r.messages); 211 - JX.DOM.prependContent( 212 - this._messagesRootCallback(), 213 - messages); 270 + this._updateTransactions(r); 214 271 })).start(); 215 272 })); 216 273 JX.Stratcom.listen( ··· 228 285 229 286 new JX.Workflow(this._getMoreMessagesURI(), data) 230 287 .setHandler(JX.bind(this, function(r) { 288 + this._deleteTransactionCaches(JX.Stratcom.getData(node).id); 231 289 JX.DOM.remove(node); 232 - var messages = JX.$H(r.messages); 233 - JX.DOM.appendContent( 234 - this._messagesRootCallback(), 235 - JX.$H(messages)); 290 + this._updateTransactions(r); 236 291 })).start(); 237 292 })); 238 293 }, ··· 254 309 return true; 255 310 }, 256 311 257 - _markUpdated: function(r) { 312 + _updateDOM: function(r) { 313 + this._updateTransactions(r); 314 + 258 315 this._updating.knownID = r.latest_transaction_id; 259 316 this._latestTransactionID = r.latest_transaction_id; 260 317 JX.Stratcom.invoke( ··· 263 320 r.aphlictDropdownData); 264 321 }, 265 322 323 + _updateTransactions: function(r) { 324 + var new_transactions = JX.$H(r.transactions).getFragment().childNodes; 325 + this._updateTransactionIDMap(new_transactions); 326 + this._updateTransactionCache(new_transactions); 327 + 328 + var transactions = this._getLoadedTransactions(); 329 + 330 + JX.DOM.setContent(this._messagesRootCallback(), transactions); 331 + }, 332 + 266 333 _updateThread: function() { 267 334 var params = this._getParams({ 268 335 action: 'load', ··· 272 339 .setData(params) 273 340 .setHandler(JX.bind(this, function(r) { 274 341 if (this._shouldUpdateDOM(r)) { 275 - this._markUpdated(r); 276 - 342 + this._updateDOM(r); 277 343 this._didUpdateThreadCallback(r); 278 344 } 279 345 })); ··· 306 372 .setData(params) 307 373 .setHandler(JX.bind(this, function(r) { 308 374 if (this._shouldUpdateDOM(r)) { 309 - this._markUpdated(r); 310 - 375 + this._updateDOM(r); 311 376 this._didUpdateWorkflowCallback(r); 312 377 } 313 378 })); ··· 357 422 r.aphlictDropdownData); 358 423 359 424 this._didLoadThreadCallback(r); 425 + var messages_root = this._messagesRootCallback(); 426 + var messages = JX.DOM.scry( 427 + messages_root, 428 + 'div', 429 + 'conpherence-transaction-view'); 430 + this._updateTransactionIDMap(messages); 431 + this._updateTransactionCache(messages); 360 432 361 433 if (force_reload) { 362 434 JX.Stratcom.invoke('hashchange'); ··· 383 455 var workflow = JX.Workflow.newFromForm(form, params, keep_enabled) 384 456 .setHandler(JX.bind(this, function(r) { 385 457 if (this._shouldUpdateDOM(r)) { 386 - this._markUpdated(r); 387 - 458 + this._updateDOM(r); 388 459 this._didSendMessageCallback(r); 389 460 } else if (r.non_update) { 390 461 this._didSendMessageCallback(r, true);
-2
webroot/rsrc/js/application/conpherence/behavior-durable-column.js
··· 159 159 return; 160 160 } 161 161 var messages = _getColumnMessagesNode(); 162 - JX.DOM.appendContent(messages, JX.$H(r.transactions)); 163 162 scrollbar.scrollTo(messages.scrollHeight); 164 163 }); 165 164 ··· 168 167 }); 169 168 threadManager.setDidUpdateWorkflowCallback(function(r) { 170 169 var messages = _getColumnMessagesNode(); 171 - JX.DOM.appendContent(messages, JX.$H(r.transactions)); 172 170 scrollbar.scrollTo(messages.scrollHeight); 173 171 JX.DOM.setContent(_getColumnTitleNode(), r.conpherence_title); 174 172 });
+3 -5
webroot/rsrc/js/application/conpherence/behavior-menu.js
··· 36 36 }); 37 37 threadManager.setDidLoadThreadCallback(function(r) { 38 38 var header = JX.$H(r.header); 39 - var messages = JX.$H(r.messages); 39 + var messages = JX.$H(r.transactions); 40 40 var form = JX.$H(r.form); 41 41 var root = JX.DOM.find(document, 'div', 'conpherence-layout'); 42 42 var header_root = JX.DOM.find(root, 'div', 'conpherence-header-pane'); ··· 51 51 }); 52 52 53 53 threadManager.setDidUpdateThreadCallback(function(r) { 54 - JX.DOM.appendContent(scrollbar.getContentNode(), JX.$H(r.transactions)); 55 54 _scrollMessageWindow(); 56 55 }); 57 56 ··· 73 72 } catch (ex) { 74 73 // Ignore; maybe no files widget 75 74 } 76 - JX.DOM.appendContent(scrollbar.getContentNode(), JX.$H(r.transactions)); 77 - _scrollMessageWindow(); 78 - 79 75 if (fileWidget) { 80 76 JX.DOM.setContent( 81 77 fileWidget, 82 78 JX.$H(r.file_widget)); 83 79 } 80 + 81 + _scrollMessageWindow(); 84 82 textarea.value = ''; 85 83 } 86 84 markThreadLoading(false);