Skip to content

Conversation

@cgsheeh
Copy link
Member

@cgsheeh cgsheeh commented Feb 18, 2022

This commit adds interactions with Bugzilla when submitting an
uplift request form on Phabricator. To do this we change the
uplift request comment action from a standard textarea populated
with remarkup for the entire request, into a form with inputs
for each question in the form. We add a custom form PHUIXControlType
which takes a spec with a single questions field to populate
the values in the form.

We change the type of DifferentialUpliftRequestCustomField::value
from a string to a mapping array in all usages, and only convert
the mapping to a string when modifying and accessing from internal
storage. The beta uplift questions array is changed to a mapping of
questions => defaults for initial form population. We add a function
getRemarkup which converts the field mapping values into a nicely
formatted Remarkup/Markdown string.

Since the above changes allow us to access form fields individually
as typed values, we can now make decisions based on the results of
inputs in the form. We add a code path to interact with Bugzilla
when applying transaction effects (ie saving the new result to
storage). We add a remarkup comment to Bugzilla on the relevant
bug and set the qe-verify flag if the user indicated manual QE
testing is required.

Comment on lines 75 to 113
public function setManualQERequiredFlag(int $bug) {
// Construct request for setting `qe-verify` flag, see
// https://bmo.readthedocs.io/en/latest/api/core/v1/bug.html#update-bug
$url = id(new PhutilURI(PhabricatorEnv::getEnvConfig('bugzilla.url')))
->setPath('/rest/bug/'.$bug);
$api_key = PhabricatorEnv::getEnvConfig('bugzilla.automation_api_key');

// Encode here because `setData` below will fail due to nested arrays.
$data = phutil_json_encode(
array(
'flags' => array(
array(
'name' => 'qe-verify',
'status' => '?',
),
),
)
);

$future = id(new HTTPSFuture($url))
->addHeader('Accept', 'application/json')
->addHeader('User-Agent', 'Phabricator')
->addHeader('X-Bugzilla-API-Key', $api_key)
->setData($data)
->setMethod('PUT')
->setTimeout(PhabricatorEnv::getEnvConfig('bugzilla.timeout'));

try {
list($status, $body) = $future->resolve();
$status_code = (int) $status->getStatusCode();

$thebody = $body;
# Return an error string and invalidate transaction if Bugzilla can't be contacted.
$body = phutil_json_decode($body);
if (array_key_exists("error", $body) && $body["error"]) {
$message = $body["message"];
throw new Exception(
'Could not set `qe-verify` on Bugzilla: '.$data.'! Status code: '.$status_code.' body: '.$thebody.'\nPlease file a bug.'
);
}

This comment was marked as outdated.

This commit adds interactions with Bugzilla when submitting an
uplift request form on Phabricator. To do this we change the
uplift request comment action from a standard textarea populated
with remarkup for the entire request, into a form with inputs
for each question in the form. We add a custom `form` PHUIXControlType
which takes a spec with a single `questions` field to populate
the values in the form.

We change the type of `DifferentialUpliftRequestCustomField::value`
from a string to a mapping array in all usages, and only convert
the mapping to a string when modifying and accessing from internal
storage. The beta uplift questions array is changed to a mapping of
`questions => defaults` for initial form population. We add a function
`getRemarkup` which converts the field mapping values into a nicely
formatted Remarkup/Markdown string.

Since the above changes allow us to access form fields individually
as typed values, we can now make decisions based on the results of
inputs in the form. We add a code path to interact with Bugzilla
when applying transaction effects (ie saving the new result to
storage). We add a remarkup comment to Bugzilla on the relevant
bug and set the `qe-verify` flag if the user indicated manual QE
testing is required.
Copy link

@dklawren dklawren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am getting the following error when trying to create a new revision for the conduit-suite locally. I am using a sample bug id (2) that I created as well:

FastCGI sent in stderr: "PHP message: [2022-03-02 16:54:06] EXCEPTION: (PhutilJSONParserException) Parse error on line 0 at column 0: 'null' is not a valid JSON object. at [/src/parser/PhutilJSONParser.php:43]PHP message: arcanist(head=stable, ref.stable=5dd6ed0da533), moz-extensions(), phabricator()PHP message: #0 <#2> PhutilJSONParser::parse(string) called at [/src/utils/utils.php:1314]PHP message: #1 <#2> phutil_json_decode(string) called at [/differential/customfield/DifferentialUpliftRequestCustomField.php:409]PHP message: #2 <#2> DifferentialUpliftRequestCustomField::setValue(string) called at [/differential/customfield/DifferentialUpliftRequestCustomField.php:403]PHP message: #3 <#2> DifferentialUpliftRequestCustomField::setValueFromApplicationTransactions(string) called at [/src/infrastructure/customfield/editor/PhabricatorCustomFieldEditField.php:70]PHP message: #4 <#2> PhabricatorCustomFieldEditField::buildControl() called at [/src/applications/transactions/editfield/PhabricatorEditField.php:376]PHP message: #5 <#2> PhabricatorEditField::renderControl() called at [/src/applications/transactions/editfield/PhabricatorEditField.php:406]PHP message: #6 <#2> PhabricatorEditField::appendToForm(AphrontFormView) called at [/src/applications/transactions/editengine/PhabricatorEditEngine.php:1384]PHP message: #7 <#2> PhabricatorEditEngine::buildEditForm(DifferentialRevision, array) called at [/src/applications/transactions/editengine/PhabricatorEditEngine.php:1240]PHP message: #8 <#2> PhabricatorEditEngine::buildEditResponse(DifferentialRevision) called at [/src/applications/transactions/editengine/PhabricatorEditEngine.php:998]PHP message: #9 <#2> PhabricatorEditEngine::buildResponse() called at [/src/applications/differential/controller/DifferentialRevisionEditController.php:53

Any thoughts?

cgsheeh added 2 commits March 2, 2022 17:01
- generalize to isFieldEnabled to hide/show fields
- add a function to check for a bug number before displaying any uplift
  request field
- if value can't be json decoded, set to blank
- don't add field edit details form since we were displaying as remarkup
  and submitting as such will cause decode error
@cgsheeh cgsheeh requested a review from dklawren March 3, 2022 03:58
Copy link

@dklawren dklawren left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works as expected now. r=dkl

@dklawren dklawren merged commit 6dc5785 into mozilla-conduit:master Mar 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants