@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
Move revision header warnings into custom fields
Summary:
Ref T5495. We currently show one warning in revision headers, about not having any reviewers.
I want to add a second warning (for missing Legalpad signatures). At least one install would like to add custom warnings (see T5495) which are so specific that we can't reasonably cover them in the upstream.
Generalize these header warnings by moving them to CustomField, so I can implement the Legalpad stuff without making a mess and the install in T5495 can use an extension.
Test Plan:
Hit all three header states, they look exactly like they did before this change:
{F173265}
Reviewers: btrahan, chad
Reviewed By: chad
Subscribers: epriestley
Maniphest Tasks: T5495
Differential Revision: https://secure.phabricator.com/D9793
+72
-29
+34
-29
src/applications/differential/controller/DifferentialRevisionViewController.php
+34
-29
src/applications/differential/controller/DifferentialRevisionViewController.php
···
115
115
}
116
116
}
117
117
118
+
$field_list = PhabricatorCustomField::getObjectFields(
119
+
$revision,
120
+
PhabricatorCustomField::ROLE_VIEW);
121
+
122
+
$field_list->setViewer($user);
123
+
$field_list->readFieldsFromStorage($revision);
124
+
125
+
$warning_handle_map = array();
126
+
foreach ($field_list->getFields() as $key => $field) {
127
+
$req = $field->getRequiredHandlePHIDsForRevisionHeaderWarnings();
128
+
foreach ($req as $phid) {
129
+
$warning_handle_map[$key][] = $phid;
130
+
$object_phids[] = $phid;
131
+
}
132
+
}
133
+
118
134
$handles = $this->loadViewerHandles($object_phids);
119
135
120
136
$request_uri = $request->getRequestURI();
···
169
185
$visible_changesets = $changesets;
170
186
}
171
187
172
-
$field_list = PhabricatorCustomField::getObjectFields(
173
-
$revision,
174
-
PhabricatorCustomField::ROLE_VIEW);
175
-
176
-
$field_list->setViewer($user);
177
-
$field_list->readFieldsFromStorage($revision);
178
-
179
188
180
189
// TODO: This should be in a DiffQuery or similar.
181
190
$need_props = array();
···
245
254
246
255
$revision_detail_box = $revision_detail->render();
247
256
248
-
$revision_warnings = $this->buildRevisionWarnings($revision, $handles);
257
+
$revision_warnings = $this->buildRevisionWarnings(
258
+
$revision,
259
+
$field_list,
260
+
$warning_handle_map,
261
+
$handles);
249
262
if ($revision_warnings) {
263
+
$revision_warnings = id(new AphrontErrorView())
264
+
->setSeverity(AphrontErrorView::SEVERITY_WARNING)
265
+
->setErrors($revision_warnings);
250
266
$revision_detail_box->setErrorView($revision_warnings);
251
267
}
252
268
···
935
951
936
952
private function buildRevisionWarnings(
937
953
DifferentialRevision $revision,
954
+
PhabricatorCustomFieldList $field_list,
955
+
array $warning_handle_map,
938
956
array $handles) {
939
957
940
-
$status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
941
-
if ($revision->getStatus() != $status_needs_review) {
942
-
return;
943
-
}
944
-
945
-
foreach ($revision->getReviewers() as $reviewer) {
946
-
if (!$handles[$reviewer]->isDisabled()) {
947
-
return;
958
+
$warnings = array();
959
+
foreach ($field_list->getFields() as $key => $field) {
960
+
$phids = idx($warning_handle_map, $key, array());
961
+
$field_handles = array_select_keys($handles, $phids);
962
+
$field_warnings = $field->getWarningsForRevisionHeader($field_handles);
963
+
foreach ($field_warnings as $warning) {
964
+
$warnings[] = $warning;
948
965
}
949
966
}
950
967
951
-
$warnings = array();
952
-
if ($revision->getReviewers()) {
953
-
$warnings[] = pht(
954
-
'This revision needs review, but all specified reviewers are '.
955
-
'disabled or inactive.');
956
-
} else {
957
-
$warnings[] = pht(
958
-
'This revision needs review, but there are no reviewers specified.');
959
-
}
960
-
961
-
return id(new AphrontErrorView())
962
-
->setSeverity(AphrontErrorView::SEVERITY_WARNING)
963
-
->setErrors($warnings);
968
+
return $warnings;
964
969
}
965
970
966
971
}
+8
src/applications/differential/customfield/DifferentialCustomField.php
+8
src/applications/differential/customfield/DifferentialCustomField.php
···
74
74
return array();
75
75
}
76
76
77
+
public function getRequiredHandlePHIDsForRevisionHeaderWarnings() {
78
+
return array();
79
+
}
80
+
81
+
public function getWarningsForRevisionHeader(array $handles) {
82
+
return array();
83
+
}
84
+
77
85
/* -( Integration with Commit Messages )----------------------------------- */
78
86
79
87
+30
src/applications/differential/customfield/DifferentialReviewersField.php
+30
src/applications/differential/customfield/DifferentialReviewersField.php
···
191
191
}
192
192
}
193
193
194
+
public function getRequiredHandlePHIDsForRevisionHeaderWarnings() {
195
+
return mpull($this->getValue(), 'getReviewerPHID');
196
+
}
197
+
198
+
public function getWarningsForRevisionHeader(array $handles) {
199
+
$revision = $this->getObject();
200
+
201
+
$status_needs_review = ArcanistDifferentialRevisionStatus::NEEDS_REVIEW;
202
+
if ($revision->getStatus() != $status_needs_review) {
203
+
return array();
204
+
}
205
+
206
+
foreach ($this->getValue() as $reviewer) {
207
+
if (!$handles[$reviewer->getReviewerPHID()]->isDisabled()) {
208
+
return array();
209
+
}
210
+
}
211
+
212
+
$warnings = array();
213
+
if ($this->getValue()) {
214
+
$warnings[] = pht(
215
+
'This revision needs review, but all specified reviewers are '.
216
+
'disabled or inactive.');
217
+
} else {
218
+
$warnings[] = pht(
219
+
'This revision needs review, but there are no reviewers specified.');
220
+
}
221
+
222
+
return $warnings;
223
+
}
194
224
195
225
}