Skip to content

Commit 0fe97ca

Browse files
authored
Bug 1987196 - remove server side uplift request assessment form validation (#69)
Bug 1987196 - remove server side uplift request assessment form validation Remove the server-side validation of the uplift request assessment form. Also remove the form comment action, so the only way to update the form is from Lando.
1 parent e2b43c6 commit 0fe97ca

File tree

3 files changed

+2
-233
lines changed

3 files changed

+2
-233
lines changed

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

Lines changed: 0 additions & 26 deletions
This file was deleted.

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

Lines changed: 2 additions & 171 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,6 @@
99
final class DifferentialUpliftRequestCustomField
1010
extends DifferentialStoredCustomField {
1111

12-
// Questions for beta, with defaults.
13-
const BETA_UPLIFT_FIELDS = array(
14-
"User impact if declined" => "",
15-
"Code covered by automated testing" => false,
16-
"Fix verified in Nightly" => false,
17-
"Needs manual QE test" => false,
18-
"Steps to reproduce for manual QE testing" => "",
19-
"Risk associated with taking this patch" => "",
20-
"Explanation of risk level" => "",
21-
"String changes made/needed" => "",
22-
"Is Android affected?" => false,
23-
);
24-
2512
// How each field is formatted in ReMarkup:
2613
// a bullet point with text in bold.
2714
const QUESTION_FORMATTING = "- **%s** %s";
@@ -76,7 +63,7 @@ public function shouldAppearInApplicationTransactions() {
7663

7764
public function shouldAppearInEditView() {
7865
// Should the field appear in Edit Revision feature
79-
return true;
66+
return false;
8067
}
8168

8269
// How the uplift text is rendered in the "Details" section.
@@ -88,80 +75,11 @@ public function renderPropertyViewValue(array $handles) {
8875
return new PHUIRemarkupView($this->getViewer(), $this->getRemarkup());
8976
}
9077

91-
// Returns `true` if the field meets all conditions to be editable.
92-
public function isFieldActive() {
93-
return $this->isUpliftTagSet() && $this->objectHasBugNumber();
94-
}
95-
96-
public function objectHasBugNumber(): bool {
97-
// Similar idea to `BugStore::resolveBug`.
98-
$bugzillaField = new DifferentialBugzillaBugIDField();
99-
$bugzillaField->setObject($this->getObject());
100-
(new PhabricatorCustomFieldStorageQuery())
101-
->addField($bugzillaField)
102-
->execute();
103-
$bug = $bugzillaField->getValue();
104-
105-
if (!$bug) {
106-
return false;
107-
}
108-
109-
return true;
110-
}
111-
11278
// How the field can be edited in the "Edit Revision" menu.
11379
public function renderEditControl(array $handles) {
11480
return null;
11581
}
11682

117-
// -- Comment action things
118-
119-
public function getCommentActionLabel() {
120-
return pht('Request Uplift');
121-
}
122-
123-
// Return `true` if the `uplift` tag is set on the repository belonging to
124-
// this revision.
125-
private function isUpliftTagSet() {
126-
$revision = $this->getObject();
127-
$viewer = $this->getViewer();
128-
129-
if ($revision == null || $viewer == null) {
130-
return false;
131-
}
132-
133-
try {
134-
$repository_projects = PhabricatorEdgeQuery::loadDestinationPHIDs(
135-
$revision->getFieldValuesForConduit()['repositoryPHID'],
136-
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST
137-
);
138-
} catch (Exception $e) {
139-
return false;
140-
}
141-
142-
if (!(bool)$repository_projects) {
143-
return false;
144-
}
145-
146-
$uplift_project = id(new PhabricatorProjectQuery())
147-
->setViewer($viewer)
148-
->withNames(array('uplift'))
149-
->executeOne();
150-
151-
// The `uplift` project isn't created or can't be found.
152-
if (!(bool)$uplift_project) {
153-
return false;
154-
}
155-
156-
// If the `uplift` project PHID is in the set of all project PHIDs
157-
// attached to the repo, return `true`.
158-
if (in_array($uplift_project->getPHID(), $repository_projects)) {
159-
return true;
160-
}
161-
162-
return false;
163-
}
164-
16583
// Convert `bool` types to readable text, or return base text.
16684
private function valueAsAnswer($value): string {
16785
if ($value === true) {
@@ -188,94 +106,7 @@ private function getRemarkup(): string {
188106
}
189107

190108
public function newCommentAction() {
191-
// Returning `null` causes no comment action to render, effectively
192-
// "disabling" the field.
193-
if (!$this->isFieldActive()) {
194-
return null;
195-
}
196-
197-
$action = id(new PhabricatorUpdateUpliftCommentAction())
198-
->setConflictKey('revision.action')
199-
->setValue($this->getValue())
200-
->setInitialValue(self::BETA_UPLIFT_FIELDS)
201-
->setSubmitButtonText(pht('Request Uplift'));
202-
203-
return $action;
204-
}
205-
206-
public function validateUpliftForm(array $form): array {
207-
$validation_errors = array();
208-
209-
# Allow clearing the form.
210-
if (empty($form)) {
211-
return $validation_errors;
212-
}
213-
214-
$valid_questions = array_keys(self::BETA_UPLIFT_FIELDS);
215-
216-
$validated_question = array();
217-
foreach($form as $question => $answer) {
218-
# Assert the question is valid.
219-
if (!in_array($question, $valid_questions)) {
220-
$validation_errors[] = "Invalid question: '$question'";
221-
continue;
222-
}
223-
224-
$default_type = gettype(self::BETA_UPLIFT_FIELDS[$question]);
225-
226-
# Assert the value is not empty.
227-
$empty_string = $default_type == "string" && empty($answer);
228-
$null_bool = $default_type == "boolean" && is_null($answer);
229-
if ($empty_string || $null_bool) {
230-
$validation_errors[] = "Need to answer '$question'";
231-
continue;
232-
}
233-
234-
# Assert the type from the response matches the type of the default.
235-
$answer_type = gettype($answer);
236-
if ($default_type != $answer_type) {
237-
$validation_errors[] = "Parsing error: type '$answer_type' for '$question' doesn't match expected '$default_type'!";
238-
continue;
239-
}
240-
241-
$validated_question[] = $question;
242-
}
243-
244-
# Make sure we have all the required fields present in the response.
245-
$missing = array_diff($valid_questions, $validated_question);
246-
if (empty($validation_errors) && $missing) {
247-
foreach($missing as $missing_question) {
248-
$validation_errors[] = "Missing response for $missing_question";
249-
}
250-
}
251-
252-
return $validation_errors;
253-
}
254-
255-
public function validateApplicationTransactions(
256-
PhabricatorApplicationTransactionEditor $editor,
257-
$type, array $xactions) {
258-
259-
$errors = parent::validateApplicationTransactions($editor, $type, $xactions);
260-
261-
foreach($xactions as $xaction) {
262-
// Validate that the form is correctly filled out.
263-
// This should always be a string (think if the value came from the remarkup edit)
264-
$validation_errors = $this->validateUpliftForm(
265-
phutil_json_decode($xaction->getNewValue()),
266-
);
267-
268-
// Push errors into the revision save stack
269-
foreach($validation_errors as $validation_error) {
270-
$errors[] = new PhabricatorApplicationTransactionValidationError(
271-
$type,
272-
'',
273-
pht($validation_error)
274-
);
275-
}
276-
}
277-
278-
return $errors;
109+
return null;
279110
}
280111

281112
// When storing the value convert the question => answer mapping to a JSON string.

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

Lines changed: 0 additions & 36 deletions
This file was deleted.

0 commit comments

Comments
 (0)