Skip to content

Commit

Permalink
Merge pull request #2886 from webcompat/issue/2725/2
Browse files Browse the repository at this point in the history
Fixes #2725: Bugform validation improvements
  • Loading branch information
Mike Taylor authored Jun 3, 2019
2 parents 7b1cf82 + 196b5c3 commit b19a963
Show file tree
Hide file tree
Showing 9 changed files with 324 additions and 182 deletions.
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) {
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

0 comments on commit b19a963

Please sign in to comment.