Skip to content
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

Feature/gcal integration #79

Open
wants to merge 29 commits into
base: master
Choose a base branch
from
Open

Conversation

filiptypjeu
Copy link
Member

Added integration of Google Calendar to FARS. You can configure a Bookable to create gcal events for every new Booking.

@filiptypjeu filiptypjeu requested a review from backjonas January 26, 2023 00:56
fars/booking/models.py Outdated Show resolved Hide resolved
@filiptypjeu filiptypjeu marked this pull request as ready for review January 29, 2023 11:45
fars/booking/gcal.py Outdated Show resolved Hide resolved
fars/booking/forms.py Outdated Show resolved Hide resolved
def __init__(self, booking):
self.booking = booking
self.gcal = GoogleCalendar(booking.bookable.google_calendar_id)
Thread.__init__(self)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IIRC threads in python are kind of a "fake" implementation. There is no actual parallelism, just async execution. I guess this is still fine though, as long as the request is allowed to finish handling and return the response without having to wait for the thread to finish.

fars/booking/gcal.py Outdated Show resolved Hide resolved
…GCal helper class to catch error (for example missing credentials file) instantly."

This reverts commit 9c9f2c3.
 - Changed Booking.save() parameter name to avoid having gcal logic in repeating bookings logic.
 - Changed GoogleCalendar helper class to mainly take event ids as method params.
 - Changed the GCal thread classes to work on GCalEvent objects.
fars/booking/gcal.py Outdated Show resolved Hide resolved
fars/booking/models.py Outdated Show resolved Hide resolved
fars/booking/migrations/0015_auto_20230201_2216.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@tlangens tlangens left a comment

Choose a reason for hiding this comment

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

I'd be content with this implementation, but I think further improvements could be made.
For one, I think the gcal sync should happen inside the GCalEvent's save/delete methods. This way the sync will work as expected when working directly with GCalEvent objects.
The GCalEventThreads should then simply call the appropriate method on the GCalEvent model.
I think I'd even have the GCalEvent model lookup the Booking from the DB rather than passing a copy to the thread. This way the threads work more independently, with less risk of race conditions. It'll be a slightly higher load on the DB, but hardly an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants