Skip to content

Commit 71c89bd

Browse files
author
epriestley
committed
Pass all adjacent card PHIDs from the client to the server when moving a card
Summary: Depends on D20321. Fixes T12175. Ref T13074. Now that before/after PHIDs are suggestions, we can give the server a more complete view of what the client is trying to do so we're more likely to get a good outcome if the client view is out of date. Instead of passing only the one directly adjacent card PHID, pass all the card PHIDs that the client thinks are in the same group. (For gigantic columns with tens of thousands of tasks this might need some tweaking -- like, slice both lists down to 10 items -- but we can cross that bridge when we come to it.) Test Plan: - Dragged some cards around to top/bottom/middle positions, saw good positioning in all cases. - In two windows, dragged stuff around on the same board. At least at first glance, conflicting simultaneous edits seemed to do reasonable things. Reviewers: amckinley Reviewed By: amckinley Maniphest Tasks: T13074, T12175 Differential Revision: https://secure.phabricator.com/D20322
1 parent 6138e50 commit 71c89bd

File tree

3 files changed

+40
-60
lines changed

3 files changed

+40
-60
lines changed

resources/celerity/map.php

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -409,7 +409,7 @@
409409
'rsrc/js/application/phortune/phortune-credit-card-form.js' => 'd12d214f',
410410
'rsrc/js/application/policy/behavior-policy-control.js' => '0eaa33a9',
411411
'rsrc/js/application/policy/behavior-policy-rule-editor.js' => '9347f172',
412-
'rsrc/js/application/projects/WorkboardBoard.js' => '106d870f',
412+
'rsrc/js/application/projects/WorkboardBoard.js' => 'c02a5497',
413413
'rsrc/js/application/projects/WorkboardCard.js' => '0392a5d8',
414414
'rsrc/js/application/projects/WorkboardCardTemplate.js' => '2a61f8d4',
415415
'rsrc/js/application/projects/WorkboardColumn.js' => 'c3d24e63',
@@ -737,7 +737,7 @@
737737
'javelin-view-renderer' => '9aae2b66',
738738
'javelin-view-visitor' => '308f9fe4',
739739
'javelin-websocket' => 'fdc13e4e',
740-
'javelin-workboard-board' => '106d870f',
740+
'javelin-workboard-board' => 'c02a5497',
741741
'javelin-workboard-card' => '0392a5d8',
742742
'javelin-workboard-card-template' => '2a61f8d4',
743743
'javelin-workboard-column' => 'c3d24e63',
@@ -1015,18 +1015,6 @@
10151015
'javelin-workflow',
10161016
'phuix-icon-view',
10171017
),
1018-
'106d870f' => array(
1019-
'javelin-install',
1020-
'javelin-dom',
1021-
'javelin-util',
1022-
'javelin-stratcom',
1023-
'javelin-workflow',
1024-
'phabricator-draggable-list',
1025-
'javelin-workboard-column',
1026-
'javelin-workboard-header-template',
1027-
'javelin-workboard-card-template',
1028-
'javelin-workboard-order-template',
1029-
),
10301018
'111bfd2d' => array(
10311019
'javelin-install',
10321020
),
@@ -1940,6 +1928,18 @@
19401928
'bde53589' => array(
19411929
'phui-inline-comment-view-css',
19421930
),
1931+
'c02a5497' => array(
1932+
'javelin-install',
1933+
'javelin-dom',
1934+
'javelin-util',
1935+
'javelin-stratcom',
1936+
'javelin-workflow',
1937+
'phabricator-draggable-list',
1938+
'javelin-workboard-column',
1939+
'javelin-workboard-header-template',
1940+
'javelin-workboard-card-template',
1941+
'javelin-workboard-order-template',
1942+
),
19431943
'c03f2fb4' => array(
19441944
'javelin-install',
19451945
),

src/applications/project/controller/PhabricatorProjectMoveController.php

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,8 @@ public function handleRequest(AphrontRequest $request) {
1212
$column_phid = $request->getStr('columnPHID');
1313
$object_phid = $request->getStr('objectPHID');
1414

15-
$after_phid = $request->getStr('afterPHID');
16-
$before_phid = $request->getStr('beforePHID');
17-
18-
$after_phids = array();
19-
if ($after_phid) {
20-
$after_phids[] = $after_phid;
21-
}
22-
23-
$before_phids = array();
24-
if ($before_phid) {
25-
$before_phids[] = $before_phid;
26-
}
15+
$after_phids = $request->getStrList('afterPHIDs');
16+
$before_phids = $request->getStrList('beforePHIDs');
2717

2818
$order = $request->getStr('order');
2919
if (!strlen($order)) {
@@ -98,13 +88,10 @@ public function handleRequest(AphrontRequest $request) {
9888
->setObjectPHIDs(array($object_phid))
9989
->executeLayout();
10090

101-
$order_params = array();
102-
if ($after_phid) {
103-
$order_params['afterPHIDs'] = $after_phids;
104-
}
105-
if ($before_phid) {
106-
$order_params['beforePHIDs'] = $before_phids;
107-
}
91+
$order_params = array(
92+
'afterPHIDs' => $after_phids,
93+
'beforePHIDs' => $before_phids,
94+
);
10895

10996
$xactions = array();
11097
$xactions[] = id(new ManiphestTransaction())

webroot/rsrc/js/application/projects/WorkboardBoard.js

Lines changed: 20 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -409,8 +409,8 @@ JX.install('WorkboardBoard', {
409409

410410
_getDropContext: function(after_node, item) {
411411
var header_key;
412-
var before_phid;
413-
var after_phid;
412+
var after_phids = [];
413+
var before_phids = [];
414414

415415
// We're going to send an "afterPHID" and a "beforePHID" if the card
416416
// was dropped immediately adjacent to another card. If a card was
@@ -424,39 +424,33 @@ JX.install('WorkboardBoard', {
424424
var after_card = after_node;
425425
while (after_card) {
426426
after_data = JX.Stratcom.getData(after_card);
427-
if (after_data.objectPHID) {
428-
break;
429-
}
427+
430428
if (after_data.headerKey) {
431429
break;
432430
}
433-
after_card = after_card.previousSibling;
434-
}
435431

436-
if (after_data) {
437432
if (after_data.objectPHID) {
438-
after_phid = after_data.objectPHID;
433+
after_phids.push(after_data.objectPHID);
439434
}
435+
436+
after_card = after_card.previousSibling;
440437
}
441438

442439
if (item) {
443440
var before_data;
444441
var before_card = item.nextSibling;
445442
while (before_card) {
446443
before_data = JX.Stratcom.getData(before_card);
447-
if (before_data.objectPHID) {
448-
break;
449-
}
444+
450445
if (before_data.headerKey) {
451446
break;
452447
}
453-
before_card = before_card.nextSibling;
454-
}
455448

456-
if (before_data) {
457449
if (before_data.objectPHID) {
458-
before_phid = before_data.objectPHID;
450+
before_phids.push(before_data.objectPHID);
459451
}
452+
453+
before_card = before_card.nextSibling;
460454
}
461455
}
462456

@@ -476,8 +470,8 @@ JX.install('WorkboardBoard', {
476470

477471
return {
478472
headerKey: header_key,
479-
afterPHID: after_phid,
480-
beforePHID: before_phid
473+
afterPHIDs: after_phids,
474+
beforePHIDs: before_phids
481475
};
482476
},
483477

@@ -496,14 +490,8 @@ JX.install('WorkboardBoard', {
496490
};
497491

498492
var context = this._getDropContext(after_node, item);
499-
500-
if (context.afterPHID) {
501-
data.afterPHID = context.afterPHID;
502-
}
503-
504-
if (context.beforePHID) {
505-
data.beforePHID = context.beforePHID;
506-
}
493+
data.afterPHIDs = context.afterPHIDs.join(',');
494+
data.beforePHIDs = context.beforePHIDs.join(',');
507495

508496
if (context.headerKey) {
509497
var properties = this.getHeaderTemplate(context.headerKey)
@@ -530,13 +518,18 @@ JX.install('WorkboardBoard', {
530518
src_phid,
531519
dst_phid);
532520

521+
var after_phid = null;
522+
if (data.afterPHIDs.length) {
523+
after_phid = data.afterPHIDs[0];
524+
}
525+
533526
var onupdate = JX.bind(
534527
this,
535528
this._oncardupdate,
536529
list,
537530
src_phid,
538531
dst_phid,
539-
data.afterPHID);
532+
after_phid);
540533

541534
new JX.Workflow(this.getController().getMoveURI(), data)
542535
.setHandler(onupdate)

0 commit comments

Comments
 (0)