-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
base: master
Are you sure you want to change the base?
Conversation
tests/unit/store/auth.module.spec.js
Outdated
}); | ||
|
||
it("SET_AUTH", () => { | ||
const stub = sinon.stub(JwtService, "saveToken"); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
src/store/auth.module.js
Outdated
@@ -15,7 +15,7 @@ const state = { | |||
isAuthenticated: !!JwtService.getToken() | |||
}; | |||
|
|||
const getters = { | |||
export const getters = { |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
src/store/auth.module.js
Outdated
async [LOGIN](context, credentials) { | ||
try { | ||
const { | ||
data: { user = {} } |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
src/store/auth.module.js
Outdated
} | ||
}; | ||
|
||
const mutations = { | ||
export const mutations = { |
There was a problem hiding this comment.
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
… refactored object destructuring
Addresses issue #58