fix(api): remove .ics suffix requirement from ICS feed URL validator#29507
fix(api): remove .ics suffix requirement from ICS feed URL validator#29507NamanYadav11 wants to merge 1 commit into
Conversation
RFC 5545 does not require a .ics file extension. Valid CalDAV feeds and dynamic export endpoints (e.g. ?format=ics) were being rejected by the overly strict pathname check. Now accepts any valid http/https URL. Fixes calcom#29286
|
Welcome to Cal.diy, @NamanYadav11! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
📝 WalkthroughWalkthroughThis PR updates ICS feed URL validation in the calendar input module to accept any valid 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/api/v2/src/platform/calendars/input/create-ics.input.ts (1)
2-9: ⚡ Quick winUse type import for
ValidatorConstraintInterface.
ValidatorConstraintInterfaceis only used in a type position (implementsclause at line 14). Per coding guidelines, type-only imports should useimport type.♻️ Proposed fix
import { ArrayNotEmpty, IsBoolean, IsOptional, Validate, ValidatorConstraint, - ValidatorConstraintInterface, } from "class-validator"; +import type { ValidatorConstraintInterface } from "class-validator"; import { IsNotEmpty, IsArray } from "class-validator";As per coding guidelines: "Use
import type { X }for TypeScript type imports".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/api/v2/src/platform/calendars/input/create-ics.input.ts` around lines 2 - 9, The import of ValidatorConstraintInterface is used only as a type in the implements clause (ValidatorConstraintInterface) and should be a type-only import; update the import statement to import type for ValidatorConstraintInterface while leaving the other imports (ArrayNotEmpty, IsBoolean, IsOptional, Validate, ValidatorConstraint) as value imports so the class that implements ValidatorConstraintInterface continues to compile.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/api/v2/src/platform/calendars/input/create-ics.input.ts`:
- Around line 2-9: The import of ValidatorConstraintInterface is used only as a
type in the implements clause (ValidatorConstraintInterface) and should be a
type-only import; update the import statement to import type for
ValidatorConstraintInterface while leaving the other imports (ArrayNotEmpty,
IsBoolean, IsOptional, Validate, ValidatorConstraint) as value imports so the
class that implements ValidatorConstraintInterface continues to compile.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15ba160d-807e-4d5d-8e4d-276a57e75b24
📒 Files selected for processing (1)
apps/api/v2/src/platform/calendars/input/create-ics.input.ts
What does this PR do?
Fixes #29286
The ICS feed URL validator was enforcing a
.icssuffix on all submitted URLs:RFC 5545 (iCalendar) has no requirement for a
.icsfile extension. Valid CalDAV feeds and dynamic export endpoints (e.g.?format=ics,?export=ics) were silently rejected even though they serve validtext/calendarcontent.Removes the suffix check and keeps only the
http/httpsprotocol validation, which is the only meaningful constraint here. Also updates the@ApiPropertydescription and example to reflect the relaxed validation.Visual Demo (For contributors especially)
N/A — validator logic change, no UI.
Before:
POST /v2/calendars/icswithhttps://calendar.example.com/feed?format=ics→ 400 "must end with .ics"After: same request → 201 accepted
Mandatory Tasks (DO NOT REMOVE)
@ApiPropertydescription.How should this be tested?
No extra environment variables needed.
Happy path:
POST /v2/calendars/icswith body{ "urls": ["https://calendar.example.com/feed?format=ics"] }Still rejected (protocol guard still applies):
ftp://example.com/feed.ics→ 400not-a-url→ 400Checklist
I haven't read the contributing guideMy code doesn't follow the style guidelines of this projectMy PR is too large (>500 lines or >10 files) and should be split into smaller PRs