411794 - T-shirt sign-up form for extension developers

Status: RESOLVED FIXED

(addons.mozilla.org Graveyard :: Administration, defect)

Description

[Alex Polvi]

When we released Firefox 2, we offered t-shirts to extension developers who got their extension in the review queue by a certain date. Is that form still around? We're going to need something similar setup for Firefox 3, just with different timeline, dates, etc.

Comment 1

[Alex Polvi]

Thanks for pointing this out fligtar... we'll need to re-implement it for remora. I'll work on coming up with a req.

Comment 2

[Alex Polvi]

Hey guys, 

We need a webpage that includes a bit of information about the contest, along with a form to collect the data. 

* Should exist at:  Home > Add-ons > Developer Control Panel > T-Shirt Request
* Eligibility: Must have at least one sandboxed (or live) addon claming Firefox 3.0b3 (or greater) compatibility by the deadline, or be an editor/reviewer
* Deadline: Beta 3 release date (TBD) + 3 weeks

I used * to indicate a required field. We do not know which fields will be required for their address -- so if they mess that up, well, no shirt. :)

Also, we're going to need to query out the collected data shortly after the deadline. A CSV would be great.

The form: 

Are you an addons developer or editor? If so, you have a shirt waiting with your name on it. Developers just need to get their addons compatible (and submitted to addons.mozilla.org) by [deadline]. The following information is required to receive your shirt.

 - Full name* - "Real name for t-shirt delivery only; will not be displayed publicly"
 - Email* 
 - Country* (drop down, ala http://tinyurl.com/2nt6tq) - "Full shipping address" 
 - Address 
 - Address 2
 - City
 - State
 - Region/Province
 - ZIP/Postal Code
 - Telephone* - "May be required by delivery service (Please include country and area code)" 
 - T-Shirt Size* - Men-S,M,L,XL,2XL, Women-S,M,L,XL

The information above is collected in accordance with Mozilla's Privacy Policy. [link to privacy policy]

Additionally, if your addon is one of the top 50 most used addons by Firefox 3 users on the Firefox 3 release day, Mozilla will offer to sponsor a party for you and your friends. That is, Mozilla will chip in for you and your friends to celebrate your tools success! Mozilla will be in contact with the top addons shortly after the Firefox 3 release. Keep those extensions up to date!

[submit]

Comment 3

[Alex Polvi]

and, I'll kick off the feature creep.

Lets also have an optional "comment" text area for who knows what. 

Comment 4

[Alex Polvi]

This is probably implied, but I wanted to state it explicitly... we should record the ID of the logged in user with the other form information.

Comment 5

[Michael Morgan [:morgamic]]

Unless there's a reason to store it elsewhere, it'll be in the user's table, along with their actual user record.  Users can only fill in stuff for themselves.  So it'll be like what we did last time.

Comment 6

[Chris Pollett]

I am not 100% sure what people would like the left menu box to look like. My
thought is to view the submission as something like a two-step wizard: step 1
being fill out the form, step 2 being receive confirmation. The only reason I
am thinking along these lines is to make the interface look similar to submit
add-on.  
On the other hand, it is a little bit silly given that it is only two-steps.

Comment 7

[Justin Scott [:fligtar]]

The way it was done before, the left sidebar of the Dev CP had a "T-shirt Request" item that went to the form. I don't think we need a multi-step process for this. For confirmation, we can do what was done before and when you go back to the T-shirt request form, it says that your request was submitted but that you can still make any modifications you need, and have the same form but with the values filled in.

Comment 8

[Chris Pollett]

So there would be no left sidebar on the page with the T-Shirt form? I don't know what the old page looked like so I tried to look up 2006 caches of the addons site at the internet archive, but it seems the archive won't serve the caches it has right now.

Comment 9

[Justin Scott [:fligtar]]

The left sidebar could just be the same one that's used in a few other Dev CP places (https://addons.mozilla.org/en-US/firefox/developers). I think it's the myaddons.thtml element. You can add the t-shirt item to it.

Comment 10

[Chris Pollett]

That's the way I had started coding it and then I began worrying. (Right now, commented out when I thought that no left sidebar was wanted.) My fear was the crazy person who decides to submit an add-on by clicking on the submit add-on link while on the t-shirt form page. I wanted to either have no sidebar or a sidebar that didn't have this kind of issue. If I do it this way, I will just code away the potential recursive bread crumb headache.

Comment 11

[Justin Scott [:fligtar]]

(In reply to comment #10)
> My fear was the crazy person who decides to submit an add-on by clicking on
> the submit add-on link while on the t-shirt form page. 

I don't understand why this would be a problem. The link to the submit add-on page should work fine.

> If I do it this way, I will just code away the potential recursive bread 
> crumb headache.

I'm also not quite sure what you mean here.

Comment 12

[Chris Pollett]

>> If I do it this way, I will just code away the potential recursive bread 
>> crumb headache.

>I'm also not quite sure what you mean here.

Sorry, I made up the name recursive bread crumb for a situation where you have a
bread crumb trail like

Home > Add-ons > Developer Control Panel > T-Shirt Request > Submit Add-on >  T-Shirt Request > Submit Add-on > ...

because after clicking T-Shirt Request, they click submit add-on, then they click T-shirt Request, etc. Presumably, allowing people to nest this too much might break something?

Comment 13

[Chris Pollett]

In quick experimenting I see the bread crumbs thing doesn't work that way so never mind. I have the panel as you suggest.

Comment 14

[Alex Polvi]

Hey -- just checking in. What is our ETA? We're going to have beta3 in week'ish. 

Comment 15

[Chris Pollett]

Hey Alex,

I am getting very close to done. Hopefully, have something finished for review today.

Comment 16

[Chris Pollett]

Attachment: Okay. Here is my first attempt at a T-shirt Request Form

This patch provides a link off the developer panel to request a t-shirt and off the admin panel to get a cvs file of those who've made request. It also creates
a country helper which may be useful, if people need to output a list of countries.

I wasn't sure on the best way to localize country names. (Or if needed to do at all). Also, debated back and forth between going to an independent page when request successful versus flash message.

Comment 17

[Michael Morgan [:morgamic]]

Comment on attachment 301614 [details] [diff] [review]
Okay. Here is my first attempt at a T-shirt Request Form

Hey Chris, looks good and thanks for the hard work.  Couple of things:
- Don't need to localize country names, for addresses this is less important in my opinion (someone feel free to disagree with me!)
- Why are there so many lines where we have substitutions for ^M?  Did you strip out line breaks in some files or something?  The diff shows you rewriting the entire app_controller.php

So next, try to strip out localized country stuff and figure out why the diff has you changing every line then re-upload another version of the patch.

Comment 18

[Chris Pollett]

Attachment: second pass

Here is a second pass at a patch. I guess Dreamweaver was only outputting linefeeds on what I was outputting before. I changed this so now an doing
windows end of lines. The made the diffs of the code I created seem more reasonable. Now the bulk of the patch is localization strings. The basic issue
here is that line numbers are printed in the file messages.po and so change with my t-shirt strings. I got rid of the country localization stuff I had before, though, so I think I've done both requested changes.

Comment 19

[Chris Pollett]

Attachment: pass 2b

Missed some files on previous patch. Trying again.

Comment 20

[Michael Morgan [:morgamic]]

Comment on attachment 301709 [details] [diff] [review]
pass 2b

Chris -- why is the entire eu messages.po changed?

Comment 21

[:Gijs (he/him)]

Probably a line ending issue?

Comment 22

[Michael Morgan [:morgamic]]

Yes, I've seen it a few times, just want to make sure we know why/how it's happening.

Comment 23

[Chris Pollett]

Attachment: pass 2c

My eyesight is bad. I think I saw eu and confused it for en, so didn't fix it in the 2b version. I have fixed the line ending issue for this file now. Also, added
to the CSV file produced in the Admin > T-shirt Viewer a line with with the column names.

Comment 24

[Alex Polvi]

checking in again... we're supposed to release Firefox beta 3 very soon ... and we had originally hoped this would be ready for that....

Comment 25

[Michael Morgan [:morgamic]]

Comment on attachment 302127 [details] [diff] [review]
pass 2c

r=morgamic, except for one fix where $attrib was used instead of array() in the form.  Fixed that, will check in shortly.

Comment 26

[Justin Scott [:fligtar]]

Comment on attachment 302127 [details] [diff] [review]
pass 2c

I haven't tested the patch, but just a few comments from me:

>    /**
>+    * T-Shirt Viewer
>+    */
>+    function tshirtviewers() {
>+        if (!$this->SimpleAcl->actionAllowed('Admin', '%', $this->Session->read('TshirtRequest'))) {
>+            $this->Amo->accessDenied();
>+        }

I think this should be actionAllowed('Admin', 'tshirtviewers', $this->Session->read('User')), although I'm not sure that it's needed at all since the ACL should check permissions by default.


>+    <?php endif; 
>+        if ($this->controller->SimpleAcl->actionAllowed('T-Shirt Request Viewer', '%', $this->controller->Session->read('User'))): ?>
>+        <li><?=$html->link('T-Shirt Request Viewer', '/admin/tshirtviewers')?></li>
>+    <?php endif; ?>	

I don't think that actionAllowed() will work, and I don't see any logic checking the developer's eligibility to receive a t-shirt.

Comment 27

[Michael Morgan [:morgamic]]

Comment on attachment 302127 [details] [diff] [review]
pass 2c

First case was fine -- only admins can see it anyway.  But I forgot about eligibility; thanks Justin.  Chris -- can you drum up a check for and submit a new patch that covers Polvi's req's:

* Eligibility: Must have at least one sandboxed (or live) addon claming Firefox
3.0b3 (or greater) compatibility by the deadline, or be an editor/reviewer

Also, the fix for the form was on line 60 of views/developers/tshirtrequest.thtml -- s/$attribe/array()/

Functionally the form worked fine (save/view) though.  :)

Comment 28

[Chris Pollett]

This text here:

Are you an addons developer or editor? If so, you have a shirt waiting with
your name on it. Developers just need to get their addons compatible (and
submitted to addons.mozilla.org) by [deadline]. The following information is
required to receive your shirt.

seems to suggest that the form gets displayed to anyone, but only after the
fact is it determined whether they have a compatible add-on. Should I just add
code to the admin view screen to only put people with a compatible add-on in
the csv?

I could also after collecting the tshirt info flash a message if they don't
have a compatible add-on that they need one by the deadline.

Comment 29

[Justin Scott [:fligtar]]

I think the way we did it previously was if the developer is eligible, we show the form and what qualified them, like:

Eligibility:
    Password Exporter (Firefox 3.0b3 compatible)
    Favloc (Firefox 3.0a1 compatible)
    AMO Editor

and if they aren't eligible, we don't show the form fields and instead say they're not eligible.

Comment 30

[Chris Pollett]

Okay. I'll do that. So that I can fill in the English string correctly in the patch I'll put up, has [deadline] gotten a value yet?

Comment 31

[Alex Polvi]

(In reply to comment #30)
> Okay. I'll do that. So that I can fill in the English string correctly in the
> patch I'll put up, has [deadline] gotten a value yet?
> 

when is this expected to land? The original plan was three weeks from beta 3 release day, but I'm not ready... and either is this. Can we figure this out once it is about to hit staging?

Comment 32

[Chris Pollett]

Attachment: Patch with eligibility logic

Here is a patch that has the eligibility logic in it. I did the check of the version just using methods from existing models so might be a little slower than if had written customized SQL. Sorry, it took me a little bit longer than said to get new patch, I was fighting with .po files in Dreamweaver (had to add my strings back a couple of times :( ).

Comment 33

[Chris Pollett]

Attachment: new patch fixed to accommodate changes that others made to remora.sql

fixed to accommodate changes that others made to remora.sql

Comment 34

[Chris Pollett]

Attachment: fix dweaver end of line issues

okay. gave up on dweaver. hopefully have fixed end of line issues

Comment 35

[Michael Morgan [:morgamic]]

Comment on attachment 303452 [details] [diff] [review]
fix dweaver end of line issues

Still have DOS linebreaks in:
+++ views/elements/developers/adminmenu.thtml   (working copy)
+++ views/elements/developers/myaddons.thtml    (working copy)
+++ views/helpers/country_list.php  (revision 0)
+++ views/admin/tshirtviewers.thtml (revision 0)
+++ controllers/developers_controller.php   (working copy)
+++ controllers/admin_controller.php    (working copy)
+++ models/tshirt_request.php   (revision 0)
Index: webroot/css/developers.css

I also get this error after applying the patch on the main /developers/ page:
Fatal error: Cannot redeclare class tshirtrequest in /home/morgamic/public_html/amo/site/app/models/tshirt_request.php on line 102

Some of the dos linebreaks are not your fault, but they should be fixed -- the dupe declaration error should definitely be fixed.

Comment 36

[Chris Pollett]

Attachment: beat end of line issue to death patch -- see comments below

My last patch I think worked (the issue was probably applying patch over old patch), but I agree there were ^M's in the tshirt model.

For this patch: I went through each file I touched to do the t-shirt, did vim to make sure not dos mode. My patch was made in the site/app directory. I tested my patch with a fresh install, verified works. This gives a patch size of 430k because several of the files in the repository already had ^M's admin_controller, developer_controller, etc. I think this might have been why my first patches last week were so large --- because dreamweaver had been set to unix mode and was deleting ^M's. To avoid this problem, I then switched dweaver into dos mode and mayhem ensued. So in creating this new patch I went for minimizing patch size avoiding introducing new ^M's (old ones left so others can tear hair out). It seems like I will be out of luck using dweaver because it makes line endings consistently one format rather than leaving things you don't change alone. This short patch was also applied to a fresh install at home to make sure worked.

The following files had pre-existing ^M's:
messages.po (english)
webroot/css/developers.css 
controllers/admin_controller.php
controllers/developers_controller.php
views/elements/developers/adminmenu.thtml
views/elements/developers/myaddons.thtml

Comment 37

[Alex Polvi]

+"get their add-ons compatible with Firefox 3.03b and submitted to addons."

s/3.03b/3.0b3/

s/addons/add-ons/

Comment 38

[Alex Polvi]

(In reply to comment #37)
> s/addons/add-ons/

well, I guess that should be addons.mozilla.org... just to be explicit.

Comment 39

[Chris Pollett]

Before I submit a patch with the one fixed string in messages.po. Let me just make sure I have it correct:

"Are you an add-ons developer or editor? If so, you could be entitled to a "
"free t-shirt. Editors are immediately eligible and developers just need to "
"get their add-ons compatible with Firefox 3.0b3 and submitted to addons."
"mozilla.org by March 10. Once you have met these criteria a form will appear "
"on this page to collect your shipping information."

That is, addons.mozilla.org stays unchanged, the firefox version typo though is corrected.

Comment 40

[Reed Loden [:reed]]

(In reply to comment #39)
> " ... Once you have met these criteria a form will appear "
> "on this page to collect your shipping information."

Comma after "criteria," please.

Comment 41

[Chris Pollett]

Attachment: fixed polvi reed typos

minor change to previous patch fixing typos as suggested

Comment 42

[Laura Thomson :laura]

Comment on attachment 303602 [details] [diff] [review]
fixed polvi reed typos

Last patch looks good to me.

Comment 43

[Laura Thomson :laura]

I should note though: svn up and regenerate, or before you commit, and also note that there are changes to remora.sql that need push.

Comment 44

[Chris Pollett]

Attachment: svn upped to this week version of previous patch

This is essentially the same patch as the last one where I have done an svn up for the last weeks changed before creating the patch. I tested this patch on a fresh install before uploading.

Comment 45

[Chris Pollett]

Attachment: Requested change March 10 -> March 18

Made Polvi's requested change of March 10 to March 18; otherwise unchanged.

Comment 46

[Michael Morgan [:morgamic]]

Checked in.  Thanks everyone -- congrats on your first patch Chris.