-
Notifications
You must be signed in to change notification settings - Fork 1
Issue 9 configure google calendar integration #43
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
base: main
Are you sure you want to change the base?
Issue 9 configure google calendar integration #43
Conversation
d8052b3 to
0b7d4a0
Compare
This reverts commit a13dd93.
b189071 to
05f0bea
Compare
server/.env.example
Outdated
| # Google API | ||
| GOOGLE_CREDENTIALS_FILE=server/api/booking/google_calendar/template_service_account.json | ||
| GOOGLE_CALENDAR_ID=xxxx | ||
| AWS_ACCESS_KEY_ID=xxxx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove aws related variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| CALENDAR_ID = os.getenv("GOOGLE_CALENDAR_ID") | ||
|
|
||
|
|
||
| def create_event(event_data: dict): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please add docs for event_data's key
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test fails when you don't set the environment variables so need to fix it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…ndency on env vars need to be set in test file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds Google Calendar integration functionality to support CRUD operations on calendar events. The implementation uses service account authentication and provides helper functions for creating, reading, updating, and deleting events.
Key Changes
- Added
google-api-python-clientdependency and related packages - Created helper functions (
create_event,get_event,update_event,delete_event) inevents.py - Implemented service account authentication in
client.py - Added integration tests with environment variable gating
Reviewed changes
Copilot reviewed 6 out of 10 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| server/pyproject.toml | Added google-api-python-client dependency |
| server/poetry.lock | Updated lock file with new dependencies and version updates |
| server/api/booking/google_calendar/test_google_calendar.py | Added integration test for CRUD operations |
| server/api/booking/google_calendar/events.py | Implemented calendar event CRUD helper functions |
| server/api/booking/google_calendar/client.py | Created service account authentication handler |
| server/api/booking/google_calendar/template_service_account.json | Added template for service account credentials |
| server/api/booking/google_calendar/init.py | Created empty module init file |
| server/.env.example | Added Google Calendar configuration placeholders |
| .gitignore | Added actual service account credentials file to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def get_calendar_service(): | ||
|
|
||
| if not cred_path: | ||
| raise FileNotFoundError( |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using FileNotFoundError is misleading here. The error is raised when the environment variable is not set, not when a file is not found. The file might exist but the environment variable is simply missing. Consider using ValueError or a custom exception instead, as this is a configuration error.
| raise FileNotFoundError( | |
| raise ValueError( |
| if not CALENDAR_ID: | ||
| raise ValueError( | ||
| "GOOGLE_CALENDAR_ID is missing. Set it in your environment.") |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CALENDAR_ID validation check is duplicated across all four functions (lines 30, 48, 66, 83). Consider extracting this validation into a helper function or decorator to reduce code duplication and improve maintainability.
| "dateTime": "2025-12-2T15:00:00", | ||
| "timeZone": "Australia/Perth", | ||
| }, | ||
| "end": { | ||
| "dateTime": "2025-12-2T16:00:00", |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date format in the test is invalid. ISO 8601 format requires two-digit day values. The date "2025-12-2" should be "2025-12-02". This applies to both the start and end dateTime fields.
| "dateTime": "2025-12-2T15:00:00", | |
| "timeZone": "Australia/Perth", | |
| }, | |
| "end": { | |
| "dateTime": "2025-12-2T16:00:00", | |
| "dateTime": "2025-12-02T15:00:00", | |
| "timeZone": "Australia/Perth", | |
| }, | |
| "end": { | |
| "dateTime": "2025-12-02T16:00:00", |
| cred_path = os.getenv("GOOGLE_CREDENTIALS_FILE") | ||
| calendar_id = os.getenv("GOOGLE_CALENDAR_ID") | ||
|
|
||
|
|
||
| def get_calendar_service(): |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Environment variables are being read at module import time (line 13-14), which means they are loaded once when the module is first imported. This can cause issues in testing scenarios or when environment variables change during runtime. Consider moving these reads inside the get_calendar_service() function or making them lazy-loaded.
| cred_path = os.getenv("GOOGLE_CREDENTIALS_FILE") | |
| calendar_id = os.getenv("GOOGLE_CALENDAR_ID") | |
| def get_calendar_service(): | |
| def get_calendar_service(): | |
| cred_path = os.getenv("GOOGLE_CREDENTIALS_FILE") | |
| calendar_id = os.getenv("GOOGLE_CALENDAR_ID") |
| import os | ||
| from .client import get_calendar_service | ||
|
|
||
| CALENDAR_ID = os.getenv("GOOGLE_CALENDAR_ID") |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CALENDAR_ID is loaded at module import time, which means it's evaluated once when the module is first imported. This can cause issues in testing environments or when environment variables change. Consider loading it inside each function or using a lazy-loading pattern.
|
|
||
|
|
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary blank line added. This creates an inconsistent spacing pattern in the dependencies section.
| GOOGLE_CREDENTIALS_FILE=server/api/booking/google_calendar/template_service_account.json | ||
| GOOGLE_CALENDAR_ID=xxxx | ||
|
|
||
|
|
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unnecessary blank line added at the end of the Google API section.
| "dateTime": "2025-12-2T15:00:00", | ||
| "timeZone": "Australia/Perth", | ||
| }, | ||
| "end": { | ||
| "dateTime": "2025-12-2T16:00:00", |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date format in the test is invalid. ISO 8601 format requires two-digit day values. The date "2025-12-2" should be "2025-12-02".
| "dateTime": "2025-12-2T15:00:00", | |
| "timeZone": "Australia/Perth", | |
| }, | |
| "end": { | |
| "dateTime": "2025-12-2T16:00:00", | |
| "dateTime": "2025-12-02T15:00:00", | |
| "timeZone": "Australia/Perth", | |
| }, | |
| "end": { | |
| "dateTime": "2025-12-02T16:00:00", |
| load_dotenv(os.path.join(BASE_DIR, ".env")) | ||
|
|
||
| cred_path = os.getenv("GOOGLE_CREDENTIALS_FILE") | ||
| calendar_id = os.getenv("GOOGLE_CALENDAR_ID") |
Copilot
AI
Dec 3, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The calendar_id variable is retrieved but never used in this module. Consider removing it to avoid confusion.
| calendar_id = os.getenv("GOOGLE_CALENDAR_ID") |
Change Summary
Added Google Calendar integration for CRUD operations.
Created helper functions (create_event, get_event, update_event, delete_event) in events.py.
Added client.py to handle service account authentication.
Added pytest for CRUD operations using personal Google API credentials
Change Form
The pull request title has an issue number — Yes
The change works by "Smoke testing" or quick testing — ran mocked CRUD tests, all passed
The change has tests — yes
The change has documentation — yes
Other Information
.env.example updated with placeholders for GOOGLE_CREDENTIALS_FILE and GOOGLE_CALENDAR_ID.