Cleaning up issues found after the previous work is what we’re up to today. We will be doing the following:
- moving showValid code into the check function
- removing the other password validation rules from elsewhere in the code
- replacing checkEmpty with hasContent throughout all of the code
Move the showValid inline into the check function
The showValid function is a lonely leftover from after cleaning up the rules functions. The showValid function doesn’t really belong by itself there now, and it’s only used in one place, so we should move it in to the check function where it belongs.
// function showValid(inputGroup) {
// const msg = " is Ok: Your data has been entered correctly";
// inputStatus.ok(inputGroup, getName(inputGroup) + msg);
// }
//...
function check(inputGroup, validators) {
//...
const isValid = validateByTypes(inputGroup);
if (isValid) {
// showValid(inputGroup);
const msg = " is Ok: Your data has been entered correctly";
inputStatus.ok(inputGroup, getName(inputGroup) + msg);
}
}
Bring password and email code into the one place
When taking care of the validate rule functions, we noticed that the password validation is being used in multiple places. Do I remove the password rules from the validate code and have it repeated in several different places? I don’t think so. As we have had email and password rules kept within the validate code, it makes sense to have it remain there.
That means making the email and password validations available from the validate code. As that’s changing the public interface to the code, we should use tests to help check that everything’s working as expected:
Here is the email test that we add to validate.test.js
validate.test.js
describe("makes matchers public", function () {
it("matches using public email matcher", function () {
emailInput.value = "email@example.com";
validate.check($(emailGroup), {
"E-mail": [
validate.fn.isEmail
]
});
expect($emailError.html()).to.contain("is Ok");
});
it("doesn't match using public email matcher", function () {
emailInput.value = "not an email";
validate.check($(emailGroup), {
"E-mail": [
validate.fn.isEmail
]
});
expect($emailError.html()).to.contain("is Incorrect");
});
});
and here are the password tests that we add to validate.test.js
describe("matches with password", function () {
const passwordGroup = $("#login .form-group").has("[name='Password']").get(0);
const passwordInput = $(passwordGroup).find("input").get(0);
const $passwordError = $(passwordGroup).find(".error");
it("is at least six characters", function () {
passwordInput.value = "abcdefg";
validate.check($(passwordGroup), {
"Password": [
validate.fn.passwordAtLeastSix
]
});
expect($passwordError.html()).to.contain("is Ok");
});
it("is less than six characters", function () {
passwordInput.value = "abcde";
validate.check($(passwordGroup), {
"E-mail": [
validate.fn.passwordAtLeastSix
]
});
expect($passwordError.html()).to.contain("is Incorrect");
});
it("is less than thirteen characters", function () {
passwordInput.value = "abcdefghijkl";
validate.check($(passwordGroup), {
"Password": [
validate.fn.passwordBelowThirteen
]
});
expect($passwordError.html()).to.contain("is Ok");
});
it("is less than six characters", function () {
passwordInput.value = "abcdefghijklm";
validate.check($(passwordGroup), {
"E-mail": [
validate.fn.passwordBelowThirteen
]
});
expect($passwordError.html()).to.contain("is Incorrect");
});
});
Those tests result help to confirm that we are returning the isEmail and password matchers from the validate code:
return {
//...
fn: {
getName,
getValue,
checkEmpty,
isEmail: defaultValidators["E-mail"][1],
passwordAtLeastSix: defaultValidators.Password[1],
passwordBelowThirteen: defaultValidators.Password[2]
}
};
We can now use those matchers from the registration code, letting us remove those parts from the registration validation:
const validators = {
//...
// passwordAtLeastSix: {
// regex: /^([a-zA-Z0-9]{6,})+$/,
// error: "Please enter at least 6 characters"
// },
// passwordBelowThirteen: {
// regex: /^[a-zA-Z0-9]{1,12}$/,
// error: "Please enter no more than 12 characters"
// },
//...
};
//...
"Password": [
validate.fn.checkEmpty,
createMatcher("differentThanFirstname"),
createMatcher("differentThanLastname"),
createMatcher("differentThanCity"),
// createMatcher("passwordAtLeastSix"),
validate.fn.passwordAtLeastSix,
// createMatcher("passwordBelowThirteen"),
validate.fn.passwordBelowThirteen
],
Replace checkEmpty with hasContent
When I remove checkEmpty from what validate returns, that leads to a few problems that we can fix up.
fn: {
getName,
getValue,
// checkEmpty,
hasContent: createMatcher(validators.hasContent),
The first issue is: TypeError: check is not a function at checkInput
function check(inputGroup, validators) {
//...
return types.every(function checkInput(check) {
return check(inputGroup);
});
}
//...
}
Having the name check used in multiple places for different things becomes confusing. The validation types contain different matchers, so we can rename check to be matcher instead.
// return types.every(function checkInput(check) {
return types.every(function checkInput(matcher) {
// return check(inputGroup);
return matcher(inputGroup);
});
Now that we have that potential confusion out of the way, we can investigate what happens there. We find that the list of types has an undefined one in there.
types: (4) [undefined, f, f, f]
and that brings us back to the test that triggered this issue, caused by us removing checkEmpty.
it("is empty", function () {
$firstnameInput.val("");
callRegistrationInputHandler($firstnameInputGroup);
expect($firstnameError.html()).to.equal("First Name is Empty: Please enter data into this input");
});
The registration input handler is where we must next look:
const nameValidationConfig = [
validate.fn.checkEmpty,
createMatcher("lessThanTwentyChars"),
createMatcher("moreThanOneChar"),
createMatcher("onlyAlphaChars")
];
and that is where the undefined part comes from. We removed checkEmpty, and it’s no longer available. We can update checkEmpty to instad be hasContent, and things should return back to working.
const nameValidationConfig = [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("lessThanTwentyChars"),
createMatcher("moreThanOneChar"),
createMatcher("onlyAlphaChars")
];
and that part of things returns back to working.
Checking for characters or no whitespace
The next issue helps to improve on an assumption that was made. The test called registration-input first name has strange characters: is failing because ## for the strange characters test is being thought of as being empty.
+ expected - actual
-First Name is Empty: Please enter data into this input
+First Name is Incorrect: Please enter upper case and lower case only
That’s because the hasContent test of /\w/ to match at a word character, is being too effective. We want the hasContent test to be less effective so that other tests can catch the situations that they need to look for.
Instead of checking for a word character, we can check that something that isn’t whitespace is there instead.
hasContent: {
// regex: /\w/,
regex: /\S/,
errorType: "Empty",
error: "Please enter data into this input"
},
and that strange characters test no longer has any trouble.
Convert remaining checkEmpty over to hasContent
We can now use the remaining failing tests to have us convert checkEmpty over to hasContent.
registration.js
validate.check($cityGroup, {
// "Your City": [validate.fn.checkEmpty]
"Your City": [validate.fn.hasContent]
});
//...
return validate.check($formGroup, {
//...
"Phone Number": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("isPhoneNumber")
],
"E-mail": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("isEmail")
],
"Postal Address": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("postalAddress")
],
"zip code": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("postcode")
],
"Your City": [
// validate.fn.checkEmpty
validate.fn.hasContent,
],
"Password": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
//...
],
"Retype Password": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
createMatcher("matchesPassword")
]
});
We are also directed to make similar changes to the change-password code:
change-password.js
const retypeValidator = {
"Password Retype": [
// validate.fn.checkEmpty,
validate.fn.hasContent,
validate.createMatcher({
fieldname: "Password",
error: "Password doesn't match retyped password"
})
]
};
On searching the code for other occurances of checkEmpty, we only find one more in the validate.js code. That’s good news, for it also means that our tests have successfully covered all other uses of it.
Remove checkEmpty from the validate code
Now that other uses of checkEmpty have been removed, it’s relatively easy to remove checkEmpty from the validate code now.
// function checkEmpty(inputGroup) {
// if (getValue(inputGroup) === "") {
// const msg = "Please enter data into this input";
// inputStatus.warning(
// inputGroup,
// getName(inputGroup) + " is Empty: " + msg
// );
// return false;
// }
// return true;
// }
//...
const validators = {
hasContent: {
regex: /\S/,
errorType: "Empty",
error: "Please enter data into this input"
},
//...
function checkFieldEmpty(formGroup) {
const $inputField = $(formGroup).find("input, textarea");
const name = $inputField.attr("name");
const validations = [
// checkEmpty
createMatcher(validators.hasContent)
];
check(formGroup, Object.fromEntries([
[name, validations]
]));
}
Summary
We moved showValid code into the check function, removed the other password validation rules from elsewhere in the code, and replaced checkEmpty with hasContent throughout all of the code.
The code as it stands today is found at v0.0.40 in releases
We only have two more sets of duplication to inspect. Next time we look at duplication in the change-password and login code.