Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #2725: Bugform validation improvements #2886

Merged
merged 3 commits into from
Jun 3, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@
"require": true,
"wcEvents": true,
"wcTmpl": true,
"WindowHelpers": true
"WindowHelpers": true,
"Validation": true
},
"rules": {
"eqeqeq": [2, "smart"],
Expand Down
1 change: 1 addition & 0 deletions grunt-tasks/concat.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ module.exports = function(grunt) {
"<%= jsPath %>/lib/flash-message.js",
"<%= jsPath %>/lib/homepage.js",
"<%= jsPath %>/lib/autogrow-textfield.js",
"<%= jsPath %>/lib/bugform-validation.js",
"<%= jsPath %>/lib/bugform.js",
"<%= jsDistPath %>/templates.js"
],
Expand Down
65 changes: 50 additions & 15 deletions tests/functional/reporting-non-auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ registerSuite("Reporting (non-auth)", {
".js-report-buttons"
)
.findAllByCssSelector(".js-report-buttons button")
.getAttribute("class")
.then(function(classNames) {
classNames.forEach(function(className) {
assert.include(className, "is-disabled");
.getAttribute("disabled")
.then(function(values) {
values.forEach(function(value) {
assert.isNotNull(value);
});
})
.end();
Expand Down Expand Up @@ -75,6 +75,10 @@ registerSuite("Reporting (non-auth)", {
.click()
.type("http:// example.com")
.end()
.execute(function() {
var elm = document.querySelector("#url");
WindowHelpers.sendEvent(elm, "blur");
})
.sleep(500)
.findByCssSelector(".form-message-error");
},
Expand All @@ -90,11 +94,15 @@ registerSuite("Reporting (non-auth)", {
.type("sup")
.end()
.sleep(500)
.execute(function() {
var elm = document.querySelector("#url");
WindowHelpers.sendEvent(elm, "blur");
})
.findByCssSelector(".form-message-error")
.getVisibleText()
.then(function(text) {
.then(function(texts) {
assert.include(
text,
texts,
"A valid URL is required",
"URL validation message is shown"
);
Expand All @@ -115,13 +123,16 @@ registerSuite("Reporting (non-auth)", {
url("/issues/new"),
".js-report-buttons"
)
.findByCssSelector("#description")
.findByCssSelector("#url")
.type("http://coolguy.biz")
.end()
// pick a problem type
.findByCssSelector("[for=problem_category-0]")
.click()
.end()
.findByCssSelector(".js-Button-wrapper")
.click()
.end()
.execute(function() {
var elm = document.querySelector("#description");
WindowHelpers.sendEvent(elm, "input");
})
.sleep(500)
.findByCssSelector(".form-message-error")
.getVisibleText()
Expand Down Expand Up @@ -250,10 +261,10 @@ registerSuite("Reporting (non-auth)", {
.sleep(250)
// now make sure the buttons aren't disabled anymore
.findAllByCssSelector(".js-report-buttons button")
.getAttribute("class")
.then(function(classNames) {
classNames.forEach(function(className) {
assert.notInclude(className, "is-disabled");
.getAttribute("disabled")
.then(function(values) {
values.forEach(function(value) {
assert.isNull(value);
});
})
.end()
Expand Down Expand Up @@ -412,6 +423,30 @@ registerSuite("Reporting (non-auth)", {
);
})
.end();
},
"Submitting form without filling anything"() {
return FunctionalHelpers.openPage(
this,
url("/issues/new"),
".js-report-buttons"
)
.findByCssSelector(".js-Button-wrapper")
.click()
.end()
.sleep(500)
.findAllByCssSelector(".form-message-error")
.getVisibleText()
.then(function(texts) {
var errorTexts = [
"A valid URL is required.",
"Problem type required.",
"A problem summary is required."
];
errorTexts.forEach(function(expectedText) {
assert.include(texts, expectedText, "Error messages don't match");
});
})
.end();
}
}
});
4 changes: 2 additions & 2 deletions webcompat/form.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,10 @@
username_message = u'A valid username must be {0} characters long'.format(
random.randrange(0, 99))

desc_label = u'Please write a short problem summary'
desc_label = u'Please write a short problem summary (mandatory)'
desc_message = u'A problem summary is required.'

url_label = u'Site URL'
url_label = u'Site URL (mandatory)'
browser_test_label = u'Did you test in another browser?'
textarea_label = u'Please describe what happened, including any steps you took before you saw the problem' # noqa

Expand Down
17 changes: 16 additions & 1 deletion webcompat/static/css/src/form.css
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,24 @@
max-width: none;
}

.form-button-multiple .button {
.form-button-multiple .button-wrapper {
cursor: pointer;
display: flex;
flex: 1 1 auto;
margin: 20px 10px 0;
position: relative;
}

.form-button-multiple .button-wrapper .button {
flex: 1;
}

.form-button-multiple .button-wrapper .button:disabled {
pointer-events: none;
}

.form-button-multiple .button-wrapper:hover .button {
opacity: .85;
}

.form-field {
Expand Down
48 changes: 48 additions & 0 deletions webcompat/static/js/lib/bugform-validation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/* exported Validation */
function Validation() {
var GITHUB_REGEXP = /^[a-z\d](?:[a-z\d]|-(?=[a-z\d])){0,38}$/i;
var ALLOWED_IMG_FORMAT = ["jpg", "jpeg", "jpe", "png", "gif", "bmp"];

var isReportableURL = function(url) {
ksy36 marked this conversation as resolved.
Show resolved Hide resolved
var ok = url && (_.startsWith(url, "http:") || _.startsWith(url, "https:"));
ok &= !(_.startsWith(url, "http:// ") || _.startsWith(url, "https:// "));
return ok;
};

/**
Check a string is a valid GitHub username
- maximum 39 chars
- alphanumerical characters and hyphens
- no two consecutive hyphens
*/
var isValidGitHubUsername = function(contact) {
return GITHUB_REGEXP.test(contact);
};

return {
isDescriptionValid: function(descField) {
var val = descField.val();
return $.trim(val) !== "";
},
isUrlValid: function(urlField) {
var val = urlField.val();
return $.trim(val) !== "" && isReportableURL(val);
},
isGithubUserNameValid: function(contactField) {
var contact = contactField.val();
return isValidGitHubUsername(contact) || $.trim(contact) === "";
},
isProblemTypeValid: function(problemTypeField) {
return problemTypeField.filter(":checked").length;
},
isImageTypeValid: function(uploadField) {
var splitImg = uploadField.val().split(".");
var ext = splitImg[splitImg.length - 1].toLowerCase();

return _.includes(ALLOWED_IMG_FORMAT, ext);
},
isOptionalValid: function(input) {
return !!input.val();
}
};
}
Loading