Closed Bug 1123275 Opened 9 years ago Closed 9 years ago

Changes to form.reps.mentorship

Categories

(bugzilla.mozilla.org :: Custom Bug Entry Forms, defect)

Product:
bugzilla.mozilla.org
Component:
Custom Bug Entry Forms
Version:
Production
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: Nukeador, Assigned: dylan)

References

(
URL
)

Details

Attachments

(1 file, 4 obsolete files)

bug-1123275-6.patch
22.48 KB, patch
glob
: review+
Details | Diff | Splinter Review 
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!
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.
URL: https://bugzilla.mozilla.org/form.rep...
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)
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)
(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)
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)
Hello,

Do we have updates regarding this?

Thanks!
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
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.
There are fairly significant problems with the voucher user list widget. Still working on this.
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.
Attached patch bug-1123275.patch (obsolete) — — Splinter Review 
Sanity check this rabbit hole if you've a moment, gov'na.
Attachment #8561796 - Flags: feedback?(glob)
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+
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)
(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!
Attached patch bug-1123275.patch (obsolete) — — Splinter Review 
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)
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.
Attachment #8562808 - Flags: review?(glob)
Attached patch bug-1123275.patch (obsolete) — — Splinter Review 
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-
https://bugzilla-dev.allizom.org/form.reps.mentorship is up, but it does not include the changes from glob's review yet.
Attached patch bug-1123275.patch (obsolete) — — Splinter Review 
(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.
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)
Attachment #8563167 - Attachment is obsolete: true
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)
Both things are fixed and available on the dev server.
Flags: needinfo?(nukeador)
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)
(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)
For auto-review if required.
Attachment #8564449 - Attachment is obsolete: true
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+
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   fab5c12..f193116  master -> master
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Everyone involved in making this happen, THANK YOU!

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

:D
Blocks: 1136687
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 → ---
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
Closed: 9 years ago → 9 years ago
Flags: needinfo?(dylan)
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: