Changes to form.reps.mentorship

RESOLVED FIXED

Status

()

bugzilla.mozilla.org
Custom Bug Entry Forms
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: Nukeador, Assigned: dylan)

Tracking

Production

Details

(URL)

Attachments

(1 attachment, 4 obsolete attachments)

(Reporter)

Description

2 years ago
Hello,

We need some changes to the Reps mentorship form.

- "Mozillians.org Account" field should be a url field and renamed to "Mozillians.org Account URL".

- "References" field should be renamed to "Mozilla contributors who vouch your application" and make it mandatory. I don't know if this field can be set as bugzilla email field and cc to the bug to these people.

- "How are you involved with Mozilla?" field should be renamed to "What are you currently doing at Mozilla?" and make it mandatory.

- "When did you first start contributing to Mozilla?" should be mandatory.

- In "What motivates you most about joining Mozilla Reps?" text area, we would like the placeholder text to be "Tell us your goals and what you would be able to do as a Rep that you can't do right now"

- New boolean field under "How did you learn about Mozilla Reps?" named "This is the first time I apply for Reps", default true.

- New url field just under the previous one "If you have already applied to the program in the past, please reference the bug url".

Thanks!
(Reporter)

Comment 1

2 years ago
Question: Is it possible to have some default first comment on the bug once it's submitted?

It would be nice if we can give people some instructions once the bug is submitted so they get them via bugzilla notification email.
(Reporter)

Updated

2 years ago
OS: Linux → All
Hardware: x86_64 → All
(In reply to Rubén Martín [:Nukeador] from comment #1)
> Question: Is it possible to have some default first comment on the bug once
> it's submitted?
> 
> It would be nice if we can give people some instructions once the bug is
> submitted so they get them via bugzilla notification email.

yes, that's possible but it's a weird way provide instructions and isn't an approach that we should take.


instead i suggest we display a page with your instructions when the bug is submitted, which includes a link to show_bug (we already do this for some other custom bug forms).  if you'd still prefer to send an email we can do that without adding it as a comment, but it seems odd to send an email when we already have the user's attention.

in either case we need the text to display/send.
Flags: needinfo?(nukeador)
(Reporter)

Comment 3

2 years ago
The point of having this first comment is that we need the people cc'ed from "Mozilla contributors who vouch your application" to chime in the bug and comment why they vouch this person.

If that's not possible, it would be good to have this text after submitting the bug:

"
Thanks for submitting your application to the Reps Program.

What happens next:

* The mozillians who vouched you should comment on the application bug endorsing your application. Please contact them to make sure they endorse you, this is needed for your application to be complete.
* The application will be reviewed after you are endorsed and if it fits all requirements it will be placed in the mentorship queue.
* Once a Rep mentor has a free slot, he will be assigned to do the final review.

This might take some time, so we apologize for any delays and thank you for your patience.
"

Oh, and I've just seen another thing we should fix in the form. On the top of the form there is a link to "contact the Reps Council for assistance", the correct email is reps-council@mozilla.com. Right now it's the mailing list email which will bounce since people are not subscribed.

Thanks!
Flags: needinfo?(nukeador)
(In reply to Rubén Martín [:Nukeador] from comment #3)
> The point of having this first comment is that we need the people cc'ed from
> "Mozilla contributors who vouch your application" to chime in the bug and
> comment why they vouch this person.

ah, sorry, i thought the audience of the instructions was the bug submitter, not the vouchers.

in light of that, a comment makes sense... especially if we needinfo the vouchers when adding that comment.

what should the content of that comment be?

> "
> Thanks for submitting your application to the Reps Program.
> 
> What happens next:
> 
> * The mozillians who vouched you should comment on the application bug
> endorsing your application. Please contact them to make sure they endorse
> you, this is needed for your application to be complete.
> * The application will be reviewed after you are endorsed and if it fits all
> requirements it will be placed in the mentorship queue.
> * Once a Rep mentor has a free slot, he will be assigned to do the final
> review.
> 
> This might take some time, so we apologize for any delays and thank you for
> your patience.
> "

this looks good.  we'd probably want to add a link to show_bug too.
Flags: needinfo?(nukeador)
(Reporter)

Comment 5

2 years ago
(In reply to Byron Jones ‹:glob› from comment #4)
> (In reply to Rubén Martín [:Nukeador] from comment #3)
> > The point of having this first comment is that we need the people cc'ed from
> > "Mozilla contributors who vouch your application" to chime in the bug and
> > comment why they vouch this person.
> 
> ah, sorry, i thought the audience of the instructions was the bug submitter,
> not the vouchers.
> 
> in light of that, a comment makes sense... especially if we needinfo the
> vouchers when adding that comment.
> 
> what should the content of that comment be?

If we can add a comment just for the people cc'ed for vouching:

"You have been added as supporter to NAME's Reps application, please access the bug<LINK> and comment why do you endorse his application. Thanks!"

> 
> > "
> > Thanks for submitting your application to the Reps Program.
> > 
> > What happens next:
> > 
> > * The mozillians who vouched you should comment on the application bug
> > endorsing your application. Please contact them to make sure they endorse
> > you, this is needed for your application to be complete.
> > * The application will be reviewed after you are endorsed and if it fits all
> > requirements it will be placed in the mentorship queue.
> > * Once a Rep mentor has a free slot, he will be assigned to do the final
> > review.
> > 
> > This might take some time, so we apologize for any delays and thank you for
> > your patience.
> > "
> 
> this looks good.  we'd probably want to add a link to show_bug too.


Yes, thanks!
Flags: needinfo?(nukeador)
(Reporter)

Comment 6

2 years ago
glob, could we have an ETA for these changes?

I need to figure out other stuff that's depending on this and just want to know more less when this is landing :)

Thanks!
Flags: needinfo?(glob)
i expect to see something up for your feedback some time next week.
Assignee: nobody → dylan
Flags: needinfo?(glob)
(Reporter)

Comment 8

2 years ago
Thanks!
(Reporter)

Comment 9

2 years ago
Hello,

Do we have updates regarding this?

Thanks!
(Assignee)

Comment 10

2 years ago
I'm pretty close to having it ready to review/feedback, definitely by Friday morning. (I am currently traveling back from FOSDEM)
Status: NEW → ASSIGNED
(Assignee)

Comment 11

2 years ago
There are some setbacks -- the bug referencing feature and the posting of the special comment #1 require a little more testing. Unless there are further problems it should be ready by US/Eastern noon.
(Assignee)

Comment 12

2 years ago
There are fairly significant problems with the voucher user list widget. Still working on this.
(Assignee)

Comment 13

2 years ago
Alright, ran into some very confusing issues with user name completion; which turned out to be a red-herring. All that is left is for me to test/fix the code that creates a needinfo for each of the vouchers. There are a few meetings on Monday, but it should be up by the end of the east coast day.
(Assignee)

Comment 14

2 years ago
Created attachment 8561796 [details] [diff] [review]
bug-1123275.patch

Sanity check this rabbit hole if you've a moment, gov'na.
Attachment #8561796 - Flags: feedback?(glob)
(Assignee)

Comment 15

2 years ago
https://bugzilla-dev.allizom.org/form.reps.mentorship is up for demo / thoughts / changes. I'm not sure adding the additional comment is a good idea. I would to add it as a different user (nobody perhaps?) but doing that has problems with group permissions. I will confer with glob/dkl in the morning.
Flags: needinfo?(nukeador)
Comment on attachment 8561796 [details] [diff] [review]
bug-1123275.patch

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

there's trailing whitespace and missing boilerplaters, but this looks mostly sane from a quick read.

::: extensions/REMO/Extension.pm
@@ +320,5 @@
> +
> +        $bug->add_comment(
> +            "You have been added as supporter to this Reps application, please comment why do you endorse their application. Thanks!"
> +        );
> +        $bug->update($bug->creation_ts);

you need to trigger bugmail::send

::: extensions/REMO/template/en/default/bug/create/create-mozreps.html.tmpl
@@ +199,5 @@
> +      </td>
> +    </tr>
> +    
> +    [% BLOCK motivation_placeholder -%]
> +    Tell us your goals and what you would be able to do as a Rep that you can't do right now

this block should be indented

::: extensions/REMO/web/js/moz_reps.js
@@ +6,5 @@
> +  var first_time = $("#first_time");
> +  first_time.prop("checked", true);
> +  first_time.change(function(evt) {
> +    if (!this.checked) {
> +      $("#prior_bug").removeClass("bz_default_hidden");

since you're refactoring anyhow, it would be nice to drop bz_default_hidding, and use style="display:none" and $(..).show() instead.

@@ +29,5 @@
> +
> +  $('#submit').click(function () {
> +    var cc = document.getElementById('cc');
> +    if (cc.value) {
> +      cc.setCustomValidity('');

setCustomValidity() isn't a widely supported method; these forms need to work in all browsers not just firefox.
Attachment #8561796 - Flags: feedback?(glob) → feedback+
(Reporter)

Comment 17

2 years ago
A few comments:

* mozillians url format doesn't allow https://mozillians.org/u/nukeador or other l10n aware urls https://mozillians.org/es/u/nukeador
* "Mozilla contributors who vouch your application" should be mandatory.
* On the top of the form " If you have questions while completing this form, please contact the Reps Council for assistance." the linked email should be reps-council@mozilla.org.
* When I submit the form I get "You must enter a summary for this bug".
Flags: needinfo?(nukeador)
(Assignee)

Comment 18

2 years ago
(In reply to Rubén Martín [:Nukeador] from comment #17)
> A few comments:
> 
> * mozillians url format doesn't allow https://mozillians.org/u/nukeador or
> other l10n aware urls https://mozillians.org/es/u/nukeador
> * "Mozilla contributors who vouch your application" should be mandatory.
> * On the top of the form " If you have questions while completing this form,
> please contact the Reps Council for assistance." the linked email should be
> reps-council@mozilla.org.
> * When I submit the form I get "You must enter a summary for this bug".

I'll attend to these and the notes from glob's feedback & have the revised version up today. Thanks!
(Assignee)

Comment 19

2 years ago
Created attachment 8562808 [details] [diff] [review]
bug-1123275.patch

We talked about checking if dependson can be set -- but there doesn't appear to be a hook to do that, and I'm not sure we could handle that (unlikely) error any better in a custom way.
Attachment #8561796 - Attachment is obsolete: true
Attachment #8562808 - Flags: review?(glob)
(Assignee)

Comment 20

2 years ago
As it would happen, the dev server is out of sync with production. As such the form cannot be demoed until dev is brought into line, which I am doing now. Unfortunately I don't have a good ETA on this -- but I imagine we'll still get this form reviewed and out in next week's push.
(Assignee)

Updated

2 years ago
Attachment #8562808 - Flags: review?(glob)
(Assignee)

Comment 21

2 years ago
Created attachment 8563167 [details] [diff] [review]
bug-1123275.patch

With copyright notice on .js file.
(note: the css has no copyright notice, but it was pre-existing. Is that a bug?)
Attachment #8562808 - Attachment is obsolete: true
Attachment #8563167 - Flags: review?(glob)
Comment on attachment 8563167 [details] [diff] [review]
bug-1123275.patch

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

this mostly looks good; i quite like the simplicity jquery brings to our js.

don't revert the changes to the itrequest form please :)

> (note: the css has no copyright notice, but it was pre-existing. Is that a bug?)
new files must have a boilerplate.
there's old files which do not; there's no harm in fixing that while touching the file however this isn't a requirement.

::: extensions/REMO/Extension.pm
@@ +319,5 @@
> +        } grep { $_->is_enabled && $original_cc{$_->login}} @{$bug->cc_users};
> +
> +        $bug->set_flags(\@new_flags, []) if @new_flags;
> +        $bug->add_comment(
> +            "You have been added as supporter to this Reps application, please comment why do you endorse their application. Thanks!"

for clarity i think this should include the endorser's name(s) in the comment.
eg.  "Spongebob, Patrick: you have been added.."

::: extensions/REMO/template/en/default/bug/create/create-mozreps.html.tmpl
@@ +82,5 @@
> +    <tr class="even">
> +      <th><label class="required" for="sex">Sex:</label></th>
> +      <td>
> +        <select id="sex" name="sex">
> +          <option value="">...</option>

... is weird here, just leave it blank

@@ +113,5 @@
> +
> +    <tr class="odd">
> +      <th><label class="required" for="mozillian">Mozillians.org Account URL:</label></th>
> +      <td>
> +        <input type="url" id="mozillian" name="mozillian" size="40" placeholder="https://mozillians.org/u/name">

the mozilla skin doesn't style type="url" fields correctly.
please add input[type=url] to the selector in the skin (don't forget there's two editions of the mozilla skin to touch).

::: extensions/REMO/web/js/moz_reps.js
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/.
> + *
> + * This Source Code Form is "Incompatible With Secondary Licenses", as
> + * defined by the Mozilla Public License, v. 2.0. */
> +/*global $ */

what's this?

@@ +30,5 @@
> +        else {
> +            $('#underage_warning').hide();
> +            $('#submit').prop("disabled", false);
> +        }
> +    });

for both of these handlers you should trigger them on page load, so the visibility is correct if the page is refreshed with values set
ie. $('#..').change(function() { .. }).change();

@@ +33,5 @@
> +        }
> +    });
> +
> +    $('#tmRequestForm').submit(function (event) {
> +        var mozillian_re = /https?:\/\/mozillians.org\/([^\/]+\/)?u\/[^\/]+/i;

probably should anchor that to the start of the string
Attachment #8563167 - Flags: review?(glob) → review-
(Assignee)

Comment 23

2 years ago
https://bugzilla-dev.allizom.org/form.reps.mentorship is up, but it does not include the changes from glob's review yet.
(Assignee)

Comment 24

2 years ago
Created attachment 8564449 [details] [diff] [review]
bug-1123275.patch

(In reply to Byron Jones ‹:glob› from comment #22)


> don't revert the changes to the itrequest form please :)
The patches are always produced for review, but are never the "original source" for my changes. :)
I will try to "git pull" on master before generating patches though.


> ::: extensions/REMO/Extension.pm
> @@ +319,5 @@
> > +        } grep { $_->is_enabled && $original_cc{$_->login}} @{$bug->cc_users};
> > +
> > +        $bug->set_flags(\@new_flags, []) if @new_flags;
> > +        $bug->add_comment(
> > +            "You have been added as supporter to this Reps application, please comment why do you endorse their application. Thanks!"
> 
> for clarity i think this should include the endorser's name(s) in the
> comment.
> eg.  "Spongebob, Patrick: you have been added.."
Done.

> ::: extensions/REMO/template/en/default/bug/create/create-mozreps.html.tmpl
> @@ +82,5 @@
> > +    <tr class="even">
> > +      <th><label class="required" for="sex">Sex:</label></th>
> > +      <td>
> > +        <select id="sex" name="sex">
> > +          <option value="">...</option>
Fixed.

> @@ +113,5 @@
> > +
> > +    <tr class="odd">
> > +      <th><label class="required" for="mozillian">Mozillians.org Account URL:</label></th>
> > +      <td>
> > +        <input type="url" id="mozillian" name="mozillian" size="40" placeholder="https://mozillians.org/u/name">
> 
> the mozilla skin doesn't style type="url" fields correctly.
> please add input[type=url] to the selector in the skin (don't forget there's
> two editions of the mozilla skin to touch).
Done. One of the files hadn't been updated with the type=email field either, I fixed that in this patch.

> ::: extensions/REMO/web/js/moz_reps.js
> @@ +3,5 @@
> > + * You can obtain one at http://mozilla.org/MPL/2.0/.
> > + *
> > + * This Source Code Form is "Incompatible With Secondary Licenses", as
> > + * defined by the Mozilla Public License, v. 2.0. */
> > +/*global $ */
> what's this?
> 
I can remove it -- but that's the standard jshint "declare global variable" thing. Several js tools use the same notation, including the javascript parser in my editor.

> for both of these handlers you should trigger them on page load, so the
> visibility is correct if the page is refreshed with values set
> ie. $('#..').change(function() { .. }).change();
Good catch!

Note: I would also do validation in change() handlers, but this does not work with the cc field so I opted to only do so at submit time.


> probably should anchor that to the start of the string
Durp. The html5 pattern= attribute seemed to be auto-anchored, so I had removed them from that RE originally.
(Assignee)

Comment 25

2 years ago
https://bugzilla-dev.allizom.org/form.reps.mentorship should be functional and have the requested changes. Let me know if it meets your requirements! (or if I've missed something!)
Flags: needinfo?(nukeador)
(Assignee)

Updated

2 years ago
Attachment #8563167 - Attachment is obsolete: true
(Reporter)

Comment 26

2 years ago
Thanks!

Some details:

* The privacy policy check is marked as mandatory but I can send the form without checking it.
* The post-submit page doesn't have the updated message.

"
Thanks for submitting your application to the Reps Program.

What happens next:

* The mozillians who vouched you should comment on the application bug endorsing your application. Please contact them to make sure they endorse you, this is needed for your application to be complete.
* The application will be reviewed after you are endorsed and if it fits all requirements it will be placed in the mentorship queue.
* Once a Rep mentor has a free slot, he will be assigned to do the final review.

This might take some time, so we apologize for any delays and thank you for your patience.
"
Flags: needinfo?(nukeador)
(Assignee)

Comment 27

2 years ago
Both things are fixed and available on the dev server.
Flags: needinfo?(nukeador)
(Reporter)

Comment 28

2 years ago
If you add more than 1 person as Mozilla contributors who vouch your application you get:

References:
ARRAY(0x7f996a1c7b10)

Also a quick change we where suggested by Glob, change "Sex" for "Gender".
Flags: needinfo?(nukeador)
(Assignee)

Comment 29

2 years ago
(In reply to Rubén Martín [:Nukeador] from comment #28)
> If you add more than 1 person as Mozilla contributors who vouch your
> application you get:
> 
> References:
> ARRAY(0x7f996a1c7b10)
> 
> Also a quick change we where suggested by Glob, change "Sex" for "Gender".

Changes made and applied to dev.
Flags: needinfo?(nukeador)
(Assignee)

Comment 30

2 years ago
Created attachment 8565682 [details] [diff] [review]
bug-1123275-6.patch

For auto-review if required.
Attachment #8564449 - Attachment is obsolete: true
(Reporter)

Comment 31

2 years ago
Woooo, I think it's perfect now!

Thank you very much for this great effort, this is going to help A LOT to the Reps program reducing unnecessary workload.

Please, apply to production as soon as you can :)
Flags: needinfo?(nukeador)
Comment on attachment 8565682 [details] [diff] [review]
bug-1123275-6.patch

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

r=glob, with a few fix-on-commit comments.

::: extensions/REMO/Extension.pm
@@ +320,5 @@
> +        } @cc_users;
> +
> +        $bug->set_flags(\@new_flags, []) if @new_flags;
> +        $bug->add_comment(
> +            join(", ", map { $_->identity } @cc_users) .

using the identity here looks weird, i think because $realname <$email> is not very conversational.

Byron Jones ‹:glob› <glob@mozilla.com>: You have been added as supporter to this Reps application, please comment why do you endorse their application. Thanks!

i think it should be { $_->realname || $_->login_name }

::: extensions/REMO/web/js/moz_reps.js
@@ +3,5 @@
> + * You can obtain one at http://mozilla.org/MPL/2.0/.
> + *
> + * This Source Code Form is "Incompatible With Secondary Licenses", as
> + * defined by the Mozilla Public License, v. 2.0. */
> +/*global $ */

remove this on commit

@@ +42,5 @@
> +        }
> +    }).change();
> +
> +    $('#tmRequestForm').submit(function (event) {
> +        var mozillian_re = /^https?:\/\/mozillians.org\/([^\/]+\/)?u\/[^\/]+$/i;

we should also support www.mozillians.org: http://www.mozillians.org/u/glob
Attachment #8565682 - Flags: review+
(Assignee)

Comment 33

2 years ago
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   fab5c12..f193116  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
(Reporter)

Comment 34

2 years ago
Everyone involved in making this happen, THANK YOU!

You have helped to reduce our daily work load a lot.

:D
Blocks: 1136687
(Reporter)

Comment 35

2 years ago
Hey, is the form working right on production? As far as I see it should be https://bugzilla.mozilla.org/form.reps.mentorship

The weird thing is that I keep seeing applications arriving using the old format/fields.
Status: RESOLVED → REOPENED
Flags: needinfo?(dylan)
Resolution: FIXED → ---
(Reporter)

Comment 36

2 years ago
Also, we are getting tons of duplicates, I don't know if related with the new form:

https://bugzilla.mozilla.org/buglist.cgi?bug_status=UNCONFIRMED&bug_status=NEW&component=Mentorship&email1=nukeador%40mozilla-hispano.org&emailtype1=substring&product=Mozilla%20Reps&query_format=advanced&query_based_on=Reps%20applicants&columnlist=product%2Ccomponent%2Cassigned_to%2Cbug_status%2Cresolution%2Cshort_desc%2Cchangeddate%2Cassigned_to_realname%2Creporter%2Cstatus_whiteboard%2Cpriority&list_id=12046412

Also these new applications doesn't seems to be using the updated form (people vouching are not in needinfo for example).
please don't reopen bugs that have been fixed and deployed; it's always better to file new bugs to track new issues.

yes, it's been deployed.  please file a new bug with examples so we can diagnose this issue.

bug 1136687 would have resulted in an error being shown during bug creation, which i suspect is causing people to resubmit their request.
Status: REOPENED → RESOLVED
Last Resolved: 2 years ago2 years ago
Flags: needinfo?(dylan)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.