From 9a8019d4a98bd06868289039496d57080286e03e Mon Sep 17 00:00:00 2001 From: epriestley Date: Sun, 10 Mar 2019 21:49:12 -0700 Subject: [PATCH] Modularize workboard column orders Summary: Depends on D20267. Depends on D20268. Ref T10333. Currently, we support "Natural" and "Priority" orders, but a lot of the particulars are pretty hard-coded, including some logic in `ManiphestTask`. Although it's not clear that we'll ever put other types of objects on workboards, it seems generally bad that you need to modify `ManiphestTask` to get a new ordering. Pull the ordering logic out into a `ProjectColumnOrder` hierarchy instead, and let each ordering define the things it needs to work (name, icon, what headers look like, how different objects are sorted, and how to apply an edit when you drop an object under a header). Then move the existing "Natural" and "Priority" orders into this new hierarchy. This has a minor bug where using the "Edit" workflow to change a card's priority on a priority-ordered board doesn't fully refresh card/header order since the response isn't ordering-aware. I'll fix that in an upcoming change. Test Plan: Grouped workboards by "Natural" and "Priority", dragged stuff around within and between columns, grepped for all touched symbols. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T10333 Differential Revision: https://secure.phabricator.com/D20269 --- resources/celerity/map.php | 102 +++++----- src/__phutil_library_map__.php | 8 + .../maniphest/storage/ManiphestTask.php | 16 -- .../PhabricatorProjectBoardViewController.php | 104 ++++------- .../PhabricatorProjectController.php | 17 +- .../PhabricatorProjectMoveController.php | 41 ++-- .../engine/PhabricatorBoardResponseEngine.php | 70 ++++++- .../order/PhabricatorProjectColumnHeader.php | 94 ++++++++++ .../PhabricatorProjectColumnNaturalOrder.php | 12 ++ .../order/PhabricatorProjectColumnOrder.php | 176 ++++++++++++++++++ .../PhabricatorProjectColumnPriorityOrder.php | 91 +++++++++ .../storage/PhabricatorProjectColumn.php | 7 - .../js/application/projects/WorkboardBoard.js | 48 +++-- .../js/application/projects/WorkboardCard.js | 7 +- .../projects/WorkboardCardTemplate.js | 17 ++ .../application/projects/WorkboardColumn.js | 16 +- .../application/projects/WorkboardHeader.js | 12 +- .../projects/WorkboardHeaderTemplate.js | 10 + .../projects/behavior-project-boards.js | 8 +- 19 files changed, 638 insertions(+), 218 deletions(-) create mode 100644 src/applications/project/order/PhabricatorProjectColumnHeader.php create mode 100644 src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php create mode 100644 src/applications/project/order/PhabricatorProjectColumnOrder.php create mode 100644 src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php diff --git a/resources/celerity/map.php b/resources/celerity/map.php index 825a5cebc9..c96d03896c 100644 --- a/resources/celerity/map.php +++ b/resources/celerity/map.php @@ -408,14 +408,14 @@ 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' => '2181739b', - 'rsrc/js/application/projects/WorkboardCard.js' => 'bc92741f', - 'rsrc/js/application/projects/WorkboardCardTemplate.js' => 'b0b5f90a', - 'rsrc/js/application/projects/WorkboardColumn.js' => '6461f58b', + 'rsrc/js/application/projects/WorkboardBoard.js' => 'fc1664ff', + 'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8', + 'rsrc/js/application/projects/WorkboardCardTemplate.js' => '2a61f8d4', + 'rsrc/js/application/projects/WorkboardColumn.js' => '533dd592', 'rsrc/js/application/projects/WorkboardController.js' => '42c7a5a7', - 'rsrc/js/application/projects/WorkboardHeader.js' => '6e75daea', - 'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => '2d641f7d', - 'rsrc/js/application/projects/behavior-project-boards.js' => 'cca3f5f8', + 'rsrc/js/application/projects/WorkboardHeader.js' => '111bfd2d', + 'rsrc/js/application/projects/WorkboardHeaderTemplate.js' => 'b65351bd', + 'rsrc/js/application/projects/behavior-project-boards.js' => '285c337a', 'rsrc/js/application/projects/behavior-project-create.js' => '34c53422', 'rsrc/js/application/projects/behavior-reorder-columns.js' => '8ac32fd9', 'rsrc/js/application/releeph/releeph-preview-branch.js' => '75184d68', @@ -656,7 +656,7 @@ return array( 'javelin-behavior-phuix-example' => 'c2c500a7', 'javelin-behavior-policy-control' => '0eaa33a9', 'javelin-behavior-policy-rule-editor' => '9347f172', - 'javelin-behavior-project-boards' => 'cca3f5f8', + 'javelin-behavior-project-boards' => '285c337a', 'javelin-behavior-project-create' => '34c53422', 'javelin-behavior-quicksand-blacklist' => '5a6f6a06', 'javelin-behavior-read-only-warning' => 'b9109f8f', @@ -728,13 +728,13 @@ return array( 'javelin-view-renderer' => '9aae2b66', 'javelin-view-visitor' => '308f9fe4', 'javelin-websocket' => 'fdc13e4e', - 'javelin-workboard-board' => '2181739b', - 'javelin-workboard-card' => 'bc92741f', - 'javelin-workboard-card-template' => 'b0b5f90a', - 'javelin-workboard-column' => '6461f58b', + 'javelin-workboard-board' => 'fc1664ff', + 'javelin-workboard-card' => '0392a5d8', + 'javelin-workboard-card-template' => '2a61f8d4', + 'javelin-workboard-column' => '533dd592', 'javelin-workboard-controller' => '42c7a5a7', - 'javelin-workboard-header' => '6e75daea', - 'javelin-workboard-header-template' => '2d641f7d', + 'javelin-workboard-header' => '111bfd2d', + 'javelin-workboard-header-template' => 'b65351bd', 'javelin-workflow' => '958e9045', 'maniphest-report-css' => '3d53188b', 'maniphest-task-edit-css' => '272daa84', @@ -909,6 +909,9 @@ return array( 'javelin-uri', 'javelin-util', ), + '0392a5d8' => array( + 'javelin-install', + ), '04023d82' => array( 'javelin-install', 'phuix-button-view', @@ -993,6 +996,9 @@ return array( 'javelin-workflow', 'phuix-icon-view', ), + '111bfd2d' => array( + 'javelin-install', + ), '1325b731' => array( 'javelin-behavior', 'javelin-uri', @@ -1048,17 +1054,6 @@ return array( 'javelin-behavior', 'javelin-request', ), - '2181739b' => 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', - ), '225bbb98' => array( 'javelin-install', 'javelin-reactor', @@ -1110,6 +1105,15 @@ return array( 'javelin-json', 'phabricator-prefab', ), + '285c337a' => array( + 'javelin-behavior', + 'javelin-dom', + 'javelin-util', + 'javelin-vector', + 'javelin-stratcom', + 'javelin-workflow', + 'javelin-workboard-controller', + ), '289bf236' => array( 'javelin-install', 'javelin-util', @@ -1119,6 +1123,9 @@ return array( 'javelin-stratcom', 'javelin-behavior', ), + '2a61f8d4' => array( + 'javelin-install', + ), '2a8b62d9' => array( 'multirow-row-manager', 'javelin-install', @@ -1142,9 +1149,6 @@ return array( 'javelin-dom', 'phabricator-keyboard-shortcut', ), - '2d641f7d' => array( - 'javelin-install', - ), '2e255291' => array( 'javelin-install', 'javelin-util', @@ -1343,6 +1347,11 @@ return array( 'javelin-dom', 'javelin-fx', ), + '533dd592' => array( + 'javelin-install', + 'javelin-workboard-card', + 'javelin-workboard-header', + ), '534f1757' => array( 'phui-oi-list-view-css', ), @@ -1427,11 +1436,6 @@ return array( '60cd9241' => array( 'javelin-behavior', ), - '6461f58b' => array( - 'javelin-install', - 'javelin-workboard-card', - 'javelin-workboard-header', - ), '65bb0011' => array( 'javelin-behavior', 'javelin-dom', @@ -1477,9 +1481,6 @@ return array( 'javelin-install', 'javelin-util', ), - '6e75daea' => array( - 'javelin-install', - ), 70245195 => array( 'javelin-behavior', 'javelin-stratcom', @@ -1839,9 +1840,6 @@ return array( 'javelin-behavior-device', 'javelin-vector', ), - 'b0b5f90a' => array( - 'javelin-install', - ), 'b105a3a6' => array( 'javelin-behavior', 'javelin-stratcom', @@ -1875,6 +1873,9 @@ return array( 'javelin-stratcom', 'javelin-dom', ), + 'b65351bd' => array( + 'javelin-install', + ), 'b7b73831' => array( 'javelin-behavior', 'javelin-dom', @@ -1896,9 +1897,6 @@ return array( 'bc16cf33' => array( 'phui-workcard-view-css', ), - 'bc92741f' => array( - 'javelin-install', - ), 'bdce4d78' => array( 'javelin-install', 'javelin-util', @@ -1968,15 +1966,6 @@ return array( 'javelin-util', 'phabricator-keyboard-shortcut-manager', ), - 'cca3f5f8' => array( - 'javelin-behavior', - 'javelin-dom', - 'javelin-util', - 'javelin-vector', - 'javelin-stratcom', - 'javelin-workflow', - 'javelin-workboard-controller', - ), 'cf32921f' => array( 'javelin-behavior', 'javelin-dom', @@ -2134,6 +2123,17 @@ return array( 'phabricator-keyboard-shortcut', 'conpherence-thread-manager', ), + 'fc1664ff' => 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', + ), 'fce5d170' => array( 'javelin-magical-init', 'javelin-util', diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 713d3e5f29..bdb1fee462 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -4052,10 +4052,14 @@ phutil_register_library_map(array( 'PhabricatorProjectColumn' => 'applications/project/storage/PhabricatorProjectColumn.php', 'PhabricatorProjectColumnDetailController' => 'applications/project/controller/PhabricatorProjectColumnDetailController.php', 'PhabricatorProjectColumnEditController' => 'applications/project/controller/PhabricatorProjectColumnEditController.php', + 'PhabricatorProjectColumnHeader' => 'applications/project/order/PhabricatorProjectColumnHeader.php', 'PhabricatorProjectColumnHideController' => 'applications/project/controller/PhabricatorProjectColumnHideController.php', + 'PhabricatorProjectColumnNaturalOrder' => 'applications/project/order/PhabricatorProjectColumnNaturalOrder.php', + 'PhabricatorProjectColumnOrder' => 'applications/project/order/PhabricatorProjectColumnOrder.php', 'PhabricatorProjectColumnPHIDType' => 'applications/project/phid/PhabricatorProjectColumnPHIDType.php', 'PhabricatorProjectColumnPosition' => 'applications/project/storage/PhabricatorProjectColumnPosition.php', 'PhabricatorProjectColumnPositionQuery' => 'applications/project/query/PhabricatorProjectColumnPositionQuery.php', + 'PhabricatorProjectColumnPriorityOrder' => 'applications/project/order/PhabricatorProjectColumnPriorityOrder.php', 'PhabricatorProjectColumnQuery' => 'applications/project/query/PhabricatorProjectColumnQuery.php', 'PhabricatorProjectColumnSearchEngine' => 'applications/project/query/PhabricatorProjectColumnSearchEngine.php', 'PhabricatorProjectColumnTransaction' => 'applications/project/storage/PhabricatorProjectColumnTransaction.php', @@ -10132,13 +10136,17 @@ phutil_register_library_map(array( ), 'PhabricatorProjectColumnDetailController' => 'PhabricatorProjectBoardController', 'PhabricatorProjectColumnEditController' => 'PhabricatorProjectBoardController', + 'PhabricatorProjectColumnHeader' => 'Phobject', 'PhabricatorProjectColumnHideController' => 'PhabricatorProjectBoardController', + 'PhabricatorProjectColumnNaturalOrder' => 'PhabricatorProjectColumnOrder', + 'PhabricatorProjectColumnOrder' => 'Phobject', 'PhabricatorProjectColumnPHIDType' => 'PhabricatorPHIDType', 'PhabricatorProjectColumnPosition' => array( 'PhabricatorProjectDAO', 'PhabricatorPolicyInterface', ), 'PhabricatorProjectColumnPositionQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', + 'PhabricatorProjectColumnPriorityOrder' => 'PhabricatorProjectColumnOrder', 'PhabricatorProjectColumnQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorProjectColumnSearchEngine' => 'PhabricatorApplicationSearchEngine', 'PhabricatorProjectColumnTransaction' => 'PhabricatorApplicationTransaction', diff --git a/src/applications/maniphest/storage/ManiphestTask.php b/src/applications/maniphest/storage/ManiphestTask.php index d6e3ec6d6c..d2700895ce 100644 --- a/src/applications/maniphest/storage/ManiphestTask.php +++ b/src/applications/maniphest/storage/ManiphestTask.php @@ -248,14 +248,6 @@ final class ManiphestTask extends ManiphestDAO return idx($this->properties, 'cover.thumbnailPHID'); } - public function getWorkboardOrderVectors() { - return array( - PhabricatorProjectColumn::ORDER_PRIORITY => array( - (int)-$this->getPriority(), - ), - ); - } - public function getPriorityKeyword() { $priority = $this->getPriority(); @@ -267,14 +259,6 @@ final class ManiphestTask extends ManiphestDAO return ManiphestTaskPriority::UNKNOWN_PRIORITY_KEYWORD; } - public function getWorkboardProperties() { - return array( - 'status' => $this->getStatus(), - 'points' => (double)$this->getPoints(), - 'priority' => $this->getPriority(), - ); - } - /* -( PhabricatorSubscribableInterface )----------------------------------- */ diff --git a/src/applications/project/controller/PhabricatorProjectBoardViewController.php b/src/applications/project/controller/PhabricatorProjectBoardViewController.php index 6df43addd9..43599fa369 100644 --- a/src/applications/project/controller/PhabricatorProjectBoardViewController.php +++ b/src/applications/project/controller/PhabricatorProjectBoardViewController.php @@ -614,49 +614,25 @@ final class PhabricatorProjectBoardViewController $board->addPanel($panel); } - // It's possible for tasks to have an invalid/unknown priority in the - // database. We still want to generate a header for these tasks so we - // don't break the workboard. - $priorities = - ManiphestTaskPriority::getTaskPriorityMap() + - mpull($all_tasks, null, 'getPriority'); - $priorities = array_keys($priorities); + $order_key = $this->sortKey; - $headers = array(); - foreach ($priorities as $priority) { - $header_key = sprintf('priority(%s)', $priority); + $ordering_map = PhabricatorProjectColumnOrder::getAllOrders(); + $ordering = id(clone $ordering_map[$order_key]) + ->setViewer($viewer); - $priority_name = ManiphestTaskPriority::getTaskPriorityName($priority); - $priority_color = ManiphestTaskPriority::getTaskPriorityColor($priority); - $priority_icon = ManiphestTaskPriority::getTaskPriorityIcon($priority); + $headers = $ordering->getHeadersForObjects($all_tasks); + $headers = mpull($headers, 'toDictionary'); - $icon_view = id(new PHUIIconView()) - ->setIcon("{$priority_icon} {$priority_color}"); - - $template = phutil_tag( - 'li', - array( - 'class' => 'workboard-group-header', - ), - array( - $icon_view, - $priority_name, - )); - - $headers[] = array( - 'order' => PhabricatorProjectColumn::ORDER_PRIORITY, - 'key' => $header_key, - 'template' => hsprintf('%s', $template), - 'vector' => array( - (int)-$priority, - PhabricatorProjectColumn::NODETYPE_HEADER, - ), - 'editProperties' => array( - PhabricatorProjectColumn::ORDER_PRIORITY => (int)$priority, - ), - ); + $vectors = $ordering->getSortVectorsForObjects($all_tasks); + $vector_map = array(); + foreach ($vectors as $task_phid => $vector) { + $vector_map[$task_phid][$order_key] = $vector; } + $header_keys = $ordering->getHeaderKeysForObjects($all_tasks); + + $properties = array(); + $behavior_config = array( 'moveURI' => $this->getApplicationURI('move/'.$project->getID().'/'), 'uploadURI' => '/file/dropupload/', @@ -667,10 +643,11 @@ final class PhabricatorProjectBoardViewController 'boardPHID' => $project->getPHID(), 'order' => $this->sortKey, 'headers' => $headers, + 'headerKeys' => $header_keys, 'templateMap' => $templates, 'columnMaps' => $column_maps, - 'orderMaps' => mpull($all_tasks, 'getWorkboardOrderVectors'), - 'propertyMaps' => mpull($all_tasks, 'getWorkboardProperties'), + 'orderMaps' => $vector_map, + 'propertyMaps' => $properties, 'boardID' => $board_id, 'projectPHID' => $project->getPHID(), @@ -680,7 +657,8 @@ final class PhabricatorProjectBoardViewController $sort_menu = $this->buildSortMenu( $viewer, $project, - $this->sortKey); + $this->sortKey, + $ordering_map); $filter_menu = $this->buildFilterMenu( $viewer, @@ -775,7 +753,7 @@ final class PhabricatorProjectBoardViewController return $default_sort; } - return PhabricatorProjectColumn::DEFAULT_ORDER; + return PhabricatorProjectColumnNaturalOrder::ORDERKEY; } private function getDefaultFilter(PhabricatorProject $project) { @@ -789,41 +767,37 @@ final class PhabricatorProjectBoardViewController } private function isValidSort($sort) { - switch ($sort) { - case PhabricatorProjectColumn::ORDER_NATURAL: - case PhabricatorProjectColumn::ORDER_PRIORITY: - return true; - } - - return false; + $map = PhabricatorProjectColumnOrder::getAllOrders(); + return isset($map[$sort]); } private function buildSortMenu( PhabricatorUser $viewer, PhabricatorProject $project, - $sort_key) { - - $sort_icon = id(new PHUIIconView()) - ->setIcon('fa-sort-amount-asc bluegrey'); - - $named = array( - PhabricatorProjectColumn::ORDER_NATURAL => pht('Natural'), - PhabricatorProjectColumn::ORDER_PRIORITY => pht('Sort by Priority'), - ); + $sort_key, + array $ordering_map) { $base_uri = $this->getURIWithState(); $items = array(); - foreach ($named as $key => $name) { - $is_selected = ($key == $sort_key); + foreach ($ordering_map as $key => $ordering) { + // TODO: It would be desirable to build a real "PHUIIconView" here, but + // the pathway for threading that through all the view classes ends up + // being fairly complex, since some callers read the icon out of other + // views. For now, just stick with a string. + $ordering_icon = $ordering->getMenuIconIcon(); + $ordering_name = $ordering->getDisplayName(); + + $is_selected = ($key === $sort_key); if ($is_selected) { - $active_order = $name; + $active_name = $ordering_name; + $active_icon = $ordering_icon; } $item = id(new PhabricatorActionView()) - ->setIcon('fa-sort-amount-asc') + ->setIcon($ordering_icon) ->setSelected($is_selected) - ->setName($name); + ->setName($ordering_name); $uri = $base_uri->alter('order', $key); $item->setHref($uri); @@ -856,8 +830,8 @@ final class PhabricatorProjectBoardViewController } $sort_button = id(new PHUIListItemView()) - ->setName($active_order) - ->setIcon('fa-sort-amount-asc') + ->setName($active_name) + ->setIcon($active_icon) ->setHref('#') ->addSigil('boards-dropdown-menu') ->setMetadata( diff --git a/src/applications/project/controller/PhabricatorProjectController.php b/src/applications/project/controller/PhabricatorProjectController.php index 7c344b0b8e..850dfa2268 100644 --- a/src/applications/project/controller/PhabricatorProjectController.php +++ b/src/applications/project/controller/PhabricatorProjectController.php @@ -149,7 +149,11 @@ abstract class PhabricatorProjectController extends PhabricatorController { return $this; } - protected function newCardResponse($board_phid, $object_phid) { + protected function newCardResponse( + $board_phid, + $object_phid, + PhabricatorProjectColumnOrder $ordering = null) { + $viewer = $this->getViewer(); $request = $this->getRequest(); @@ -158,12 +162,17 @@ abstract class PhabricatorProjectController extends PhabricatorController { $visible_phids = array(); } - return id(new PhabricatorBoardResponseEngine()) + $engine = id(new PhabricatorBoardResponseEngine()) ->setViewer($viewer) ->setBoardPHID($board_phid) ->setObjectPHID($object_phid) - ->setVisiblePHIDs($visible_phids) - ->buildResponse(); + ->setVisiblePHIDs($visible_phids); + + if ($ordering) { + $engine->setOrdering($ordering); + } + + return $engine->buildResponse(); } public function renderHashtags(array $tags) { diff --git a/src/applications/project/controller/PhabricatorProjectMoveController.php b/src/applications/project/controller/PhabricatorProjectMoveController.php index 7a771ea7e8..6d7902a733 100644 --- a/src/applications/project/controller/PhabricatorProjectMoveController.php +++ b/src/applications/project/controller/PhabricatorProjectMoveController.php @@ -13,7 +13,15 @@ final class PhabricatorProjectMoveController $object_phid = $request->getStr('objectPHID'); $after_phid = $request->getStr('afterPHID'); $before_phid = $request->getStr('beforePHID'); - $order = $request->getStr('order', PhabricatorProjectColumn::DEFAULT_ORDER); + + $order = $request->getStr('order'); + if (!strlen($order)) { + $order = PhabricatorProjectColumnNaturalOrder::ORDERKEY; + } + + $ordering = PhabricatorProjectColumnOrder::getOrderByKey($order); + $ordering = id(clone $ordering) + ->setViewer($viewer); $edit_header = null; $raw_header = $request->getStr('header'); @@ -88,9 +96,8 @@ final class PhabricatorProjectMoveController ) + $order_params, )); - $header_xactions = $this->newHeaderTransactions( + $header_xactions = $ordering->getColumnTransactions( $object, - $order, $edit_header); foreach ($header_xactions as $header_xaction) { $xactions[] = $header_xaction; @@ -104,33 +111,7 @@ final class PhabricatorProjectMoveController $editor->applyTransactions($object, $xactions); - return $this->newCardResponse($board_phid, $object_phid); - } - - private function newHeaderTransactions( - ManiphestTask $task, - $order, - array $header) { - - $xactions = array(); - - switch ($order) { - case PhabricatorProjectColumn::ORDER_PRIORITY: - $new_priority = idx($header, $order); - - if ($task->getPriority() !== $new_priority) { - $keyword_map = ManiphestTaskPriority::getTaskPriorityKeywordsMap(); - $keyword = head(idx($keyword_map, $new_priority)); - - $xactions[] = id(new ManiphestTransaction()) - ->setTransactionType( - ManiphestTaskPriorityTransaction::TRANSACTIONTYPE) - ->setNewValue($keyword); - } - break; - } - - return $xactions; + return $this->newCardResponse($board_phid, $object_phid, $ordering); } } diff --git a/src/applications/project/engine/PhabricatorBoardResponseEngine.php b/src/applications/project/engine/PhabricatorBoardResponseEngine.php index 969dfa3bc8..36c5e81150 100644 --- a/src/applications/project/engine/PhabricatorBoardResponseEngine.php +++ b/src/applications/project/engine/PhabricatorBoardResponseEngine.php @@ -6,6 +6,7 @@ final class PhabricatorBoardResponseEngine extends Phobject { private $boardPHID; private $objectPHID; private $visiblePHIDs; + private $ordering; public function setViewer(PhabricatorUser $viewer) { $this->viewer = $viewer; @@ -43,10 +44,20 @@ final class PhabricatorBoardResponseEngine extends Phobject { return $this->visiblePHIDs; } + public function setOrdering(PhabricatorProjectColumnOrder $ordering) { + $this->ordering = $ordering; + return $this; + } + + public function getOrdering() { + return $this->ordering; + } + public function buildResponse() { $viewer = $this->getViewer(); $object_phid = $this->getObjectPHID(); $board_phid = $this->getBoardPHID(); + $ordering = $this->getOrdering(); // Load all the other tasks that are visible in the affected columns and // perform layout for them. @@ -74,10 +85,17 @@ final class PhabricatorBoardResponseEngine extends Phobject { ->setViewer($viewer) ->withPHIDs($visible_phids) ->execute(); + $all_visible = mpull($all_visible, null, 'getPHID'); - $order_maps = array(); - foreach ($all_visible as $visible) { - $order_maps[$visible->getPHID()] = $visible->getWorkboardOrderVectors(); + if ($ordering) { + $vectors = $ordering->getSortVectorsForObjects($all_visible); + $header_keys = $ordering->getHeaderKeysForObjects($all_visible); + $headers = $ordering->getHeadersForObjects($all_visible); + $headers = mpull($headers, 'toDictionary'); + } else { + $vectors = array(); + $header_keys = array(); + $headers = array(); } $object = id(new ManiphestTaskQuery()) @@ -91,14 +109,50 @@ final class PhabricatorBoardResponseEngine extends Phobject { $template = $this->buildTemplate($object); + $cards = array(); + foreach ($all_visible as $card_phid => $object) { + $card = array( + 'vectors' => array(), + 'headers' => array(), + 'properties' => array(), + 'nodeHTMLTemplate' => null, + ); + + if ($ordering) { + $order_key = $ordering->getColumnOrderKey(); + + $vector = idx($vectors, $card_phid); + if ($vector !== null) { + $card['vectors'][$order_key] = $vector; + } + + $header = idx($header_keys, $card_phid); + if ($header !== null) { + $card['headers'][$order_key] = $header; + } + + $card['properties'] = array( + 'points' => (double)$object->getPoints(), + 'status' => $object->getStatus(), + ); + } + + if ($card_phid === $object_phid) { + $card['nodeHTMLTemplate'] = hsprintf('%s', $template); + } + + $card['vectors'] = (object)$card['vectors']; + $card['headers'] = (object)$card['headers']; + $card['properties'] = (object)$card['properties']; + + $cards[$card_phid] = $card; + } + $payload = array( 'objectPHID' => $object_phid, - 'cardHTML' => $template, 'columnMaps' => $natural, - 'orderMaps' => $order_maps, - 'propertyMaps' => array( - $object_phid => $object->getWorkboardProperties(), - ), + 'cards' => $cards, + 'headers' => $headers, ); return id(new AphrontAjaxResponse()) diff --git a/src/applications/project/order/PhabricatorProjectColumnHeader.php b/src/applications/project/order/PhabricatorProjectColumnHeader.php new file mode 100644 index 0000000000..432b78279b --- /dev/null +++ b/src/applications/project/order/PhabricatorProjectColumnHeader.php @@ -0,0 +1,94 @@ +orderKey = $order_key; + return $this; + } + + public function getOrderKey() { + return $this->orderKey; + } + + public function setHeaderKey($header_key) { + $this->headerKey = $header_key; + return $this; + } + + public function getHeaderKey() { + return $this->headerKey; + } + + public function setSortVector(array $sort_vector) { + $this->sortVector = $sort_vector; + return $this; + } + + public function getSortVector() { + return $this->sortVector; + } + + public function setName($name) { + $this->name = $name; + return $this; + } + + public function getName() { + return $this->name; + } + + public function setIcon(PHUIIconView$icon) { + $this->icon = $icon; + return $this; + } + + public function getIcon() { + return $this->icon; + } + + public function setEditProperties(array $edit_properties) { + $this->editProperties = $edit_properties; + return $this; + } + + public function getEditProperties() { + return $this->editProperties; + } + + public function toDictionary() { + return array( + 'order' => $this->getOrderKey(), + 'key' => $this->getHeaderKey(), + 'template' => hsprintf('%s', $this->newView()), + 'vector' => $this->getSortVector(), + 'editProperties' => $this->getEditProperties(), + ); + } + + private function newView() { + $icon_view = $this->getIcon(); + $name = $this->getName(); + + $template = phutil_tag( + 'li', + array( + 'class' => 'workboard-group-header', + ), + array( + $icon_view, + $name, + )); + + return $template; + } + +} diff --git a/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php b/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php new file mode 100644 index 0000000000..26f4d28601 --- /dev/null +++ b/src/applications/project/order/PhabricatorProjectColumnNaturalOrder.php @@ -0,0 +1,12 @@ +viewer = $viewer; + return $this; + } + + final public function getViewer() { + return $this->viewer; + } + + final public function getColumnOrderKey() { + return $this->getPhobjectClassConstant('ORDERKEY'); + } + + final public static function getAllOrders() { + return id(new PhutilClassMapQuery()) + ->setAncestorClass(__CLASS__) + ->setUniqueMethod('getColumnOrderKey') + ->execute(); + } + + final public static function getOrderByKey($key) { + $map = self::getAllOrders(); + + if (!isset($map[$key])) { + throw new Exception( + pht( + 'No column ordering exists with key "%s".', + $key)); + } + + return $map[$key]; + } + + final public function getColumnTransactions($object, array $header) { + $result = $this->newColumnTransactions($object, $header); + + if (!is_array($result) && !is_null($result)) { + throw new Exception( + pht( + 'Expected "newColumnTransactions()" on "%s" to return "null" or a '. + 'list of transactions, but got "%s".', + get_class($this), + phutil_describe_type($result))); + } + + if ($result === null) { + $result = array(); + } + + assert_instances_of($result, 'PhabricatorApplicationTransaction'); + + return $result; + } + + final public function getMenuIconIcon() { + return $this->newMenuIconIcon(); + } + + protected function newMenuIconIcon() { + return 'fa-sort-amount-asc'; + } + + abstract public function getDisplayName(); + + protected function newColumnTransactions($object, array $header) { + return array(); + } + + final public function getHeadersForObjects(array $objects) { + $headers = $this->newHeadersForObjects($objects); + + if (!is_array($headers)) { + throw new Exception( + pht( + 'Expected "newHeadersForObjects()" on "%s" to return a list '. + 'of headers, but got "%s".', + get_class($this), + phutil_describe_type($headers))); + } + + assert_instances_of($headers, 'PhabricatorProjectColumnHeader'); + + // Add a "0" to the end of each header. This makes them sort above object + // cards in the same group. + foreach ($headers as $header) { + $vector = $header->getSortVector(); + $vector[] = 0; + $header->setSortVector($vector); + } + + return $headers; + } + + protected function newHeadersForObjects(array $objects) { + return array(); + } + + final public function getSortVectorsForObjects(array $objects) { + $vectors = $this->newSortVectorsForObjects($objects); + + if (!is_array($vectors)) { + throw new Exception( + pht( + 'Expected "newSortVectorsForObjects()" on "%s" to return a '. + 'map of vectors, but got "%s".', + get_class($this), + phutil_describe_type($vectors))); + } + + assert_same_keys($objects, $vectors); + + return $vectors; + } + + protected function newSortVectorsForObjects(array $objects) { + $vectors = array(); + + foreach ($objects as $key => $object) { + $vectors[$key] = $this->newSortVectorForObject($object); + } + + return $vectors; + } + + protected function newSortVectorForObject($object) { + return array(); + } + + final public function getHeaderKeysForObjects(array $objects) { + $header_keys = $this->newHeaderKeysForObjects($objects); + + if (!is_array($header_keys)) { + throw new Exception( + pht( + 'Expected "newHeaderKeysForObject()" on "%s" to return a '. + 'map of header keys, but got "%s".', + get_class($this), + phutil_describe_type($header_keys))); + } + + assert_same_keys($objects, $header_keys); + + return $header_keys; + } + + protected function newHeaderKeysForObjects(array $objects) { + $header_keys = array(); + + foreach ($objects as $key => $object) { + $header_keys[$key] = $this->newHeaderKeyForObject($object); + } + + return $header_keys; + } + + protected function newHeaderKeyForObject($object) { + return null; + } + + final protected function newTransaction($object) { + return $object->getApplicationTransactionTemplate(); + } + + final protected function newHeader() { + return id(new PhabricatorProjectColumnHeader()) + ->setOrderKey($this->getColumnOrderKey()); + } + +} diff --git a/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php b/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php new file mode 100644 index 0000000000..e08e7c07ef --- /dev/null +++ b/src/applications/project/order/PhabricatorProjectColumnPriorityOrder.php @@ -0,0 +1,91 @@ +newHeaderKeyForPriority($object->getPriority()); + } + + private function newHeaderKeyForPriority($priority) { + return sprintf('priority(%d)', $priority); + } + + protected function newSortVectorForObject($object) { + return $this->newSortVectorForPriority($object->getPriority()); + } + + private function newSortVectorForPriority($priority) { + return array( + (int)-$priority, + ); + } + + protected function newHeadersForObjects(array $objects) { + $priorities = ManiphestTaskPriority::getTaskPriorityMap(); + + // It's possible for tasks to have an invalid/unknown priority in the + // database. We still want to generate a header for these tasks so we + // don't break the workboard. + $priorities = $priorities + mpull($objects, null, 'getPriority'); + + $priorities = array_keys($priorities); + + $headers = array(); + foreach ($priorities as $priority) { + $header_key = $this->newHeaderKeyForPriority($priority); + $sort_vector = $this->newSortVectorForPriority($priority); + + $priority_name = ManiphestTaskPriority::getTaskPriorityName($priority); + $priority_color = ManiphestTaskPriority::getTaskPriorityColor($priority); + $priority_icon = ManiphestTaskPriority::getTaskPriorityIcon($priority); + + $icon_view = id(new PHUIIconView()) + ->setIcon($priority_icon, $priority_color); + + $header = $this->newHeader() + ->setHeaderKey($header_key) + ->setSortVector($sort_vector) + ->setName($priority_name) + ->setIcon($icon_view) + ->setEditProperties( + array( + 'value' => (int)$priority, + )); + + $headers[] = $header; + } + + return $headers; + } + + protected function newColumnTransactions($object, array $header) { + $new_priority = idx($header, 'value'); + + if ($object->getPriority() === $new_priority) { + return null; + } + + $keyword_map = ManiphestTaskPriority::getTaskPriorityKeywordsMap(); + $keyword = head(idx($keyword_map, $new_priority)); + + $xactions = array(); + $xactions[] = $this->newTransaction($object) + ->setTransactionType(ManiphestTaskPriorityTransaction::TRANSACTIONTYPE) + ->setNewValue($keyword); + + return $xactions; + } + + +} diff --git a/src/applications/project/storage/PhabricatorProjectColumn.php b/src/applications/project/storage/PhabricatorProjectColumn.php index 03b9cbff70..febb2eb647 100644 --- a/src/applications/project/storage/PhabricatorProjectColumn.php +++ b/src/applications/project/storage/PhabricatorProjectColumn.php @@ -12,13 +12,6 @@ final class PhabricatorProjectColumn const STATUS_ACTIVE = 0; const STATUS_HIDDEN = 1; - const DEFAULT_ORDER = 'natural'; - const ORDER_NATURAL = 'natural'; - const ORDER_PRIORITY = 'priority'; - - const NODETYPE_HEADER = 0; - const NODETYPE_CARD = 1; - protected $name; protected $status; protected $projectPHID; diff --git a/webroot/rsrc/js/application/projects/WorkboardBoard.js b/webroot/rsrc/js/application/projects/WorkboardBoard.js index a746461325..cda48bde11 100644 --- a/webroot/rsrc/js/application/projects/WorkboardBoard.js +++ b/webroot/rsrc/js/application/projects/WorkboardBoard.js @@ -289,7 +289,6 @@ JX.install('WorkboardBoard', { var columns = this.getColumns(); var phid = response.objectPHID; - var card = this.getCardTemplate(phid); for (var add_phid in response.columnMaps) { var target_column = this.getColumn(add_phid); @@ -302,18 +301,6 @@ JX.install('WorkboardBoard', { target_column.newCard(phid); } - card.setNodeHTMLTemplate(response.cardHTML); - - var order_maps = response.orderMaps; - for (var order_phid in order_maps) { - var card_template = this.getCardTemplate(order_phid); - for (var order_key in order_maps[order_phid]) { - card_template.setSortVector( - order_key, - order_maps[order_phid][order_key]); - } - } - var column_maps = response.columnMaps; var natural_column; for (var natural_phid in column_maps) { @@ -329,10 +316,37 @@ JX.install('WorkboardBoard', { natural_column.setNaturalOrder(column_maps[natural_phid]); } - var property_maps = response.propertyMaps; - for (var property_phid in property_maps) { - this.getCardTemplate(property_phid) - .setObjectProperties(property_maps[property_phid]); + for (var card_phid in response.cards) { + var card_data = response.cards[card_phid]; + var card_template = this.getCardTemplate(card_phid); + + if (card_data.nodeHTMLTemplate) { + card_template.setNodeHTMLTemplate(card_data.nodeHTMLTemplate); + } + + var order; + for (order in card_data.vectors) { + card_template.setSortVector(order, card_data.vectors[order]); + } + + for (order in card_data.headers) { + card_template.setHeaderKey(order, card_data.headers[order]); + } + + for (var key in card_data.properties) { + card_template.setObjectProperty(key, card_data.properties[key]); + } + } + + var headers = response.headers; + for (var jj = 0; jj < headers.length; jj++) { + var header = headers[jj]; + + this.getHeaderTemplate(header.key) + .setOrder(header.order) + .setNodeHTMLTemplate(header.template) + .setVector(header.vector) + .setEditProperties(header.editProperties); } for (var column_phid in columns) { diff --git a/webroot/rsrc/js/application/projects/WorkboardCard.js b/webroot/rsrc/js/application/projects/WorkboardCard.js index e0eab6b53b..4a3be2a51d 100644 --- a/webroot/rsrc/js/application/projects/WorkboardCard.js +++ b/webroot/rsrc/js/application/projects/WorkboardCard.js @@ -29,7 +29,8 @@ JX.install('WorkboardCard', { }, getProperties: function() { - return this.getColumn().getBoard().getCardTemplate(this.getPHID()) + return this.getColumn().getBoard() + .getCardTemplate(this.getPHID()) .getObjectProperties(); }, @@ -41,10 +42,6 @@ JX.install('WorkboardCard', { return this.getProperties().status; }, - getPriority: function(order) { - return this.getProperties().priority; - }, - getNode: function() { if (!this._root) { var phid = this.getPHID(); diff --git a/webroot/rsrc/js/application/projects/WorkboardCardTemplate.js b/webroot/rsrc/js/application/projects/WorkboardCardTemplate.js index 70080b5364..58f3f9e97f 100644 --- a/webroot/rsrc/js/application/projects/WorkboardCardTemplate.js +++ b/webroot/rsrc/js/application/projects/WorkboardCardTemplate.js @@ -9,6 +9,7 @@ JX.install('WorkboardCardTemplate', { construct: function(phid) { this._phid = phid; this._vectors = {}; + this._headerKeys = {}; this.setObjectProperties({}); }, @@ -19,7 +20,9 @@ JX.install('WorkboardCardTemplate', { members: { _phid: null, + _html: null, _vectors: null, + _headerKeys: null, getPHID: function() { return this._phid; @@ -39,8 +42,22 @@ JX.install('WorkboardCardTemplate', { return this._vectors[order]; }, + setHeaderKey: function(order, key) { + this._headerKeys[order] = key; + return this; + }, + + getHeaderKey: function(order) { + return this._headerKeys[order]; + }, + newNode: function() { return JX.$H(this._html).getFragment().firstChild; + }, + + setObjectProperty: function(key, value) { + this.getObjectProperties()[key] = value; + return this; } } diff --git a/webroot/rsrc/js/application/projects/WorkboardColumn.js b/webroot/rsrc/js/application/projects/WorkboardColumn.js index 1c5d6f1a59..a560234a2a 100644 --- a/webroot/rsrc/js/application/projects/WorkboardColumn.js +++ b/webroot/rsrc/js/application/projects/WorkboardColumn.js @@ -189,6 +189,9 @@ JX.install('WorkboardColumn', { var board = this.getBoard(); var order = board.getOrder(); + // TODO: This should be modularized into "ProjectColumnOrder" classes, + // but is currently hard-coded. + switch (order) { case 'natural': return false; @@ -197,15 +200,6 @@ JX.install('WorkboardColumn', { return true; }, - _getCardHeaderKey: function(card, order) { - switch (order) { - case 'priority': - return 'priority(' + card.getPriority() + ')'; - default: - return null; - } - }, - redraw: function() { var board = this.getBoard(); var order = board.getOrder(); @@ -235,7 +229,9 @@ JX.install('WorkboardColumn', { // cards in a column. if (has_headers) { - var header_key = this._getCardHeaderKey(card, order); + var header_key = board.getCardTemplate(card.getPHID()) + .getHeaderKey(order); + if (!seen_headers[header_key]) { while (header_keys.length) { var next = header_keys.pop(); diff --git a/webroot/rsrc/js/application/projects/WorkboardHeader.js b/webroot/rsrc/js/application/projects/WorkboardHeader.js index 0a8f4d9681..a0cbfc13c7 100644 --- a/webroot/rsrc/js/application/projects/WorkboardHeader.js +++ b/webroot/rsrc/js/application/projects/WorkboardHeader.js @@ -27,12 +27,16 @@ JX.install('WorkboardHeader', { getNode: function() { if (!this._root) { var header_key = this.getHeaderKey(); - var board = this.getColumn().getBoard(); - var template = board.getHeaderTemplate(header_key).getTemplate(); - this._root = JX.$H(template).getFragment().firstChild; - JX.Stratcom.getData(this._root).headerKey = header_key; + var root = this.getColumn().getBoard() + .getHeaderTemplate(header_key) + .newNode(); + + JX.Stratcom.getData(root).headerKey = header_key; + + this._root = root; } + return this._root; }, diff --git a/webroot/rsrc/js/application/projects/WorkboardHeaderTemplate.js b/webroot/rsrc/js/application/projects/WorkboardHeaderTemplate.js index add37d9c25..8376359270 100644 --- a/webroot/rsrc/js/application/projects/WorkboardHeaderTemplate.js +++ b/webroot/rsrc/js/application/projects/WorkboardHeaderTemplate.js @@ -19,9 +19,19 @@ JX.install('WorkboardHeaderTemplate', { members: { _headerKey: null, + _html: null, getHeaderKey: function() { return this._headerKey; + }, + + setNodeHTMLTemplate: function(html) { + this._html = html; + return this; + }, + + newNode: function() { + return JX.$H(this._html).getFragment().firstChild; } } diff --git a/webroot/rsrc/js/application/projects/behavior-project-boards.js b/webroot/rsrc/js/application/projects/behavior-project-boards.js index da82c02040..51c067925f 100644 --- a/webroot/rsrc/js/application/projects/behavior-project-boards.js +++ b/webroot/rsrc/js/application/projects/behavior-project-boards.js @@ -116,11 +116,17 @@ JX.behavior('project-boards', function(config, statics) { board.getHeaderTemplate(header.key) .setOrder(header.order) - .setTemplate(header.template) + .setNodeHTMLTemplate(header.template) .setVector(header.vector) .setEditProperties(header.editProperties); } + var header_keys = config.headerKeys; + for (var header_phid in header_keys) { + board.getCardTemplate(header_phid) + .setHeaderKey(config.order, header_keys[header_phid]); + } + board.start(); });