penny-university / penny_university

5 stars 6 forks source link

Gcal Integration #378

Open mrshwah opened 3 years ago

mrshwah commented 3 years ago

Opening a draft PR for now. Includes the work that I demoed on Tuesday. Please comment with any concerns!

Some todos:

related

closes https://github.com/penny-university/penny_university/issues/122 because of addition of lru_cache decorator

JnBrymn commented 3 years ago
ghost commented 3 years ago

Agreed with John on everything here.

If we're moving to a .env here, it might also be good to do that for SLACK_API_KEY as well.

From a user experience perspective, we might want to include a short blurb on the connection modal detailing what data we access (from what I remember Google's OAuth page for this is very vague) and what benefit they get from connecting Google. ex. When you connect with Google, we use your calendar to keep you updated on your upcoming Penny Chats and automatically create video chats for you!

Really like the way you structured this in the code, makes it simple to expand on for other integrations. I think the more an app integrates with other existing tools, the more a company or organization is willing to adopt it into that workflow, and starting with this we'll have a lot of room to grow on that idea.

mrshwah commented 3 years ago

@JnBrymn

In the final state I think that we share the chat as soon as the modal closes. Is that right? I just want to make sure that we don't have a new, required step at the end.

Yes, this is correct! I didn't mean to originally commit the wording change on the button. I was messing around with having several modals at one point and forgot to change it back before committing.

What do you think about just making that an ephemeral post?

I actually moved to this in my latest commit before seeing your message! Great minds, yada yada...

@deadbender

If we're moving to a .env here, it might also be good to do that for SLACK_API_KEY as well.

My intention isn't for everyone to move to a .env, I just did so because I prefer it to direnv which I was using before. Contributors can use whatever they like for setting up their environments which is why it is gitignored. We don't want to commit those files with our API keys to the repo.

From a user experience perspective, we might want to include a short blurb on the connection modal detailing what data we access (from what I remember Google's OAuth page for this is very vague) and what benefit they get from connecting Google.

The message from Google says that it will allow us to "view and edit events" on their calendar. I think that is clear enough. I will definitely add that language you suggested for the benefits though!

mrshwah commented 3 years ago

Before merging todos:

mrshwah commented 3 years ago

I have submitted to google for verification... And now we wait.