recaptime.dev / phorge
@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 - improve stack re: non-update updates

Summary:
Fixes T7761. Fixes T7318.

When we send an empty message to the server, pretend its just a request to load the page. Make load a bit smarter such that if we don't get back any transactions, rather than error like the fool, just send down to the client the notion of a 'non_update'. Instrument the client to just turn off the appropriate loading state, etc for a non update.

T7318 is a tricky beast since we don't know exactly how to reproduce it but if / when it occurs again it would be some other bizarre application behavior maybe? We won't be getting the execption anymore, that's for sure.

Test Plan: removed code in `ConpherenceThreadManager.sendMessage` that protects against sending empty messages. sent empty messages (non updates) like whoa and everything worked on both durable column and main column view. re-added the code in `ConpherenceThreadManager.sendMessage` and noted empty messages did not send while any text including a space sent up nicely

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin, epriestley

Maniphest Tasks: T7318, T7761

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

Bob Trahan b14ca389 540e38d2

+92 -70
+39 -39
resources/celerity/map.php
··· 354 354 'rsrc/js/application/aphlict/behavior-aphlict-status.js' => 'ea681761', 355 355 'rsrc/js/application/auth/behavior-persona-login.js' => '9414ff18', 356 356 'rsrc/js/application/config/behavior-reorder-fields.js' => '14a827de', 357 - 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => 'bb928342', 358 - 'rsrc/js/application/conpherence/behavior-durable-column.js' => 'a0216452', 359 - 'rsrc/js/application/conpherence/behavior-menu.js' => 'db6c0ba7', 357 + 'rsrc/js/application/conpherence/ConpherenceThreadManager.js' => '0a5192c4', 358 + 'rsrc/js/application/conpherence/behavior-durable-column.js' => '7ffa744f', 359 + 'rsrc/js/application/conpherence/behavior-menu.js' => 'e67cfd8c', 360 360 'rsrc/js/application/conpherence/behavior-pontificate.js' => '21ba5861', 361 361 'rsrc/js/application/conpherence/behavior-quicksand-blacklist.js' => '7927a7d3', 362 362 'rsrc/js/application/conpherence/behavior-widget-pane.js' => '1ec93bcf', ··· 519 519 'conpherence-menu-css' => '7c900089', 520 520 'conpherence-message-pane-css' => 'e44b667b', 521 521 'conpherence-notification-css' => '04a6e10a', 522 - 'conpherence-thread-manager' => 'bb928342', 522 + 'conpherence-thread-manager' => '0a5192c4', 523 523 'conpherence-update-css' => '1099a660', 524 524 'conpherence-widget-pane-css' => 'a9082fd0', 525 525 'differential-changeset-view-css' => 'e19cfd6e', ··· 559 559 'javelin-behavior-boards-dropdown' => '0ec56e1d', 560 560 'javelin-behavior-choose-control' => '6153c708', 561 561 'javelin-behavior-config-reorder-fields' => '14a827de', 562 - 'javelin-behavior-conpherence-menu' => 'db6c0ba7', 562 + 'javelin-behavior-conpherence-menu' => 'e67cfd8c', 563 563 'javelin-behavior-conpherence-pontificate' => '21ba5861', 564 564 'javelin-behavior-conpherence-widget-pane' => '1ec93bcf', 565 565 'javelin-behavior-countdown-timer' => 'e4cc26b3', ··· 586 586 'javelin-behavior-diffusion-locate-file' => '6d3e1947', 587 587 'javelin-behavior-diffusion-pull-lastmodified' => '2b228192', 588 588 'javelin-behavior-doorkeeper-tag' => 'e5822781', 589 - 'javelin-behavior-durable-column' => 'a0216452', 589 + 'javelin-behavior-durable-column' => '7ffa744f', 590 590 'javelin-behavior-error-log' => '6882e80a', 591 591 'javelin-behavior-fancy-datepicker' => 'c51ae228', 592 592 'javelin-behavior-global-drag-and-drop' => 'bbdf75ca', ··· 874 874 'javelin-dom', 875 875 'javelin-router', 876 876 ), 877 + '0a5192c4' => array( 878 + 'javelin-dom', 879 + 'javelin-util', 880 + 'javelin-stratcom', 881 + 'javelin-install', 882 + 'javelin-workflow', 883 + 'javelin-router', 884 + 'javelin-behavior-device', 885 + 'javelin-vector', 886 + ), 877 887 '0c6946e7' => array( 878 888 'javelin-install', 879 889 'javelin-dom', ··· 1429 1439 'javelin-uri', 1430 1440 'phabricator-file-upload', 1431 1441 ), 1442 + '7ffa744f' => array( 1443 + 'javelin-behavior', 1444 + 'javelin-dom', 1445 + 'javelin-stratcom', 1446 + 'javelin-behavior-device', 1447 + 'javelin-scrollbar', 1448 + 'javelin-quicksand', 1449 + 'phabricator-keyboard-shortcut', 1450 + 'conpherence-thread-manager', 1451 + ), 1432 1452 82439934 => array( 1433 1453 'javelin-behavior', 1434 1454 'javelin-dom', ··· 1578 1598 'javelin-request', 1579 1599 'phabricator-shaped-request', 1580 1600 ), 1581 - 'a0216452' => array( 1582 - 'javelin-behavior', 1583 - 'javelin-dom', 1584 - 'javelin-stratcom', 1585 - 'javelin-behavior-device', 1586 - 'javelin-scrollbar', 1587 - 'javelin-quicksand', 1588 - 'phabricator-keyboard-shortcut', 1589 - 'conpherence-thread-manager', 1590 - ), 1591 1601 'a0b57eb8' => array( 1592 1602 'javelin-behavior', 1593 1603 'javelin-dom', ··· 1680 1690 'javelin-stratcom', 1681 1691 'javelin-dom', 1682 1692 'javelin-util', 1683 - ), 1684 - 'bb928342' => array( 1685 - 'javelin-dom', 1686 - 'javelin-util', 1687 - 'javelin-stratcom', 1688 - 'javelin-install', 1689 - 'javelin-workflow', 1690 - 'javelin-router', 1691 - 'javelin-behavior-device', 1692 - 'javelin-vector', 1693 1693 ), 1694 1694 'bba9eedf' => array( 1695 1695 'javelin-behavior', ··· 1803 1803 'javelin-util', 1804 1804 'phabricator-shaped-request', 1805 1805 ), 1806 - 'db6c0ba7' => array( 1807 - 'javelin-behavior', 1808 - 'javelin-dom', 1809 - 'javelin-util', 1810 - 'javelin-stratcom', 1811 - 'javelin-workflow', 1812 - 'javelin-behavior-device', 1813 - 'javelin-history', 1814 - 'javelin-vector', 1815 - 'phabricator-title', 1816 - 'phabricator-shaped-request', 1817 - 'conpherence-thread-manager', 1818 - ), 1819 1806 'dbbf48b6' => array( 1820 1807 'javelin-behavior', 1821 1808 'javelin-stratcom', ··· 1900 1887 'javelin-mask', 1901 1888 'javelin-behavior-device', 1902 1889 'phabricator-keyboard-shortcut', 1890 + ), 1891 + 'e67cfd8c' => array( 1892 + 'javelin-behavior', 1893 + 'javelin-dom', 1894 + 'javelin-util', 1895 + 'javelin-stratcom', 1896 + 'javelin-workflow', 1897 + 'javelin-behavior-device', 1898 + 'javelin-history', 1899 + 'javelin-vector', 1900 + 'phabricator-title', 1901 + 'phabricator-shaped-request', 1902 + 'conpherence-thread-manager', 1903 1903 ), 1904 1904 'e723c323' => array( 1905 1905 'javelin-behavior',
+17 -6
src/applications/conpherence/controller/ConpherenceUpdateController.php
··· 76 76 break; 77 77 case ConpherenceUpdateActions::MESSAGE: 78 78 $message = $request->getStr('text'); 79 - $xactions = $editor->generateTransactionsFromText( 80 - $user, 81 - $conpherence, 82 - $message); 83 - $delete_draft = true; 79 + if (strlen($message)) { 80 + $xactions = $editor->generateTransactionsFromText( 81 + $user, 82 + $conpherence, 83 + $message); 84 + $delete_draft = true; 85 + } else { 86 + $action = ConpherenceUpdateActions::LOAD; 87 + $updated = false; 88 + $response_mode = 'ajax'; 89 + } 84 90 break; 85 91 case ConpherenceUpdateActions::ADD_PERSON: 86 92 $person_phids = $request->getArr('add_person'); ··· 397 403 ->withIDs(array($conpherence_id)) 398 404 ->executeOne(); 399 405 400 - if ($need_transactions) { 406 + $non_update = false; 407 + if ($need_transactions && $conpherence->getTransactions()) { 401 408 $data = ConpherenceTransactionView::renderTransactions( 402 409 $user, 403 410 $conpherence, 404 411 !$this->getRequest()->getExists('minimal_display')); 405 412 $participant_obj = $conpherence->getParticipant($user->getPHID()); 406 413 $participant_obj->markUpToDate($conpherence, $data['latest_transaction']); 414 + } else if ($need_transactions) { 415 + $non_update = true; 416 + $data = array(); 407 417 } else { 408 418 $data = array(); 409 419 } ··· 451 461 } 452 462 $data = $conpherence->getDisplayData($user); 453 463 $content = array( 464 + 'non_update' => $non_update, 454 465 'transactions' => hsprintf('%s', $rendered_transactions), 455 466 'conpherence_title' => (string) $data['title'], 456 467 'latest_transaction_id' => $new_latest_transaction_id,
+12
webroot/rsrc/js/application/conpherence/ConpherenceThreadManager.js
··· 183 183 _shouldUpdateDOM: function(r) { 184 184 if (this._updating && 185 185 this._updating.threadPHID == this._loadedThreadPHID) { 186 + 187 + if (r.non_update) { 188 + return false; 189 + } 190 + 186 191 // we have a different, more current update in progress so 187 192 // return early 188 193 if (r.latest_transaction_id < this._updating.knownID) { ··· 282 287 }, 283 288 284 289 sendMessage: function(form, params) { 290 + // don't bother sending up text if there is nothing to submit 291 + var textarea = JX.DOM.find(form, 'textarea'); 292 + if (!textarea.value.length) { 293 + return; 294 + } 285 295 params = this._getParams(params); 286 296 287 297 var keep_enabled = true; ··· 292 302 this._markUpdated(r); 293 303 294 304 this._didSendMessageCallback(r); 305 + } else if (r.non_update) { 306 + this._didSendMessageCallback(r, true); 295 307 } 296 308 })); 297 309 this.syncWorkflow(workflow, 'finally');
+4 -7
webroot/rsrc/js/application/conpherence/behavior-durable-column.js
··· 140 140 _focusColumnTextareaNode(); 141 141 }); 142 142 143 - threadManager.setDidSendMessageCallback(function(r) { 143 + threadManager.setDidSendMessageCallback(function(r, non_update) { 144 + if (non_update) { 145 + return; 146 + } 144 147 var messages = _getColumnMessagesNode(); 145 148 JX.DOM.appendContent(messages, JX.$H(r.transactions)); 146 149 scrollbar.scrollTo(messages.scrollHeight); ··· 304 307 // From here on, interpret this as a "send" action, not a literal 305 308 // newline. 306 309 e.kill(); 307 - 308 - var textarea = _getColumnTextareaNode(); 309 - if (!textarea.value.length) { 310 - // If there's no text, don't try to submit the form. 311 - return; 312 - } 313 310 314 311 _sendMessage(e); 315 312 });
+20 -18
webroot/rsrc/js/application/conpherence/behavior-menu.js
··· 57 57 markThreadLoading(true); 58 58 JX.DOM.alterClass(form_root, 'loading', true); 59 59 }); 60 - threadManager.setDidSendMessageCallback(function (r) { 60 + threadManager.setDidSendMessageCallback(function (r, non_update) { 61 61 var root = JX.DOM.find(document, 'div', 'conpherence-layout'); 62 62 var form_root = JX.DOM.find(root, 'div', 'conpherence-form'); 63 - var messages_root = JX.DOM.find(root, 'div', 'conpherence-message-pane'); 64 - var messages = JX.DOM.find(messages_root, 'div', 'conpherence-messages'); 65 - var fileWidget = null; 66 - try { 67 - fileWidget = JX.DOM.find(root, 'div', 'widgets-files'); 68 - } catch (ex) { 69 - // Ignore; maybe no files widget 70 - } 71 - JX.DOM.appendContent(messages, JX.$H(r.transactions)); 72 - messages.scrollTop = messages.scrollHeight; 63 + var textarea = JX.DOM.find(form_root, 'textarea'); 64 + if (!non_update) { 65 + var messages_root = JX.DOM.find(root, 'div', 'conpherence-message-pane'); 66 + var messages = JX.DOM.find(messages_root, 'div', 'conpherence-messages'); 67 + var fileWidget = null; 68 + try { 69 + fileWidget = JX.DOM.find(root, 'div', 'widgets-files'); 70 + } catch (ex) { 71 + // Ignore; maybe no files widget 72 + } 73 + JX.DOM.appendContent(messages, JX.$H(r.transactions)); 74 + messages.scrollTop = messages.scrollHeight; 73 75 74 - if (fileWidget) { 75 - JX.DOM.setContent( 76 - fileWidget, 77 - JX.$H(r.file_widget) 78 - ); 76 + if (fileWidget) { 77 + JX.DOM.setContent( 78 + fileWidget, 79 + JX.$H(r.file_widget) 80 + ); 81 + } 82 + textarea.value = ''; 79 83 } 80 - var textarea = JX.DOM.find(form_root, 'textarea'); 81 - textarea.value = ''; 82 84 markThreadLoading(false); 83 85 84 86 setTimeout(function() { JX.DOM.focus(textarea); }, 100);