Skip to content

feat: implement authenticated calendar subscriptions with stable URLs#217

Open
abe-101 wants to merge 6 commits intomainfrom
feature/calendar-migration
Open

feat: implement authenticated calendar subscriptions with stable URLs#217
abe-101 wants to merge 6 commits intomainfrom
feature/calendar-migration

Conversation

@abe-101
Copy link
Copy Markdown
Owner

@abe-101 abe-101 commented Oct 20, 2025

Add user-based calendar subscription system with persistent alarm preferences. Each subscription gets a unique 11-character short UUID for clean, shareable URLs.

Key changes:

  • Add UserCalendarSubscription model with shortuuid and alarm_time field
  • Auto-create subscriptions for calendar owners on creation
  • Auto-create subscriptions for authenticated users viewing calendars
  • Store alarm preferences in database instead of URL query parameters
  • Add HTMX endpoint for updating alarm time without changing subscription URL
  • Migrate calendar templates to unified subscription link components
  • Add migration prompt injection for legacy calendar access
  • Add subscribe endpoint and admin interface for subscription management

Database migrations:

  • 0009: Add UserCalendarSubscription model and migrated_at tracking
  • 0010: Create subscriptions for all existing calendar owners
  • 0011: Add subscribers many-to-many relationship to Calendar
  • 0012: Add alarm_time field to subscriptions (default: 9 AM)

Technical details:

  • Subscription URLs: /c/<subscription_id>.ics (11 chars, base57 alphabet)
  • Alarm time stored in DB ensures stable URLs for calendar apps
  • HTMX updates subscription preferences without page refresh
  • Migration-aware calendar serving with backward compatibility

🤖 Generated with Claude Code

Add user-based calendar subscription system with persistent alarm preferences.
Each subscription gets a unique 11-character short UUID for clean, shareable URLs.

Key changes:
- Add UserCalendarSubscription model with shortuuid and alarm_time field
- Auto-create subscriptions for calendar owners on creation
- Auto-create subscriptions for authenticated users viewing calendars
- Store alarm preferences in database instead of URL query parameters
- Add HTMX endpoint for updating alarm time without changing subscription URL
- Migrate calendar templates to unified subscription link components
- Add migration prompt injection for legacy calendar access
- Add subscribe endpoint and admin interface for subscription management

Database migrations:
- 0009: Add UserCalendarSubscription model and migrated_at tracking
- 0010: Create subscriptions for all existing calendar owners
- 0011: Add subscribers many-to-many relationship to Calendar
- 0012: Add alarm_time field to subscriptions (default: 9 AM)

Technical details:
- Subscription URLs: /c/<subscription_id>.ics (11 chars, base57 alphabet)
- Alarm time stored in DB ensures stable URLs for calendar apps
- HTMX updates subscription preferences without page refresh
- Migration-aware calendar serving with backward compatibility

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@abe-101 abe-101 requested a review from Copilot October 20, 2025 02:22
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Implements authenticated, user-specific calendar subscriptions with stable URLs and alarm preferences stored in the database.

  • Adds UserCalendarSubscription model, migration flow, and owner auto-subscription logic.
  • Introduces subscription-aware calendar rendering, alarm update HTMX endpoint, and unified subscription link templates.
  • Adds new endpoints for per-subscription ICS access and migration prompting for legacy calendars.

Reviewed Changes

Copilot reviewed 18 out of 18 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
requirements/base.txt Adds shortuuid dependency for generating compact subscription IDs.
users/models.py Adds comment clarifying reverse relation access for subscribed calendars.
templates/hebcal/calendar_list.html Adds migrated badge and subscribed calendars section with share URL updates.
templates/hebcal/calendar_detail.html Introduces subscription-specific UI, alarm selector, and conditional ICS URL logic.
templates/hebcal/_calendar_subscribe_links.html New reusable subscription link component including platform-specific subscribe actions.
templates/hebcal/_calendar_links_htmx.html Wrapper for HTMX calendar link updates using new shared partial.
static/js/project.js Refactors sharing/copy logic to operate directly on provided URLs.
hebcal/views.py Adds subscription creation, legacy/migrated handling, alarm update endpoint, and subscription ICS serving.
hebcal/utils.py Adds migration prompt event injection and placeholder email notification function.
hebcal/urls.py Registers new subscribe and alarm update endpoints.
hebcal/models.py Adds migration state, subscribers M2M via new subscription model, and subscription utilities.
hebcal/migrations/* Introduces subscription model, owner backfill, M2M subscribers, and alarm_time field.
hebcal/admin.py Adds admin integration for subscriptions and migration actions.
config/urls.py Adds top-level short subscription ICS endpoint.
Comments suppressed due to low confidence (1)

my_hebrew_dates/hebcal/views.py:1

  • The subscription ICS endpoint introduces last_accessed tracking and uses stored alarm_time; add tests to assert last_accessed updates on each request and that alarm_time influences the generated ICS alarm trigger.
import base64

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines +102 to +110
<div class="flex-grow-1">
<h5 class="card-title mb-1">
<i class="bi bi-calendar-heart text-secondary me-2"></i>{{ subscription.calendar.name }}
</h5>
<div class="text-muted small">
<span class="badge bg-secondary">{{ subscription.calendar.calendarOf.all.count }} event
{% if subscription.calendar.calendarOf.all.count != 1 %}s{% endif %}
</span>
<span class="ms-2">Owner: {{ subscription.calendar.owner.email }}</span>
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Accessing subscription.calendar.owner.email without selecting the owner will cause an extra query per subscription. In the view, extend select_related to include 'calendar__owner' (e.g., .select_related('calendar', 'calendar__owner')) to avoid N+1 database queries.

Copilot uses AI. Check for mistakes.
Comment on lines +52 to +53
@admin.action(description="Enable migration (set migrated_at)")
def enable_migration(self, request, queryset):
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The actions list references 'migrate_and_notify' which is not defined; the declared actions are enable_migration and send_migration_email. Update actions = ['enable_migration', 'send_migration_email'] or implement migrate_and_notify to avoid a missing attribute error in the admin.

Copilot uses AI. Check for mistakes.
- Fix malformed Django template conditional in calendar_detail.html (inline if/else)
- Add select_related('calendar__owner') to avoid N+1 queries in calendar_list
- Replace slice:'8:' with safer URL transformation using cut/add filters
- Add @login_required decorator to create_calendar_view for security
- Add validation for alarm_time to prevent extreme values
- Centralize alarm time options in UserCalendarSubscription.ALARM_TIME_CHOICES
- Fix admin actions list to reference actual method names

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@abe-101 abe-101 requested a review from Copilot October 20, 2025 02:51
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

my_hebrew_dates/hebcal/views.py:476

  • Corrected spelling of 'expirimental' to 'experimental'. Consider supporting the old query param temporarily for backward compatibility.
    expirimental = request.GET.get("expirimental", False)
    if expirimental:
        calendar_str = generate_ical_experimental(
            model_calendar=calendar,

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@@ -355,33 +422,54 @@ def serve_pixel(request, pixel_id: UUID, pk: int):

@cache_page(60 * 60) # Cache the page for 15 minutes
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

The comment states 15 minutes but 60 * 60 caches for 60 minutes. Update the comment or adjust to 60 * 15 to match the intended duration.

Suggested change
@cache_page(60 * 60) # Cache the page for 15 minutes
@cache_page(60 * 15) # Cache the page for 15 minutes

Copilot uses AI. Check for mistakes.
Comment on lines +586 to +599
@cache_page(60 * 60)
def calendar_subscription_file(request: HttpRequest, subscription_id: str):
"""
Serve calendar file via authenticated subscription short ID.
Uses the subscription's alarm_time preference from database.
"""
subscription = get_object_or_404(
UserCalendarSubscription.objects.select_related("calendar", "user"),
subscription_id=subscription_id,
)

# Update last accessed timestamp
subscription.last_accessed = timezone.now()
subscription.save(update_fields=["last_accessed"])
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

last_accessed will only update on the first request per cache period; subsequent requests serve cached responses and skip this code. Move calendar_str generation into a cached fragment (e.g., low-level cache) or remove the view-level cache if accurate access tracking is required.

Copilot uses AI. Check for mistakes.
Allow users to set alarm reminders at any time interval (hours before event).
Users may want reminders days or even weeks in advance for important events
like yahrzeits or anniversaries. Calendar systems support arbitrary alarm
triggers, so we shouldn't artificially limit this.

Changes:
- Remove choices constraint from alarm_time field
- Remove validation against allowed_alarm_times in view
- Keep ALARM_TIME_CHOICES constant for UI preset display only
- Update help text to clarify flexible time support

Migration 0013 removes the database choices constraint.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@abe-101 abe-101 requested a review from Copilot October 20, 2025 02:57
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 473 to 474
expirimental = request.GET.get("expirimental", False)
if expirimental:
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

Corrected spelling of 'expirimental' to 'experimental' (both variable name and expected query parameter key) to improve clarity.

Suggested change
expirimental = request.GET.get("expirimental", False)
if expirimental:
experimental = request.GET.get("experimental", False)
if experimental:

Copilot uses AI. Check for mistakes.
Comment on lines +226 to +242
# Alarm time choices (hours before event, negative = previous day)
ALARM_TIME_CHOICES = [
(-5, "7 PM (previous day)"),
(-4, "8 PM (previous day)"),
(-3, "9 PM (previous day)"),
(-2, "10 PM (previous day)"),
(-1, "11 PM (previous day)"),
(0, "12 AM"),
(5, "5 AM"),
(6, "6 AM"),
(7, "7 AM"),
(8, "8 AM"),
(9, "9 AM"),
(10, "10 AM"),
(11, "11 AM"),
(12, "12 PM"),
]
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

[nitpick] ALARM_TIME_CHOICES is defined but the alarm_time field does not reference it (no choices=...), which can cause confusion and drift between UI options and persisted values. Either add choices=ALARM_TIME_CHOICES to the field or clarify (e.g., rename to DEFAULT_ALARM_TIME_CHOICES) that it's only for display mapping.

Copilot uses AI. Check for mistakes.
Comment on lines +268 to +274
alarm_time = models.IntegerField(
default=9,
help_text=(
"Hours for alarm reminder (negative = before midnight, "
"positive = after midnight, supports any value)"
),
)
Copy link

Copilot AI Oct 20, 2025

Choose a reason for hiding this comment

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

[nitpick] ALARM_TIME_CHOICES is defined but the alarm_time field does not reference it (no choices=...), which can cause confusion and drift between UI options and persisted values. Either add choices=ALARM_TIME_CHOICES to the field or clarify (e.g., rename to DEFAULT_ALARM_TIME_CHOICES) that it's only for display mapping.

Copilot uses AI. Check for mistakes.
abe-101 and others added 3 commits October 19, 2025 23:02
Security and clarity improvements:
- Combine admin actions into single migrate_and_notify action
- Change alarm update from GET to POST with CSRF protection (@require_POST)
- Fix cache comment (correctly states 60 minutes instead of 15)
- Remove view-level cache from calendar_subscription_file to ensure accurate last_accessed tracking
- Fix spelling: expirimental -> experimental
- Rename ALARM_TIME_CHOICES to DEFAULT_ALARM_TIME_CHOICES to clarify it's for UI display only, not model validation

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The cut filter removes ALL occurrences throughout the string, not just the
protocol prefix. This caused malformed URLs like:
  webcal://127.0.0.1:8000/c/abc.icswebcal://

Reverted to slice:'8:' which correctly strips only "https://" prefix.
Production will use HTTPS, so this works correctly. For local dev with http://,
the URL will be slightly off but webcal:// still functions.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
Email Functionality:
- Implement send_migration_notification_email() using Django's send_mail
- Send HTML and plain text versions of migration notification
- Include personalized subscribe URL and clear migration instructions
- Use logger.exception for better error tracking

Calendar Name Indicators:
- Prefix calendar name with "⚠️ Action Needed:" when migrated
- Clear and non-technical language (not "legacy")
- Shows at the beginning for better visibility
- Helps users immediately identify calendars needing migration

User Experience:
- Users receive email with migration instructions
- Calendar name changes to show action needed status
- Migration prompt event appears in their calendar
- Old URL continues to work (backward compatible)
- Clear path to get personal subscription URL

Documentation:
- Add MIGRATION_USER_EXPERIENCE.md explaining full user journey
- Details email content, calendar behavior, and migration timeline
- Explains how different calendar apps refresh
- Includes FAQs and success metrics

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
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