Add option to import from/save to Google Sheets #4
Conversation
This allows importing from and saving to Google Sheets. This is totally optional and the URLs will still work as they always have. The flow is something like this: * Use the Authorize button to authorize the app to access your Google Sheets with OAuth. * Then you have an input that accepts a sheet ID and three buttons: import, create/save, and sign out * The sheet ID field accepts a sheet ID. You can simply paste in a URL from an open Google doc and the page extracts the sheet ID automatically. * Import imports a Google Sheet into the Snowflake page, using the entered sheet ID. * The create button is shown when no sheet ID has been entered. This creates a new sheet, adds the sheet ID to the input, and shows a link that takes you to the sheet. * If you've entered a sheet ID, the create button becomes a save button and you may click it to update the sheet corresponding to the ID. * Finally, the sign out button revokes access to Google Sheets and takes you back to the original view, with only an Authorize button. There's not a ton of error-checking. If you try to import a sheet that doesn't have data in the expected format, it's not going to go well. Don't do that. The sheet doesn't yet store name or title, but that's coming up in another PR.
|
|
||
| const API_KEY = 'AIzaSyCPZccI1B543VHblD__af_JvV2b8Z5-Lis' | ||
| const CLIENT_ID = '124466069863-0uic3ahingc9bst2oc95h29nvu30lrnu.apps.googleusercontent.com' | ||
|
|
There was a problem hiding this comment.
For that tool this is likely fine. But is that usually ok to have thing like secret keys and password pushed to Github?
There was a problem hiding this comment.
It feels kinda weird, but I don't believe either of these things are actually a secret. I can't think of anything you could do with them as an attacker. See also my comment with the email screenshot.
There was a problem hiding this comment.
Yes. I was asking because we had a blank policy of nothing like that in Github. I prefer we are more pragmatic and not just over cautious here instead.
| <script | ||
| async | ||
| defer | ||
| src="https://apis.google.com/js/api.js" |
There was a problem hiding this comment.
On modern browsers async will take precedence over defer and defer will not apply.
I am wondering if you just want defer here to make sure it executes after the Google API and HTML is ready.
There was a problem hiding this comment.
I just copied this out of the Google example. 🤷♂️
There was a problem hiding this comment.
Got it. They probably thought about giving an option that would work well in all browsers not just the most recent. Progressive degradation.
| src="https://apis.google.com/js/api.js" | ||
| onLoad="this.onLoad = function(){};handleClientLoad()" | ||
| onreadystatechange="if (this.readyState === 'complete') this.onload()" /> | ||
| </Head> |
There was a problem hiding this comment.
export default () => (
<div>
<Head>
<script defer src="https://apis.google.com/js/api.js" onLoad="handleClientLoad()" />
</Head>
<SnowflakeApp />
</div>
);This should be good enough because as soon as window.sheetsControl is available the checkInit call is not repeated.
There was a problem hiding this comment.
Again, basically copied out of the Google example. 🤷♂️ https://developers.google.com/sheets/api/quickstart/js
There was a problem hiding this comment.
Maybe some older browser support again 🤔 ? Fair enough.
I can tell the simplified snippet worked for recent Chrome on my machine. Behaviour was as expected.
|
|
||
| componentDidMount() { | ||
| window.sheetsControl = this | ||
| } |
There was a problem hiding this comment.
This is a bit odd to do that but until we have a better idea...
There was a problem hiding this comment.
Yeah, I know it's bad. I just wasn't sure of any better way.
| console.log('handleClientLoad') | ||
| const checkInit = () => { | ||
| if (window.sheetsControl) { | ||
| gapi.load('client', window.sheetsControl.initClient.bind(window.sheetsControl)) |
| console.log('error saving') | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Ideally you could split this React component into several more granular components (each buttons) and compose them and move the Google API logic into a separate functions/file that you import here.
The dumber the component, the better 😄 !
Being said that can be done later in that kind of project.
|
Let's merge this! Do you usually leave that to the reviewer or the developer that created the PR? |
|
Usually the developer that created the PR will merge it. 👍 🚢 |
This allows importing from and saving to Google Sheets. It's totally optional and the URLs will still work as they always have. The flow is something like this:
Use the Authorize button to authorize the app to access your Google Sheets with OAuth.
Then you have an input that accepts a sheet ID and three buttons: import, create/save, and sign out
The sheet ID field accepts a sheet ID. You can simply paste in a URL from an open Google doc and the page extracts the sheet ID automatically.
There's not a ton of error-checking. If you try to import a sheet that doesn't have data in the expected format, it's not going to go well. Don't do that.
The sheet doesn't yet store name or title, but that's coming up in another PR. We'll have notes too. Don't worry, one thing at a time.