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

Add option fetchUser #27

Merged

Conversation

ernestas-poskus
Copy link
Contributor

Adding option fetchUser which allows control of fetching authenticated user.

cc @pi0

@@ -146,7 +146,9 @@ export default {
<% } %>

// Fetch authenticated user
<% if (options.user.fetchUser) { %>
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for this PR. What's your idea if we conditionally remove fetchUser action from store too when this option is false?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my project I don't keep a state of user, token is enough therefore I wanted to make this conditional

@pi0
Copy link
Member

pi0 commented Dec 23, 2017

@ernestas-poskus I think we need to update this lines too :

@ernestas-poskus
Copy link
Contributor Author

https://github.com/nuxt-community/auth-module/blob/master/lib/templates/auth.store.js#L16 (Only checking user when fetchUser is true)
https://github.com/nuxt-community/auth-module/blob/master/lib/templates/auth.store.js#L11 (Remove extra variable here when fetchUser is false)

maybe better name would be user.enabled to be consistent with token.enabled?

@ernestas-poskus ernestas-poskus force-pushed the features/add_option_fetch_user branch from 6b4f4ac to 885d7a5 Compare December 24, 2017 10:19
@ernestas-poskus
Copy link
Contributor Author

updated and renamed to .user.enabled

@ernestas-poskus
Copy link
Contributor Author

offtopic: is there a way to handle API error after dispatching?

store.dispatch('auth/login', {
  fields: {
    username: 'your_username',
    password: 'your_password'
  }
})

@pi0
Copy link
Member

pi0 commented Dec 24, 2017

Thanks for your contribution. Now changes seems good enough for merging...

For dispatch error handling can't ce just await on store.dispatch like this ?

try {
 await store.dispatch('auth/login', {
  fields: {
    username: 'your_username',
    password: 'your_password'
  }
 })
} catch (e) {
 // Handle error
}

@pi0 pi0 merged commit 1b8856c into nuxt-community:master Dec 24, 2017
@ernestas-poskus ernestas-poskus deleted the features/add_option_fetch_user branch December 25, 2017 14:34
@ernestas-poskus
Copy link
Contributor Author

I will try this, thanks !

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.

2 participants