From d02beaf8161a15e3babaef5ef99f433bf260cd02 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 17 Jul 2019 10:55:17 -0700 Subject: [PATCH] Make reloading workboards with "R" respect workboard ordering Summary: Depends on D20653. Ref T4900. Pass ordering details to the reload endpoint so it can give the client accurate ordering/header information in the response. The removed comment mentions this, but here's why this is a difficult mess: - In window A, view a board with "Group by: Owner" and no tasks owned by "Alice". Since "Alice" owns no tasks, this means the columns do not have an "Assigned to: Alice" header! - In window B, edit task T and assign it to Alice. - In window A, press "R". Window A now not only needs to update to properly reflect the state of task T, it actually needs to draw a new "Assigned to: Alice" header in every column. Fortunately, the "group by" code anticipates this being a big mess, is fairly careful about handling it, and the client can handle this state change and the actual code change here isn't too involved. This is just causing a lot of not-very-obvious indirect effects in the pipeline to handle these situations that need complex redraws. Test Plan: - After making various normal edits/creates/moves in window A, pressed "R" in window B. Saw ordering reflected correctly after sync. - Went through the whole "Group by: Owner" + assign to unrepresented owner flow above. After pressing "R", saw "Assigned to: Alice" appear on the board. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T4900 Differential Revision: https://secure.phabricator.com/D20654 --- resources/celerity/map.php | 28 +++++++++---------- ...habricatorProjectBoardReloadController.php | 15 ++++++---- .../js/application/projects/WorkboardBoard.js | 1 + 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 675cdebaac..febcddca82 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -412,7 +412,7 @@ return array( 'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f', 'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9', 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172', - 'rsrc/js/application/projects/WorkboardBoard.js' => '50147a89', + 'rsrc/js/application/projects/WorkboardBoard.js' => '19df903f', 'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8', 'rsrc/js/application/projects/WorkboardCardTemplate.js' => '84f82dad', 'rsrc/js/application/projects/WorkboardColumn.js' => 'c3d24e63', @@ -743,7 +743,7 @@ return array( 'javelin-view-renderer' => '9aae2b66', 'javelin-view-visitor' => '308f9fe4', 'javelin-websocket' => 'fdc13e4e', - 'javelin-workboard-board' => '50147a89', + 'javelin-workboard-board' => '19df903f', 'javelin-workboard-card' => '0392a5d8', 'javelin-workboard-card-template' => '84f82dad', 'javelin-workboard-column' => 'c3d24e63', @@ -1030,6 +1030,18 @@ return array( '17b71bbc' => array( 'phui-theme-css', ), + '19df903f' => array( + 'javelin-install', + 'javelin-dom', + 'javelin-util', + 'javelin-stratcom', + 'javelin-workflow', + 'phabricator-draggable-list', + 'javelin-workboard-column', + 'javelin-workboard-header-template', + 'javelin-workboard-card-template', + 'javelin-workboard-order-template', + ), '1b6acc2a' => array( 'javelin-magical-init', 'javelin-util', @@ -1357,18 +1369,6 @@ return array( '4feea7d3' => array( 'trigger-rule-control', ), - '50147a89' => array( - 'javelin-install', - 'javelin-dom', - 'javelin-util', - 'javelin-stratcom', - 'javelin-workflow', - 'phabricator-draggable-list', - 'javelin-workboard-column', - 'javelin-workboard-header-template', - 'javelin-workboard-card-template', - 'javelin-workboard-order-template', - ), '506aa3f4' => array( 'javelin-behavior', 'javelin-stratcom', diff --git a/src/applications/project/controller/PhabricatorProjectBoardReloadController.php b/src/applications/project/controller/PhabricatorProjectBoardReloadController.php index e029cba896..6204671505 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardReloadController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardReloadController.php @@ -11,6 +11,15 @@ final class PhabricatorProjectBoardReloadController return $response; } + $order = $request->getStr('order'); + if (!strlen($order)) { + $order = PhabricatorProjectColumnNaturalOrder::ORDERKEY; + } + + $ordering = PhabricatorProjectColumnOrder::getOrderByKey($order); + $ordering = id(clone $ordering) + ->setViewer($viewer); + $project = $this->getProject(); $state = $this->getViewState(); $board_uri = $state->newWorkboardURI(); @@ -53,15 +62,11 @@ final class PhabricatorProjectBoardReloadController $engine = id(new PhabricatorBoardResponseEngine()) ->setViewer($viewer) ->setBoardPHID($board_phid) + ->setOrdering($ordering) ->setObjects($objects) ->setUpdatePHIDs($update_phids) ->setVisiblePHIDs($visible_phids); - // TODO: We don't currently process "order" properly. If a user is viewing - // a board grouped by "Owner", and another user changes a task to be owned - // by a user who currently owns nothing on the board, the new header won't - // generate correctly if the first user presses "R". - return $engine->buildResponse(); } diff --git a/webroot/rsrc/js/application/projects/WorkboardBoard.js b/webroot/rsrc/js/application/projects/WorkboardBoard.js index a6fc97beb4..c78fcd4eaf 100644 --- a/webroot/rsrc/js/application/projects/WorkboardBoard.js +++ b/webroot/rsrc/js/application/projects/WorkboardBoard.js @@ -746,6 +746,7 @@ JX.install('WorkboardBoard', { var data = { state: JX.JSON.stringify(state), + order: this.getOrder() }; var on_reload = JX.bind(this, this._onReloadResponse);