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

Transform Login call to async/await #58

Open
igeligel opened this issue Oct 12, 2018 · 2 comments
Open

Transform Login call to async/await #58

igeligel opened this issue Oct 12, 2018 · 2 comments

Comments

@igeligel
Copy link
Collaborator

Issue Summary:

In the auth.module.js here you have got some code like:

[LOGIN](context, credentials) {
  return new Promise(resolve => {
    ApiService.post("users/login", { user: credentials })
      .then(({ data }) => {
        context.commit(SET_AUTH, data.user);
        resolve(data);
      })
      .catch(({ response }) => {
        context.commit(SET_ERROR, response.data.errors);
      });
  });
}

Those promise calls could be simpliefied with async and await calls.

Resources:

Acceptance Criteria:

  • A test is introduced, this is perfect work for TDD
  • There is no .then(... or .catch(... block or any promise like structure

Main issue is here #5

@ljones92
Copy link

I'll take a look at this

@moozzyk
Copy link
Contributor

moozzyk commented Oct 13, 2018

The problem with this one is that the original code never resolves or rejects the promise in case of errors. This is by itself incorrect. When converting this to async/await the promise will always be either resolved or rejected. This results in unwanted redirection to home in case of an error (because the error is not propagated, hence the promise gets resolved). This can be fixed by re-throwing the error from the catch block and swallowing it here like this:

    onSubmit(email, password) {
      this.$store
        .dispatch(LOGIN, { email, password })
        .then(() => this.$router.push({ name: "home" }))
        .catch(e => { /* the exception was already handled */});
    }

or by returning the data and checking inside the then for an error to know whether to redirect or not.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants