Change Password - Part 5 - Adding More Tests
In this video we are continuing on with our errorHelper implementation. This function takes the error object returned from a failing "change password" call, and helps extracts the message(s) into a format our front end can work with.
Towards the end of the previous video we had a simple implementation whereby we could correctly display a single error. In reality, we must be able to handle multiple errors, and we'd likely like to do so in a visually pleasing format.
To begin with, we will create a new failing test, this time covering an array with multiple error entries:
// /__tests__/helpers/formErrorHelpers.react-test.js
import errorHelper from '../../src/helpers/formErrorHelper';
describe('Symfony Form Error Helper', () => {
it('returns multiple errors if found', () => {
let errors = {
children: {
current_password: {
errors: [
"This value is invalid",
"Some other issue",
"A third issue"
]
}
}
};
expect(
errorHelper(errors, 'children.current_password.errors')
).toEqual('This value is invalid, Some other issue, and A third issue');
});
});The immediately obvious change here is going from one error message to three error messages.
Less obvious is the formatting of the output:
'This value is invalid, Some other issue, and A third issue'
I want to include each error up until the very last with a separating comma. For the very last element, I want to join with: ', and'.
Now, being software development, there are likely various ways to do this. Likely many are more elegant. This is my implementation. There are many others like it, but this one is mine.
I'm going to start by changing our original implementation slightly. To begin with, this will be more verbose, and then later we will extract and tidy:
// /src/helpers/formErrorHelper.js
import _ from 'lodash';
const errorHelper = (errors, errorPath) => {
let errorInfo;
try {
errorInfo = _.get(errors, errorPath, undefined);
if (Array.isArray(errorInfo)) {
if (errorInfo.length === 1) {
errorInfo = errorInfo[0];
}
}
} catch (e) {
console.error('errorHelper e', e);
}
return errorInfo;
};
export default errorHelper;Ok, so a couple of nasty issues here.
Firstly, nested conditionals. This is such a personal bugbear of mine. I find a single if to be quite enough for my brain to handle. An if inside an if? It doesn't seem like much, but when you factor in that this is likely just one of several functions being called to get from request to response, it quickly can become taxing keeping a bunch of potential state paths in your head.
The second nasty issue is mutation. errorInfo is potentially being changed here from an array to a string. Again, not quite a show stopper here, but it adds a layer of misty (not hidden, but not super obvious) complexity to the code, and is a point in this code that is strong candidate for the introduction of bugs.
Anyway, one of the nice parts about having tests is that no matter how unhealthy the implementation, so long as the tests pass we can assume the code is behaving as expected. Well, so long as the tests are good, but that's a different can of worms.
At this point, our test outcomes should still be the same. The first test passes, the second test still fails.
Let's add an else here to address the issue where errorInfo.length > 1:
// /src/helpers/formErrorHelper.js
import _ from 'lodash';
const errorHelper = (errors, errorPath) => {
let errorInfo;
try {
errorInfo = _.get(errors, errorPath, undefined);
if (Array.isArray(errorInfo)) {
if (errorInfo.length === 1) {
errorInfo = errorInfo[0];
} else {
errorInfo = errorInfo
.slice(0, -1)
.join(', ') +
', and ' +
errorInfo.slice(-1)
;
}
}
} catch (e) {
console.error('errorHelper e', e);
}
return errorInfo;
};
export default errorHelper;Ok, so this seems to be getting worse :)
Well, anyway let's continue and look at just the new code in isolation:
errorInfo = errorInfo
.slice(0, -1)
.join(', ') +
', and ' +
errorInfo.slice(-1)
;Firstly, it's still got mutation of errorInfo. Don't worry, this will be addressed.
We start by taking the errorInfo array and slice'ing from the first element (0 indexed) to the penultimate element (-1 - or one from the end).
If we had two elements in the errorInfo array, we would have the first element here.
If we had three elements in the errorInfo array, we would have the first two elements here.
And so on.
We don't need to concern ourselves with errorInfo only having one element, as that's covered in the earlier if condition.
Taking these however many elements, we join them together with a ', ' - a comma and a space. Easy enough.
As join is going to convert our array to a string, we can now use the + operator as normal to join multiple strings together. In this instance, as we are after the final element we add the ', and ' text to the string.
Finally, we slice the last element off the original array errorInfo.slice(-1) and join that onto the output string also. This is a quirk of JavaScript - different to PHP - in that we can convert the array to a string without issue. Strange, but true.
At this point our poor errorInfo variable has been mutated more times than your standard Marvel super hero trilogy reboot.
As nasty as this is, we do now have two passing tests.
Extract, And Make Better
The overall outcome of this could still be improved, but in extracting the nested conditionals (if inside if), we will at least have made a start towards improving readability / understanding for the next person to work with this code:
// /src/helpers/formErrorHelper.js
import _ from 'lodash';
const errorHelper = (errors, errorPath) => {
let errorInfo;
try {
errorInfo = _.get(errors, errorPath, undefined);
if (Array.isArray(errorInfo)) {
errorInfo = formatErrors(errorInfo);
}
} catch (e) {
// console.error('errorHelper e', e);
}
return errorInfo;
};
const formatErrors = (errors) => {
let outputString = "";
if (errors.length === 1) {
outputString = errors[0];
} else {
outputString = errors
.slice(0, -1)
.join(', ') +
', and ' +
errors.slice(-1)
;
}
return outputString;
};
export default errorHelper;Ok, so not my finest code.
The nested conditional has been extracted into formatErrors. We still have a nested conditional, but it's much easier to understand when viewed in isolation.
Inside formatErrors we have now removed the mutation of errors (previously this was called errorInfo).
We need to return a string, so the variable naming makes this more clear.
Because formatErrors isn't being exported, this is effectively a private function.
Overall, as I say, not the world's greatest code. Feel free to improve / rewrite / laugh at. But hey, it works.
To wrap up, I want to ensure we are covering some of the edge cases that I can think of:
// /__tests__/helpers/formErrorHelpers.react-test.js
import errorHelper from '../../src/helpers/formErrorHelper';
describe('Symfony Form Error Helper', () => {
it('handles empty objects', () => {
expect(errorHelper({}, 'any.path')).toEqual(undefined);
});
it('handles object with data but the requested path is not found', () => {
let errors = {
children: {}
};
expect(errorHelper(errors, 'some.path')).toEqual(undefined);
});
it('returns errors if found', () => {
let errors = {
children: {
current_password: {
errors: [
"This value is invalid"
]
}
}
};
expect(
errorHelper(errors, 'children.current_password.errors')
).toEqual('This value is invalid');
});
it('returns multiple errors if found', () => {
let errors = {
children: {
current_password: {
errors: [
"This value is invalid",
"Some other issue",
"A third issue"
]
}
}
};
expect(
errorHelper(errors, 'children.current_password.errors')
).toEqual('This value is invalid, Some other issue, and A third issue');
});
it('handles very deeply nested errors', () => {
let errors = {
children: {
current_password: {
errors: [
"This value is invalid"
]
},
plainPassword: {
children: {
first: {
errors: [
"These do not match"
]
},
second: {}
}
}
}
};
expect(
errorHelper(errors, 'children.plainPassword.children.first.errors')
).toEqual('These do not match');
})
});At this stage, all of these tests should be passing.
We now have a working utility / helper function which we can call from our saga to extract the required errors, and display them on the form. We will do so in the very next video.
