Gcal Integration by mrshwah · Pull Request #378 · penny-university/penny_university

mrshwah · 2020-11-15T17:46:46Z

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

Some todos:

After google auth, redirect to frontend page with user friendly message and button to redirect to slack
Send user message to set up google integration after creating penny chat (if no credentials exist)
Update google event when penny chat is updated
Fix Add to Google Calendar link in penny chat share
Send signal to create google events for all of the existing penny chat invites

Keep in mind any place where you might be adding any more effort on the part of our users. You changed the Share to Submit for a bit but changed it back. 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.
If I choose not to connect google, I'm going to see the same modal pop up after every chat I create. (Right?) That's going to feel a bit intrusive. What do you think about just making that an ephemeral post?
get_or_create_social_profile_from_slack_id was written a little sloppily in that it calls to slack upon every request. Maybe memoize the function so that it's used at most once per call.

JnBrymn · 2020-11-15T23:58:11Z

bot/tasks/pennychat.py

        start_date = penny_chat_invitation.date.astimezone(utc).strftime('%Y%m%dT%H%M%SZ')
        end_date = (penny_chat_invitation.date.astimezone(utc) + timedelta(hours=1)).strftime('%Y%m%dT%H%M%SZ')
+
+        description = f'{penny_chat_invitation.description} [Video Link]({penny_chat_invitation.video_conference_link})'


maybe add a new line? ...description}\n[Video...

Lol, I totally forgot to revisit this. Adding to todos...

ghost · 2020-11-16T04:35:05Z

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.

JnBrymn

I think it looks good so far. The main thing I'm wondering about is the user experience in the various paths through this. Would you mind demoing that again this week? (Especially if anything has changed.)

JnBrymn · 2020-11-16T04:09:02Z

bot/tasks/pennychat.py

        date_time_block
    ]

+    if penny_chat_invitation.video_conference_link:


probably a nit pick, but rather than have 3 sections here, I'd just have a {PREVIEW, INVITE, UPDATE} section with a parenthetical (A video link will be provided shortly before the chat starts) and a REMIND section with the full details.

So just remove the Video Link header from the invite and add parentheses?

yup, just like you did - looks good

bot/tasks/pennychat.py

JnBrymn · 2020-11-16T04:31:40Z

integrations/google.py

+        self.calendar_id = calendar_id
+
+    @property
+    def events(self):


I'm curious, why not just create self.events = self.service.events() in the init? 🤔 I wonder what calling events() each time you want self.events does.

Good point, I should do that instead.

JnBrymn · 2020-11-16T04:44:30Z

integrations/views.py

+    error = request.GET.get('error')
+    # TODO: Redirect to frontend page
+    if error:
+        return HttpResponse(f"<h1/>There was an error authorizing with Google.</h1><p>{request.GET.get('error')}</p>")


We should probably log details to sentry here. Probably sentry_sdk.capture_message since this isn't a raised exception.

mrshwah · 2020-11-16T12:43:15Z

@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!

JnBrymn · 2020-11-25T02:25:39Z

bot/processors/pennychat.py

    share_penny_chat_invitation,
    add_google_meet,
-    add_google_integration_blocks,
+    add_google_integration_blocks, update_google_meet,


nit, new line

JnBrymn · 2020-11-25T02:33:36Z

bot/tasks/pennychat.py

+def update_google_meet(penny_chat_id):
+    penny_chat_invitation = PennyChatSlackInvitation.objects.get(id=penny_chat_id)
+
+    calendar = get_user_google_calendar_from_slack_id(penny_chat_invitation.organizer_slack_id)


if the user retracts google permissions then calendar will be None here, add an if calendar is None here or we'll error on line 112. In this case I guess we just keep the calendar invite as is? (I presume it will still work)

JnBrymn · 2020-11-25T02:41:11Z

bot/tasks/pennychat.py

        date_time_block
    ]

+    if penny_chat_invitation.video_conference_link:


yup, just like you did - looks good

JnBrymn · 2020-11-25T02:51:02Z

integrations/signals.py

+    if credentials:
+        user = credentials.user
+        slack_ids = [profile.slack_id for profile in user.social_profiles.all()]
+        invites = PennyChatSlackInvitation.objects.filter(organizer_slack_id__in=slack_ids, date__gt=timezone.now())


would there ever be any invites with date__gt=timezone.now() ? is this a bug

disregard ... hey this box wine is great!

JnBrymn · 2020-11-25T03:16:38Z

bot/tasks/pennychat.py

-            f'{urllib.parse.quote(penny_chat_invitation.description)}'
+                         f'{urllib.parse.quote(penny_chat_invitation.title)}&dates=' \
+                         f'{start_date}/{end_date}&details=' \
+                         f'{urllib.parse.quote(description)}'


this function has grown in complexity, can you pull out the contents of this if statement into it's own helpful func that lives in this file?

mrshwah · 2020-11-25T03:45:45Z

Before merging todos:

Add production google api and add to secrets
Update docs
Privacy policy (another PR)

mrshwah · 2020-12-30T02:11:04Z

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

mrshwah added 2 commits October 25, 2020 16:52

google auth flow

137f73e

Integrate w/Google via Slack

0ab0f57

mrshwah requested review from a user and JnBrymn November 15, 2020 17:46

mrshwah added 2 commits November 15, 2020 11:54

Fix urls

35f82bd

Fix tests

e1b9ada

JnBrymn reviewed Nov 15, 2020

View reviewed changes

JnBrymn reviewed Nov 16, 2020

View reviewed changes

Add frontend page and task for adding a gcal event

0168e78

mrshwah and others added 5 commits November 16, 2020 08:45

PR updates

ccbdd7e

Add signal to create integration after save

9bea3e1

Fix test

581c692

Merge branch 'master' into gcal-integration

89d636d

Fix lint

858324f

mrshwah marked this pull request as ready for review November 25, 2020 01:58

JnBrymn reviewed Nov 25, 2020

View reviewed changes

JnBrymn approved these changes Nov 25, 2020

View reviewed changes

Add tests

f242563

mrshwah mentioned this pull request Dec 9, 2020

Incorporate hangout or zoom link invitation into penny chat invitation #130

Open

Conversation

mrshwah commented Nov 15, 2020 • edited by JnBrymn Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

related

Uh oh!

JnBrymn commented Nov 15, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ghost commented Nov 16, 2020

Uh oh!

JnBrymn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrshwah commented Nov 16, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrshwah commented Nov 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrshwah commented Dec 30, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mrshwah commented Nov 15, 2020 •

edited by JnBrymn

Loading

mrshwah commented Nov 25, 2020 •

edited

Loading