@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Add EditEngine + Modular Transactions for reviewers
Summary: Ref T11114. This one is a bit more complex, but I think I covered everything.
Test Plan:
- Added reviewers.
- Removed reviewers.
- Made reviewers blocking.
- Made reviewers nonblocking.
- Tried to make the author a reviewer.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11114
Differential Revision: https://secure.phabricator.com/D17050
+385
-2
+2
src/__phutil_library_map__.php
+2
src/__phutil_library_map__.php
···
549
549
'DifferentialRevisionRequiredActionResultBucket' => 'applications/differential/query/DifferentialRevisionRequiredActionResultBucket.php',
550
550
'DifferentialRevisionResultBucket' => 'applications/differential/query/DifferentialRevisionResultBucket.php',
551
551
'DifferentialRevisionReviewersHeraldField' => 'applications/differential/herald/DifferentialRevisionReviewersHeraldField.php',
552
+
'DifferentialRevisionReviewersTransaction' => 'applications/differential/xaction/DifferentialRevisionReviewersTransaction.php',
552
553
'DifferentialRevisionSearchConduitAPIMethod' => 'applications/differential/conduit/DifferentialRevisionSearchConduitAPIMethod.php',
553
554
'DifferentialRevisionSearchEngine' => 'applications/differential/query/DifferentialRevisionSearchEngine.php',
554
555
'DifferentialRevisionStatus' => 'applications/differential/constants/DifferentialRevisionStatus.php',
···
5202
5203
'DifferentialRevisionRequiredActionResultBucket' => 'DifferentialRevisionResultBucket',
5203
5204
'DifferentialRevisionResultBucket' => 'PhabricatorSearchResultBucket',
5204
5205
'DifferentialRevisionReviewersHeraldField' => 'DifferentialRevisionHeraldField',
5206
+
'DifferentialRevisionReviewersTransaction' => 'DifferentialRevisionTransactionType',
5205
5207
'DifferentialRevisionSearchConduitAPIMethod' => 'PhabricatorSearchEngineAPIMethod',
5206
5208
'DifferentialRevisionSearchEngine' => 'PhabricatorApplicationSearchEngine',
5207
5209
'DifferentialRevisionStatus' => 'Phobject',
+14
-1
src/applications/differential/editor/DifferentialRevisionEditEngine.php
+14
-1
src/applications/differential/editor/DifferentialRevisionEditEngine.php
···
32
32
}
33
33
34
34
protected function newObjectQuery() {
35
-
return new DifferentialRevisionQuery();
35
+
return id(new DifferentialRevisionQuery())
36
+
->needReviewerStatus(true);
36
37
}
37
38
38
39
protected function getObjectCreateTitleText($object) {
···
102
103
->setConduitTypeDescription(pht('New test plan.'))
103
104
->setValue($object->getTestPlan());
104
105
}
106
+
107
+
$fields[] = id(new PhabricatorDatasourceEditField())
108
+
->setKey('reviewerPHIDs')
109
+
->setLabel(pht('Reviewers'))
110
+
->setDatasource(new DifferentialReviewerDatasource())
111
+
->setUseEdgeTransactions(true)
112
+
->setTransactionType(
113
+
DifferentialRevisionReviewersTransaction::TRANSACTIONTYPE)
114
+
->setDescription(pht('Reviewers for this revision.'))
115
+
->setConduitDescription(pht('Change the reviewers for this revision.'))
116
+
->setConduitTypeDescription(pht('New reviewers.'))
117
+
->setValue($object->getReviewerPHIDsForEdit());
105
118
106
119
$fields[] = id(new PhabricatorDatasourceEditField())
107
120
->setKey('repositoryPHID')
+18
src/applications/differential/storage/DifferentialRevision.php
+18
src/applications/differential/storage/DifferentialRevision.php
···
409
409
return $this;
410
410
}
411
411
412
+
public function getReviewerPHIDsForEdit() {
413
+
$reviewers = $this->getReviewerStatus();
414
+
415
+
$status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
416
+
417
+
$value = array();
418
+
foreach ($reviewers as $reviewer) {
419
+
$phid = $reviewer->getReviewerPHID();
420
+
if ($reviewer->getStatus() == $status_blocking) {
421
+
$value[] = 'blocking('.$phid.')';
422
+
} else {
423
+
$value[] = $phid;
424
+
}
425
+
}
426
+
427
+
return $value;
428
+
}
429
+
412
430
public function getRepository() {
413
431
return $this->assertAttached($this->repository);
414
432
}
+350
src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
+350
src/applications/differential/xaction/DifferentialRevisionReviewersTransaction.php
···
1
+
<?php
2
+
3
+
final class DifferentialRevisionReviewersTransaction
4
+
extends DifferentialRevisionTransactionType {
5
+
6
+
const TRANSACTIONTYPE = 'differential.revision.reviewers';
7
+
8
+
public function generateOldValue($object) {
9
+
$reviewers = $object->getReviewerStatus();
10
+
$reviewers = mpull($reviewers, 'getStatus', 'getReviewerPHID');
11
+
return $reviewers;
12
+
}
13
+
14
+
public function generateNewValue($object, $value) {
15
+
$actor = $this->getActor();
16
+
17
+
$datasource = id(new DifferentialBlockingReviewerDatasource())
18
+
->setViewer($actor);
19
+
20
+
$reviewers = $this->generateOldValue($object);
21
+
22
+
// First, remove any reviewers we're getting rid of.
23
+
$rem = idx($value, '-', array());
24
+
$rem = $datasource->evaluateTokens($rem);
25
+
foreach ($rem as $phid) {
26
+
unset($reviewers[$phid]);
27
+
}
28
+
29
+
$add = idx($value, '+', array());
30
+
$add = $datasource->evaluateTokens($add);
31
+
$add_map = array();
32
+
foreach ($add as $spec) {
33
+
if (!is_array($spec)) {
34
+
$phid = $spec;
35
+
$status = DifferentialReviewerStatus::STATUS_ADDED;
36
+
} else {
37
+
$phid = $spec['phid'];
38
+
$status = $spec['type'];
39
+
}
40
+
41
+
$add_map[$phid] = $status;
42
+
}
43
+
44
+
$set = idx($value, '=', null);
45
+
if ($set !== null) {
46
+
$set = $datasource->evaluateTokens($set);
47
+
foreach ($set as $spec) {
48
+
if (!is_array($spec)) {
49
+
$phid = $spec;
50
+
$status = DifferentialReviewerStatus::STATUS_ADDED;
51
+
} else {
52
+
$phid = $spec['phid'];
53
+
$status = $spec['type'];
54
+
}
55
+
56
+
$add_map[$phid] = $status;
57
+
}
58
+
59
+
// We treat setting reviewers as though they were being added to an
60
+
// empty list, so we can share more code between pathways.
61
+
$reviewers = array();
62
+
}
63
+
64
+
$status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
65
+
foreach ($add_map as $phid => $new_status) {
66
+
$old_status = idx($reviewers, $phid);
67
+
68
+
// If we have an old status and this didn't make the reviewer blocking
69
+
// or nonblocking, just retain the old status. This makes sure we don't
70
+
// throw away rejects, accepts, etc.
71
+
if ($old_status) {
72
+
$was_blocking = ($old_status == $status_blocking);
73
+
$now_blocking = ($new_status == $status_blocking);
74
+
75
+
$is_block = ($now_blocking && !$was_blocking);
76
+
$is_unblock = (!$now_blocking && $was_blocking);
77
+
78
+
if (!$is_block && !$is_unblock) {
79
+
continue;
80
+
}
81
+
}
82
+
83
+
$reviewers[$phid] = $new_status;
84
+
}
85
+
86
+
return $reviewers;
87
+
}
88
+
89
+
public function applyExternalEffects($object, $value) {
90
+
$src_phid = $object->getPHID();
91
+
92
+
$old = $this->generateOldValue($object);
93
+
$new = $value;
94
+
$edge_type = DifferentialRevisionHasReviewerEdgeType::EDGECONST;
95
+
96
+
$editor = new PhabricatorEdgeEditor();
97
+
98
+
$rem = array_diff_key($old, $new);
99
+
foreach ($rem as $dst_phid => $status) {
100
+
$editor->removeEdge($src_phid, $edge_type, $dst_phid);
101
+
}
102
+
103
+
foreach ($new as $dst_phid => $status) {
104
+
$old_status = idx($old, $dst_phid);
105
+
if ($old_status === $status) {
106
+
continue;
107
+
}
108
+
109
+
$data = array(
110
+
'data' => array(
111
+
'status' => $status,
112
+
113
+
// TODO: This seemes like it's buggy before the Modular Transactions
114
+
// changes. Figure out what's going on here? We don't have a very
115
+
// clean way to get the active diff ID right now.
116
+
'diffID' => null,
117
+
),
118
+
);
119
+
120
+
$editor->addEdge($src_phid, $edge_type, $dst_phid, $data);
121
+
}
122
+
123
+
$editor->save();
124
+
}
125
+
126
+
public function getTitle() {
127
+
return $this->renderReviewerEditTitle(false);
128
+
}
129
+
130
+
public function getTitleForFeed() {
131
+
return $this->renderReviewerEditTitle(true);
132
+
}
133
+
134
+
private function renderReviewerEditTitle($is_feed) {
135
+
$old = $this->getOldValue();
136
+
$new = $this->getNewValue();
137
+
138
+
$rem = array_diff_key($old, $new);
139
+
$add = array_diff_key($new, $old);
140
+
$rem_phids = array_keys($rem);
141
+
$add_phids = array_keys($add);
142
+
$total_count = count($rem) + count($add);
143
+
144
+
$parts = array();
145
+
146
+
if ($rem && $add) {
147
+
if ($is_feed) {
148
+
$parts[] = pht(
149
+
'%s edited %s reviewer(s) for %s, added %s: %s; removed %s: %s.',
150
+
$this->renderAuthor(),
151
+
new PhutilNumber($total_count),
152
+
$this->renderObject(),
153
+
phutil_count($add_phids),
154
+
$this->renderHandleList($add_phids),
155
+
phutil_count($rem_phids),
156
+
$this->renderHandleList($rem_phids));
157
+
} else {
158
+
$parts[] = pht(
159
+
'%s edited %s reviewer(s), added %s: %s; removed %s: %s.',
160
+
$this->renderAuthor(),
161
+
new PhutilNumber($total_count),
162
+
phutil_count($add_phids),
163
+
$this->renderHandleList($add_phids),
164
+
phutil_count($rem_phids),
165
+
$this->renderHandleList($rem_phids));
166
+
}
167
+
} else if ($add) {
168
+
if ($is_feed) {
169
+
$parts[] = pht(
170
+
'%s added %s reviewer(s) for %s: %s.',
171
+
$this->renderAuthor(),
172
+
phutil_count($add_phids),
173
+
$this->renderObject(),
174
+
$this->renderHandleList($add_phids));
175
+
} else {
176
+
$parts[] = pht(
177
+
'%s added %s reviewer(s): %s.',
178
+
$this->renderAuthor(),
179
+
phutil_count($add_phids),
180
+
$this->renderHandleList($add_phids));
181
+
}
182
+
} else if ($rem) {
183
+
if ($is_feed) {
184
+
$parts[] = pht(
185
+
'%s removed %s reviewer(s) for %s: %s.',
186
+
$this->renderAuthor(),
187
+
phutil_count($rem_phids),
188
+
$this->renderObject(),
189
+
$this->renderHandleList($rem_phids));
190
+
} else {
191
+
$parts[] = pht(
192
+
'%s removed %s reviewer(s): %s.',
193
+
$this->renderAuthor(),
194
+
phutil_count($rem_phids),
195
+
$this->renderHandleList($rem_phids));
196
+
}
197
+
}
198
+
199
+
$status_blocking = DifferentialReviewerStatus::STATUS_BLOCKING;
200
+
$blocks = array();
201
+
$unblocks = array();
202
+
foreach ($new as $phid => $new_status) {
203
+
$old_status = idx($old, $phid);
204
+
if (!$old_status) {
205
+
continue;
206
+
}
207
+
208
+
$was_blocking = ($old_status == $status_blocking);
209
+
$now_blocking = ($new_status == $status_blocking);
210
+
211
+
$is_block = ($now_blocking && !$was_blocking);
212
+
$is_unblock = (!$now_blocking && $was_blocking);
213
+
214
+
if ($is_block) {
215
+
$blocks[] = $phid;
216
+
}
217
+
if ($is_unblock) {
218
+
$unblocks[] = $phid;
219
+
}
220
+
}
221
+
222
+
$total_count = count($blocks) + count($unblocks);
223
+
224
+
if ($blocks && $unblocks) {
225
+
if ($is_feed) {
226
+
$parts[] = pht(
227
+
'%s changed %s blocking reviewer(s) for %s, added %s: %s; removed '.
228
+
'%s: %s.',
229
+
$this->renderAuthor(),
230
+
new PhutilNumber($total_count),
231
+
$this->renderObject(),
232
+
phutil_count($blocks),
233
+
$this->renderHandleList($blocks),
234
+
phutil_count($unblocks),
235
+
$this->renderHandleList($unblocks));
236
+
} else {
237
+
$parts[] = pht(
238
+
'%s changed %s blocking reviewer(s), added %s: %s; removed %s: %s.',
239
+
$this->renderAuthor(),
240
+
new PhutilNumber($total_count),
241
+
phutil_count($blocks),
242
+
$this->renderHandleList($blocks),
243
+
phutil_count($unblocks),
244
+
$this->renderHandleList($unblocks));
245
+
}
246
+
} else if ($blocks) {
247
+
if ($is_feed) {
248
+
$parts[] = pht(
249
+
'%s added %s blocking reviewer(s) for %s: %s.',
250
+
$this->renderAuthor(),
251
+
phutil_count($blocks),
252
+
$this->renderObject(),
253
+
$this->renderHandleList($blocks));
254
+
} else {
255
+
$parts[] = pht(
256
+
'%s added %s blocking reviewer(s): %s.',
257
+
$this->renderAuthor(),
258
+
phutil_count($blocks),
259
+
$this->renderHandleList($blocks));
260
+
}
261
+
} else if ($unblocks) {
262
+
if ($is_feed) {
263
+
$parts[] = pht(
264
+
'%s removed %s blocking reviewer(s) for %s: %s.',
265
+
$this->renderAuthor(),
266
+
phutil_count($unblocks),
267
+
$this->renderObject(),
268
+
$this->renderHandleList($unblocks));
269
+
} else {
270
+
$parts[] = pht(
271
+
'%s removed %s blocking reviewer(s): %s.',
272
+
$this->renderAuthor(),
273
+
phutil_count($unblocks),
274
+
$this->renderHandleList($unblocks));
275
+
}
276
+
}
277
+
278
+
if ($this->isTextMode()) {
279
+
return implode(' ', $parts);
280
+
} else {
281
+
return phutil_implode_html(' ', $parts);
282
+
}
283
+
}
284
+
285
+
public function validateTransactions($object, array $xactions) {
286
+
$actor = $this->getActor();
287
+
$errors = array();
288
+
289
+
$author_phid = $object->getAuthorPHID();
290
+
$config_self_accept_key = 'differential.allow-self-accept';
291
+
$allow_self_accept = PhabricatorEnv::getEnvConfig($config_self_accept_key);
292
+
293
+
$old = $this->generateOldValue($object);
294
+
foreach ($xactions as $xaction) {
295
+
$new = $this->generateNewValue($object, $xaction->getNewValue());
296
+
297
+
$add = array_diff_key($new, $old);
298
+
if (!$add) {
299
+
continue;
300
+
}
301
+
302
+
$objects = id(new PhabricatorObjectQuery())
303
+
->setViewer($actor)
304
+
->withPHIDs(array_keys($add))
305
+
->execute();
306
+
$objects = mpull($objects, null, 'getPHID');
307
+
308
+
foreach ($add as $phid => $status) {
309
+
if (!isset($objects[$phid])) {
310
+
$errors[] = $this->newInvalidError(
311
+
pht(
312
+
'Reviewer "%s" is not a valid object.',
313
+
$phid),
314
+
$xaction);
315
+
continue;
316
+
}
317
+
318
+
switch (phid_get_type($phid)) {
319
+
case PhabricatorPeopleUserPHIDType::TYPECONST:
320
+
case PhabricatorOwnersPackagePHIDType::TYPECONST:
321
+
case PhabricatorProjectProjectPHIDType::TYPECONST:
322
+
break;
323
+
default:
324
+
$errors[] = $this->newInvalidError(
325
+
pht(
326
+
'Reviewer "%s" must be a user, a package, or a project.',
327
+
$phid),
328
+
$xaction);
329
+
continue 2;
330
+
}
331
+
332
+
// NOTE: This weird behavior around commandeering is a bit unorthodox,
333
+
// but this restriction is an unusual one.
334
+
335
+
$is_self = ($phid === $author_phid);
336
+
if ($is_self && !$allow_self_accept) {
337
+
if (!$xaction->getIsCommandeerSideEffect()) {
338
+
$errors[] = $this->newInvalidError(
339
+
pht('The author of a revision can not be a reviewer.'),
340
+
$xaction);
341
+
continue;
342
+
}
343
+
}
344
+
}
345
+
}
346
+
347
+
return $errors;
348
+
}
349
+
350
+
}
+1
-1
src/applications/transactions/storage/PhabricatorModularTransactionType.php
+1
-1
src/applications/transactions/storage/PhabricatorModularTransactionType.php