Skip to content

Tell us more app. #40

Closed
wants to merge 32 commits into from

3 participants

@nigelbabu

Eventually, this will get merged into input, but for now, its for status checking + ongoing code review.

@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/forms.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
@davedash
Mozilla member
davedash added a note Dec 29, 2011

I find it odd to add this line, since this file isn't meant to be cmd line executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/forms.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
@davedash
Mozilla member
davedash added a note Dec 29, 2011

I don't see any UTF-8 in this file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/forms.py
@@ -0,0 +1,36 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+from django import forms
+
+from tower import ugettext as _, ugettext_lazy as _lazy
+
+class TellUsMoreForm(forms.Form):
+ summary = forms.CharField(max_length=100,
+ required=True,
+ label=_lazy("What's a short summary of what you're seeing!"))
@davedash
Mozilla member
davedash added a note Dec 29, 2011

"Short summary of what you are seeing"

No exclamation points.

@davedash
Mozilla member
davedash added a note Dec 29, 2011

also, align your parameters better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/templates/tellusmore/form.html
+ <p>{{ _('Your feedback helps us improve Firefox.') }}</p>
+ </header>
+
+ <aside>
+ <span>
+ {% trans support_url='http://support.mozilla.com' %}
+ If you need help or have a problem
+ with Firefox, please visit <a href="{{ support_url }}">Firefox Support</a>.
+ {% endtrans %}
+ </span>
+ </aside>
+
+ <section class="content" id="bugform">
+ <form action="" method="post" autocomplete="off">
+ <div class="fields">
+ {% for field in form %}
@davedash
Mozilla member
davedash added a note Dec 29, 2011

is the order guaranteed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/templates/tellusmore/form.html
+
+ <section class="content" id="bugform">
+ <form action="" method="post" autocomplete="off">
+ <div class="fields">
+ {% for field in form %}
+ <div>
+ {{ field.label_tag() }}
+ <p>{{ field }}</p>
+ {{ field.errors }}
+ </div>
+ {% endfor %}
+ </div>
+
+ {{ form.errors['__all__'] }}
+
+ <div id="{{ type }}-submit" class="submit">
@davedash
Mozilla member
davedash added a note Dec 29, 2011

I feel like you can do this with just the button, and no noscript tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Dec 29, 2011
media/css/feedback.css
@@ -463,7 +463,9 @@ footer form option {
#intro aside a {
font-weight: normal;
}
-
+#intro #bugform {
+text-align: left;
@davedash
Mozilla member
davedash added a note Dec 29, 2011

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash
Mozilla member

so far, so good.

@davedash davedash commented on an outdated diff Dec 29, 2011
apps/tellusmore/forms.py
@@ -0,0 +1,42 @@
+from django import forms
+
+from tower import ugettext as _, ugettext_lazy as _lazy
+
+class TellUsMoreForm(forms.Form):
+ summary = forms.CharField(max_length=100,
+ required=True,
@davedash
Mozilla member
davedash added a note Dec 29, 2011

When it's this weird, I'd put a new line after ( and indent max_length 8 spaces. You could fit this all cleanly in 2 lines.

Do the same with all these defs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@nigelbabu

Things start to work now. The attachment handling bit is not ready and the thanks pages could use some love. Some of the things I need a sanity check and/or I need clarifications are

  • I had to create a template filter is_checkbox to check if a field is a checkbox, I wonder if there's a simpler way
  • I haven't figured out the point of updates form field. I don't know what I'd do different in case it was checked.
  • The bugzilla interface takes an optional name field, we don't ask for this. Should we?
  • I don't think the form has CSRF protection. Is that fine?
  • After this form is submitted, redirecting to our usual thanks page is fine?
  • I'm using enforce_ua to get the UA. Is that fine?
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/forms.py
@@ -0,0 +1,36 @@
+from django import forms
+
+from tower import ugettext as _, ugettext_lazy as _lazy
+
@davedash
Mozilla member
davedash added a note Jan 25, 2012

run check.py

@davedash
Mozilla member
davedash added a note Jan 26, 2012

which should say here "expected 2 lines, got 1"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/forms.py
@@ -0,0 +1,36 @@
+from django import forms
+
+from tower import ugettext as _, ugettext_lazy as _lazy
+
+class TellUsMoreForm(forms.Form):
+ summary = forms.CharField(
+ max_length=100, required=True, label=_lazy("What's a short "
@davedash
Mozilla member
davedash added a note Jan 25, 2012

split it before label, so the string isn't cut in half

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/helpers.py
@@ -0,0 +1,9 @@
+from jingo import register
+
+@register.filter
+def is_checkbox(ob):
+ """Checks if a form element is a checkbox."""
+ if ob.widget.__class__.__name__ == 'CheckboxInput':
@davedash
Mozilla member
davedash added a note Jan 25, 2012

or

def whatever(ob):  
    return ob.widget.__class__.__name__ == 'CheckboxInput'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/views.py
+
+from django import http
+from django.conf import settings
+from django.shortcuts import render
+
+from session_csrf import anonymous_csrf_exempt
+import xmlrpclib
+
+from input.urlresolvers import reverse
+from feedback.views import enforce_ua
+from tellusmore.forms import TellUsMoreForm
+
+@enforce_ua
+@anonymous_csrf_exempt
+def bugform(request, ua):
+ """
@davedash
Mozilla member
davedash added a note Jan 25, 2012

one line full stop at the end.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/views.py
+from input.urlresolvers import reverse
+from feedback.views import enforce_ua
+from tellusmore.forms import TellUsMoreForm
+
+@enforce_ua
+@anonymous_csrf_exempt
+def bugform(request, ua):
+ """
+ Page to file a bug via Tell Us More
+ """
+ if request.method == 'POST':
+ form = TellUsMoreForm(request.POST)
+ if form.is_valid():
+ # Get the data from the cleaned form data
+ description = ("%s\n\nExpected Result\n%s\n\nActual Result\n%s" %
+ (form.cleaned_data['description'],
@davedash
Mozilla member
davedash added a note Jan 25, 2012

indent so this is one space in (under the "

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/views.py
+from feedback.views import enforce_ua
+from tellusmore.forms import TellUsMoreForm
+
+@enforce_ua
+@anonymous_csrf_exempt
+def bugform(request, ua):
+ """
+ Page to file a bug via Tell Us More
+ """
+ if request.method == 'POST':
+ form = TellUsMoreForm(request.POST)
+ if form.is_valid():
+ # Get the data from the cleaned form data
+ description = ("%s\n\nExpected Result\n%s\n\nActual Result\n%s" %
+ (form.cleaned_data['description'],
+ form.cleaned_data['actual_result'],
@davedash
Mozilla member
davedash added a note Jan 25, 2012

these should be inside the open parens for the tuple

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Jan 25, 2012
apps/tellusmore/views.py
+ print form.cleaned_data['security']
+ data = {
+ 'creator': form.cleaned_data['email'],
+ 'product': 'Firefox',
+ 'summary': form.cleaned_data['summary'],
+ 'description': description,
+ 'user_agent': ua,
+ 'restricted': 0,
+ 'Bugzilla_login': settings.BMO_LOGIN,
+ 'Bugzilla_password': settings.BMO_PASSWORD,
+ }
+ if form.cleaned_data['security']:
+ data['restricted'] = 1
+ if len(form.cleaned_data['url']) > 0:
+ data['url'] = form.cleaned_data['url']
+ print form.cleaned_data
@davedash
Mozilla member
davedash added a note Jan 25, 2012

print?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on the diff Feb 1, 2012
apps/tellusmore/forms.py
+
+
+class TellUsMoreForm(forms.Form):
+ summary = forms.CharField(
+ max_length=100, required=True, label=_lazy(
+ "What's a short summary of what you're seeing"))
+ description = forms.CharField(
+ required=True, widget=forms.Textarea, label=_lazy("Please "
+ "describe your issue"))
+ actual_result = forms.CharField(
+ required=True, widget=forms.Textarea, label=_lazy("What happened?"
+ ))
+ expected_result = forms.CharField(
+ required=True, widget=forms.Textarea, label=_lazy("What should "
+ "have happened?"))
+ #TODO: Make this required and deal with file uploads later.
@davedash
Mozilla member
davedash added a note Feb 1, 2012

# TODO

and can't people submit errors without screenshots?

@nigelbabu
nigelbabu added a note Feb 2, 2012

I'm not entirely sure. It was a required field in the mockup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on the diff Feb 3, 2012
apps/tellusmore/forms.py
+ expected_result = forms.CharField(
+ required=True, widget=forms.Textarea, label=_lazy("What should "
+ "have happened?"))
+ #TODO: Make this required and deal with file uploads later.
+ attachment = forms.FileField(
+ required=False, label=_lazy("Attach a screenshot of your issue"))
+ email = forms.EmailField(
+ required=True, label=_lazy("We'll need to get back to your if "
+ "we're having trouble reproduction the issue, so please give us a"
+ "contact e-mail address (or bugzilla e-email address if you have "
+ "it) in order to file this bug."))
+ url = forms.URLField(
+ required=False, label=_lazy("Is there a specific website related "
+ "to your issue? If so, please add its URL."))
+ security = forms.BooleanField(
+ required=False, label=_lazy("Many users could be harmed by this "
@davedash
Mozilla member
davedash added a note Feb 3, 2012

These labels are huge... ok, but huge...

Maybe they can be class constants at the top?

FOO = _lazy('foo bar')
form.Field(label=FOO)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@davedash davedash commented on an outdated diff Feb 3, 2012
apps/tellusmore/tasks.py
@@ -0,0 +1,28 @@
+from celery.decorators import task
+from django.conf import settings
+import xmlrpclib
+
@davedash
Mozilla member
davedash added a note Feb 3, 2012

check.py hasn't complained

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
nigelbabu and others added some commits Dec 29, 2011
@nigelbabu nigelbabu First iteration, display the tellusmore form de3572f
@nigelbabu nigelbabu Updates after dave's initial review 99e4fb6
@nigelbabu nigelbabu textareas where necessary 3672bb5
@nigelbabu nigelbabu Whitespace cleanup cfc134d
@nigelbabu nigelbabu Got checkbox to render properly b74dda4
@nigelbabu nigelbabu WORKS! OMG IT WORKS! OMGOMGOMG 4732c0a
@nigelbabu nigelbabu Removed some code and and removed helper 4637dbf
@nigelbabu nigelbabu Removed helper, changed some indentation to keep davedash happy c18e4e3
@nigelbabu nigelbabu Removed the shebang and encoding line aecf060
@nigelbabu nigelbabu Fixed a typo 97288a0
@nigelbabu nigelbabu Mobile layout added as well faaba55
@nigelbabu nigelbabu Move bugzilla calls into a celery task 2fd3e34
@nigelbabu nigelbabu Start handling file uploads 518210d
@nigelbabu nigelbabu File upload done 8145335
@nigelbabu nigelbabu Added bug image b2cdd30
@nigelbabu nigelbabu Added the boxes to link to bug page 66b9b7a
@nigelbabu nigelbabu update with check.py 0ab3185
Nigel Babu Copied the thank you pages over ebdde8e
Nigel Babu Updated tellusmore urls 728ccab
Nigel Babu Added thanks page, and template f7689dc
Nigel Babu Updated thanks page. Switch to POST b284e20
Nigel Babu cast into int, and fix the dict bug fe88258
Nigel Babu Added mobile version of thanks page 63c1fe9
Nigel Babu Add mobile templates for everything 254a327
Nigel Babu Get everything working with forms and POST 3c8c918
Nigel Babu Removed print 39bb405
Nigel Babu Try to display errors from the form validation f7000ce
Nigel Babu Show errors form validation correctly d46b646
Nigel Babu Added the checksum option and corresponding validation 2f79a6d
Nigel Babu Documentation for tell us more updated df1c65a
Nigel Babu Updated headers with the right message 3df511e
Nigel Babu updated gitignore 4a79e76
@mythmon
Mozilla member
mythmon commented Nov 21, 2012

Not entirely sure what this is... but it is almost a year old with no action, so I'm closing it. Re-open if desired.

@mythmon mythmon closed this Nov 21, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.