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

[Bug]: Fix async validations #107

Merged
merged 5 commits into from
Feb 25, 2021
Merged

[Bug]: Fix async validations #107

merged 5 commits into from
Feb 25, 2021

Conversation

snewcomer
Copy link
Collaborator

@snewcomer snewcomer commented Feb 24, 2021

close #106 #105

@snewcomer snewcomer self-assigned this Feb 24, 2021
@snewcomer snewcomer added the bug Something isn't working label Feb 24, 2021
@snewcomer snewcomer force-pushed the sn/fix-async-validation branch 2 times, most recently from 1649ba0 to b7e92db Compare February 24, 2021 17:31
@snewcomer snewcomer force-pushed the sn/fix-async-validation branch from b7e92db to 3d705ff Compare February 24, 2021 17:33
@snewcomer snewcomer added the WIP label Feb 24, 2021
@snewcomer snewcomer removed the WIP label Feb 25, 2021
const expectedError = { delayedAsync: { validation: false, value: 'second' } };
expect(dummyChangeset.changes).toEqual(expectedChanges);
expect(dummyChangeset.error).toEqual(expectedError);
});
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

let running: RunningValidations = this[RUNNING_VALIDATIONS];
let promises = Object.entries(running);

return Promise.all(promises).then(() => {
Copy link
Collaborator Author

@snewcomer snewcomer Feb 25, 2021

Choose a reason for hiding this comment

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

So the idea here is to finish all existing validations before starting the next one in line. This won't block user interaction since the key already has been set on the changeset. This is just an async path that will update the changset *sometime later.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant