Skip to content

Commit 51c6b98

Browse files
author
Mike Taylor
committed
Merge pull request #579 from /issues/432/2
Fixes #432 - Give common problem types as a choice for the user
2 parents 430a6e0 + bc8be6b commit 51c6b98

6 files changed

Lines changed: 134 additions & 123 deletions

File tree

‎requirements.txt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ Flask-Cache==0.13.1
44
Flask-Limiter==0.7.4
55
Flask-SQLAlchemy==1.0
66
GitHub-Flask==0.3.4
7-
WTForms==1.0.5
7+
WTForms==2.0.2
88
ua-parser==0.3.5
99
nose==1.3.1
1010
blinker==1.3

‎tests/functional/reporting.js‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,22 +36,37 @@ define([
3636
.get(require.toUrl(url + '?open=1'))
3737
.findByCssSelector('#url').click()
3838
.end()
39-
.findByCssSelector('#summary').click()
39+
.findByCssSelector('#browser').click()
4040
.end()
41-
.findByCssSelector('#url').type('hi')
41+
.findByXpath('//*[@id="new-report"]/div/form/div[1]/div[2]/div[1]').getAttribute('class')
42+
.then(function (className) {
43+
assert.include(className, 'has-error');
44+
assert.notInclude(className, 'no-error');
45+
})
4246
.end()
43-
.findByCssSelector('#summary').click()
47+
.findByCssSelector('#url').type('sup')
4448
.end()
45-
.findByCssSelector('.u-formGroup').getAttribute('class')
49+
// xpath to the #url formGroup
50+
.findByXpath('//*[@id="new-report"]/div/form/div[1]/div[2]/div[1]').getAttribute('class')
4651
.then(function (className) {
4752
assert.include(className, 'no-error');
4853
assert.notInclude(className, 'has-error');
4954
})
5055
.end()
51-
.findByCssSelector('#summary').type('sup')
56+
// click in the textarea to trigger validation for radios
57+
.findByCssSelector('#description').click()
58+
.end()
59+
.findByXpath('//*[@id="new-report"]/div/form/div[1]/div[1]/fieldset').getAttribute('class')
60+
.then(function (className) {
61+
assert.include(className, 'has-error');
62+
assert.notInclude(className, 'no-error');
63+
})
64+
.end()
65+
// pick a problem type
66+
.findByCssSelector('#problem_category-0').click()
5267
.end()
53-
// xpath to the #summary formGroup
54-
.findByXpath('//*[@id="new-report"]/div/form/div[1]/div[2]/div').getAttribute('class')
68+
// validation message should be removed now
69+
.findByXpath('//*[@id="new-report"]/div/form/div[1]/div[1]/fieldset').getAttribute('class')
5570
.then(function (className) {
5671
assert.include(className, 'no-error');
5772
assert.notInclude(className, 'has-error');

‎webcompat/form.py‎

Lines changed: 37 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,24 @@
1414
from wtforms import RadioField
1515
from wtforms import StringField
1616
from wtforms import TextAreaField
17+
from wtforms.validators import InputRequired
1718
from wtforms.validators import Length
1819
from wtforms.validators import Optional
19-
from wtforms.validators import Required
2020

2121
AUTH_REPORT = 'github-auth-report'
2222
PROXY_REPORT = 'github-proxy-report'
2323
SCHEMES = ('http://', 'https://')
2424

25-
owner_choices = [(u'True', u'Yes'), (u'False', u'No')]
26-
problem_choices = [(u'browser_bug', u'Looks like the browser has a bug'),
27-
(u'site_bug', u'Looks like the website has a bug.'),
28-
(u'unknown_bug', u'Don\'t know but something\'s wrong.')]
25+
problem_choices = [
26+
(u'detection_bug', u'Desktop site instead of mobile site'),
27+
(u'mobile_site_bug', u'Mobile site is not usable'),
28+
(u'video_bug', u'Video doesn\'t play'),
29+
(u'layout_bug', u'Layout is messed up'),
30+
(u'text_bug', u'Text is not visible'),
31+
(u'unknown_bug', u'Somethign else - I\'ll add details below')
32+
]
2933
url_message = u'A URL is required.'
30-
summary_message = u'Please give a summary.'
34+
radio_message = u'Problem type required.'
3135
username_message = u'A valid username must be {0} characters long'.format(
3236
random.randrange(0, 99))
3337

@@ -41,19 +45,16 @@
4145

4246
class IssueForm(Form):
4347
'''Define form fields and validation for our bug reporting form.'''
44-
url = StringField(u'Site URL*', [Required(message=url_message)])
48+
url = StringField(u'Site URL*', [InputRequired(message=url_message)])
4549
browser = StringField(u'Browser / Version', [Optional()])
4650
os = StringField(u'Operating System', [Optional()])
47-
summary = StringField(u'Problem in 5 words*',
48-
[Required(message=summary_message)])
4951
username = StringField(u'Username',
5052
[Length(max=0, message=username_message)])
51-
description = TextAreaField(u'How can we replicate this?', [Optional()],
53+
description = TextAreaField(u'Give more details', [Optional()],
5254
default=desc_default)
53-
site_owner = RadioField(u'Is this your website?', [Optional()],
54-
choices=owner_choices)
55-
problem_category = RadioField(u'What seems to be the trouble?',
56-
[Optional()], choices=problem_choices)
55+
problem_category = RadioField(u'What seems to be the trouble?*',
56+
[InputRequired(message=radio_message)],
57+
choices=problem_choices)
5758

5859

5960
def get_problem(category):
@@ -65,14 +66,12 @@ def get_problem(category):
6566
return u'Unknown'
6667

6768

68-
def get_owner(is_site_owner):
69-
'''Return human-readable language (Y/N) for site owner form value.'''
70-
if is_site_owner == 'True':
71-
return u'Yes'
72-
elif is_site_owner == 'False':
73-
return u'No'
69+
def get_problem_summary(category):
70+
'''Allows us to special case the "Other" radio choice summary.'''
71+
if category == 'unknown_bug':
72+
return u'see bug description'
7473
else:
75-
return u'Unknown'
74+
return get_problem(category).lower()
7675

7776

7877
def wrap_label(label):
@@ -127,7 +126,6 @@ def build_formdata(form_object):
127126
URL -> part of body
128127
Description -> part of body
129128
Category -> labels
130-
Owner -> labels
131129
132130
We'll try to parse the Browser and come up with a browser label, as well
133131
as labels like mobile, desktop, tablet.
@@ -150,27 +148,28 @@ def build_formdata(form_object):
150148
normalized_url = normalize_url(url)
151149
# Domain extraction
152150
domain = domain_name(normalized_url)
151+
problem_summary = get_problem_summary(form_object.get('problem_category'))
153152
if domain:
154-
summary = '{0} - {1}'.format(domain, form_object.get('summary'))
153+
summary = '{0} - {1}'.format(domain, problem_summary)
155154
else:
156-
summary = '{0}'.format(form_object.get('summary'))
155+
summary = '{0} - {1}'.format(normalized_url, problem_summary)
157156
# Preparing the body
158-
body = u'''{0}{1}
159-
**URL**: {2}
160-
**Browser / Version**: {3}
161-
**Operating System**: {4}
162-
**Problem type**: {5}
163-
**Site owner**: {6}
157+
body = u'''{browser_label}{ua_label}
158+
**URL**: {url}
159+
**Browser / Version**: {browser}
160+
**Operating System**: {os}
161+
**Problem type**: {problem_type}
164162
165163
**Steps to Reproduce**
166-
{7}'''.format(get_labels(form_object.get('browser')),
167-
wrap_label(('ua_header', form_object.get('ua_header'))),
168-
form_object.get('url'),
169-
form_object.get('browser'),
170-
form_object.get('os'),
171-
get_problem(form_object.get('problem_category')),
172-
get_owner(form_object.get('site_owner')),
173-
form_object.get('description'))
164+
{description}'''.format(
165+
browser_label=get_labels(form_object.get('browser')),
166+
ua_label=wrap_label(('ua_header', form_object.get('ua_header'))),
167+
url=form_object.get('url'),
168+
browser=form_object.get('browser'),
169+
os=form_object.get('os'),
170+
problem_type=get_problem(form_object.get('problem_category')),
171+
description=form_object.get('description')
172+
)
174173
result = {}
175174
result['title'] = summary
176175
result['body'] = body

‎webcompat/static/css/development/components/form.css‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,9 @@ input[type="radio"] {
5757
.u-formGroup {
5858
margin-bottom:1.6em;
5959
}
60+
legend.u-formLabel {
61+
width: 100%;
62+
}
6063
/* bootstrap overrides */
6164
.radio-inline, .checkbox-inline {
6265
/* Hack to get Chrome 24+ to behave.

‎webcompat/static/js/lib/bugform.js‎

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@
55
function BugForm() {
66
var urlField = $('#url');
77
var descField = $('#description');
8-
var summaryField = $('#summary');
8+
var problemType = $('[name=problem_category]');
99
var submitButtons = $('.Report-form button.Button');
1010
var inputMap = {
1111
'url': {
1212
'elm': urlField, // elm is a jQuery object
13-
'valid': false,
13+
'valid': null,
1414
'helpText': 'A URL is required.'
1515
},
16-
'summary' : {
17-
'elm': summaryField,
18-
'valid': false,
19-
'helpText': 'Please give a summary.'
16+
'problem_type': {
17+
'elm': problemType,
18+
'valid': null,
19+
'helpText': 'Problem type required.'
2020
}
2121
};
2222

@@ -25,8 +25,9 @@ function BugForm() {
2525
self.checkParams();
2626
urlField.on('input', self.copyURL);
2727
self.disableSubmits();
28-
urlField.on('blur input', self.checkValidity);
29-
summaryField.on('blur input', self.checkValidity);
28+
descField.on('focus', self.checkProblemTypeValidity);
29+
problemType.on('change', self.checkProblemTypeValidity);
30+
urlField.on('blur input', self.checkURLValidity);
3031
},
3132
checkParams: function() {
3233
// Assumes a URI like: /?open=1&url=http://webpy.org/, for use by addons
@@ -60,40 +61,58 @@ function BugForm() {
6061
submitButtons.prop('disabled', false);
6162
submitButtons.removeClass('is-disabled');
6263
},
63-
/* Check to see that the form element is not empty.
64+
checkProblemTypeValidity: function() {
65+
if (!$('[name=problem_category]:checked').length) {
66+
self.makeInvalid('problem_type');
67+
} else {
68+
self.makeValid('problem_type');
69+
}
70+
},
71+
/* Check to see that the URL input element is not empty.
6472
We don't do any other kind of validation yet. */
65-
checkValidity: function(e) {
66-
if ($.trim(e.target.value) === "") {
67-
self.makeInvalid(e.target.id);
73+
checkURLValidity: function() {
74+
if ($.trim(urlField.val()) === "") {
75+
self.makeInvalid('url');
6876
} else {
69-
self.makeValid(e.target.id);
77+
self.makeValid('url');
7078
}
7179
},
7280
makeInvalid: function(id) {
7381
// Early return if inline help is already in place.
74-
if (inputMap[id].elm.parent().prev('.help-inline').length) {
82+
if (inputMap[id].valid === false) {
7583
return;
7684
}
7785

86+
var inlineHelp = $('<span></span>', {
87+
'class': 'help-inline wc-bold',
88+
'text': inputMap[id].helpText
89+
});
90+
91+
7892
inputMap[id].valid = false;
7993
inputMap[id].elm.parents('.u-formGroup')
8094
.removeClass('no-error')
8195
.addClass('has-error');
8296

83-
$('<span></span>', {
84-
'class': 'help-inline wc-bold',
85-
'text': inputMap[id].helpText
86-
}).insertAfter('label[for='+id+']');
97+
if (id === 'url') {
98+
inlineHelp.insertAfter('label[for='+id+']');
99+
}
100+
101+
if (id === 'problem_type') {
102+
inlineHelp.appendTo('legend.u-formLabel');
103+
}
104+
87105
self.disableSubmits();
88106
},
89107
makeValid: function(id) {
90108
inputMap[id].valid = true;
91109
inputMap[id].elm.parents('.u-formGroup')
92110
.removeClass('has-error')
93111
.addClass('no-error');
94-
inputMap[id].elm.prev('.help-inline').remove();
95112

96-
if (inputMap['url'].valid && inputMap['summary'].valid) {
113+
inputMap[id].elm.parents('.u-formGroup').find('.help-inline').remove();
114+
115+
if (inputMap['url'].valid && inputMap['problem_type'].valid) {
97116
self.enableSubmits();
98117
}
99118
},

0 commit comments

Comments
 (0)