Skip to content

zulip-integrations: Update google oauth and reminders for google calendar integration. #850

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

Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@ module = [
"feedparser.*",
"gitlint.*",
"googleapiclient.*",
"google_api_python_client.*",
"google_auth_httplib2.*",
"google_auth_oauthlib.*",
"irc.*",
"mercurial.*",
"nio.*",
Expand Down
68 changes: 52 additions & 16 deletions zulip/integrations/google/get-google-credentials
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,43 @@
import argparse
import os

from oauth2client import client, tools
from oauth2client.file import Storage
from google.auth.transport.requests import Request # type: ignore[import-not-found]
from google.oauth2.credentials import Credentials # type: ignore[import-not-found]
from google_auth_oauthlib.flow import InstalledAppFlow

parser = argparse.ArgumentParser(add_help=False)
parser.add_argument(
"--auth_host_name", default="localhost", help="Hostname when running a local web server."
)
parser.add_argument(
"--noauth_local_webserver",
action="store_true",
default=False,
help="Do not run a local web server.",
)
parser.add_argument(
"--auth_host_port",
default=[8080, 8090],
type=int,
nargs="*",
help="Port web server should listen on.",
)
flags = parser.parse_args()

flags = argparse.ArgumentParser(parents=[tools.argparser]).parse_args()

# If modifying these scopes, delete your previously saved credentials
# at zulip/bots/gcal/
# NOTE: When adding more scopes, add them after the previous one in the same field, with a space
# seperating them.
SCOPES = "https://www.googleapis.com/auth/calendar.readonly"
SCOPES = ["https://www.googleapis.com/auth/calendar.readonly"]
# This file contains the information that google uses to figure out which application is requesting
# this client's data.
CLIENT_SECRET_FILE = "client_secret.json" # noqa: S105
APPLICATION_NAME = "Zulip Calendar Bot"
HOME_DIR = os.path.expanduser("~")


def get_credentials() -> client.Credentials:
def get_credentials() -> Credentials:
"""Gets valid user credentials from storage.

If nothing has been stored, or if the stored credentials are invalid,
Expand All @@ -28,19 +47,36 @@ def get_credentials() -> client.Credentials:
Returns:
Credentials, the obtained credential.
"""

credentials = None
credential_path = os.path.join(HOME_DIR, "google-credentials.json")

store = Storage(credential_path)
credentials = store.get()
if not credentials or credentials.invalid:
flow = client.flow_from_clientsecrets(os.path.join(HOME_DIR, CLIENT_SECRET_FILE), SCOPES)
flow.user_agent = APPLICATION_NAME
# This attempts to open an authorization page in the default web browser, and asks the user
# to grant the bot access to their data. If the user grants permission, the run_flow()
# function returns new credentials.
credentials = tools.run_flow(flow, store, flags)
if os.path.exists(credential_path):
credentials = Credentials.from_authorized_user_file(credential_path, SCOPES)
if not credentials or not credentials.valid:
if credentials and credentials.expired and credentials.refresh_token:
credentials.refresh(Request())
else:
flow = InstalledAppFlow.from_client_secrets_file(
os.path.join(HOME_DIR, CLIENT_SECRET_FILE), SCOPES
)
if not flags.noauth_local_webserver:
credentials = flow.run_local_server(
host=flags.auth_host_name, port=flags.auth_host_port[0]
)
# This attempts to open an authorization page in the default web browser, and asks the user
# to grant the bot access to their data. If the user grants permission, the run_flow()
# function returns new credentials.
else:
auth_url, _ = flow.authorization_url(prompt="consent")
print(
"Proceed to the following link in your browser:",
auth_url,
)
auth_code = input("Enter the authorization code: ")
credentials = flow.fetch_token(code=auth_code)
with open(credential_path, "w") as token:
token.write(credentials.to_json())
print("Storing credentials to " + credential_path)
return credentials


get_credentials()
192 changes: 139 additions & 53 deletions zulip/integrations/google/google-calendar
Original file line number Diff line number Diff line change
Expand Up @@ -9,33 +9,45 @@ import logging
import os
import sys
import time
from typing import List, Optional, Set, Tuple
from typing import List, Optional, Set, Tuple, TypedDict

import dateutil.parser
import httplib2
import pytz
from oauth2client import client
from oauth2client.file import Storage

try:
from googleapiclient import discovery
from google.oauth2.credentials import Credentials # type: ignore[import-not-found]
from googleapiclient.discovery import build
except ImportError:
logging.exception("Install google-api-python-client")
logging.exception("Install google-api-python-client and google-auth-oauthlib")
sys.exit(1)

sys.path.append(os.path.join(os.path.dirname(__file__), "../../"))
import zulip

SCOPES = "https://www.googleapis.com/auth/calendar.readonly"
SCOPES = ["https://www.googleapis.com/auth/calendar.readonly"]
CLIENT_SECRET_FILE = "client_secret.json" # noqa: S105
APPLICATION_NAME = "Zulip"
HOME_DIR = os.path.expanduser("~")


class Event(TypedDict):
id: int
start: datetime.datetime
end: datetime.datetime
summary: str
html_link: str
status: str
location: str
description: str
organizer: str
hangout_link: str
reminder: int
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a comment explaining this field here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will do.



# Our cached view of the calendar, updated periodically.
events: List[Tuple[int, datetime.datetime, str]] = []
events: List[Event] = []

# Unique keys for events we've already sent, so we don't remind twice.
sent: Set[Tuple[int, datetime.datetime]] = set()
# Unique keys for reminders we've already sent, so we don't remind twice.
sent: Set[Tuple[int, datetime.datetime, int]] = set()
Copy link
Contributor

Choose a reason for hiding this comment

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

How about instead of adding a separate reminder field, we actually switch the start_time field in the tuple to instead use the reminder_time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This would be better will make this change


sys.path.append(os.path.dirname(__file__))

Expand All @@ -62,12 +74,11 @@ google-calendar --calendar [email protected]


parser.add_argument(
"--interval",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not change this.
This is not meant to be an override, this is the default value for the interval we're going to be using.
This shouldn't be a required argument, but an optional one with the default value as 30 mins.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you explain this argument properly? What is it supposed to signify?

dest="interval",
default=30,
"--override",
dest="override",
type=int,
action="store",
help="Minutes before event for reminder [default: 30]",
help="Override the reminder time for all events.",
metavar="MINUTES",
)

Expand All @@ -88,7 +99,7 @@ if not options.zulip_email:
zulip_client = zulip.init_from_options(options)


def get_credentials() -> client.Credentials:
def get_credentials() -> Credentials:
"""Gets valid user credentials from storage.

If nothing has been stored, or if the stored credentials are invalid,
Expand All @@ -100,95 +111,170 @@ def get_credentials() -> client.Credentials:
"""
try:
credential_path = os.path.join(HOME_DIR, "google-credentials.json")

store = Storage(credential_path)
return store.get()
except client.Error:
credentials = Credentials.from_authorized_user_file(credential_path, SCOPES)
except ValueError:
logging.exception("Error while trying to open the `google-credentials.json` file.")
sys.exit(1)
except OSError:
logging.error("Run the get-google-credentials script from this directory first.")
sys.exit(1)
else:
return credentials


def populate_events() -> Optional[None]:
credentials = get_credentials()
creds = credentials.authorize(httplib2.Http())
service = discovery.build("calendar", "v3", http=creds)

now = datetime.datetime.now(pytz.utc).isoformat()
creds = get_credentials()
service = build("calendar", "v3", credentials=creds)
feed = (
service.events()
.list(
calendarId=options.calendarID,
timeMin=now,
maxResults=5,
timeMin=datetime.datetime.now(pytz.utc).isoformat(),
timeMax=datetime.datetime.now(pytz.utc).isoformat().split("T")[0] + "T23:59:59Z",
Copy link
Contributor

Choose a reason for hiding this comment

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

maxResults=5 is definitely restricting and needs to be updated for the new algorithm, but I'm not sure we want to pick only events for the same day.
For example, what if the user wants to send reminders like "7 days to go!", "3 days to go", etc. We should be able to support those too.
So, I'm not yet sure what we can do here, to avoid fetching too many events every single call, especially since we do events.clear() each time.

There are other options like not clearing our cache, and getting notifications from Google, on the calendar being updated instead. But again, I haven't looked into the feasibility of it, and whether that's necessary or optimal. And that would warrant more commits.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can deal with this later I guess. Will try to think of a feasible solution.

singleEvents=True,
orderBy="startTime",
)
.execute()
)

events.clear()
for event in feed["items"]:
try:
start = dateutil.parser.parse(event["start"]["dateTime"])
end = dateutil.parser.parse(event["end"]["dateTime"])
# According to the API documentation, a time zone offset is required
# for start.dateTime unless a time zone is explicitly specified in
# start.timeZone.
if start.tzinfo is None:
if start.tzinfo is None or end.tzinfo is None:
event_timezone = pytz.timezone(event["start"]["timeZone"])
# pytz timezones include an extra localize method that's not part
# of the tzinfo base class.
start = event_timezone.localize(start)
end = event_timezone.localize(end)
except KeyError:
# All-day events can have only a date.
start_naive = dateutil.parser.parse(event["start"]["date"])

end_naive = dateutil.parser.parse(event["end"]["date"])
# All-day events don't have a time zone offset; instead, we use the
# time zone of the calendar.
calendar_timezone = pytz.timezone(feed["timeZone"])
# pytz timezones include an extra localize method that's not part
# of the tzinfo base class.
start = calendar_timezone.localize(start_naive)
end = calendar_timezone.localize(end_naive)
now = datetime.datetime.now(tz=start.tzinfo)
if start < now:
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure why this is required.
We're using timeMin = now, so why would start be < now?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fetching events with timeMin = now also fetches events that are ongoing for some reason and we want to ignore those events since they have already started. Hence a small check to ignore these events.

Example:

  • This event is ongoing:

    image

  • When I print the feed of events received (print(feed)) I see this event popping up:

    image

continue
id = event["id"]
summary = event.get("summary", "(No Title)")
html_link = event["htmlLink"]
status = event.get("status", "confirmed")
location = event.get("location", "")
description = event.get("description", "")
organizer = (
""
if (
event["organizer"]["email"] == options.zulip_email or event["organizer"].get("self")
)
else event["organizer"].get("displayName", event["organizer"]["email"])
)
hangout_link = event.get("hangoutLink", "")
reminders = event["reminders"]
# If the user has specified an override, we use that for all events.
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's actually do this instead.

Let's ignore the calendar's default reminders value.
The only value we'll use is event["reminders"]["overrides"]["minutes"].

We'll use 30 mins as the default interval.
If the user wants something else, they can pass it in with --interval.

And for particular events with reminders set for x intervals, we override using the interval.

So, something like the below.

Suggested change
# If the user has specified an override, we use that for all events.
if event.get("reminders", {}).get("overrides"):
for override in event["reminders"]["overrides"]:
reminder_interval = override["minutes"]
event["reminder"] = event["start"] - datetime.timedelta(minutes=reminder_interval)
# Call events.append() for each override

Note that I'm suggesting setting event["reminder"] not to event["reminders"]["overrides"]["minutes"], but actually the time it should be called, as a replacement for event["start"].

And I know I had previously mentioned not sending notifications if it's turned off for particular events, after taking a look at the API. But, event["reminders"]["useDefault"] seems to return False by default, and I haven't yet figured out how to set it to True through the Google Calendar UI. So, unless there's an easy way to do that, and its commonly used (and you can figure it out), I think we can drop that, at least for now.

# If the event uses the calendar's default reminders, we use that.
# If the event has overrides on Google Calendar, we use that.
# If none of the above, we don't set a reminder.
if options.override:
reminder_minutes = [options.override]
elif reminders.get("useDefault"):
calendar_list = service.calendarList().get(calendarId=options.calendarID).execute()
reminder_minutes = (
[reminder["minutes"] for reminder in calendar_list["defaultReminders"]]
if calendar_list.get("defaultReminders")
else []
)
elif reminders.get("overrides"):
reminder_minutes = [reminder["minutes"] for reminder in reminders["overrides"]]
else:
reminder_minutes = []
events.extend(
{
"id": id,
"start": start,
"end": end,
"summary": summary,
"html_link": html_link,
"status": status,
"location": location,
"description": description,
"organizer": organizer,
"hangout_link": hangout_link,
"reminder": reminder,
}
for reminder in reminder_minutes
)

try:
events.append((event["id"], start, event["summary"]))
except KeyError:
events.append((event["id"], start, "(No Title)"))

def event_to_message(event: Event) -> str:
"""Parse the event dictionary and return a string that can be sent as a message.

The message includes the event title, start and end times, location, organizer, hangout link, and description.

Returns:
str: The message to be sent.
"""
line = f"**[{event['summary']}]({event['html_link']})**\n"
if event["start"].hour == 0 and event["start"].minute == 0:
line += "Scheduled for today.\n"
else:
line += f"Scheduled from **{event['start'].strftime('%H:%M')}** to **{event['end'].strftime('%H:%M')}**.\n"
line += f"**Location:** {event['location']}\n" if event["location"] else ""
line += f"**Organizer:** {event['organizer']}\n" if event["organizer"] else ""
line += (
f"**Hangout Link:** [{event['hangout_link'].split('/')[2]}]({event['hangout_link']})\n"
if event["hangout_link"]
else ""
)
line += f"**Status:** {event['status']}\n" if event["status"] else ""
line += f"**Description:** {event['description']}\n" if event["description"] else ""
return line


def send_reminders() -> Optional[None]:
messages = []
messages: List[str] = []
keys = set()
now = datetime.datetime.now(tz=pytz.utc)

for id, start, summary in events:
dt = start - now
if dt.days == 0 and dt.seconds < 60 * options.interval:
# The unique key includes the start time, because of
# repeating events.
key = (id, start)
# Sort events by the time of the reminder.
events.sort(
Copy link
Contributor

Choose a reason for hiding this comment

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

Switching to using "reminder" instead of "start" can now simplify this to:

Suggested change
events.sort(
events.sort(key=lambda event: (event["reminder"]))

key=lambda event: (event["start"] - datetime.timedelta(minutes=event["reminder"])),
reverse=True,
)
# Iterate through the events and send reminders for those whose reminder time has come or passed and remove them from the list.
# The instant a reminder's time is greater than the current time, we stop sending reminders and break out of the loop.
while len(events):
Copy link
Contributor

@Niloth-p Niloth-p Feb 20, 2025

Choose a reason for hiding this comment

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

Switching to using "reminder" instead of "start" can now simplify this to using the previous logic itself, so this section of changes wouldn't be needed.

Just "start" -> "reminder".

Suggested change
while len(events):
dt = event["reminder"] - now
if dt.days == 0 and dt.seconds > 0:
# The unique key includes the reminder time due to repeating events, and reminder overrides.
key = (event["id"], event["reminder"])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes makes sense will do.

event = events[-1]
now = datetime.datetime.now(tz=event["start"].tzinfo)
dt = event["start"] - datetime.timedelta(minutes=event["reminder"])
if dt <= now:
key = (event["id"], event["start"], event["reminder"])
if key not in sent:
if start.hour == 0 and start.minute == 0:
line = f"{summary} is today."
else:
line = "{} starts at {}".format(summary, start.strftime("%H:%M"))
line = event_to_message(event)
print("Sending reminder:", line)
messages.append(line)
messages = [line, *messages]
keys.add(key)
events.pop()
else:
break

if not messages:
return

if len(messages) == 1:
message = "Reminder: " + messages[0]
message = "**Reminder:**\n\n " + messages[0]
else:
message = "Reminder:\n\n" + "\n".join("* " + m for m in messages)
message = "**Reminders:**\n\n" + "\n".join(
str(i + 1) + ". " + m for i, m in enumerate(messages)
)

zulip_client.send_message(
dict(type="private", to=options.zulip_email, sender=options.zulip_email, content=message)
)
zulip_client.send_message(dict(type="private", to=options.zulip_email, content=message))

sent.update(keys)

Expand Down
Loading