Make "Move Panel" on dashboards use the new storage and transactions

Summary: Depends on D20408. Ref T13272. The actual JS is still a little bit iffy, but this makes the server side "move" operation work correctly by updating it to use the same code as everything else.

Test Plan: Moved panels around on single-column and multi-column dashboards, saw them move to reasonable places and stay there when I reloaded the page.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13272

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

resources/celerity/map.php

@@ -9,7 +9,7 @@ return array(
   'names' => array(
     'conpherence.pkg.css' => '3c8a0668',
     'conpherence.pkg.js' => '020aebcf',
-    'core.pkg.css' => '294e365c',
+    'core.pkg.css' => '4011a01e',
     'core.pkg.js' => '69247edd',
     'differential.pkg.css' => '8d8360fb',
     'differential.pkg.js' => '67e02996',
@@ -132,7 +132,7 @@ return array(
     'rsrc/css/phui/object-item/phui-oi-color.css' => 'b517bfa0',
     'rsrc/css/phui/object-item/phui-oi-drag-ui.css' => 'da15d3dc',
     'rsrc/css/phui/object-item/phui-oi-flush-ui.css' => '490e2e2e',
-    'rsrc/css/phui/object-item/phui-oi-list-view.css' => 'f14f2422',
+    'rsrc/css/phui/object-item/phui-oi-list-view.css' => 'd7723ecc',
     'rsrc/css/phui/object-item/phui-oi-simple-ui.css' => '6a30fa46',
     'rsrc/css/phui/phui-action-list.css' => '48a45c51',
     'rsrc/css/phui/phui-action-panel.css' => '6c386cbf',
@@ -372,7 +372,7 @@ return array(
     'rsrc/js/application/countdown/timer.js' => '6a162524',
     'rsrc/js/application/daemon/behavior-bulk-job-reload.js' => '3829a3cf',
     'rsrc/js/application/dashboard/behavior-dashboard-async-panel.js' => 'a871fe00',
-    'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '076bd092',
+    'rsrc/js/application/dashboard/behavior-dashboard-move-panels.js' => '7d33143d',
     'rsrc/js/application/dashboard/behavior-dashboard-query-panel-select.js' => '1e413dc9',
     'rsrc/js/application/dashboard/behavior-dashboard-tab-panel.js' => '0116d3e8',
     'rsrc/js/application/diff/DiffChangeset.js' => 'd0a85a85',
@@ -595,7 +595,7 @@ return array(
     'javelin-behavior-countdown-timer' => '6a162524',
     'javelin-behavior-dark-console' => 'f39d968b',
     'javelin-behavior-dashboard-async-panel' => 'a871fe00',
-    'javelin-behavior-dashboard-move-panels' => '076bd092',
+    'javelin-behavior-dashboard-move-panels' => '7d33143d',
     'javelin-behavior-dashboard-query-panel-select' => '1e413dc9',
     'javelin-behavior-dashboard-tab-panel' => '0116d3e8',
     'javelin-behavior-day-view' => '727a5a61',
@@ -853,7 +853,7 @@ return array(
     'phui-oi-color-css' => 'b517bfa0',
     'phui-oi-drag-ui-css' => 'da15d3dc',
     'phui-oi-flush-ui-css' => '490e2e2e',
-    'phui-oi-list-view-css' => 'f14f2422',
+    'phui-oi-list-view-css' => 'd7723ecc',
     'phui-oi-simple-ui-css' => '6a30fa46',
     'phui-pager-css' => 'd022c7ad',
     'phui-pinboard-view-css' => '1f08f5d8',
@@ -964,14 +964,6 @@ return array(
       'javelin-request',
       'javelin-uri',
     ),
-    '076bd092' => array(
-      'javelin-behavior',
-      'javelin-dom',
-      'javelin-util',
-      'javelin-stratcom',
-      'javelin-workflow',
-      'phabricator-draggable-list',
-    ),
     '0889b835' => array(
       'javelin-install',
       'javelin-event',
@@ -1589,6 +1581,14 @@ return array(
       'javelin-install',
       'javelin-dom',
     ),
+    '7d33143d' => array(
+      'javelin-behavior',
+      'javelin-dom',
+      'javelin-util',
+      'javelin-stratcom',
+      'javelin-workflow',
+      'phabricator-draggable-list',
+    ),
     '80bff3af' => array(
       'javelin-install',
       'javelin-typeahead-source',

src/__phutil_library_map__.php

@@ -2930,7 +2930,6 @@ phutil_register_library_map(array(
     'PhabricatorDashboardLayoutMode' => 'applications/dashboard/layoutconfig/PhabricatorDashboardLayoutMode.php',
     'PhabricatorDashboardLayoutTransaction' => 'applications/dashboard/xaction/dashboard/PhabricatorDashboardLayoutTransaction.php',
     'PhabricatorDashboardListController' => 'applications/dashboard/controller/PhabricatorDashboardListController.php',
-    'PhabricatorDashboardMovePanelController' => 'applications/dashboard/controller/PhabricatorDashboardMovePanelController.php',
     'PhabricatorDashboardNameTransaction' => 'applications/dashboard/xaction/dashboard/PhabricatorDashboardNameTransaction.php',
     'PhabricatorDashboardNgrams' => 'applications/dashboard/storage/PhabricatorDashboardNgrams.php',
     'PhabricatorDashboardObjectInstallWorkflow' => 'applications/dashboard/install/PhabricatorDashboardObjectInstallWorkflow.php',
@@ -8929,7 +8928,6 @@ phutil_register_library_map(array(
     'PhabricatorDashboardLayoutMode' => 'Phobject',
     'PhabricatorDashboardLayoutTransaction' => 'PhabricatorDashboardTransactionType',
     'PhabricatorDashboardListController' => 'PhabricatorDashboardController',
-    'PhabricatorDashboardMovePanelController' => 'PhabricatorDashboardController',
     'PhabricatorDashboardNameTransaction' => 'PhabricatorDashboardTransactionType',
     'PhabricatorDashboardNgrams' => 'PhabricatorSearchNgrams',
     'PhabricatorDashboardObjectInstallWorkflow' => 'PhabricatorDashboardInstallWorkflow',

src/applications/dashboard/application/PhabricatorDashboardApplication.php

@@ -48,8 +48,7 @@ final class PhabricatorDashboardApplication extends PhabricatorApplication {
           '(?:(?P<modeKey>[^/]+)/)?)?' =>
           'PhabricatorDashboardInstallController',
         'console/' => 'PhabricatorDashboardConsoleController',
-        'movepanel/(?P<id>\d+)/' => 'PhabricatorDashboardMovePanelController',
-        'adjust/(?P<op>remove|add)/'
+        'adjust/(?P<op>remove|add|move)/'
           => 'PhabricatorDashboardAdjustController',
         'panel/' => array(
           'install/(?P<engineKey>[^/]+)/(?:(?P<queryKey>[^/]+)/)?' =>

src/applications/dashboard/controller/PhabricatorDashboardMovePanelController.php

@@ -1,70 +0,0 @@
-<?php
-
-final class PhabricatorDashboardMovePanelController
-  extends PhabricatorDashboardController {
-
-  public function handleRequest(AphrontRequest $request) {
-    $viewer = $request->getViewer();
-    $id = $request->getURIData('id');
-
-    $column_id = $request->getStr('columnID');
-    $panel_phid = $request->getStr('objectPHID');
-    $after_phid = $request->getStr('afterPHID');
-    $before_phid = $request->getStr('beforePHID');
-
-    $dashboard = id(new PhabricatorDashboardQuery())
-      ->setViewer($viewer)
-      ->withIDs(array($id))
-      ->requireCapabilities(
-        array(
-          PhabricatorPolicyCapability::CAN_VIEW,
-          PhabricatorPolicyCapability::CAN_EDIT,
-        ))
-      ->executeOne();
-    if (!$dashboard) {
-      return new Aphront404Response();
-    }
-    $panels = mpull($dashboard->getPanels(), null, 'getPHID');
-    $panel = idx($panels, $panel_phid);
-    if (!$panel) {
-      return new Aphront404Response();
-    }
-
-    $layout_config = $dashboard->getLayoutConfigObject();
-    $layout_config->removePanel($panel_phid);
-    $panel_location_grid = $layout_config->getPanelLocations();
-
-    $column_phids = idx($panel_location_grid, $column_id, array());
-    $column_phids = array_values($column_phids);
-    if ($column_phids) {
-      $insert_at = 0;
-      foreach ($column_phids as $index => $phid) {
-        if ($phid === $before_phid) {
-          $insert_at = $index;
-          break;
-        }
-        if ($phid === $after_phid) {
-          $insert_at = $index + 1;
-          break;
-        }
-      }
-
-      $new_column_phids = $column_phids;
-      array_splice(
-        $new_column_phids,
-        $insert_at,
-        0,
-        array($panel_phid));
-    } else {
-      $new_column_phids = array(0 => $panel_phid);
-    }
-
-    $panel_location_grid[$column_id] = $new_column_phids;
-    $layout_config->setPanelLocations($panel_location_grid);
-    $dashboard->setLayoutConfigFromObject($layout_config);
-    $dashboard->save();
-
-    return id(new AphrontAjaxResponse())->setContent('');
- }
-
-}

src/applications/dashboard/controller/dashboard/PhabricatorDashboardAdjustController.php

@@ -39,8 +39,6 @@ final class PhabricatorDashboardAdjustController
       }
 
       $this->panelKey = $panel_key;
-    } else {
-      $panel_ref = null;
     }
 
     $column_key = $request->getStr('columnKey');
@@ -52,6 +50,15 @@ final class PhabricatorDashboardAdjustController
       $this->columnKey = $column_key;
     }
 
+    $after_ref = null;
+    $after_key = $request->getStr('afterKey');
+    if (strlen($after_key)) {
+      $after_ref = $ref_list->getPanelRef($after_key);
+      if (!$after_ref) {
+        return new Aphront404Response();
+      }
+    }
+
     switch ($request->getURIData('op')) {
       case 'add':
         return $this->handleAddRequest($dashboard, $done_uri);
@@ -60,6 +67,8 @@ final class PhabricatorDashboardAdjustController
           return new Aphront404Response();
         }
         return $this->handleRemoveRequest($dashboard, $panel_ref, $done_uri);
+      case 'move':
+        return $this->handleMoveRequest($dashboard, $panel_ref, $after_ref);
     }
   }
 
@@ -192,6 +201,38 @@ final class PhabricatorDashboardAdjustController
       ->addSubmitButton(pht('Remove Panel'));
   }
 
+  private function handleMoveRequest(
+    PhabricatorDashboard $dashboard,
+    PhabricatorDashboardPanelRef $panel_ref,
+    PhabricatorDashboardPanelRef $after_ref = null) {
+
+    $request = $this->getRequest();
+    $request->validateCSRF();
+    $viewer = $this->getViewer();
+
+    $xactions = array();
+
+    $ref_list = clone $dashboard->getPanelRefList();
+    $ref_list->movePanelRef($panel_ref, $this->columnKey, $after_ref);
+    $new_panels = $ref_list->toDictionary();
+
+    $xactions[] = $dashboard->getApplicationTransactionTemplate()
+      ->setTransactionType(
+        PhabricatorDashboardPanelsTransaction::TRANSACTIONTYPE)
+      ->setNewValue($new_panels);
+
+    $editor = $dashboard->getApplicationTransactionEditor()
+      ->setActor($viewer)
+      ->setContentSourceFromRequest($request)
+      ->setContinueOnNoEffect(true)
+      ->setContinueOnMissingFields(true);
+
+    $editor->applyTransactions($dashboard, $xactions);
+
+    return id(new AphrontAjaxResponse())->setContent(array());
+  }
+
+
   private function newEditDialog() {
     return $this->newDialog()
       ->addHiddenInput('contextPHID', $this->contextPHID)

src/applications/dashboard/engine/PhabricatorDashboardPanelRenderingEngine.php

@@ -280,11 +280,11 @@ final class PhabricatorDashboardPanelRenderingEngine extends Phobject {
     if ($panel) {
       $box->setMetadata(
         array(
-          'objectPHID' => $panel->getPHID(),
+          'panelKey' => $this->getPanelKey(),
         ));
     }
 
-    return phutil_tag_div('dashboard-pane', $box);
+    return $box;
   }
 
 

src/applications/dashboard/engine/PhabricatorDashboardRenderingEngine.php

@@ -111,11 +111,16 @@ final class PhabricatorDashboardRenderingEngine extends Phobject {
     }
 
     if ($is_editable) {
+      $params = array(
+        'contextPHID' => $dashboard->getPHID(),
+      );
+      $move_uri = new PhutilURI('/dashboard/adjust/move/', $params);
+
       Javelin::initBehavior(
         'dashboard-move-panels',
         array(
-          'dashboardID' => $dashboard_id,
-          'moveURI' => '/dashboard/movepanel/'.$dashboard->getID().'/',
+          'dashboardNodeID' => $dashboard_id,
+          'moveURI' => (string)$move_uri,
         ));
     }
 

src/applications/dashboard/layoutconfig/PhabricatorDashboardColumn.php

@@ -25,6 +25,12 @@ final class PhabricatorDashboardColumn
     return $this->classes;
   }
 
+  public function setPanelRefs(array $refs) {
+    assert_instances_of($refs, 'PhabricatorDashboardPanelRef');
+    $this->refs = $refs;
+    return $this;
+  }
+
   public function addPanelRef(PhabricatorDashboardPanelRef $ref) {
     $this->refs[] = $ref;
     return $this;

src/applications/dashboard/layoutconfig/PhabricatorDashboardPanelRefList.php

@@ -111,6 +111,43 @@ final class PhabricatorDashboardPanelRefList
     return null;
   }
 
+  public function movePanelRef(
+    PhabricatorDashboardPanelRef $target,
+    $column_key,
+    PhabricatorDashboardPanelRef $after = null) {
+
+    $target->setColumnKey($column_key);
+
+    $results = array();
+
+    if (!$after) {
+      $results[] = $target;
+    }
+
+    foreach ($this->refs as $ref) {
+      if ($ref->getPanelKey() === $target->getPanelKey()) {
+        continue;
+      }
+
+      $results[] = $ref;
+
+      if ($after) {
+        if ($ref->getPanelKey() === $after->getPanelKey()) {
+          $results[] = $target;
+        }
+      }
+    }
+
+    $this->refs = $results;
+
+    $column_map = mgroup($results, 'getColumnKey');
+    foreach ($this->columns as $column_key => $column) {
+      $column->setPanelRefs(idx($column_map, $column_key, array()));
+    }
+
+    return $ref;
+  }
+
   private function newPanelKey() {
     return Filesystem::readRandomCharacters(8);
   }

webroot/rsrc/css/phui/object-item/phui-oi-list-view.css

@@ -611,11 +611,6 @@ ul.phui-oi-list-view .phui-oi-selected
    border-top: none;
  }
 
-.dashboard-pane .phui-oi-empty .phui-info-view {
-  border: none;
-  margin: 0;
-}
-
 .device-desktop .aphront-multi-column-fluid .aphront-multi-column-2-up
  .aphront-multi-column-column-outer.third .phui-oi-col2 {
    display: none;

webroot/rsrc/js/application/dashboard/behavior-dashboard-move-panels.js

@@ -33,40 +33,16 @@ JX.behavior('dashboard-move-panels', function(config) {
     list.lock();
     JX.DOM.alterClass(item, 'drag-sending', true);
 
-    var item_phid = JX.Stratcom.getData(item).objectPHID;
     var data = {
-      objectPHID: item_phid,
-      columnID: JX.Stratcom.getData(list.getRootNode()).columnID
+      panelKey: JX.Stratcom.getData(item).panelKey,
+      columnKey: JX.Stratcom.getData(list.getRootNode()).columnKey
     };
 
-    var after_phid = null;
-    var items = finditems(list.getRootNode());
     if (after) {
-      after_phid = JX.Stratcom.getData(after).objectPHID;
-      data.afterPHID = after_phid;
-    }
-    var ii;
-    var ii_item;
-    var ii_item_phid;
-    var ii_prev_item_phid = null;
-    var before_phid = null;
-    for (ii = 0; ii < items.length; ii++) {
-      ii_item = items[ii];
-      ii_item_phid = JX.Stratcom.getData(ii_item).objectPHID;
-      if (ii_item_phid == item_phid) {
-        // skip the item we just dropped
-        continue;
+      var after_data = JX.Stratcom.getData(after);
+      if (after_data.panelKey) {
+        data.afterKey = after_data.panelKey;
       }
-      // note this handles when there is no after phid - we are at the top of
-      // the list - quite nicely
-      if (ii_prev_item_phid == after_phid) {
-        before_phid = ii_item_phid;
-        break;
-      }
-      ii_prev_item_phid = ii_item_phid;
-    }
-    if (before_phid) {
-      data.beforePHID = before_phid;
     }
 
     var workflow = new JX.Workflow(config.moveURI, data)
@@ -77,23 +53,24 @@ JX.behavior('dashboard-move-panels', function(config) {
     workflow.start();
   }
 
-  var lists = [];
-  var ii;
-  var cols = JX.DOM.scry(JX.$(config.dashboardID), 'div', 'dashboard-column');
-  var col = null;
+  var dashboard_node = JX.$(config.dashboardNodeID);
 
+  var lists = [];
+  var cols = JX.DOM.scry(dashboard_node, 'div', 'dashboard-column');
+
+  var ii;
   for (ii = 0; ii < cols.length; ii++) {
-    col = cols[ii];
+    var col = cols[ii];
     var list = new JX.DraggableList(itemSigil, col)
       .setFindItemsHandler(JX.bind(null, finditems, col))
       .setCanDragX(true);
 
     list.listen('didSend', JX.bind(list, onupdate, col));
     list.listen('didReceive', JX.bind(list, onupdate, col));
-
     list.listen('didDrop', JX.bind(null, ondrop, list));
 
     lists.push(list);
+
     markcolempty(col, finditems(col).length === 0);
   }