995000 - Please create an Automation Request Form in Bugzilla

Status: RESOLVED FIXED

(bugzilla.mozilla.org :: Administration, task)

Description

[Jonathan Griffin (:jgriffin)]

We'd like to create an Automation Request Form in bugzilla that would take the answers to a number of questions and automatically file a single bug using the contents of the form.

This is a bit similar to the Project Kickoff Form, but less complex, as this doesn't involve a series of bugs, notifications, etc.

The bug that gets filed based on this form should be put in Testing:General, and the default cc list should be: jgriffin, mcote, dburns, jeads, jmaher, and ctalbert.

The questions I'd like in this form are as follows:

* Describe the problem that you'd like automation to help solve
* Describe how you'd like to automate a solution for this
* Describe the top-level project goal which this is supporting
* Existing bug (if any)
* Does this automation need to be run per-commit and report to TBPL?  Can it be run less frequently?
* If this automation will report data other than pass/fail (e.g.,, some sort of performance metric), describe the data that you'd like to have the automation produce.  Do we already have a method of capturing this kind of data, or do we need to develop one?
* When is a prototype needed?
* When is a finished project running in production needed?
* If there are multiple pieces, tests, or features in the proposed automation, what is the single most valuable piece?
* Which engineer is responsible for working with the automation engineer for information, support, and troubleshooting?
* Which manager/project manager is responsible for issues related to milestones and priorities?
* What other teams are involved and are there any other external dependencies?
* Additional information

Comment 1

[Dylan Hardison [:dylan] (he/him)]

The form is ready for testing on https://bugzilla-dev.allizom.org/form:automative

I'm curious which fields should be required, which fields should be textarea vs. line input,
and what (if anything) I do with Responsible Engineer and Manager. Add them to the cc list?
Is one of them the default assignee?

Comment 2

[Mark Côté [:mcote]]

The default assignee should probably be the default for Testing ([e-mail]).  I think I would just put the responsible engineer and manager into the bug comment, since they may not map cleanly to a Bugzilla user.

I would make "Describe the problem that you'd like automation to help solve" a textarea, since it might be long (and we want to encourage a detailed account of the problem to make sure their proposed solution fits). Other than that, I think your guesses are good.

Also I see jgriffin made a typo; please remove one comma from "e.g.,, some sort of performance metric" :)

Comment 3

[Mark Côté [:mcote]]

Dylan points out that we need a bug short_desc (bug "title"), so we should split the first question into two, with the first being the short_desc (line input), and the second going into the comment with the rest of the info (textarea):

* One-line description of the problem you'd like automation to help solve
* Detailed description of the problem

Comment 4

[Joel Maher ( :jmaher ) (UTC -8)]

this is good stuff!

Comment 5

[Mark Côté [:mcote]]

Sorry, another tiny correction.  First question should be

* One-line summary of the problem you'd like automation to help solve

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Form updated on bugzilla-dev: https://bugzilla-dev.allizom.org/form:automative

I changed the description of "desc_solution" to match the style of "desc_problem", let me know if that is acceptable.

Comment 7

[Dylan Hardison [:dylan] (he/him)]

My word, I failed to remove the double comma (",,"), but that will be fixed shortly.

Comment 8

[Mark Côté [:mcote]]

Just a slight change: "Detailed description of the proposed automation solution"... should be obvious, but just want to make sure they will focus on how automation is involved in the solution.

Comment 9

[Dylan Hardison [:dylan] (he/him)]

Change applied, should show up on bugzilla-dev shortly.

Comment 10

[Mark Côté [:mcote]]

I just realized that "Does this automation need to be run per-commit and report to TBPL?  Can it be run less frequently?" is kind of a terrible question.  It implies that a one-word answer is okay, except that the each question is the opposite of the other, so "yes" is ambiguous.  Even just changing it to "... and report to TBPL, or can it be run ..." will improve this a bit.

Comment 11

[Mark Côté [:mcote]]

Also, there are some spacing issue in the output; goal, bug, and per-commit have no extra blank lines between them.  Also trailing colon usage is inconsistent.

See https://bugzilla-dev.allizom.org/show_bug.cgi?id=1004381

Comment 12

[David Lawrence [:dkl]]

(In reply to Mark Côté ( :mcote ) from comment #10)
> I just realized that "Does this automation need to be run per-commit and
> report to TBPL?  Can it be run less frequently?" is kind of a terrible
> question.  It implies that a one-word answer is okay, except that the each
> question is the opposite of the other, so "yes" is ambiguous.  Even just
> changing it to "... and report to TBPL, or can it be run ..." will improve
> this a bit.

Seems like it would be better as a drop down in that case

"--Select One--"
"None"
"Per Commit"
"Less Frequently"

or something like that.

Also seems like Data Capture? would be better suited as a textarea.

dkl

Comment 13

[Mark Côté [:mcote]]

Well I think we would like them to specify a frequency if per-commit isn't required.

Comment 14

[Dylan Hardison [:dylan] (he/him)]

I was originally thinking Per-Commit was going to be a group of radio boxes, but I went with a text field because I didn't know what the domain of the frequency would be if not per-commit. 

Should I embiggen Data Capture to a text area?

Comment 15

[Mark Côté [:mcote]]

Yeah, actually that's a good idea.

Comment 16

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-995000.patch

here's the automation request form.

Comment 17

[:glob ✱]

Comment on attachment 8419473 [details] [diff] [review]
bug-995000.patch

Review of attachment 8419473 [details] [diff] [review]:
-----------------------------------------------------------------

t/009bugwords.t ...... 1/597
#   Failed test 'extensions/BMO/template/en/default/bug/create/comment-automative.txt.tmpl contains invalid bare words (e.g. 'bug') --WARNING'
#   at t/009bugwords.t line 90.

#   Failed test 'extensions/BMO/template/en/default/bug/create/create-automative.html.tmpl contains invalid bare words (e.g. 'bug') --WARNING'
#   at t/009bugwords.t line 90.

::: extensions/BMO/template/en/default/bug/create/create-automative.html.tmpl
@@ +47,5 @@
> +[% END %]
> +
> +[% inline_javascript = BLOCK %]
> +(function(){
> +'use strict';

this doesn't work; submitting the form results in:
> ReferenceError: validateAndSubmit is not defined

inline just the function, there's no need to wrap it or to 'use strict'.

Comment 18

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-995000-b.patch

Fixed the two minor issues.

Comment 19

[Mark Côté [:mcote]]

I think the CC list should actually be empty, and the bug should be assigned to jgriffin (but left as NEW).

Comment 20

[:glob ✱]

Comment on attachment 8419911 [details] [diff] [review]
bug-995000-b.patch

Review of attachment 8419911 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/BMO/template/en/default/bug/create/comment-automative.txt.tmpl
@@ +17,5 @@
> +[%+ cgi.param('desc_top_level_goal') %]
> +
> +[% IF cgi.param("existing_bug") %]
> +>>Existing [% terms.Bug %]:
> +[%+ cgi.param("existing_bug") %]

change to:

[%+ terms.Bug %] [% cgi.param("existing_bug") %]

to allow linkification of the bug number

@@ +38,5 @@
> +[%+ cgi.param('most_valuable_piece') %]
> +[% END %]
> +
> +>>Responsible Engineer:
> +[%+ cgi.param('responsible_engineer') %]

you're treating optional fields without values in three different ways:

1. 'not provided' placeholder (eg. prototype_date)
2. removed from comment (eg. most_valuable_piece)
3. empty value in comment (eg. responsible_engineeer)

select either (1) or (2), and make it applicable to all optional fields.  i have no strong preference either way.

::: extensions/BMO/template/en/default/bug/create/create-automative.html.tmpl
@@ +58,5 @@
> +        '#automative_form *[name="' + name + '"]'
> +      ).map(function(e) { return e.id });
> +
> +      if (ids && ids[0]) {
> +        console.log(name, ids[0]);

remove debugging

@@ +82,5 @@
> +   javascript = inline_javascript
> +   javascript_urls = [ 'extensions/BMO/web/js/form_validate.js',
> +                       'js/field.js', 'js/util.js',
> +                       'js/yui/selector/selector-min.js' ]
> +   yui = [ "autocomplete", "calendar" ]

you should never include yui javascript files via javascript_urls.

move the selector include from javascript_urls to yui..
yui = [ "autocomplete", "calendar", "selector" ]

@@ +100,5 @@
> +  <input type="hidden" name="bug_severity" id="bug_severity" value="normal">
> +  <input type="hidden" name="token" value="[% token FILTER html %]">
> +  <input type="hidden" name="cc" value="[e-mail], [e-mail],
> +  [e-mail], [e-mail], [e-mail],
> +  [e-mail]">

as per mcote's comment (made after this patch, but i'll mention it here so it isn't lost), hardcoding a long list of users is setting ourselves up for unnecessary administration overhead in the future.

remove the cc list, and change the default assignee to jgriffin.

@@ +103,5 @@
> +  [e-mail], [e-mail], [e-mail],
> +  [e-mail]">
> +
> +  <div class="head_desc">
> +      Welcome to the Automation Request Form!

nit: incorrect indentation

@@ +132,5 @@
> +  </div>
> +
> +  <div class="form_section">
> +    <label for="desc_top_level_goal" class="field_label required">Top Level
> +    Goal</label>

the line break after 'level' ends up in the javascript alert if this value is missing.

it's fine to not wrap at 80 chars here, and this fix should be applied to all labels.

@@ +140,5 @@
> +    rows="5"></textarea>
> +  </div>
> +
> +  <div class="form_section">
> +    <label for="existing_bug" class="field_label">Existing [% terms.Bug %] </label>

nit: remove space before </label>

@@ +141,5 @@
> +  </div>
> +
> +  <div class="form_section">
> +    <label for="existing_bug" class="field_label">Existing [% terms.Bug %] </label>
> +    <div class="field_desc"> Existing [% terms.bug %] (if any) </div>

i think this would be clearer as "existing bug number", instead of just "existing bug".

@@ +169,5 @@
> +  <div class="form_section">
> +    <label for="prototype_date" class="field_label">Prototype
> +    Date</label>
> +    <div class="field_desc">
> +    When is a prototype needed?

nit: incorrect indentation

@@ +217,5 @@
> +    <label for="responsible_engineer" class="field_label">Responsible
> +    Engineer</label>
> +    <div class="field_desc">
> +    Which engineer is responsible for working with the automation engineer for
> +    information, support, and troubleshooting?

nit: incorrect indentation

Comment 21

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-995000-c.patch

(In reply to Byron Jones ‹:glob› from comment #20)
> to allow linkification of the bug number
Ah, yes.

> you're treating optional fields without values in three different ways:
> 
> 1. 'not provided' placeholder (eg. prototype_date)
> 2. removed from comment (eg. most_valuable_piece)
> 3. empty value in comment (eg. responsible_engineeer)

> select either (1) or (2), and make it applicable to all optional fields.  i
> have no strong preference either way.

I think I copied that from the example. I went with #1, except for Existing Bug, where it is either terms.Bug existing_bug
or "No bug".

> remove debugging
Done.

> you should never include yui javascript files via javascript_urls.
Fixed.

> as per mcote's comment (made after this patch, but i'll mention it here so
> it isn't lost), hardcoding a long list of users is setting ourselves up for
> unnecessary administration overhead in the future.
> remove the cc list, and change the default assignee to jgriffin.

Fixed.

> nit: incorrect indentation
Reflowed identation for the entire file, then tweaked a few things that looked wrong.

> @@ +132,5 @@
> > +  </div>
> > +
> > +  <div class="form_section">
> > +    <label for="desc_top_level_goal" class="field_label required">Top Level
> > +    Goal</label>
> 
> the line break after 'level' ends up in the javascript alert if this value
> is missing.
> 
> it's fine to not wrap at 80 chars here, and this fix should be applied to
> all labels.

Actually, I like keeping the file consistent. I added javascript to strip newlines and repeated spaces.

> nit: remove space before </label>
fixed, and similar problems.

> i think this would be clearer as "existing bug number", instead of just
> "existing bug".
I agree. I almost think this field should be a bug selection field, but I didn't know if we had a widget for such. Changed the label.

> nit: incorrect indentation
> nit: incorrect indentation

Resolved.

Comment 22

[:glob ✱]

Comment on attachment 8420380 [details] [diff] [review]
bug-995000-c.patch

Review of attachment 8420380 [details] [diff] [review]:
-----------------------------------------------------------------

::: extensions/BMO/template/en/default/bug/create/comment-automative.txt.tmpl
@@ +17,5 @@
> +[%+ cgi.param('desc_top_level_goal') %]
> +
> +>>Existing [% terms.Bug %]:
> +[% IF cgi.param('existing_bug') %]
> +[%+ terms.Bug %] [% cgi.param("existing_bug") %]

terms.Bug doesn't work; you haven't imported global/variables.none.tmpl

@@ +19,5 @@
> +>>Existing [% terms.Bug %]:
> +[% IF cgi.param('existing_bug') %]
> +[%+ terms.Bug %] [% cgi.param("existing_bug") %]
> +[% ELSE %]
> +No bug

#   Failed test 'extensions/BMO/template/en/default/bug/create/comment-automative.txt.tmpl contains invalid bare words (e.g. 'bug') --WARNING'

Comment 23

[Dylan Hardison [:dylan] (he/him)]

Attachment: bug-995000-d.patch

Meh. I have added "prove t" as a pre-commit script in my bmo checkout.
This does not contain any bare/bug words.

Comment 24

[:glob ✱]

Comment on attachment 8420947 [details] [diff] [review]
bug-995000-d.patch

Review of attachment 8420947 [details] [diff] [review]:
-----------------------------------------------------------------

r=glob

Comment 25

[:glob ✱]

committed to land prior to this week's push:

To ssh://[e-mail]/webtools/bmo/bugzilla.git
   5a29794..46dc79d  master -> master