Skip to content

Commit 7a31578

Browse files
author
epriestley
committedJun 30, 2016
When using the "Close as Duplicate" relationship action, limit the UI to 1 task
Summary: Ref T4788. When closing a task as a duplicate of another task, you can only select one task, since it doesn't really make sense to merge one task into several other tasks (this operation is //possible//, but probably not what anyone ever wants to do, I think?). Make the UI understand this: after you select a task, disable all of the "select" buttons in the UI to make this clear. Test Plan: - Used "Close as Duplicate", only allowed to select 1 task. - Used other editors like "Merge Duplicates In", allowed to select lots of tasks. Reviewers: chad Reviewed By: chad Maniphest Tasks: T4788 Differential Revision: https://secure.phabricator.com/D16203
1 parent 23ec515 commit 7a31578

File tree

6 files changed

+129
-52
lines changed

6 files changed

+129
-52
lines changed
 

‎resources/celerity/map.php

+9-9
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
'core.pkg.js' => 'f2139810',
1212
'darkconsole.pkg.js' => 'e7393ebb',
1313
'differential.pkg.css' => '3e81ae60',
14-
'differential.pkg.js' => '01a010d6',
14+
'differential.pkg.js' => '634399e9',
1515
'diffusion.pkg.css' => '91c5d3a6',
1616
'diffusion.pkg.js' => '3a9a8bfa',
1717
'maniphest.pkg.css' => '4845691a',
@@ -495,7 +495,7 @@
495495
'rsrc/js/core/behavior-lightbox-attachments.js' => 'f8ba29d7',
496496
'rsrc/js/core/behavior-line-linker.js' => '1499a8cb',
497497
'rsrc/js/core/behavior-more.js' => 'a80d0378',
498-
'rsrc/js/core/behavior-object-selector.js' => '9030ebef',
498+
'rsrc/js/core/behavior-object-selector.js' => 'e0ec7f2f',
499499
'rsrc/js/core/behavior-oncopy.js' => '2926fff2',
500500
'rsrc/js/core/behavior-phabricator-nav.js' => '56a1ca03',
501501
'rsrc/js/core/behavior-phabricator-remarkup-assist.js' => '116cf19b',
@@ -658,7 +658,7 @@
658658
'javelin-behavior-phabricator-line-linker' => '1499a8cb',
659659
'javelin-behavior-phabricator-nav' => '56a1ca03',
660660
'javelin-behavior-phabricator-notification-example' => '8ce821c5',
661-
'javelin-behavior-phabricator-object-selector' => '9030ebef',
661+
'javelin-behavior-phabricator-object-selector' => 'e0ec7f2f',
662662
'javelin-behavior-phabricator-oncopy' => '2926fff2',
663663
'javelin-behavior-phabricator-remarkup-assist' => '116cf19b',
664664
'javelin-behavior-phabricator-reveal-content' => '60821bc7',
@@ -1608,12 +1608,6 @@
16081608
'javelin-dom',
16091609
'javelin-request',
16101610
),
1611-
'9030ebef' => array(
1612-
'javelin-behavior',
1613-
'javelin-dom',
1614-
'javelin-request',
1615-
'javelin-util',
1616-
),
16171611
'9196fb06' => array(
16181612
'javelin-install',
16191613
'javelin-dom',
@@ -2036,6 +2030,12 @@
20362030
'df5e11d2' => array(
20372031
'javelin-install',
20382032
),
2033+
'e0ec7f2f' => array(
2034+
'javelin-behavior',
2035+
'javelin-dom',
2036+
'javelin-request',
2037+
'javelin-util',
2038+
),
20392039
'e10f8e18' => array(
20402040
'javelin-behavior',
20412041
'javelin-dom',

‎src/applications/maniphest/relationship/ManiphestTaskCloseAsDuplicateRelationship.php

+4-9
Original file line numberDiff line numberDiff line change
@@ -53,16 +53,11 @@ public function canUndoRelationship() {
5353
return false;
5454
}
5555

56-
public function willUpdateRelationships($object, array $add, array $rem) {
57-
58-
// TODO: Communicate this in the UI before users hit this error.
59-
if (count($add) > 1) {
60-
throw new Exception(
61-
pht(
62-
'A task can only be closed as a duplicate of exactly one other '.
63-
'task.'));
64-
}
56+
public function getMaximumSelectionSize() {
57+
return 1;
58+
}
6559

60+
public function willUpdateRelationships($object, array $add, array $rem) {
6661
$task = head($add);
6762
return $this->newMergeIntoTransactions($task);
6863
}

‎src/applications/search/controller/PhabricatorSearchRelationshipController.php

+13
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,23 @@ public function handleRequest(AphrontRequest $request) {
3939
$done_uri = $src_handle->getURI();
4040
$initial_phids = $dst_phids;
4141

42+
$maximum = $relationship->getMaximumSelectionSize();
43+
4244
if ($request->isFormPost()) {
4345
$phids = explode(';', $request->getStr('phids'));
4446
$phids = array_filter($phids);
4547
$phids = array_values($phids);
4648

49+
// The UI normally enforces this with Javascript, so this is just a
50+
// sanity check and does not need to be particularly user-friendly.
51+
if ($maximum && (count($phids) > $maximum)) {
52+
throw new Exception(
53+
pht(
54+
'Too many relationships (%s, of type "%s").',
55+
phutil_count($phids),
56+
$relationship->getRelationshipConstant()));
57+
}
58+
4759
$initial_phids = $request->getStrList('initialPHIDs');
4860

4961
// Apply the changes as adds and removes relative to the original state
@@ -175,6 +187,7 @@ public function handleRequest(AphrontRequest $request) {
175187
->setHeader($dialog_header)
176188
->setButtonText($dialog_button)
177189
->setInstructions($dialog_instructions)
190+
->setMaximumSelectionSize($maximum)
178191
->buildDialog();
179192
}
180193

‎src/applications/search/relationship/PhabricatorObjectRelationship.php

+4
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ private function getActionURI($object) {
104104
return "/search/rel/{$type}/{$phid}/";
105105
}
106106

107+
public function getMaximumSelectionSize() {
108+
return null;
109+
}
110+
107111
public function canUndoRelationship() {
108112
return true;
109113
}

‎src/view/control/PhabricatorObjectSelectorDialog.php

+13
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ final class PhabricatorObjectSelectorDialog extends Phobject {
1111
private $selectedFilter;
1212
private $excluded;
1313
private $initialPHIDs;
14+
private $maximumSelectionSize;
1415

1516
private $title;
1617
private $header;
@@ -87,6 +88,15 @@ public function getInitialPHIDs() {
8788
return $this->initialPHIDs;
8889
}
8990

91+
public function setMaximumSelectionSize($maximum_selection_size) {
92+
$this->maximumSelectionSize = $maximum_selection_size;
93+
return $this;
94+
}
95+
96+
public function getMaximumSelectionSize() {
97+
return $this->maximumSelectionSize;
98+
}
99+
90100
public function buildDialog() {
91101
$user = $this->user;
92102

@@ -190,6 +200,8 @@ public function buildDialog() {
190200
$dialog->addHiddenInput('initialPHIDs', $initial_phids);
191201
}
192202

203+
$maximum = $this->getMaximumSelectionSize();
204+
193205
Javelin::initBehavior(
194206
'phabricator-object-selector',
195207
array(
@@ -202,6 +214,7 @@ public function buildDialog() {
202214
'exclude' => $this->excluded,
203215
'uri' => $this->searchURI,
204216
'handles' => $handle_views,
217+
'maximum' => $maximum,
205218
));
206219

207220
$dialog->setResizeX(true);

‎webroot/rsrc/js/core/behavior-object-selector.js

+86-34
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,13 @@ JX.behavior('phabricator-object-selector', function(config) {
1010
var n = 0;
1111

1212
var phids = {};
13+
var display = [];
14+
1315
var handles = config.handles;
1416
for (var k in handles) {
1517
phids[k] = true;
1618
}
17-
var button_list = {};
19+
1820
var query_timer = null;
1921
var query_delay = 50;
2022

@@ -41,37 +43,82 @@ JX.behavior('phabricator-object-selector', function(config) {
4143
return;
4244
}
4345

44-
var display = [];
45-
button_list = {};
46+
display = [];
4647
for (var k in r) {
4748
handles[r[k].phid] = r[k];
48-
display.push(renderHandle(r[k], true));
49+
display.push({phid: r[k].phid});
4950
}
5051

51-
if (!display.length) {
52-
display = renderNote('No results.');
53-
}
54-
55-
JX.DOM.setContent(JX.$(config.results), display);
52+
redrawList(true);
5653
}
5754

5855
function redrawAttached() {
59-
var display = [];
56+
var attached = [];
6057

6158
for (var k in phids) {
62-
display.push(renderHandle(handles[k], false));
59+
attached.push(renderHandle(handles[k], false).item);
6360
}
6461

65-
if (!display.length) {
66-
display = renderNote('Nothing attached.');
62+
if (!attached.length) {
63+
attached = renderNote('Nothing attached.');
6764
}
6865

69-
JX.DOM.setContent(JX.$(config.current), display);
66+
JX.DOM.setContent(JX.$(config.current), attached);
7067
phid_input.value = JX.keys(phids).join(';');
7168
}
7269

73-
function renderHandle(h, attach) {
70+
function redrawList(rebuild) {
71+
var ii;
72+
var content;
73+
74+
if (rebuild) {
75+
if (display.length) {
76+
var handle;
77+
78+
content = [];
79+
for (ii = 0; ii < display.length; ii++) {
80+
handle = handles[display[ii].phid];
81+
82+
display[ii].node = renderHandle(handle, true);
83+
content.push(display[ii].node.item);
84+
}
85+
} else {
86+
content = renderNote('No results.');
87+
}
88+
89+
JX.DOM.setContent(JX.$(config.results), content);
90+
}
91+
92+
var phid;
93+
var is_disabled;
94+
var button;
95+
96+
var at_maximum = !canSelectMore();
97+
98+
for (ii = 0; ii < display.length; ii++) {
99+
phid = display[ii].phid;
100+
101+
is_disabled = false;
102+
103+
// If this object is already selected, you can not select it again.
104+
if (phids.hasOwnProperty(phid)) {
105+
is_disabled = true;
106+
}
107+
108+
// If the maximum number of objects are already selected, you can
109+
// not select more.
110+
if (at_maximum) {
111+
is_disabled = true;
112+
}
113+
114+
button = display[ii].node.button;
115+
JX.DOM.alterClass(button, 'disabled', is_disabled);
116+
button.disabled = is_disabled;
117+
}
118+
119+
}
74120

121+
function renderHandle(h, attach) {
75122
var some_icon = JX.$N(
76123
'span',
77124
{className: 'phui-icon-view phui-font-fa ' +
@@ -111,15 +158,10 @@ JX.behavior('phabricator-object-selector', function(config) {
111158
meta: {handle: h, table:table}},
112159
cells));
113160

114-
if (attach) {
115-
button_list[h.phid] = select_object_button;
116-
if (h.phid in phids) {
117-
JX.DOM.alterClass(select_object_button, 'disabled', true);
118-
select_object_button.disabled = true;
119-
}
120-
}
121-
122-
return table;
161+
return {
162+
item: table,
163+
button: select_object_button
164+
};
123165
}
124166

125167
function renderNote(note) {
@@ -138,6 +180,18 @@ JX.behavior('phabricator-object-selector', function(config) {
138180
.send();
139181
}
140182

183+
function canSelectMore() {
184+
if (!config.maximum) {
185+
return true;
186+
}
187+
188+
if (JX.keys(phids).length < config.maximum) {
189+
return true;
190+
}
191+
192+
return false;
193+
}
194+
141195
JX.DOM.listen(
142196
JX.$(config.results),
143197
'click',
@@ -151,10 +205,13 @@ JX.behavior('phabricator-object-selector', function(config) {
151205
return;
152206
}
153207

208+
if (!canSelectMore()) {
209+
return;
210+
}
211+
154212
phids[phid] = true;
155-
JX.DOM.alterClass(button_list[phid], 'disabled', true);
156-
button_list[phid].disabled = true;
157213

214+
redrawList(false);
158215
redrawAttached();
159216
});
160217

@@ -170,13 +227,7 @@ JX.behavior('phabricator-object-selector', function(config) {
170227

171228
delete phids[phid];
172229

173-
// NOTE: We may not have a button in the button list, if this result is
174-
// not visible in the current search results.
175-
if (button_list[phid]) {
176-
JX.DOM.alterClass(button_list[phid], 'disabled', false);
177-
button_list[phid].disabled = false;
178-
}
179-
230+
redrawList(false);
180231
redrawAttached();
181232
});
182233

@@ -205,6 +256,7 @@ JX.behavior('phabricator-object-selector', function(config) {
205256
});
206257

207258
sendQuery();
208-
redrawAttached();
209259

260+
redrawList(true);
261+
redrawAttached();
210262
});

0 commit comments

Comments
 (0)
Failed to load comments.