[FIX] scopes comma-delimited to space-delimited #2
Conversation
There was a problem hiding this comment.
Hey, thanks so much for your pull request!
Good catch on the wrong scopes delimiter. I'm happy to merge that change in after the test is corrected. Can you update the snapshot as well? You can do that with by running yarn test --updateSnapshot (which runs jest --updateSnapshot. See more here)
The other changes have a few things that need to be addressed; see my comments. We can move those to another PR if you want the scopes correct in more quickly.
| @@ -1,12 +1,15 @@ | |||
| { | |||
| "name": "@lourd/react-google-api", | |||
| "name": "@not/react-google-api", | |||
| this.auth.grantOfflineAccess({ | ||
| scope: `${this.props.scopes.join(' ')}`, | ||
| prompt: 'select_account' | ||
| }).then(({code }) => this.setState({ code })) |
There was a problem hiding this comment.
What should happen if the promise rejects?
There was a problem hiding this comment.
Also what is the code parameter here?
| }).then(({code }) => this.setState({ code })) | ||
| } | ||
|
|
||
| if (!this.props.offline) this.auth.signIn() |
There was a problem hiding this comment.
These blocks can be simplified:
if (this.auth) {
if (this.props.offline) {
// grantOfflineAccess
} else {
this.auth.signIn()
}
}| @@ -62,6 +72,8 @@ class GoogleApi extends React.Component { | |||
| clientId: this.props.clientId, | |||
| discoveryDocs: this.props.discoveryDocs, | |||
| scope: this.props.scopes.join(' '), | |||
| fetch_basic_profile: this.props.profile, | |||
| redirect_uri: 'http://localhost:3000' | |||
There was a problem hiding this comment.
If this.props.profile isn't set, what happens? redirect_uri should also be configurable by props
As the documentation reference from JavaScript Google API Client says:
"The scopes to request, as a space-delimited string."
https://developers.google.com/api-client-library/javascript/reference/referencedocs#gapiclientinitargs