Conversation
- Comprehensive documentation for the iCalendar demo feature - Includes API specs, client compatibility matrix, and RFC compliance notes - Documents known vendor quirks and workarounds - Refs #296
- Define Event interface following RFC 5545 spec - Add types for parsed events and iCal options - Include vendor extension types for client compatibility - Add query parameter types for API endpoint - Refs #296
- ical-generator v9.0.0 for RFC-compliant iCalendar generation - rev-hash v4.1.0 for stable UID generation from event data - Refs #296
- Add GET /api/ical/events.ics endpoint for dynamic event generation - Support timezone handling with automatic VTIMEZONE generation - Implement cancellation logic with proper METHOD and SEQUENCE handling - Add vendor-specific extensions for client compatibility - Include comprehensive error handling and validation - Refs #296
- Create interactive demo form at /demos/ical - Support all query parameters with datetime-local inputs - Include timezone selection with common options - Generate both download and webcal:// subscription URLs - Add client-specific testing instructions - Include copy-to-clipboard functionality - Refs #296
- Add link to /demos/ical in the HTML Demos section - Include brief description of the feature - Refs #296
- Test demo page form functionality - Test API endpoint with various parameters - Test error handling for missing/invalid params - Test cancellation and timezone handling - Test copy-to-clipboard functionality - Refs #296
- Extract parseEventParams and addVendorExtensions to utils/ical-helpers.ts - Add comprehensive unit tests for parameter parsing and validation - Test error cases, cancellation logic, and UID generation - Add test script to package.json using Node.js built-in test runner - Refs #296
- Explain project purpose as demo/testing platform - Document key design principles - Add development and feature addition guidelines - Reference existing documentation structure
- Apply prettier formatting to iCal feature files - Remove invalid JSON comments from devcontainer.json - Fix quote styles and indentation issues
|
CI Status Update:
The E2E test failure is due to a CI infrastructure issue where the Cypress binary is not being properly cached/installed. The error message indicates: All code changes have been validated and formatting/linting issues have been resolved. The code is ready for review. |
6845b08 to
e6e8799
Compare
- Update to latest main branch changes - Integrate Playwright-BDD E2E testing framework - Update dependency versions to match main - Add ical-generator dependency for iCalendar feature
- Remove packages/e2e-tests/src/ical-demo.test.ts which was unused after Playwright-BDD migration
…ommits - Update CLAUDE.md to clarify that only CI fix commits should be squashed - Preserve logical feature development commits in PR history - Add clear examples of when to squash vs preserve commits - Fix misleading workflow that suggested squashing entire PR history This ensures PRs maintain meaningful commit history while keeping CI fixes clean and integrated.
e323298 to
a578627
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive iCalendar demo feature for testing calendar client compatibility. The implementation follows RFC 5545 specifications while including vendor-specific workarounds for real-world client behavior.
Key changes include:
- API endpoint for generating dynamic iCalendar feeds with event cancellation support
- Interactive demo page with form-based feed generation and testing instructions
- Comprehensive unit tests and detailed documentation covering client compatibility
Reviewed Changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/app/src/views/Home.ts | Adds navigation link to the new iCalendar demo |
| packages/app/src/utils/ical-helpers.ts | Core utility functions for parsing parameters and adding vendor extensions |
| packages/app/src/utils/ical-helpers.test.ts | Comprehensive unit tests for the helper functions |
| packages/app/src/types/events.ts | Type definitions for event data structures and iCalendar options |
| packages/app/src/routes/demos/ical.ts | Demo page with interactive form and client testing instructions |
| packages/app/src/routes/api/ical.ts | API endpoint for generating iCalendar feeds |
| packages/app/src/app.ts | Router configuration for new demo and API routes |
| packages/app/package.json | Dependencies for iCalendar generation and testing |
| docs/features/ical.md | Comprehensive feature documentation with client compatibility details |
| CLAUDE.md | Updates to git workflow guidelines for commit squashing |
| AGENTS.md | New AI agent guidelines for development standards |
| .devcontainer/init-firewall.sh | Firewall configuration update |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
|
|
||
| // Parse duration | ||
| const durationMin = parseInt(duration, 10); | ||
| if (isNaN(durationMin) || durationMin <= 0) { |
There was a problem hiding this comment.
[nitpick] Consider extracting the magic number 0 into a named constant like MIN_DURATION_MINUTES for better maintainability and clarity.
| if (isNaN(durationMin) || durationMin <= 0) { | |
| if (isNaN(durationMin) || durationMin <= MIN_DURATION_MINUTES) { |
| return { error: "Invalid startAt: must be a valid ISO 8601 datetime" }; | ||
| } | ||
|
|
||
| const endDate = new Date(startDate.getTime() + durationMin * 60000); |
There was a problem hiding this comment.
[nitpick] The magic number 60000 (milliseconds in a minute) should be extracted into a named constant like MILLISECONDS_PER_MINUTE for better readability.
| const endDate = new Date(startDate.getTime() + durationMin * 60000); | |
| const endDate = new Date(startDate.getTime() + durationMin * MILLISECONDS_PER_MINUTE); |
| document.getElementById("copy-btn").addEventListener("click", () => { | ||
| const input = document.getElementById("feed-url"); | ||
| input.select(); | ||
| document.execCommand("copy"); | ||
|
|
||
| const btn = document.getElementById("copy-btn"); | ||
| const originalText = btn.textContent; | ||
| btn.textContent = "Copied!"; | ||
| setTimeout(() => (btn.textContent = originalText), 2000); |
There was a problem hiding this comment.
document.execCommand('copy') is deprecated. Use the modern Clipboard API (navigator.clipboard.writeText()) with fallback for better browser compatibility.
| document.getElementById("copy-btn").addEventListener("click", () => { | |
| const input = document.getElementById("feed-url"); | |
| input.select(); | |
| document.execCommand("copy"); | |
| const btn = document.getElementById("copy-btn"); | |
| const originalText = btn.textContent; | |
| btn.textContent = "Copied!"; | |
| setTimeout(() => (btn.textContent = originalText), 2000); | |
| document.getElementById("copy-btn").addEventListener("click", async () => { | |
| const input = document.getElementById("feed-url"); | |
| input.select(); | |
| try { | |
| if (navigator.clipboard && navigator.clipboard.writeText) { | |
| await navigator.clipboard.writeText(input.value); | |
| } else { | |
| document.execCommand("copy"); // Fallback for older browsers | |
| } | |
| const btn = document.getElementById("copy-btn"); | |
| const originalText = btn.textContent; | |
| btn.textContent = "Copied!"; | |
| setTimeout(() => (btn.textContent = originalText), 2000); | |
| } catch (err) { | |
| console.error("Failed to copy text: ", err); | |
| } |
…plan - Add comprehensive vendor extension rationale (Microsoft, Apple, Google) - Document specific calendar client compatibility issues and solutions - Include refresh rate comparison table across major calendar clients - Create detailed E2E test plan with Node.js library research - Update CLAUDE.md with git history cleanup strategy 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add ical.js dependency for parsing and validating iCalendar data - Create iCalendar parsing utilities with RFC 5545 compliance validation - Add BDD feature files for web interface and API testing - Implement step definitions and page object model - Test coverage includes: - Web interface functionality (form, URL generation, downloads) - API response validation and error handling - RFC 5545 compliance checks - Vendor extension validation (Microsoft, Apple, Google) - Timezone and cancellation handling 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…rrors - Restructured CLAUDE.md to emphasize local-first CI workflow - Added clear 4-phase workflow: Local Quality → Local CI → Push → GitHub CI - Made it explicit that GitHub CI should never fail if local CI passed - Added troubleshooting section with common issues and quick fixes - Reorganized content for better flow and clarity - Fixed ESLint errors in E2E test step definitions by prefixing unused parameters with underscore - Fixed timezone handling in iCal API (moved from event-level to calendar-level) - Fixed lit-html template literal issue in iCal demo page 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Added detailed "Timezone Handling" section to ical.md explaining: - How timezone implementation works (calendar-level setting) - RFC 5545 requirements for TZID and VTIMEZONE - Why VTIMEZONE components are mandatory with TZID parameters - Why we don't use global timezone IDs (non-standard) - Valid vs invalid timezone usage examples - Client behavior expectations - Enhanced API documentation to clarify tz parameter generates VTIMEZONE - Added detailed code comments in ical.ts explaining RFC compliance - Referenced Stack Overflow discussion as authoritative source 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements a minimal iCalendar demo feature for testing calendar client compatibility.
Closes #296
See issue for implementation details.