Skip to content

Commit d171c0d

Browse files
cgsheehcgsheeh
committedDec 21, 2021
Bug 1737835: add an "uplift request" custom field
Adds a new custom field that stores the uplift request form. This form is to be filled out on the tip commit of a stack to be uplifted and will replace the similar uplift request present in Bugzilla. The field will only present itself in the UI if the revisions's repository is associated with the `uplift` project tag.
1 parent e88ba42 commit d171c0d

File tree

4 files changed

+347
-0
lines changed

4 files changed

+347
-0
lines changed
 

‎moz-extensions/src/__phutil_library_map__.php

+6
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
'DifferentialBugzillaBugIDCustomFieldTestCase' => 'differential/customfield/__tests__/DifferentialBugzillaIdCustomFieldTestCase.php',
1919
'DifferentialBugzillaBugIDField' => 'differential/customfield/DifferentialBugzillaBugIDField.php',
2020
'DifferentialBugzillaBugIDValidator' => 'differential/customfield/DifferentialBugzillaBugIDValidator.php',
21+
'DifferentialUpliftRequestCustomField' => 'differential/customfield/DifferentialUpliftRequestCustomField.php',
22+
'DifferentialUpliftRequestCustomFieldTestCase' => 'differential/customfield/__tests__/DifferentialUpliftRequestCustomFieldTestCase.php',
23+
'PhabricatorUpdateUpliftCommentAction' => 'applications/transactions/commentaction/PhabricatorUpdateUpliftCommentAction.php',
2124
'DifferentialRevisionWarning' => 'differential/view/DifferentialRevisionWarning.php',
2225
'EmailAPIAuthorization' => 'email/EmailAPIAuthorization.php',
2326
'EmailAffectedFile' => 'email/model/EmailAffectedFile.php',
@@ -119,8 +122,11 @@
119122
'CreatePolicyConduitAPIMethod' => 'ConduitAPIMethod',
120123
'DifferentialBugzillaBugIDCommitMessageField' => 'DifferentialCommitMessageCustomField',
121124
'DifferentialBugzillaBugIDCustomFieldTestCase' => 'PhabricatorTestCase',
125+
'DifferentialUpliftRequestCustomFieldTestCase' => 'PhabricatorTestCase',
122126
'DifferentialBugzillaBugIDField' => 'DifferentialStoredCustomField',
123127
'DifferentialBugzillaBugIDValidator' => 'Phobject',
128+
'DifferentialUpliftRequestCustomField' => 'DifferentialStoredCustomField',
129+
'PhabricatorUpdateUpliftCommentAction' => 'PhabricatorEditEngineCommentAction',
124130
'DifferentialRevisionWarning' => 'Phobject',
125131
'EmailRevisionAbandoned' => 'PublicEmailBody',
126132
'EmailRevisionAccepted' => 'PublicEmailBody',

‎moz-extensions/src/applications/transactions/commentaction/PhabricatorUpdateUpliftCommentAction.php

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
class PhabricatorUpdateUpliftCommentAction
4+
extends PhabricatorEditEngineCommentAction {
5+
6+
public function getPHUIXControlType() {
7+
return 'remarkup';
8+
}
9+
10+
public function getPHUIXControlSpecification() {
11+
$value = $this->getValue();
12+
13+
if (empty($value)) {
14+
$value = $this->getInitialValue();
15+
}
16+
17+
if (empty($value)) {
18+
$value = null;
19+
}
20+
21+
return array(
22+
'value' => pht($value),
23+
);
24+
}
25+
26+
}

‎moz-extensions/src/differential/customfield/DifferentialUpliftRequestCustomField.php

+288
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,288 @@
1+
<?php
2+
// This Source Code Form is subject to the terms of the Mozilla Public
3+
// License, v. 2.0. If a copy of the MPL was not distributed with this
4+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
/**
7+
* Extends Differential with a 'Uplift Request' field.
8+
*/
9+
final class DifferentialUpliftRequestCustomField
10+
extends DifferentialStoredCustomField {
11+
12+
const BETA_UPLIFT_FIELDS = array(
13+
"User impact if declined",
14+
"Code covered by automated testing",
15+
"Fix verified in Nightly",
16+
"Needs manual QE test",
17+
"Steps to reproduce for manual QE testing",
18+
"Risk associated with taking this patch",
19+
"Explanation of risk level",
20+
"String changes made/needed",
21+
);
22+
23+
// How each field is formatted in ReMarkup.
24+
const QUESTION_FORMATTING = "==== %s ====";
25+
26+
private $proxy;
27+
28+
/* -( Core Properties and Field Identity )--------------------------------- */
29+
30+
public function readValueFromRequest(AphrontRequest $request) {
31+
$uplift_data = $request->getStr($this->getFieldKey());
32+
$this->setValue($uplift_data);
33+
}
34+
35+
public function getFieldKey() {
36+
return 'differential:uplift-request';
37+
}
38+
39+
public function getFieldKeyForConduit() {
40+
return 'uplift.request';
41+
}
42+
43+
public function getFieldValue() {
44+
return $this->getValue();
45+
}
46+
47+
public function getFieldName() {
48+
return pht('Uplift Request form');
49+
}
50+
51+
public function getFieldDescription() {
52+
// Rendered in 'Config > Differential > differential.fields'
53+
return pht('Renders uplift request form.');
54+
}
55+
56+
public function isFieldEnabled() {
57+
return true;
58+
}
59+
60+
public function canDisableField() {
61+
// Field can't be switched off in configuration
62+
return false;
63+
}
64+
65+
/* -( ApplicationTransactions )-------------------------------------------- */
66+
67+
public function shouldAppearInApplicationTransactions() {
68+
// Required to be editable
69+
return true;
70+
}
71+
72+
/* -( Edit View )---------------------------------------------------------- */
73+
74+
public function shouldAppearInEditView() {
75+
// Should the field appear in Edit Revision feature
76+
return true;
77+
}
78+
79+
// How the uplift text is rendered in the "Details" section.
80+
public function renderPropertyViewValue(array $handles) {
81+
if (!strlen($this->getValue())) {
82+
return null;
83+
}
84+
85+
return new PHUIRemarkupView($this->getViewer(), pht($this->getValue()));
86+
}
87+
88+
// How the field can be edited in the "Edit Revision" menu.
89+
public function renderEditControl(array $handles) {
90+
if (!$this->isUpliftTagSet()) {
91+
return null;
92+
}
93+
94+
return id(new PhabricatorRemarkupControl())
95+
->setLabel($this->getFieldName())
96+
->setCaption(pht('Please answer all questions.'))
97+
->setName($this->getFieldKey())
98+
->setValue($this->getValue(), '');
99+
}
100+
101+
// -- Comment action things
102+
103+
public function getCommentActionLabel() {
104+
return pht('Request Uplift');
105+
}
106+
107+
// Return `true` if the `uplift` tag is set on the repository belonging to
108+
// this revision.
109+
private function isUpliftTagSet() {
110+
$revision = $this->getObject();
111+
$viewer = $this->getViewer();
112+
113+
if ($revision == null || $viewer == null) {
114+
return false;
115+
}
116+
117+
try {
118+
$repository_projects = PhabricatorEdgeQuery::loadDestinationPHIDs(
119+
$revision->getFieldValuesForConduit()['repositoryPHID'],
120+
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST);
121+
} catch (Exception $e) {
122+
return false;
123+
}
124+
125+
if (!(bool)$repository_projects) {
126+
return false;
127+
}
128+
129+
$uplift_project = id(new PhabricatorProjectQuery())
130+
->setViewer($viewer)
131+
->withNames(array('uplift'))
132+
->executeOne();
133+
134+
// If the `uplift` project PHID is in the set of all project PHIDs
135+
// attached to the repo, return `true`.
136+
if (in_array($uplift_project->getPHID(), $repository_projects)) {
137+
return true;
138+
}
139+
140+
return false;
141+
}
142+
143+
private function getUpliftFormQuestions() {
144+
$questions = array();
145+
146+
foreach (self::BETA_UPLIFT_FIELDS as $section) {
147+
$questions[] = sprintf(self::QUESTION_FORMATTING, $section);
148+
$questions[] = "\n";
149+
}
150+
151+
return implode("\n", $questions);
152+
}
153+
154+
public function newCommentAction() {
155+
// Returning `null` causes no comment action to render, effectively
156+
// "disabling" the field.
157+
if (!$this->isUpliftTagSet()) {
158+
return null;
159+
}
160+
161+
$action = id(new PhabricatorUpdateUpliftCommentAction())
162+
->setConflictKey('revision.action')
163+
->setValue($this->getValue())
164+
->setInitialValue($this->getUpliftFormQuestions())
165+
->setSubmitButtonText(pht('Request Uplift'));
166+
167+
return $action;
168+
}
169+
170+
public function validateUpliftForm($form) {
171+
$validation_errors = array();
172+
173+
# Allow clearing the form.
174+
if (empty($form)) {
175+
return $validation_errors;
176+
}
177+
178+
# Check each question in the form is present as a header
179+
# in the field.
180+
foreach(self::BETA_UPLIFT_FIELDS as $section) {
181+
if (strpos($form, sprintf(self::QUESTION_FORMATTING, $section)) === false) {
182+
$validation_errors[] = "Missing the '$section' field";
183+
}
184+
}
185+
186+
return $validation_errors;
187+
}
188+
189+
public function validateApplicationTransactions(
190+
PhabricatorApplicationTransactionEditor $editor,
191+
$type, array $xactions) {
192+
193+
$errors = parent::validateApplicationTransactions($editor, $type, $xactions);
194+
195+
foreach($xactions as $xaction) {
196+
// Validate that the form is correctly filled out
197+
$validation_errors = $this->validateUpliftForm(
198+
$xaction->getNewValue(),
199+
);
200+
201+
// Push errors into the revision save stack
202+
foreach($validation_errors as $validation_error) {
203+
$errors[] = new PhabricatorApplicationTransactionValidationError(
204+
$type,
205+
'',
206+
pht($validation_error)
207+
);
208+
}
209+
}
210+
211+
return $errors;
212+
}
213+
214+
/* -( Property View )------------------------------------------------------ */
215+
216+
public function shouldAppearInPropertyView() {
217+
return true;
218+
}
219+
220+
/* -( List View )---------------------------------------------------------- */
221+
222+
// Switched of as renderOnListItem is undefined
223+
// public function shouldAppearInListView() {
224+
// return true;
225+
// }
226+
227+
// TODO Find out if/how to implement renderOnListItem
228+
// It throws Incomplete if not overriden, but doesn't appear anywhere else
229+
// except of it's definition in `PhabricatorCustomField`
230+
231+
/* -( Global Search )------------------------------------------------------ */
232+
233+
public function shouldAppearInGlobalSearch() {
234+
return true;
235+
}
236+
237+
/* -( Conduit )------------------------------------------------------------ */
238+
239+
public function shouldAppearInConduitDictionary() {
240+
// Should the field appear in `differential.revision.search`
241+
return true;
242+
}
243+
244+
public function shouldAppearInConduitTransactions() {
245+
// Required if needs to be saved via Conduit (i.e. from `arc diff`)
246+
return true;
247+
}
248+
249+
protected function newConduitSearchParameterType() {
250+
return new ConduitStringParameterType();
251+
}
252+
253+
protected function newConduitEditParameterType() {
254+
// Define the type of the parameter for Conduit
255+
return new ConduitStringParameterType();
256+
}
257+
258+
public function readFieldValueFromConduit(string $value) {
259+
return $value;
260+
}
261+
262+
public function isFieldEditable() {
263+
// Has to be editable to be written from `arc diff`
264+
return true;
265+
}
266+
267+
// TODO see what this controls and consider using it
268+
public function shouldDisableByDefault() {
269+
return false;
270+
}
271+
272+
public function shouldOverwriteWhenCommitMessageIsEdited() {
273+
return false;
274+
}
275+
276+
public function getApplicationTransactionTitle(
277+
PhabricatorApplicationTransaction $xaction) {
278+
279+
if($this->proxy) {
280+
return $this->proxy->getApplicationTransactionTitle($xaction);
281+
}
282+
283+
$author_phid = $xaction->getAuthorPHID();
284+
285+
return pht('%s updated the uplift request field.', $xaction->renderHandleLink($author_phid));
286+
}
287+
}
288+

‎moz-extensions/src/differential/customfield/__tests__/DifferentialUpliftRequestCustomFieldTestCase.php

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
// This Source Code Form is subject to the terms of the Mozilla Public
3+
// License, v. 2.0. If a copy of the MPL was not distributed with this
4+
// file, You can obtain one at http://mozilla.org/MPL/2.0/.
5+
6+
final class DifferentialUpliftRequestCustomFieldTestCase
7+
extends PhabricatorTestCase {
8+
9+
public function testFormValidation() {
10+
$field = new DifferentialUpliftRequestCustomField();
11+
// Ensure the field can't be filled without answering all questions
12+
$errors = $field->validateUpliftForm("=== junk ===");
13+
14+
$expected = array();
15+
foreach(DifferentialUpliftRequestCustomField::BETA_UPLIFT_FIELDS as $err) {
16+
$expected[] = "Missing the '$err' field";
17+
}
18+
$this->assertEqual($expected, $errors);
19+
20+
// Ensure the field can be set as empty
21+
$errors = $field->validateUpliftForm("");
22+
$this->assertEqual(
23+
array(),
24+
$errors,
25+
"The empty form leads to errors - should be allowed.");
26+
}
27+
}

0 commit comments

Comments
 (0)
Failed to load comments.