From fae23e086049d8484006bc9eb0522761fac7304d Mon Sep 17 00:00:00 2001 From: epriestley Date: Thu, 10 Jul 2014 10:19:03 -0700 Subject: [PATCH] Make column reordering after edits on workboards more general Summary: Ref T5476. Currently, the task edit code assumes it knows what the UI looks like and sends back where on the column an item should be inserted. This is buggy after adding filters, and relatively complex. Instead, send down the ordering on the whole column and sort it in the UI. This is a bit simpler overall and more general. It makes it easier to further generalize this code for T5476. Test Plan: - Edited a task on a board, changing priority. Saw it reorder properly. - Edited a task on a board in a field of other tasks at the same top-level priority. Saw it refresh without reordering. Reviewers: chad Reviewed By: chad Subscribers: epriestley Maniphest Tasks: T5476 Differential Revision: https://secure.phabricator.com/D9832 --- resources/celerity/map.php | 22 ++++---- .../ManiphestTaskEditController.php | 22 +++----- .../maniphest/storage/ManiphestTask.php | 7 +++ .../projects/behavior-project-boards.js | 55 ++++++++++++------- 4 files changed, 60 insertions(+), 46 deletions(-) diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 2f2f83307f..cb8c9c1e8b 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -411,7 +411,7 @@ return array( 'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '92918fcb', 'rsrc/js/application/ponder/behavior-votebox.js' => '4e9b766b', 'rsrc/js/application/projects/behavior-boards-dropdown.js' => '0ec56e1d', - 'rsrc/js/application/projects/behavior-project-boards.js' => '1cb113dc', + 'rsrc/js/application/projects/behavior-project-boards.js' => 'c6b95cbd', 'rsrc/js/application/projects/behavior-project-create.js' => '065227cc', 'rsrc/js/application/releeph/releeph-preview-branch.js' => 'b2b4fbaf', 'rsrc/js/application/releeph/releeph-request-state-change.js' => 'ab836011', @@ -637,7 +637,7 @@ return array( 'javelin-behavior-policy-control' => 'f3fef818', 'javelin-behavior-policy-rule-editor' => '92918fcb', 'javelin-behavior-ponder-votebox' => '4e9b766b', - 'javelin-behavior-project-boards' => '1cb113dc', + 'javelin-behavior-project-boards' => 'c6b95cbd', 'javelin-behavior-project-create' => '065227cc', 'javelin-behavior-refresh-csrf' => '7814b593', 'javelin-behavior-releeph-preview-branch' => 'b2b4fbaf', @@ -967,15 +967,6 @@ return array( 1 => 'javelin-util', 2 => 'phabricator-keyboard-shortcut-manager', ), - '1cb113dc' => - array( - 0 => 'javelin-behavior', - 1 => 'javelin-dom', - 2 => 'javelin-util', - 3 => 'javelin-stratcom', - 4 => 'javelin-workflow', - 5 => 'phabricator-draggable-list', - ), '1d8ad5c3' => array( 0 => 'javelin-install', @@ -1849,6 +1840,15 @@ return array( 2 => 'javelin-uri', 3 => 'javelin-util', ), + 'c6b95cbd' => + array( + 0 => 'javelin-behavior', + 1 => 'javelin-dom', + 2 => 'javelin-util', + 3 => 'javelin-stratcom', + 4 => 'javelin-workflow', + 5 => 'phabricator-draggable-list', + ), 'ca3f91eb' => array( 0 => 'javelin-behavior', diff --git a/src/applications/maniphest/controller/ManiphestTaskEditController.php b/src/applications/maniphest/controller/ManiphestTaskEditController.php index 68fc82e3b7..08c9414fe4 100644 --- a/src/applications/maniphest/controller/ManiphestTaskEditController.php +++ b/src/applications/maniphest/controller/ManiphestTaskEditController.php @@ -365,8 +365,6 @@ final class ManiphestTaskEditController extends ManiphestController { $potential_col_tasks = id(new ManiphestTaskQuery()) ->setViewer($user) ->withAllProjects(array($column->getProjectPHID())) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) ->execute(); $potential_col_tasks = mpull( $potential_col_tasks, @@ -395,21 +393,17 @@ final class ManiphestTaskEditController extends ManiphestController { $column_tasks = id(new ManiphestTaskQuery()) ->setViewer($user) ->withPHIDs($column_task_phids) - ->withStatuses(ManiphestTaskStatus::getOpenStatusConstants()) - ->setOrderBy(ManiphestTaskQuery::ORDER_PRIORITY) ->execute(); } - $column_task_phids = mpull($column_tasks, 'getPHID'); - $task_phid = $task->getPHID(); - $after_phid = null; - foreach ($column_task_phids as $phid) { - if ($phid == $task_phid) { - break; - } - $after_phid = $phid; - } + + $sort_map = mpull( + $column_tasks, + 'getPrioritySortVector', + 'getPHID'); + $data = array( - 'insertAfterPHID' => $after_phid); + 'sortMap' => $sort_map, + ); break; case 'task': default: diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index f380495b5c..9477f6af7c 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -161,6 +161,13 @@ final class ManiphestTask extends ManiphestDAO return ManiphestTaskStatus::isClosedStatus($this->getStatus()); } + public function getPrioritySortVector() { + return array( + $this->getPriority(), + -$this->getSubpriority(), + ); + } + /* -( Markup Interface )--------------------------------------------------- */ diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index 452f647e5a..90320c60f4 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -24,6 +24,22 @@ JX.behavior('project-boards', function(config) { JX.DOM.replace(item, JX.$H(response.task)); } + function colsort(u, v) { + var ud = JX.Stratcom.getData(u).sort || []; + var vd = JX.Stratcom.getData(v).sort || []; + + for (var ii = 0; ii < ud.length; ii++) { + if (ud[ii] < vd[ii]) { + return 1; + } + if (ud[ii] > vd[ii]) { + return -1; + } + } + + return 0; + } + function ondrop(list, item, after) { list.lock(); JX.DOM.alterClass(item, 'drag-sending', true); @@ -93,30 +109,27 @@ JX.behavior('project-boards', function(config) { } var onedit = function(card, column, r) { - var new_card = JX.$H(r.tasks); + var new_card = JX.$H(r.tasks).getNode(); + var new_data = JX.Stratcom.getData(new_card); var items = finditems(column); - var insert_after = r.data.insertAfterPHID; - if (!insert_after) { - JX.DOM.prependContent(column, new_card); - if (card) { - JX.DOM.remove(card); - } - return; - } - var ii; - var item; - var item_phid; - for (ii = 0; ii< items.length; ii++) { - item = items[ii]; - item_phid = JX.Stratcom.getData(item).objectPHID; - if (item_phid == insert_after) { - JX.DOM.replace(item, [item, new_card]); - if (card) { - JX.DOM.remove(card); - } - return; + + for (var ii = 0; ii < items.length; ii++) { + var item = items[ii]; + + var data = JX.Stratcom.getData(item); + var phid = data.objectPHID; + + if (phid == new_data.objectPHID) { + items[ii] = new_card; + data = new_data; } + + data.sort = r.data.sortMap[data.objectPHID] || data.sort; } + + items.sort(colsort); + + JX.DOM.setContent(column, items); }; JX.Stratcom.listen(