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

Use async/await in auth store, added tests #88

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
"lint-staged": "^7.2.2",
"node-sass": "^4.9.0",
"sass-loader": "^7.0.1",
"sinon": "^6.3.5",
"vue-template-compiler": "^2.5.17"
},
"gitHooks": {
Expand Down
85 changes: 46 additions & 39 deletions src/store/auth.module.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const state = {
isAuthenticated: !!JwtService.getToken()
};

const getters = {
export const getters = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do we export this now?

Copy link
Author

Choose a reason for hiding this comment

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

Good point, I'll remove these exports

currentUser(state) {
return state.user;
},
Expand All @@ -24,50 +24,50 @@ const getters = {
}
};

const actions = {
[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);
});
});
export const actions = {
async [LOGIN](context, credentials) {
try {
const {
data: { user = {} }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we keep Object destructuring at one level? Otherwise, it gets unreadable.

Copy link
Author

Choose a reason for hiding this comment

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

I tend to quite like nested destructuring since it allows you to easily assign default values without having to do a separate check at each level and assign new variable names. I accept that it can make it a bit less readable for people not used to it though so I'll simplify it.

} = await ApiService.post("users/login", { user: credentials });
context.commit(SET_AUTH, user);
} catch ({ response: { data: { errors = {} } = {} } = {} }) {
context.commit(SET_ERROR, errors);
throw new Error(errors);
}
},
[LOGOUT](context) {
context.commit(PURGE_AUTH);
},
[REGISTER](context, credentials) {
return new Promise((resolve, reject) => {
ApiService.post("users", { user: credentials })
.then(({ data }) => {
context.commit(SET_AUTH, data.user);
resolve(data);
})
.catch(({ response }) => {
context.commit(SET_ERROR, response.data.errors);
reject(response);
});
});
async [REGISTER](context, credentials) {
try {
const {
data: { user = {} }
} = await ApiService.post("users", { user: credentials });
context.commit(SET_AUTH, user);
} catch ({ response: { data: { errors = {} } = {} } = {} }) {
context.commit(SET_ERROR, errors);
throw new Error(errors);
}
},
[CHECK_AUTH](context) {
async [CHECK_AUTH](context) {
if (JwtService.getToken()) {
ApiService.setHeader();
ApiService.get("user")
.then(({ data }) => {
context.commit(SET_AUTH, data.user);
})
.catch(({ response }) => {
context.commit(SET_ERROR, response.data.errors);
});

try {
const {
data: { user = {} }
} = await ApiService.get("user");
context.commit(SET_AUTH, user);
} catch ({ response: { data: { errors = {} } = {} } = {} }) {
context.commit(SET_ERROR, errors);
throw new Error(errors);
}
} else {
context.commit(PURGE_AUTH);
}
},
[UPDATE_USER](context, payload) {
async [UPDATE_USER](context, payload) {
const { email, username, password, image, bio } = payload;
const user = {
email,
Expand All @@ -79,14 +79,21 @@ const actions = {
user.password = password;
}

return ApiService.put("user", user).then(({ data }) => {
context.commit(SET_AUTH, data.user);
return data;
});
try {
const {
data: { user: updatedUser = {} }
} = await ApiService.put("user", user);
context.commit(SET_AUTH, updatedUser);

return user;
} catch ({ response: { data: { errors = {} } = {} } = {} }) {
context.commit(SET_ERROR, errors);
throw new Error(errors);
}
}
};

const mutations = {
export const mutations = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not think we need to export this

[SET_ERROR](state, error) {
state.errors = error;
},
Expand Down
3 changes: 2 additions & 1 deletion src/views/Login.vue
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ export default {
onSubmit(email, password) {
this.$store
.dispatch(LOGIN, { email, password })
.then(() => this.$router.push({ name: "home" }));
.then(() => this.$router.push({ name: "home" }))
.catch(() => {});
}
},
computed: {
Expand Down
143 changes: 143 additions & 0 deletions tests/unit/store/auth.module.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
import sinon from "sinon";

import { mutations, actions, getters } from "@/store/auth.module";
import { SET_AUTH, PURGE_AUTH, SET_ERROR } from "@/store/mutations.type";
import JwtService from "@/common/jwt.service";
import ApiService from "@/common/api.service";
import {
LOGIN,
LOGOUT,
REGISTER,
CHECK_AUTH,
UPDATE_USER
} from "@/store/actions.type";

describe("getters", () => {
const state = {
user: "Test User",
isAuthenticated: true
};

it("currentUser", () => {
expect(getters.currentUser(state)).toBe("Test User");
});

it("isAuthenticated", () => {
expect(getters.isAuthenticated(state)).toBe(true);
});
});

describe("mutations", () => {
it("SET_ERROR", () => {
const setError = mutations[SET_ERROR];
const state = { errors: null };
setError(state, "Test Error");

expect(state.errors).toBe("Test Error");
});

it("SET_AUTH", () => {
const stub = sinon.stub(JwtService, "saveToken");
Copy link
Collaborator

Choose a reason for hiding this comment

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

We do not need sinon here. Jest is capable of mocking JavaScript.

Resources

Copy link
Author

Choose a reason for hiding this comment

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

Good point, too used to the mocha+sinon combo

const setAuth = mutations[SET_AUTH];
const state = {
isAuthenticated: false,
user: null,
errors: "Some Error"
};
setAuth(state, { token: "Test Token" });

expect(state).toEqual({
isAuthenticated: true,
user: { token: "Test Token" },
errors: {}
});

expect(stub.calledOnceWith("Test Token")).toBe(true);
});

it("PURGE_AUTH", () => {
const destroyToken = sinon.stub(JwtService, "destroyToken");
const purgeAuth = mutations[PURGE_AUTH];
const state = {
isAuthenticated: true,
user: "Test User",
errors: "Some Error"
};
purgeAuth(state);

expect(state).toEqual({
isAuthenticated: false,
user: {},
errors: {}
});

expect(destroyToken.calledOnce).toBe(true);
});
});

describe("actions", () => {
let commit;

const userPromise = Promise.resolve({ data: { user: "Test User" } });
const tokenStub = sinon.stub(JwtService, "getToken");

sinon.stub(ApiService, "post").returns(userPromise);
sinon.stub(ApiService, "get").returns(userPromise);
sinon.stub(ApiService, "put").returns(userPromise);
sinon.stub(ApiService, "setHeader");

beforeEach(() => {
commit = sinon.spy();
});

it("LOGIN", done => {
actions[LOGIN]({ commit }, {}).then(() => {
expect(commit.calledOnceWith(SET_AUTH, "Test User")).toBe(true);
done();
});
});

it("LOGOUT", () => {
actions[LOGOUT]({ commit });
expect(commit.calledOnceWith(PURGE_AUTH)).toBe(true);
});

it("REGISTER", done => {
actions[REGISTER]({ commit }, {}).then(() => {
expect(commit.calledOnceWith(SET_AUTH, "Test User")).toBe(true);
done();
});
});

describe("CHECK_AUTH", () => {
it("with token", done => {
tokenStub.returns(true);

actions[CHECK_AUTH]({ commit }).then(() => {
expect(commit.calledOnceWith(SET_AUTH, "Test User")).toBe(true);
done();
});
});

it("without token", () => {
tokenStub.returns(false);
actions[CHECK_AUTH]({ commit });
expect(commit.calledOnceWith(PURGE_AUTH)).toBe(true);
});
});

it("UPDATE_USER", done => {
const testUser = {
email: "[email protected]",
username: "testuser",
password: "testpass123",
image: "test.png",
bio: "Test Bio"
};

actions[UPDATE_USER]({ commit }, testUser).then(() => {
expect(commit.calledOnceWith(SET_AUTH, "Test User")).toBe(true);
done();
});
});
});
Loading