Closed
Bug 1123275
Opened 10 years ago
Closed 10 years ago
Changes to form.reps.mentorship
Categories
(bugzilla.mozilla.org :: Custom Bug Entry Forms, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: Nukeador, Assigned: dylan)
References
()
Details
Attachments
(1 file, 4 obsolete files)
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!
Reporter | ||
Comment 1•10 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•10 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•10 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•10 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•10 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•10 years ago
|
||
Thanks!
Reporter | ||
Comment 9•10 years ago
|
||
Hello,
Do we have updates regarding this?
Thanks!
Assignee | ||
Comment 10•10 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•10 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•10 years ago
|
||
There are fairly significant problems with the voucher user list widget. Still working on this.
Assignee | ||
Comment 13•10 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•10 years ago
|
||
Sanity check this rabbit hole if you've a moment, gov'na.
Attachment #8561796 -
Flags: feedback?(glob)
Assignee | ||
Comment 15•10 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 16•10 years ago
|
||
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•10 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•10 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•10 years ago
|
||
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•10 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•10 years ago
|
Attachment #8562808 -
Flags: review?(glob)
Assignee | ||
Comment 21•10 years ago
|
||
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 22•10 years ago
|
||
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•10 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•10 years ago
|
||
(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•10 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•10 years ago
|
Attachment #8563167 -
Attachment is obsolete: true
Reporter | ||
Comment 26•10 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•10 years ago
|
||
Both things are fixed and available on the dev server.
Flags: needinfo?(nukeador)
Reporter | ||
Comment 28•10 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•10 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•10 years ago
|
||
For auto-review if required.
Attachment #8564449 -
Attachment is obsolete: true
Reporter | ||
Comment 31•10 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 32•10 years ago
|
||
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•10 years ago
|
||
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
fab5c12..f193116 master -> master
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 34•10 years ago
|
||
Everyone involved in making this happen, THANK YOU!
You have helped to reduce our daily work load a lot.
:D
Reporter | ||
Comment 35•10 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•10 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).
Comment 37•10 years ago
|
||
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: 10 years ago → 10 years ago
Flags: needinfo?(dylan)
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•