@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
Allow non-authors to "Request Review" of draft revisions
Summary:
See PHI1810. In situations where:
- An author submits an urgent change for review.
- The author pings reviewers to ask them to look at it.
...the reviewers may not be able to move the review forward if the review is currently a "Draft". They can only "Commandeer" or ask the author to "Request Review" as ways forward.
Although I'm hesitant to support review actions (particularly, "Accept") on draft revisions, I think there's no harm in allowing reviewers to skip tests and promote the revision out of draft as an explicit action.
Additionally, lightly specialize some of the transaction strings to distinguish between "request review from draft" and other state transitions.
Test Plan:
- As an author, used "Request Review" to promote a draft and to return a change to reviewers for consideration. These behaviors are unchanged, except "promote a draft" has different timeline text.
- As a non-author, used "Begin Review" to promote a draft.
Differential Revision: https://secure.phabricator.com/D21403
+113
-18
+11
src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
+11
src/applications/differential/xaction/DifferentialRevisionActionTransaction.php
···
66
66
return null;
67
67
}
68
68
69
+
protected function getRevisionActionMetadata(
70
+
DifferentialRevision $revision,
71
+
PhabricatorUser $viewer) {
72
+
return array();
73
+
}
74
+
69
75
public static function loadAllActions() {
70
76
return id(new PhutilClassMapQuery())
71
77
->setAncestorClass(__CLASS__)
···
149
155
if ($can_multi || $can_force || $not_self) {
150
156
$field->setOptions($options);
151
157
$field->setValue($value);
158
+
}
159
+
160
+
$metadata = $this->getRevisionActionMetadata($revision, $viewer);
161
+
foreach ($metadata as $metadata_key => $metadata_value) {
162
+
$field->setMetadataValue($metadata_key, $metadata_value);
152
163
}
153
164
}
154
165
}
+102
-18
src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
+102
-18
src/applications/differential/xaction/DifferentialRevisionRequestReviewTransaction.php
···
6
6
const TRANSACTIONTYPE = 'differential.revision.request';
7
7
const ACTIONKEY = 'request-review';
8
8
9
+
const SOURCE_HARBORMASTER = 'harbormaster';
10
+
const SOURCE_AUTHOR = 'author';
11
+
const SOURCE_VIEWER = 'viewer';
12
+
9
13
protected function getRevisionActionLabel(
10
14
DifferentialRevision $revision,
11
15
PhabricatorUser $viewer) {
16
+
17
+
// See PHI1810. Allow non-authors to "Request Review" on draft revisions
18
+
// to promote them out of the draft state. This smoothes over the workflow
19
+
// where an author asks for review of an urgent change but has not used
20
+
// "Request Review" to skip builds.
21
+
22
+
if ($revision->isDraft()) {
23
+
if (!$this->isViewerRevisionAuthor($revision, $viewer)) {
24
+
return pht('Begin Review Now');
25
+
}
26
+
}
27
+
12
28
return pht('Request Review');
13
29
}
14
30
···
16
32
DifferentialRevision $revision,
17
33
PhabricatorUser $viewer) {
18
34
if ($revision->isDraft()) {
19
-
return pht('This revision will be submitted to reviewers for feedback.');
35
+
if (!$this->isViewerRevisionAuthor($revision, $viewer)) {
36
+
return pht(
37
+
'This revision will be moved out of the draft state so you can '.
38
+
'review it immediately.');
39
+
} else {
40
+
return pht(
41
+
'This revision will be submitted to reviewers for feedback.');
42
+
}
20
43
} else {
21
44
return pht('This revision will be returned to reviewers for feedback.');
22
45
}
23
46
}
24
47
48
+
protected function getRevisionActionMetadata(
49
+
DifferentialRevision $revision,
50
+
PhabricatorUser $viewer) {
51
+
$map = array();
52
+
53
+
if ($revision->isDraft()) {
54
+
$action_source = $this->getActorSourceType(
55
+
$revision,
56
+
$viewer);
57
+
$map['promotion.source'] = $action_source;
58
+
}
59
+
60
+
return $map;
61
+
}
62
+
25
63
protected function getRevisionActionSubmitButtonText(
26
64
DifferentialRevision $revision,
27
65
PhabricatorUser $viewer) {
···
76
114
'revisions.'));
77
115
}
78
116
79
-
// When revisions automatically promote out of "Draft" after builds finish,
80
-
// the viewer may be acting as the Harbormaster application.
81
-
if (!$viewer->isOmnipotent()) {
82
-
if (!$this->isViewerRevisionAuthor($object, $viewer)) {
83
-
throw new Exception(
84
-
pht(
85
-
'You can not request review of this revision because you are not '.
86
-
'the author of the revision.'));
87
-
}
88
-
}
117
+
$this->getActorSourceType($object, $viewer);
89
118
}
90
119
91
120
public function getTitle() {
92
-
return pht(
93
-
'%s requested review of this revision.',
94
-
$this->renderAuthor());
121
+
$source = $this->getDraftPromotionSource();
122
+
123
+
switch ($source) {
124
+
case self::SOURCE_HARBORMASTER:
125
+
case self::SOURCE_VIEWER:
126
+
case self::SOURCE_AUTHOR:
127
+
return pht(
128
+
'%s published this revision for review.',
129
+
$this->renderAuthor());
130
+
default:
131
+
return pht(
132
+
'%s requested review of this revision.',
133
+
$this->renderAuthor());
134
+
}
95
135
}
96
136
97
137
public function getTitleForFeed() {
98
-
return pht(
99
-
'%s requested review of %s.',
100
-
$this->renderAuthor(),
101
-
$this->renderObject());
138
+
$source = $this->getDraftPromotionSource();
139
+
140
+
switch ($source) {
141
+
case self::SOURCE_HARBORMASTER:
142
+
case self::SOURCE_VIEWER:
143
+
case self::SOURCE_AUTHOR:
144
+
return pht(
145
+
'%s published %s for review.',
146
+
$this->renderAuthor(),
147
+
$this->renderObject());
148
+
default:
149
+
return pht(
150
+
'%s requested review of %s.',
151
+
$this->renderAuthor(),
152
+
$this->renderObject());
153
+
}
102
154
}
103
155
104
156
public function getTransactionTypeForConduit($xaction) {
···
107
159
108
160
public function getFieldValuesForConduit($object, $data) {
109
161
return array();
162
+
}
163
+
164
+
private function getDraftPromotionSource() {
165
+
return $this->getMetadataValue('promotion.source');
166
+
}
167
+
168
+
private function getActorSourceType(
169
+
DifferentialRevision $revision,
170
+
PhabricatorUser $viewer) {
171
+
172
+
$is_harbormaster = $viewer->isOmnipotent();
173
+
$is_author = $this->isViewerRevisionAuthor($revision, $viewer);
174
+
$is_draft = $revision->isDraft();
175
+
176
+
if ($is_harbormaster) {
177
+
// When revisions automatically promote out of "Draft" after builds
178
+
// finish, the viewer may be acting as the Harbormaster application.
179
+
$source = self::SOURCE_HARBORMASTER;
180
+
} else if ($is_author) {
181
+
$source = self::SOURCE_AUTHOR;
182
+
} else if ($is_draft) {
183
+
// Non-authors are allowed to "Request Review" on draft revisions, to
184
+
// force them into review immediately.
185
+
$source = self::SOURCE_VIEWER;
186
+
} else {
187
+
throw new Exception(
188
+
pht(
189
+
'You can not request review of this revision because you are not '.
190
+
'the author of the revision and it is not currently a draft.'));
191
+
}
192
+
193
+
return $source;
110
194
}
111
195
112
196
}