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

#1198 Fixed issue of a QT results download unable to start if candidates skip questions and submit with no answers. #1202

Conversation

mbrookeswebdev
Copy link
Contributor

@mbrookeswebdev mbrookeswebdev commented Mar 3, 2021

Testing instructions are in the ticket description along with test scripts for reference.

…tes skip questions and submit with no answers.
Copy link
Member

@warrensearle warrensearle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is working, so have approved. 👍
Please see comments re perhaps simplifying the code

Comment on lines +164 to +166
if ((responseSelection.mostAppropriate !== undefined && responseSelection.mostAppropriate !== null)
&& (responseSelection.leastAppropriate !== undefined && responseSelection.leastAppropriate !== null))
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could perhaps simplify this by using == null to cover both the null and undefined scenarios. So

if (
  responseSelection.mostAppropriate != null && 
  responseSelection.leastAppropriate != null
)

Copy link
Contributor Author

@mbrookeswebdev mbrookeswebdev Mar 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I try not to use == anymore and go for ===, to avoid any conversions taking place and to make clear what we need to check.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's best to use === but there's a camp, which includes me, that thinks it's ok to use == null and != null.
e.g. see here: http://adripofjavascript.com/blog/drips/equals-equals-null-in-javascript.html
It's fine either way

@@ -202,7 +204,7 @@ export default {
}
if (responses) {
responses.forEach((response) => {
row.push(response.text === null ? 'Question skipped' : response.text);
row.push(response.text === undefined || response.text === null ? 'Question skipped' : response.text);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps use != null to check for null and undefined

@@ -219,7 +221,7 @@ export default {
}
if (response) {
const responseSelection = response.selection;
if (responseSelection !== undefined) {
if (responseSelection !== undefined && responseSelection !== null) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps use != null to check for null and undefined

@mbrookeswebdev mbrookeswebdev merged commit a0993f3 into develop Mar 8, 2021
@mbrookeswebdev mbrookeswebdev deleted the feature/1198-unable-to-download-critical-analysis-test-result branch March 8, 2021 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants