Starred google calander fix #398
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's GuideThis PR refactors calendar export to support webcal-based calendars and tokenized starred session exports by consolidating calendar redirect logic into a generic view, implementing a signed token mechanism for “my” calendars with expiry checks, extending exporter classes for webcal, and updating URL patterns and signals to register the new exporters. ER diagram for tokenized starred session exporterDiagram
USER ||--o{ SUBMISSION_FAVOURITE : has
SUBMISSION_FAVOURITE }o--|| SUBMISSION : refers
USER ||--o{ TOKEN : generates
TOKEN {
user_id int
exp int
}
Class diagram for updated and new calendar exporter classesclassDiagram
class BaseExporter
class BaseCalendarExporter {
+public
+show_qrcode
+icon
+show_public
}
class GoogleCalendarExporter {
+identifier
+verbose_name
+icon
+ical_exporter_cls
}
class MyGoogleCalendarExporter {
+identifier
+icon
+verbose_name
+ical_exporter_cls
}
class WebcalExporter {
+identifier
+verbose_name
+ical_exporter_cls
}
class MyWebcalExporter {
+identifier
+verbose_name
+ical_exporter_cls
}
BaseExporter <|-- BaseCalendarExporter
BaseCalendarExporter <|-- GoogleCalendarExporter
BaseCalendarExporter <|-- MyGoogleCalendarExporter
BaseCalendarExporter <|-- WebcalExporter
BaseCalendarExporter <|-- MyWebcalExporter
Class diagram for updated CalendarRedirectView and token logicclassDiagram
class CalendarRedirectView {
+MY_STARRED_ICS_TOKEN_SESSION_KEY
+get(request, *args, **kwargs)
+generate_ics_token(user_id)
+parse_ics_token(token)
+check_token_expiry(token)
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Saksham-Sirohi - I've reviewed your changes - here's some feedback:
- Extract magic values like the token salt, expiry durations (15 days, 4 days), and session key into module‐level constants or settings so they aren’t hard‐coded in multiple places.
- The token generation/validation currently maintains its own “exp” field in addition to using signing.loads(max_age); consider relying on one mechanism for expiry (either the JWT exp claim or Django signing max_age) to simplify the logic.
- CalendarRedirectView contains duplicated URL‐building and HTML redirect snippets for Google vs webcal—pull that into helper methods or a small template to reduce code repetition.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Extract magic values like the token salt, expiry durations (15 days, 4 days), and session key into module‐level constants or settings so they aren’t hard‐coded in multiple places.
- The token generation/validation currently maintains its own “exp” field in addition to using signing.loads(max_age); consider relying on one mechanism for expiry (either the JWT exp claim or Django signing max_age) to simplify the logic.
- CalendarRedirectView contains duplicated URL‐building and HTML redirect snippets for Google vs webcal—pull that into helper methods or a small template to reduce code repetition.
## Individual Comments
### Comment 1
<location> `src/pretalx/agenda/views/schedule.py:79` </location>
<code_context>
+ return signing.dumps(value, salt="my-starred-ics")
+
+ @staticmethod
+ def parse_ics_token(token):
+ """Parse and validate the token, return user_id if valid"""
+ try:
+ value = signing.loads(token, salt="my-starred-ics", max_age=15*24*60*60)
+ if value["exp"] < int(timezone.now().timestamp()):
+ raise ValueError("Token expired")
+ return value["user_id"]
+ except (signing.BadSignature, signing.SignatureExpired, KeyError, ValueError) as e:
+ logger.warning('Failed to parse ICS token: %s', e)
+ return None
+
</code_context>
<issue_to_address>
Token expiry is checked both by signing.loads and manually, which may be redundant.
Since signing.loads with max_age already enforces expiry, manually checking the 'exp' field is unnecessary and could introduce inconsistencies. Consider using only one expiry check.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
@staticmethod
def parse_ics_token(token):
"""Parse and validate the token, return user_id if valid"""
try:
value = signing.loads(token, salt="my-starred-ics", max_age=15*24*60*60)
if value["exp"] < int(timezone.now().timestamp()):
raise ValueError("Token expired")
return value["user_id"]
except (signing.BadSignature, signing.SignatureExpired, KeyError, ValueError) as e:
logger.warning('Failed to parse ICS token: %s', e)
return None
=======
@staticmethod
def parse_ics_token(token):
"""Parse and validate the token, return user_id if valid"""
try:
value = signing.loads(token, salt="my-starred-ics", max_age=15*24*60*60)
return value["user_id"]
except (signing.BadSignature, signing.SignatureExpired, KeyError) as e:
logger.warning('Failed to parse ICS token: %s', e)
return None
>>>>>>> REPLACE
</suggested_fix>
### Comment 2
<location> `src/pretalx/agenda/views/schedule.py:91` </location>
<code_context>
+ return None
+
+ @staticmethod
+ def check_token_expiry(token):
+ """Check if a token exists and has more than 4 days until expiry
+
+ Returns:
+ - None if token is invalid
+ - False if token is valid but expiring soon (< 4 days)
+ - True if token is valid and not expiring soon (>= 4 days)
+ """
+ try:
+ value = signing.loads(token, salt="my-starred-ics")
+ expiry_date = timezone.datetime.fromtimestamp(value["exp"], tz=timezone.utc)
+ time_until_expiry = expiry_date - timezone.now()
+ return time_until_expiry >= timedelta(days=4)
+ except Exception as e:
+ logger.warning('Failed to check token expiry: %s', e)
+ return None # Invalid token
class ExporterView(EventPermissionRequired, ScheduleMixin, TemplateView):
</code_context>
<issue_to_address>
check_token_expiry does not enforce max_age, which could allow expired tokens to be considered valid.
Unlike parse_ics_token, this method does not use max_age in signing.loads, which may lead to inconsistent token validation. Please add max_age for consistency and to ensure expired tokens are rejected.
</issue_to_address>
### Comment 3
<location> `src/pretalx/agenda/views/schedule.py:130` </location>
<code_context>
url = resolve(self.request.path_info)
- if url.url_name == "export":
+ calendar_exports = ["export.google-calendar", "export.my-google-calendar", "export.other-calendar", "export.my-other-calendar"]
+ if url.url_name in ["export", "export-tokenized"]:
exporter = url.kwargs.get("name") or unquote(
self.request.GET.get("exporter")
</code_context>
<issue_to_address>
Hardcoded calendar_exports list includes non-existent URL names.
'export.other-calendar' and 'export.my-other-calendar' are not defined in the URL config. Please remove or update these entries to reflect actual URL patterns.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
calendar_exports = ["export.google-calendar", "export.my-google-calendar", "export.other-calendar", "export.my-other-calendar"]
=======
calendar_exports = ["export.google-calendar", "export.my-google-calendar"]
>>>>>>> REPLACE
</suggested_fix>
### Comment 4
<location> `src/pretalx/agenda/views/schedule.py:162` </location>
<code_context>
- if "-my" in exporter.identifier and self.request.user.id is None:
+ # Handle tokenized access for Google Calendar integration
+ token = kwargs.get('token')
+ if token and "-my" in exporter.identifier:
+ user_id = ScheduleMixin.parse_ics_token(token)
+ if not user_id:
+ raise Http404()
+
+ # Set up exporter for this user without requiring login
+ favs_talks = SubmissionFavourite.objects.filter(user=user_id)
+ if favs_talks.exists():
+ exporter.talk_ids = list(
+ favs_talks.values_list("submission_id", flat=True)
</code_context>
<issue_to_address>
No check for user existence when using user_id from token.
Please verify that the user exists and is active before filtering SubmissionFavourite with user_id from the token, to prevent issues with deleted or deactivated users.
</issue_to_address>
### Comment 5
<location> `src/pretalx/agenda/views/schedule.py:173` </location>
<code_context>
+ exporter.talk_ids = list(
+ favs_talks.values_list("submission_id", flat=True)
+ )
+ elif "-my" in exporter.identifier and self.request.user.id is None:
if request.GET.get("talks"):
exporter.talk_ids = request.GET.get("talks").split(",")
else:
return HttpResponseRedirect(self.request.event.urls.login)
- favs_talks = SubmissionFavourite.objects.filter(
- user=self.request.user.id
</code_context>
<issue_to_address>
Redirect to login may not preserve intended destination after authentication.
Consider including a 'next' parameter in the login redirect to retain the user's original request, ensuring they can complete their intended action after authentication.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
else:
return HttpResponseRedirect(self.request.event.urls.login)
=======
else:
from django.utils.http import urlquote
next_url = urlquote(self.request.get_full_path())
login_url = f"{self.request.event.urls.login}?next={next_url}"
return HttpResponseRedirect(login_url)
>>>>>>> REPLACE
</suggested_fix>
### Comment 6
<location> `src/pretalx/agenda/views/schedule.py:362` </location>
<code_context>
+class CalendarRedirectView(EventPermissionRequired, ScheduleMixin, TemplateView):
</code_context>
<issue_to_address>
Redirect for unauthenticated users does not preserve the original destination.
Consider adding a 'next' parameter to the redirect so users return to their intended destination after login.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Saksham-Sirohi
force-pushed
the
starred-google-calander-fix
branch
from
c21eed4 to
3515791
Compare
Saksham-Sirohi
force-pushed
the
starred-google-calander-fix
branch
from
e18e472 to
7d8cf44
Compare
hongquan
requested changes
Aug 3, 2025
Saksham-Sirohi
force-pushed
the
starred-google-calander-fix
branch
3 times, most recently
from
7815ff6 to
cd7e558
Compare
Saksham-Sirohi
force-pushed
the
starred-google-calander-fix
branch
from
cd7e558 to
98fa758
Compare
hongquan
approved these changes
Aug 3, 2025
|
It looks like adding sessions to Google cal creates a "Google Meet" link for each session. How to avoid this? |
|
@mariobehling Unfortunately, this specific Google Calendar setting ("Automatically add Google Meet video conferences to events I create") cannot be changed programmatically via the Google Calendar API or any .ics file directive. It must be set by the user (or their admin) in the Google Calendar UI or Admin Console. There is no API parameter, property, or value you can add to your .ics export or Google Calendar API call that disables Meet links for the user. So, The user will need to manually disable disable this under event settings in google calender settings I am still looking for a workaround will update regarding this |
|
@Saksham-Sirohi confirmed No google link was created when I tested on Chrome Browser |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #397
Google Exporters now uses HTTP, and a new calendar option has been added to support webcal-based calendars
Summary by Sourcery
Add support for tokenized starred session exports and a unified calendar redirect handler for both Google and other calendar protocols
New Features:
Enhancements: