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

Small async changes to assessment request #606

Merged
merged 4 commits into from
Jul 7, 2021

Conversation

tomlovesgithub
Copy link
Contributor

I was unsure of where the function was failing, as there was no errors logged and the function was returning true.
I believe that these changes, specifically where noted below have fixed the issue, and testing between this branch and develop, confirms this.

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.

@tomlovesgithub can you walk me through these changes. I'm a little confused!


// get assessments
let assessmentsRef = db.collection('assessments')
let assessmentsRef = await db.collection('assessments')
Copy link
Member

Choose a reason for hiding this comment

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

Using an await here doesn't look right. We are constructing assessmentsRef in lines 289 - 301 and actually using it, with await, on line 303

return res;
});

let result = await validateAssessorEmailAddresses(assessments).then((res) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here ^^

@@ -301,6 +301,7 @@ module.exports = (config, firebase, db) => {
const assessments = await getDocuments(assessmentsRef);

let result = validateAssessorEmailAddresses(assessments);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😚👌

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.

Thanks @tomlovesgithub this is a good fix 👍

@tomlovesgithub tomlovesgithub requested a review from HalcyonJAC July 6, 2021 17:11
@KateMJAC KateMJAC merged commit e8f4af3 into develop Jul 7, 2021
@KateMJAC KateMJAC deleted the independent-assessment-issues branch July 7, 2021 09:03
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.

4 participants