Skip to content

Commit 6d96527

Browse files
committedNov 1, 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 10c373b commit 6d96527

File tree

4 files changed

+371
-0
lines changed

4 files changed

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

0 commit comments

Comments
 (0)
Failed to load comments.