feat: add Proton Calendar integration via read-only ICS feed#29004
Open
nehemiyawicks wants to merge 9 commits intocalcom:mainfrom
Open
feat: add Proton Calendar integration via read-only ICS feed#29004nehemiyawicks wants to merge 9 commits intocalcom:mainfrom
nehemiyawicks wants to merge 9 commits intocalcom:mainfrom
Conversation
github-actions
Bot
added
app-store
Contributor
|
Welcome to Cal.diy, @nehemiyawicks! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
Adds a dedicated Proton Calendar app that imports availability from Proton's shareable ICS feed URLs for conflict checking.
nehemiyawicks
force-pushed
the
feat/proton-calendar-integration
branch
from
02c6e60 to
ffca817
Compare
There was a problem hiding this comment.
Pull request overview
Adds a new read-only Proton Calendar integration for Cal.com via shareable ICS feed URLs, enabling conflict checking against Proton events without OAuth/API access.
Changes:
- Introduces a new app-store package (
proton-calendarcalendar) with API handler, calendar service, metadata, UI setup page, and assets. - Implements ICS fetching/parsing with Proton-specific handling (e.g., filtering
STATUS:CANCELLED, recurrence expansion limits, fetch redirect restrictions). - Registers the integration across app-store generated registries and adds i18n copy + unit tests.
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds workspace entry for the new Proton Calendar app package. |
| packages/i18n/locales/en/common.json | Adds connect_proton_calendar UI label. |
| packages/app-store/proton-calendarcalendar/static/icon.svg | Adds Proton Calendar icon asset. |
| packages/app-store/proton-calendarcalendar/package.json | Declares the new app-store workspace package metadata/deps. |
| packages/app-store/proton-calendarcalendar/lib/index.ts | Exposes the calendar service factory export. |
| packages/app-store/proton-calendarcalendar/lib/tests/CalendarService.test.ts | Adds unit tests for Proton calendar service behavior. |
| packages/app-store/proton-calendarcalendar/lib/CalendarService.ts | Implements Proton ICS fetch/parse + availability mapping. |
| packages/app-store/proton-calendarcalendar/index.ts | Exposes api/lib entrypoints for the integration. |
| packages/app-store/proton-calendarcalendar/config.json | Adds app-store metadata/config for Proton Calendar integration. |
| packages/app-store/proton-calendarcalendar/api/index.ts | Exposes the add API handler. |
| packages/app-store/proton-calendarcalendar/api/add.ts | Adds credential creation endpoint + Proton URL validation. |
| packages/app-store/proton-calendarcalendar/api/tests/add.test.ts | Adds unit tests for Proton URL validation helper. |
| packages/app-store/proton-calendarcalendar/DESCRIPTION.md | Adds integration description/help text for the app store. |
| packages/app-store/calendar.services.generated.ts | Registers the Proton calendar service in the calendar service map. |
| packages/app-store/apps.server.generated.ts | Registers Proton app API handlers for server routing. |
| packages/app-store/apps.metadata.generated.ts | Registers Proton app metadata in the app-store metadata map. |
| apps/web/components/apps/proton-calendarcalendar/Setup.tsx | Adds the UI setup page for entering the Proton ICS URL. |
| apps/web/components/apps/AppSetupPage.tsx | Adds Proton Calendar to the setup component router map. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Fix test name: userinfo URLs are accepted, not rejected - Scope cancelledInstances per-calendar to prevent cross-calendar UID collisions - Update error message to list both allowed hostnames
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Proton Calendar integration: a React setup UI and route; a Next.js API endpoint (GET/POST) that validates Proton ICS URLs, encrypts and stores credentials; a calendar service that decrypts keys, fetches and parses ICS feeds, expands recurrences, handles cancellations and timezones, and exposes read-only calendar operations. Also adds package metadata, app-store registry and service/API mappings, config and description files, tests for URL validation and calendar parsing, and a new localization string. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (8)
packages/app-store/proton-calendarcalendar/lib/CalendarService.ts (2)
214-218: Silent skip of dense recurrence frequencies.The parallel
ics-feedcalendarimplementation logsWon't handle [${type}] recurrencebefore continuing; here it's silently dropped. Adding alog.warn/log.erroraids debugging when a Proton user reports "my recurring events aren't blocking slots".♻️ Suggested log
- if (["HOURLY", "SECONDLY", "MINUTELY"].includes(event.getRecurrenceTypes())) { - continue; - } + const recurrenceType = event.getRecurrenceTypes(); + if (["HOURLY", "SECONDLY", "MINUTELY"].includes(recurrenceType)) { + log.warn(`Skipping unsupported recurrence frequency: ${recurrenceType}`); + continue; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts` around lines 214 - 218, The recurrence branch in CalendarService (inside the event.isRecurring() check) silently skips dense frequencies; update the logic where you check if (["HOURLY", "SECONDLY", "MINUTELY"].includes(event.getRecurrenceTypes())) to log a warning before continuing (e.g., use the module logger to emit `Won't handle [${type}] recurrence` or similar, where `type` is derived from event.getRecurrenceTypes()), so callers can see why recurring events are ignored; keep the existing continue and MAX_RECURRENCE_ITERATIONS handling unchanged.
1-303: Heavy duplication withics-feedcalendar— consider extracting shared ICS logic.This service is largely a copy of
packages/app-store/ics-feedcalendar/lib/CalendarService.ts(recurrence expansion, vtimezone synthesis, travel-duration handling,fetchCalendarsshape,listCalendars). Bug fixes (e.g., the recurrence-id matching above, future ICS quirks) will need to be applied in two places. Once this lands, a follow-up to extract a shared helper module (e.g.,packages/app-store/_utils/ics/parseAvailability.ts) and have both Proton and ICS-feed services delegate to it would significantly reduce maintenance cost.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts` around lines 1 - 303, This file duplicates ICS parsing/availability logic found in the ics-feedcalendar service; extract the shared logic (fetchCalendars, recurrence expansion, vtimezone synthesis, travel-duration handling, cancellations and events accumulation used in getAvailability, and listCalendars mapping) into a reusable helper module (e.g., parseAvailability or buildIcsService) and have ProtonCalendarService delegate to it by importing that helper and calling its functions from ProtonCalendarService.fetchCalendars, ProtonCalendarService.getAvailability and ProtonCalendarService.listCalendars; keep unique integrationName and credential handling in ProtonCalendarService while moving ICU-specific functions getTravelDurationInSeconds, applyTravelDuration and recurrence/iterator handling into the shared module so bug fixes and parsing quirks live in one place.packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts (1)
86-92:global.fetchis left mutated across tests.
mockFetchResponseoverwritesglobal.fetchandvi.clearAllMocks()only resets call history, not the assignment itself. Tests that don't callmockFetchResponsewill inherit the previous test's mock (e.g., the "fetch fails" test sets a rejecting fetch that persists). For the current ordering this happens to work, but it's a subtle source of order-dependent flakes when new tests are added. Consider stashing/restoringglobal.fetchinbeforeEach/afterEachor usingvi.stubGlobal("fetch", ...)+vi.unstubAllGlobals().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts` around lines 86 - 92, mockFetchResponse directly assigns to global.fetch which mutates the global across tests and can cause order-dependent flakes; update tests to stash and restore the original global.fetch in beforeEach/afterEach or replace mockFetchResponse to use vi.stubGlobal("fetch", ...) and ensure tests call vi.unstubAllGlobals() (or restore the saved original) in afterEach; reference the mockFetchResponse helper and any test setup/teardown blocks so you replace the direct global.fetch assignment with vi.stubGlobal or implement saving const originalFetch = global.fetch in beforeEach and restoring global.fetch = originalFetch in afterEach.apps/web/components/apps/proton-calendarcalendar/Setup.tsx (3)
8-8:Toasteris mounted but notoast()calls exist in this component.Errors are surfaced via the
<Alert>component. If toasts aren't intended, drop thesonnerimport and<Toaster>to avoid a redundant top-level toaster (the app likely already mounts one globally).Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/apps/proton-calendarcalendar/Setup.tsx` at line 8, The component mounts a sonner Toaster but never calls toast(), creating a redundant top-level toaster; remove the import of Toaster and the <Toaster /> JSX from the Setup component (which already surfaces errors via the Alert UI) so you don't mount an extra global toaster—search for the Toaster symbol and the Setup component's render/return to remove both the import and the JSX usage.
13-64:react-hook-formis initialized but bypassed — controlleduseStateruns the show.
useForm({ defaultValues: {} })only providesformState.isSubmittinghere. TheTextFieldisn't registered ({...form.register("url")}),handleSubmitdiscards its argument (async (_)), andurlis held in a paralleluseState. This is a confusing dead abstraction.Pick one of two paths:
- Drop
react-hook-formentirely and use a plain<form onSubmit={...}>with a localisSubmittingstate.- Actually wire RHF:
const { register, handleSubmit, formState } = useForm<{ url: string }>({ defaultValues: { url: "" } });then<TextField {...register("url", { required: true })} />and readdata.urlinsidehandleSubmit(async (data) => …).Option 2 also gives you proper required-field validation on the client, which the current
requiredHTML attribute on a non-registered field doesn't enforce through RHF.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/apps/proton-calendarcalendar/Setup.tsx` around lines 13 - 64, The form is wired to useForm but the TextField is managed by local state and handleSubmit ignores RHF data—switch to real RHF wiring: change useForm to const { register, handleSubmit, formState } = useForm<{ url: string }>({ defaultValues: { url: "" } }), remove the local url state and the value/onChange on TextField, spread {...register("url", { required: true })} into TextField, and replace the inline submit handler async (_) with handleSubmit(async (data) => { /* use data.url in the fetch body */ }); keep using formState.isSubmitting for the fieldset and keep setErrorMessage/router.push logic but read/POST data.url instead of the removed url state.
47-52: Guard against missingjson.urlbefore navigating.If the backend ever returns a 200 with an unexpected body (or
jsonis non-object),router.push(json.url)becomesrouter.push(undefined). A defensive check avoids cryptic navigation failures:- const json = await res.json(); - if (!res.ok) { - setErrorMessage(json?.message || t("something_went_wrong")); - } else { - router.push(json.url); - } + const json = await res.json().catch(() => ({})); + if (!res.ok) { + setErrorMessage(json?.message || t("something_went_wrong")); + } else if (typeof json?.url === "string") { + router.push(json.url); + } else { + setErrorMessage(t("something_went_wrong")); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/components/apps/proton-calendarcalendar/Setup.tsx` around lines 47 - 52, When handling the successful response in the Setup component, guard against a missing or non-string json.url before calling router.push; replace the unconditional router.push(json.url) in the res.ok branch with a check like if (json && typeof json.url === "string") { router.push(json.url) } else { setErrorMessage(json?.message || t("something_went_wrong")) } so you never call router.push(undefined) and surface a proper error via setErrorMessage instead; update the block that currently reads const json = await res.json(); if (!res.ok) { ... } else { router.push(json.url); } to use this guarded logic.packages/app-store/proton-calendarcalendar/api/add.ts (2)
64-83: Catch block discards the error — production failures will be undebuggable.
catch {drops the original exception, and the log message has no error context. When this returns 500 (e.g., feed unreachable, validation mismatch, Prisma constraint), there is no signal of why. Capture and log it:- } catch { - log.error("Could not add Proton Calendar feeds"); + } catch (err) { + log.error("Could not add Proton Calendar feeds", err instanceof Error ? err.message : err); return res.status(500).json({ message: "Could not add Proton Calendar feeds" }); }Also note: per coding guidelines, line 74's
throw new Error(...)in a non-tRPC file should useErrorWithCode(caught locally so impact is small, but consistency matters).As per coding guidelines: "Use
ErrorWithCodefor errors in non-tRPC files (services, repositories, utilities); useTRPCErroronly in tRPC routers".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/api/add.ts` around lines 64 - 83, The catch block currently swallows exceptions and the thrown Error at the validation step uses plain Error; update the try/catch to catch (err) and log the error (e.g., include err in log.error) before returning the 500 response so failures from BuildCalendarService, listedCals validation, or prisma.credential.create are recorded, and replace the throw new Error(...) used around listedCals validation with ErrorWithCode to follow non-tRPC error conventions; reference BuildCalendarService, listedCals, prisma.credential.create, and ErrorWithCode when making these changes.
8-8: Avoid barrel import; import directly from the source module.
"../lib"resolves tolib/index.ts, which is a barrel. ImportBuildCalendarServicefrom its defining file directly:-import { BuildCalendarService } from "../lib"; +import { BuildCalendarService } from "../lib/CalendarService";As per coding guidelines: "Import directly from source files, not barrel files" / "Never use barrel imports from index.ts files".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/api/add.ts` at line 8, The import in add.ts uses a barrel ("../lib"); change it to a direct import from the module that defines BuildCalendarService (replace import { BuildCalendarService } from "../lib" with an import that points to the concrete source file that exports BuildCalendarService) so the file imports BuildCalendarService directly from its defining module rather than via lib/index.ts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/proton-calendarcalendar/api/__tests__/add.test.ts`:
- Around line 51-54: The test name says "rejects URLs with credentials in
userinfo" but the assertion expects true; update the behavior and test: modify
isValidProtonUrl to explicitly reject URLs containing userinfo (check
url.username or url.password and return false) and change the test in
add.test.ts (the "rejects URLs with credentials in userinfo" case) to expect
false, or alternatively update the test name to reflect acceptance if you intend
to keep current behavior; prefer rejecting credentials for security and set the
assertion to expect(false).
In `@packages/app-store/proton-calendarcalendar/api/add.ts`:
- Around line 32-42: Move the authentication check before any input validation:
verify req.session?.user?.id and return the 401 response immediately at the
start of the POST handling branch (before iterating over urls), so
unauthenticated requests don't run the isValidProtonUrl loop or reveal validator
behavior; update the code around the urls loop to assume an authenticated user
and keep the existing error responses (res.status(401).json(...),
res.status(400).json(...)) otherwise.
- Around line 24-93: The handler function currently only handles "POST" and
"GET" and falls through for other HTTP methods causing the request to hang;
update the default branch in the exported handler to return a 405 Method Not
Allowed with an Allow header listing "GET, POST" (e.g., call
res.setHeader("Allow", "GET, POST") and res.status(405).json({ message: "Method
Not Allowed" })); locate the exported default async function handler and add
this explicit response for unsupported methods after the existing GET/POST
branches.
- Line 56: The code silently falls back to an empty CALENDSO_ENCRYPTION_KEY when
calling symmetricEncrypt, which will break AES-256 or produce insecure behavior;
update the POST handling in add.ts to validate
process.env.CALENDSO_ENCRYPTION_KEY (e.g., non-empty and correct length/bytes
for AES-256) before calling symmetricEncrypt and fail fast (throw or return a
5xx/4xx error) with a clear message if the env var is missing/invalid so
symmetricEncrypt is never invoked with a bad key.
In
`@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts`:
- Around line 185-200: The test currently assumes non-recurring ICS events are
returned unfiltered; explicitly document this assumption in the test file and
add a new test that verifies availability/conflict-checking respects the
requested range by either (A) asserting getAvailability filters out events
outside dateFrom/dateTo when used for availability checks, or (B) if filtering
is intentionally upstream, add a documented test that simulates a large ICS feed
(reuse BuildCalendarService and MULTI_EVENT_ICS with many historical events) and
asserts getAvailability still returns only the expected in-range results for
performance/accuracy, and include a comment referencing getAvailability and
BuildCalendarService to explain the expected responsibility for date filtering.
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 47-50: The constructor currently calls
JSON.parse(symmetricDecrypt(...)) unguarded and assigns urls directly; wrap the
decryption+parse in a try/catch inside the constructor (the code path using
symmetricDecrypt, CALENDSO_ENCRYPTION_KEY, and JSON.parse) and validate that the
parsed object has a urls property that is an array of strings before assigning
to this.urls; on any error or invalid shape, log the error and set this.urls =
[] so construction never throws and the service degrades gracefully like the
fetch path.
- Around line 142-158: The cancelled-instances logic collects keys into
cancelledInstances using vevent.getFirstPropertyValue("uid") and "recurrence-id"
(in the block around cancelledInstances, vevent, recurrence-id) but there is no
test ensuring a RECURRENCE-ID-based cancelled instance matches the runtime key
built from currentEvent.startDate (the comparison
`${uid}:${currentEvent.startDate}`); add a unit test fixture that creates a
recurring VEVENT (RRULE) plus a separate VEVENT with STATUS:CANCELLED and
RECURRENCE-ID for one occurrence, then run the CalendarService codepath that
builds cancelledInstances and asserts the specific instance is filtered out;
ensure the test constructs the RECURRENCE-ID using the same ICAL.Time string
form used at runtime (or compare before timezone conversion) so the
`${uid}:${recurrenceId}` key matches `${uid}:${currentEvent.startDate}`.
- Around line 169-178: The code is accessing ical.js internals via
(dtstartProperty as any).jCal[1].tzid; replace that with the public API by
calling dtstartProperty.getParameter("tzid") (use the existing dtstartProperty
from vevent.getFirstProperty("dtstart")) to obtain the TZID string, and remove
the unsafe cast; ensure tzidFromDtstart is assigned the result of
dtstartProperty.getParameter("tzid") (or undefined) so the subsequent tzid
computation (tzidFromDtstart || vevent?.getFirstPropertyValue("tzid") || (isUTC
? "UTC" : timezone)) remains valid.
---
Nitpick comments:
In `@apps/web/components/apps/proton-calendarcalendar/Setup.tsx`:
- Line 8: The component mounts a sonner Toaster but never calls toast(),
creating a redundant top-level toaster; remove the import of Toaster and the
<Toaster /> JSX from the Setup component (which already surfaces errors via the
Alert UI) so you don't mount an extra global toaster—search for the Toaster
symbol and the Setup component's render/return to remove both the import and the
JSX usage.
- Around line 13-64: The form is wired to useForm but the TextField is managed
by local state and handleSubmit ignores RHF data—switch to real RHF wiring:
change useForm to const { register, handleSubmit, formState } = useForm<{ url:
string }>({ defaultValues: { url: "" } }), remove the local url state and the
value/onChange on TextField, spread {...register("url", { required: true })}
into TextField, and replace the inline submit handler async (_) with
handleSubmit(async (data) => { /* use data.url in the fetch body */ }); keep
using formState.isSubmitting for the fieldset and keep
setErrorMessage/router.push logic but read/POST data.url instead of the removed
url state.
- Around line 47-52: When handling the successful response in the Setup
component, guard against a missing or non-string json.url before calling
router.push; replace the unconditional router.push(json.url) in the res.ok
branch with a check like if (json && typeof json.url === "string") {
router.push(json.url) } else { setErrorMessage(json?.message ||
t("something_went_wrong")) } so you never call router.push(undefined) and
surface a proper error via setErrorMessage instead; update the block that
currently reads const json = await res.json(); if (!res.ok) { ... } else {
router.push(json.url); } to use this guarded logic.
In `@packages/app-store/proton-calendarcalendar/api/add.ts`:
- Around line 64-83: The catch block currently swallows exceptions and the
thrown Error at the validation step uses plain Error; update the try/catch to
catch (err) and log the error (e.g., include err in log.error) before returning
the 500 response so failures from BuildCalendarService, listedCals validation,
or prisma.credential.create are recorded, and replace the throw new Error(...)
used around listedCals validation with ErrorWithCode to follow non-tRPC error
conventions; reference BuildCalendarService, listedCals,
prisma.credential.create, and ErrorWithCode when making these changes.
- Line 8: The import in add.ts uses a barrel ("../lib"); change it to a direct
import from the module that defines BuildCalendarService (replace import {
BuildCalendarService } from "../lib" with an import that points to the concrete
source file that exports BuildCalendarService) so the file imports
BuildCalendarService directly from its defining module rather than via
lib/index.ts.
In
`@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts`:
- Around line 86-92: mockFetchResponse directly assigns to global.fetch which
mutates the global across tests and can cause order-dependent flakes; update
tests to stash and restore the original global.fetch in beforeEach/afterEach or
replace mockFetchResponse to use vi.stubGlobal("fetch", ...) and ensure tests
call vi.unstubAllGlobals() (or restore the saved original) in afterEach;
reference the mockFetchResponse helper and any test setup/teardown blocks so you
replace the direct global.fetch assignment with vi.stubGlobal or implement
saving const originalFetch = global.fetch in beforeEach and restoring
global.fetch = originalFetch in afterEach.
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 214-218: The recurrence branch in CalendarService (inside the
event.isRecurring() check) silently skips dense frequencies; update the logic
where you check if (["HOURLY", "SECONDLY",
"MINUTELY"].includes(event.getRecurrenceTypes())) to log a warning before
continuing (e.g., use the module logger to emit `Won't handle [${type}]
recurrence` or similar, where `type` is derived from
event.getRecurrenceTypes()), so callers can see why recurring events are
ignored; keep the existing continue and MAX_RECURRENCE_ITERATIONS handling
unchanged.
- Around line 1-303: This file duplicates ICS parsing/availability logic found
in the ics-feedcalendar service; extract the shared logic (fetchCalendars,
recurrence expansion, vtimezone synthesis, travel-duration handling,
cancellations and events accumulation used in getAvailability, and listCalendars
mapping) into a reusable helper module (e.g., parseAvailability or
buildIcsService) and have ProtonCalendarService delegate to it by importing that
helper and calling its functions from ProtonCalendarService.fetchCalendars,
ProtonCalendarService.getAvailability and ProtonCalendarService.listCalendars;
keep unique integrationName and credential handling in ProtonCalendarService
while moving ICU-specific functions getTravelDurationInSeconds,
applyTravelDuration and recurrence/iterator handling into the shared module so
bug fixes and parsing quirks live in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f80055ce-de09-4b07-95f6-4dd2015d1baa
⛔ Files ignored due to path filters (2)
packages/app-store/proton-calendarcalendar/static/icon.svgis excluded by!**/*.svgyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (16)
apps/web/components/apps/AppSetupPage.tsxapps/web/components/apps/proton-calendarcalendar/Setup.tsxpackages/app-store/apps.metadata.generated.tspackages/app-store/apps.server.generated.tspackages/app-store/calendar.services.generated.tspackages/app-store/proton-calendarcalendar/DESCRIPTION.mdpackages/app-store/proton-calendarcalendar/api/__tests__/add.test.tspackages/app-store/proton-calendarcalendar/api/add.tspackages/app-store/proton-calendarcalendar/api/index.tspackages/app-store/proton-calendarcalendar/config.jsonpackages/app-store/proton-calendarcalendar/index.tspackages/app-store/proton-calendarcalendar/lib/CalendarService.tspackages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.tspackages/app-store/proton-calendarcalendar/lib/index.tspackages/app-store/proton-calendarcalendar/package.jsonpackages/i18n/locales/en/common.json
- Reject URLs with userinfo credentials (user:pass@)
- Move auth check before URL validation
- Fail fast if CALENDSO_ENCRYPTION_KEY is missing
- Return 405 for unsupported HTTP methods
- Replace unsafe `as any` jCal access with getParameter("tzid")
nehemiyawicks
commented
Apr 26, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 138-139: The current assignment of userTimeZone uses userId ?
await this.getUserTimezoneFromDB(userId) : "Europe/London", which leaves
userTimeZone undefined when the DB row has timeZone null and causes
timezoneToUse (used later) to skip the fallback; update the logic in
CalendarService (around getUserId / getUserTimezoneFromDB usage) to coalesce the
DB result to a safe default (e.g., const userTimeZone = userId ? (await
this.getUserTimezoneFromDB(userId)) || "Europe/London" : "Europe/London") so
that userTimeZone is never undefined and timezoneToUse = tzid || userTimeZone
behaves correctly.
- Around line 257-263: The isBetween call currently uses exclusive bounds and
drops events that start exactly at the window boundary; update the Dayjs checks
(e.g., the currentStart.isBetween(start, end) call in CalendarService.ts and any
equivalent isBetween usages in proton-calendar, ics-feed, and lib integrations)
to use inclusive bounds by passing the bounds parameter (the fourth argument)
set to '[]' so boundary-start events are considered inside the window; search
for occurrences of .isBetween(start, end) and replace them with the inclusive
variant in all calendar integration files.
- Around line 37-41: The applyTravelDuration function currently mutates
event.startDate.second directly causing unnormalized negative seconds; replace
that raw field mutation by calling the ICAL.Time normalizer: use
event.startDate.adjust(0, 0, 0, -seconds) inside applyTravelDuration and return
the event, ensuring proper cross-boundary normalization so recurrence expansion
and cancelled-instance keys remain consistent (refer to applyTravelDuration and
event.startDate.adjust).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b156f9ca-af1a-4275-8999-ad467c2ce5a3
📒 Files selected for processing (3)
packages/app-store/proton-calendarcalendar/api/__tests__/add.test.tspackages/app-store/proton-calendarcalendar/api/add.tspackages/app-store/proton-calendarcalendar/lib/CalendarService.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/app-store/proton-calendarcalendar/api/tests/add.test.ts
- packages/app-store/proton-calendarcalendar/api/add.ts
- Add defensive guard on credential.key in constructor - Validate URL array items are strings in isValidProtonUrl - Add RECURRENCE-ID cancellation test for per-instance filtering
- Use ICAL.Time.adjust() for travel duration instead of raw second mutation - Fallback to Europe/London when user has no timeZone in DB - Use inclusive start bound in isBetween for recurring events
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 47-49: The constructor in class CalendarService currently throws a
plain Error when credential.key is missing; replace this with the
project-standard ErrorWithCode: import and use ErrorWithCode and ErrorCode and
throw new ErrorWithCode(ErrorCode.BadRequest, "Missing Proton Calendar
credential key") (or add MISSING_CREDENTIAL_KEY to the ErrorCode enum if you
prefer a distinct code), ensuring the change is made in the CalendarService
constructor that accepts CredentialPayload and retains the same message and
behavior.
- Around line 171-179: The dtstart variable is incorrectly annotated as { [key:
string]: string }; change its type to ICAL.Time | undefined (the actual return
type of vevent.getFirstPropertyValue("dtstart")) and update any member access to
use ICAL.Time properties (e.g., inspect dtstart.timezone or dtstart.isUTC as
appropriate) so you don't rely on unsafe string indexing; also remove the
redundant fallback vevent?.getFirstPropertyValue("tzid") when computing tzid
since TZID is a DTSTART parameter already captured in tzidFromDtstart (keep tzid
logic using tzidFromDtstart || (isUTC ? "UTC" : timezone)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89bc2701-d955-4877-ad6c-617295778524
📒 Files selected for processing (4)
packages/app-store/proton-calendarcalendar/api/__tests__/add.test.tspackages/app-store/proton-calendarcalendar/api/add.tspackages/app-store/proton-calendarcalendar/lib/CalendarService.tspackages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts
✅ Files skipped from review due to trivial changes (1)
- packages/app-store/proton-calendarcalendar/api/tests/add.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/app-store/proton-calendarcalendar/api/add.ts
- packages/app-store/proton-calendarcalendar/lib/tests/CalendarService.test.ts
…t typing
- Use ErrorWithCode(ErrorCode.InternalServerError) instead of bare Error
- Wrap JSON.parse(symmetricDecrypt(...)) in try/catch with urls validation
- Fix dtstart type annotation to use ICAL.Time generic
- Remove no-op getFirstPropertyValue("tzid") fallback
- Add constructor, timezone, travel duration, and recurrence tests (32 total)
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/app-store/proton-calendarcalendar/api/add.ts (2)
73-92: Bind the error incatchso the failure mode is diagnosable.
catch {(line 89) swallows the underlying exception, so when install fails users see only the generic"Could not add Proton Calendar feeds"and operators have no signal to distinguish a fetch timeout, a parse failure, alistCalendarslength mismatch, or a Prisma error. Capture and log it.♻️ Proposed refactor
- } catch { - log.error("Could not add Proton Calendar feeds"); + } catch (error) { + log.error("Could not add Proton Calendar feeds", error); return res.status(500).json({ message: "Could not add Proton Calendar feeds" }); }Also worth noting: the
trywraps both validation (listCalendars) and persistence (prisma.credential.create). If you want to surface different status codes for "validation failed" vs "DB write failed", split the two. Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/api/add.ts` around lines 73 - 92, The catch block currently swallows the exception; change the anonymous catch to capture the error (e.g., catch (err)) and include that error when logging from log.error so failures are diagnosable (reference BuildCalendarService, listedCals/listCalendars validation and prisma.credential.create); optionally split the flow into two try/catch blocks—one around the validation (BuildCalendarService() + listedCals length check) to return a 400/422 on validation failures and a second around prisma.credential.create to return a 500 on DB errors—so logs and status codes reflect whether listCalendars or the DB write failed.
38-42: Consider deduplicatingurlsbefore validation/storage.If a user submits the same feed twice (e.g.
["u","u"]), each URL is fetched once per array entry,listedCals.length === urls.lengthstill holds, and the duplicate gets persisted into the encrypted credential. That doubles fetch traffic on every availability lookup and could create duplicate busy entries for the same event. A simpleconst urls = Array.from(new Set(rawUrls))after type validation avoids it.♻️ Proposed refactor
- const { urls } = req.body; - - if (!Array.isArray(urls) || urls.length === 0) { + const { urls: rawUrls } = req.body; + + if (!Array.isArray(rawUrls) || rawUrls.length === 0) { return res.status(400).json({ message: "At least one Proton Calendar ICS URL is required" }); } + const urls = Array.from(new Set(rawUrls));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/api/add.ts` around lines 38 - 42, The request body `urls` array should be deduplicated after the Array.isArray/type validation and before any fetching or persisting to avoid duplicate fetches and duplicate stored credentials; change the handler to assign a deduplicated list (e.g., collapse duplicates from `req.body.urls` into a unique `urls` array) before using it in the fetch loop that produces `listedCals` and before persisting into the encrypted credential so `listedCals.length` and stored feeds reflect unique URLs only.packages/app-store/proton-calendarcalendar/lib/CalendarService.ts (1)
173-291: One malformed VEVENT can take down the whole feed's availability.
new ICAL.Event(vevent)(line 180),event.iterator(...)/event.getOccurrenceDetails(...)(lines 237, 251), andnew ICAL.Timezone(vtimezone)(lines 263, 281) can all throw on edge-case input (missing DTSTART, bad RRULE, malformed VTIMEZONE). Because the per-event work runs inside a plainfor…ofinside aforEach, any throw aborts processing of every remaining event in that calendar — and silently, sinceforEachcallbacks don't surface errors to a caller.The fetch/parse path already degrades gracefully (skip on failure, log). Recommend matching that pattern here:
♻️ Proposed refactor
for (const vevent of vevents) { const status = vevent.getFirstPropertyValue("status"); if (typeof status === "string" && status.toUpperCase() === "CANCELLED") continue; const uid = vevent.getFirstPropertyValue("uid"); if (uid && cancelledInstances.has(`${uid}`)) continue; - const event = new ICAL.Event(vevent); + let event: ICAL.Event; + try { + event = new ICAL.Event(vevent); + } catch (err) { + log.error("Skipping malformed Proton Calendar VEVENT", err); + continue; + } ... }You can keep it scoped (just the construction) or wrap the entire per-event body — either way, one bad event shouldn't blank out the user's busy times.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts` around lines 173 - 291, The loop currently lets exceptions from new ICAL.Event(vevent), event.iterator(...)/event.getOccurrenceDetails(...), and new ICAL.Timezone(...) abort processing of the whole calendar; wrap the per-event work in a try/catch (e.g., immediately after entering the for (const vevent of vevents) block or around the blocks that call ICAL.Event, event.iterator/getOccurrenceDetails, and new ICAL.Timezone) to catch any thrown errors, skip the offending VEVENT on error, and log/debug the error; ensure you still call applyTravelDuration and continue to the next vevent when an error occurs so a single malformed VEVENT cannot stop processing remaining events.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 199-200: The synthetic STANDARD block currently sets tzoffsetfrom
and tzoffsetto using the same timestamp, which yields identical offsets and can
misrepresent DST transitions; update the code that computes tzoffsetfrom (the
variable assigned from
dayjs(event.startDate.toJSDate()).tz(timezoneToUse).format("Z")) to use a time
just before the transition (for example event.startDate minus a small delta like
1 minute or day) so tzoffsetfrom reflects the pre-transition offset, or
alternatively skip adding the synthetic STANDARD when offsets are identical and
let ical.js handle floating/userTimeZone resolution; adjust the logic around
tzoffsetfrom, tzoffsetto, event.startDate, event.endDate and the synthetic
STANDARD creation accordingly.
- Around line 191-221: The code injects at most one synthetic VTIMEZONE by
checking vcalendar.getFirstSubcomponent("vtimezone"), causing wrong timezone
mapping for later events with different tzids; change the injection logic to
check for an existing VTIMEZONE matching the event tzid (use
vcalendar.getAllSubcomponents("vtimezone") and compare each
vtz.getFirstPropertyValue("tzid") to tzid or userTimeZone) and only inject a
synthetic VTIMEZONE when no matching tzid exists (create the ICAL.Component with
tzid set to the event's tzid when present), and later when selecting vtimezone
keep the current lookup by tzid but return null if none found so callers (e.g.,
convertToZone) can skip conversion when no matching VTIMEZONE was added.
---
Nitpick comments:
In `@packages/app-store/proton-calendarcalendar/api/add.ts`:
- Around line 73-92: The catch block currently swallows the exception; change
the anonymous catch to capture the error (e.g., catch (err)) and include that
error when logging from log.error so failures are diagnosable (reference
BuildCalendarService, listedCals/listCalendars validation and
prisma.credential.create); optionally split the flow into two try/catch
blocks—one around the validation (BuildCalendarService() + listedCals length
check) to return a 400/422 on validation failures and a second around
prisma.credential.create to return a 500 on DB errors—so logs and status codes
reflect whether listCalendars or the DB write failed.
- Around line 38-42: The request body `urls` array should be deduplicated after
the Array.isArray/type validation and before any fetching or persisting to avoid
duplicate fetches and duplicate stored credentials; change the handler to assign
a deduplicated list (e.g., collapse duplicates from `req.body.urls` into a
unique `urls` array) before using it in the fetch loop that produces
`listedCals` and before persisting into the encrypted credential so
`listedCals.length` and stored feeds reflect unique URLs only.
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 173-291: The loop currently lets exceptions from new
ICAL.Event(vevent), event.iterator(...)/event.getOccurrenceDetails(...), and new
ICAL.Timezone(...) abort processing of the whole calendar; wrap the per-event
work in a try/catch (e.g., immediately after entering the for (const vevent of
vevents) block or around the blocks that call ICAL.Event,
event.iterator/getOccurrenceDetails, and new ICAL.Timezone) to catch any thrown
errors, skip the offending VEVENT on error, and log/debug the error; ensure you
still call applyTravelDuration and continue to the next vevent when an error
occurs so a single malformed VEVENT cannot stop processing remaining events.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6a9bea8b-3bbd-40c7-aed6-a7a561de8288
📒 Files selected for processing (3)
packages/app-store/proton-calendarcalendar/api/add.tspackages/app-store/proton-calendarcalendar/lib/CalendarService.tspackages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/app-store/proton-calendarcalendar/lib/tests/CalendarService.test.ts
…d offset - Check for matching TZID before injecting synthetic VTIMEZONE (not just any) - Each distinct TZID now gets its own synthetic VTIMEZONE component - Use single utcOffset for both tzoffsetfrom/tzoffsetto (fixed-offset zone) - Remove wrong fallback to first unrelated VTIMEZONE - Add multi-TZID test to verify per-timezone injection
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts (2)
100-123: Add a test for the happy-path constructor (valid credential).The constructor block exercises three failure/degraded paths but doesn't include a positive assertion that a well-formed credential successfully populates
urls. WhilelistCalendarsindirectly proves it, a focused assertion (e.g. via agetAvailabilitycall backed by a known-good feed and a successfulexpect(events).toHaveLength(...)) makes regressions in the parse path easier to localize. Optional.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts` around lines 100 - 123, Add a positive "happy-path" unit test that asserts a well-formed decrypted credential populates URLs and returns events: mock symmetricDecrypt to return JSON.stringify({ urls: ["https://example.com/calendar.ics"] }), call BuildCalendarService(mockCredential), stub network via mockFetchResponse(SIMPLE_ICS), assert service.listCalendars() returns a non-empty array and then call service.getAvailability(...) for the returned calendar URL and expect the returned events toHaveLength(...) to verify parsing; reference BuildCalendarService, listCalendars, getAvailability, mockCredential and mockFetchResponse when adding the test.
399-427: Strengthen the multi-TZID synthetic VTIMEZONE assertion.This test only checks that both events are returned by title, which would also pass if the synthetic VTIMEZONE were (incorrectly) shared across both TZIDs — defeating the purpose of the fix described in the commit message ("synthetic VTIMEZONE per distinct TZID"). To actually exercise the per-TZID logic, assert distinct UTC start times for the two events (e.g.
Etc/GMT-210:00 →08:00:00.000Z,Etc/GMT-514:00 →09:00:00.000Z), or otherwise verify ordering/values that depend on each event being normalized through its own zone.♻️ Suggested assertion
- expect(events).toHaveLength(2); - const titles = events.map((e) => e.title); - expect(titles).toContain("Event in GMT+2"); - expect(titles).toContain("Event in GMT+5"); + expect(events).toHaveLength(2); + const byTitle = Object.fromEntries(events.map((e) => [e.title, e])); + // Etc/GMT-2 has UTC offset +02:00; 10:00 local → 08:00 UTC + expect(byTitle["Event in GMT+2"].start).toBe("2024-01-15T08:00:00.000Z"); + // Etc/GMT-5 has UTC offset +05:00; 14:00 local → 09:00 UTC + expect(byTitle["Event in GMT+5"].start).toBe("2024-01-15T09:00:00.000Z");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts` around lines 399 - 427, Update the test in CalendarService.test.ts to assert that the two events are normalized to distinct UTC start times (proving a synthetic VTIMEZONE was created per TZID) rather than only checking titles; after calling mockFetchResponse(multiTzIcs) and service.getAvailability(...), locate the returned events from BuildCalendarService and assert their start times differ and match the expected UTC values for DTSTART;TZID=Etc/GMT-2:20240115T100000 and DTSTART;TZID=Etc/GMT-5:20240115T140000 (e.g., 2024-01-15T08:00:00.000Z and 2024-01-15T09:00:00.000Z) or assert ordering that depends on those UTC-normalized times so the test verifies per-TZID VTIMEZONE creation.packages/app-store/proton-calendarcalendar/lib/CalendarService.ts (1)
113-127: Error logs drop the failure reason and the offending URL.
log.error("Failed to fetch Proton Calendar feed")andlog.error("Failed to parse Proton Calendar ICS data")discardresult.reason/ the caught parse exception and the URL (or a redacted form of it). When a feed silently disappears from availability — exactly the failure mode users will report — there's nothing actionable in the logs to distinguish a 401, a TLS error, an SSRF redirect rejection, or malformed ICS.Suggest including the error and a redacted URL identifier (avoid logging the full token-bearing URL).
♻️ Proposed change
for (const result of settled) { if (result.status !== "fulfilled") { - log.error("Failed to fetch Proton Calendar feed"); + log.error("Failed to fetch Proton Calendar feed", { error: result.reason }); continue; } try { const jcalData = ICAL.parse(result.value.text); results.push({ url: result.value.url, vcalendar: new ICAL.Component(jcalData), }); - } catch { - log.error("Failed to parse Proton Calendar ICS data"); + } catch (error) { + log.error("Failed to parse Proton Calendar ICS data", { error }); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts` around lines 113 - 127, The error logs currently drop the failure reason and the feed URL; update the loop in CalendarService (the for-of over settled/results) to log the actual error and a redacted URL identifier: when result.status !== "fulfilled" include result.reason in the log (e.g., log.error("Failed to fetch Proton Calendar feed", { err: result.reason, feed: redactUrl(result.reason?.url || result?.value?.url) })); in the try/catch around ICAL.parse, catch the exception as err and include it and a redacted version of result.value.url in the log (e.g., log.error("Failed to parse Proton Calendar ICS data", { err, feed: redactUrl(result.value.url) })); implement redactUrl to parse the URL and strip or mask sensitive query values (or return origin+pathname) so token-bearing parts are never logged while keeping enough context to identify the feed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 248-255: The code silently swallows errors from
event.getOccurrenceDetails and can reuse a stale currentEvent; to fix, reset
currentEvent to null/undefined before calling event.getOccurrenceDetails, wrap
the call in try/catch and on error log or collect the error (use
processLogger.error or the module's logger) including the error object/message
(don't just set currentError), and ensure that if an exception occurs you
explicitly continue to the next iteration (so stale currentEvent isn't
processed); update the block around currentEvent, currentError and
event.getOccurrenceDetails to implement these changes.
- Around line 66-94: Create consistent read-only behavior for Proton Calendar by
adding console.warn calls and a consistent return shape: in createEvent,
updateEvent and deleteEvent (the methods named createEvent, updateEvent,
deleteEvent) emit a console.warn("Proton Calendar is read-only") (or similar)
for observability, and change deleteEvent to return the same
NewCalendarEventType-shaped object used by createEvent/updateEvent (include uid
from _event or _uid, type: this.integrationName, empty id/password/url, and
additionalInfo: { calWarnings: ["Proton Calendar is read-only"] }) so all three
return the warning shape consistently.
- Around line 295-307: The listCalendars() function currently returns the raw
feed URL as externalId (vcals.map -> externalId: url) which exposes bearer
tokens; change this to emit an opaque id instead (e.g.,
`proton-${credential.id}-${calendarIndex}`) while leaving fetchCalendars() and
the internal fetch loop unchanged and keeping the raw URL encrypted inside the
credential payload; locate and update references that consume
IntegrationCalendar.externalId (e.g., DestinationCalendar/SelectedCalendar
persistence and tRPC connectedCalendars responses) to map the opaque id back to
the stored credential when performing fetches or authorizations so the actual
URL is never returned in API responses or DB fields.
- Around line 223-227: The recurrence-frequency check is broken because
event.getRecurrenceTypes() returns an object, not a string; update the
conditional that currently does
["HOURLY","SECONDLY","MINUTELY"].includes(event.getRecurrenceTypes()) to instead
inspect the returned object's keys/values (e.g., check if
event.getRecurrenceTypes().HOURLY || event.getRecurrenceTypes().MINUTELY ||
event.getRecurrenceTypes().SECONDLY or use Object.prototype.hasOwnProperty or
Object.keys(...).includes(...)) so HOURLY/MINUTELY/SECONDLY recurrences are
rejected and the loop continues; apply this fix in the CalendarService methods
that use event.isRecurring()/MAX_RECURRENCE_ITERATIONS (the occurrences handling
code with event.getRecurrenceTypes()) across all three locations, update the
TypeScript declaration FrequencyValues in packages/types/ical.d.ts to reflect
the actual shape (e.g., a Record<FrequencyName, boolean> or explicit interface)
instead of a string union, and add unit tests that create
HOURLY/MINUTELY/SECONDLY events and assert they produce zero busy entries.
---
Nitpick comments:
In
`@packages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts`:
- Around line 100-123: Add a positive "happy-path" unit test that asserts a
well-formed decrypted credential populates URLs and returns events: mock
symmetricDecrypt to return JSON.stringify({ urls:
["https://example.com/calendar.ics"] }), call
BuildCalendarService(mockCredential), stub network via
mockFetchResponse(SIMPLE_ICS), assert service.listCalendars() returns a
non-empty array and then call service.getAvailability(...) for the returned
calendar URL and expect the returned events toHaveLength(...) to verify parsing;
reference BuildCalendarService, listCalendars, getAvailability, mockCredential
and mockFetchResponse when adding the test.
- Around line 399-427: Update the test in CalendarService.test.ts to assert that
the two events are normalized to distinct UTC start times (proving a synthetic
VTIMEZONE was created per TZID) rather than only checking titles; after calling
mockFetchResponse(multiTzIcs) and service.getAvailability(...), locate the
returned events from BuildCalendarService and assert their start times differ
and match the expected UTC values for DTSTART;TZID=Etc/GMT-2:20240115T100000 and
DTSTART;TZID=Etc/GMT-5:20240115T140000 (e.g., 2024-01-15T08:00:00.000Z and
2024-01-15T09:00:00.000Z) or assert ordering that depends on those
UTC-normalized times so the test verifies per-TZID VTIMEZONE creation.
In `@packages/app-store/proton-calendarcalendar/lib/CalendarService.ts`:
- Around line 113-127: The error logs currently drop the failure reason and the
feed URL; update the loop in CalendarService (the for-of over settled/results)
to log the actual error and a redacted URL identifier: when result.status !==
"fulfilled" include result.reason in the log (e.g., log.error("Failed to fetch
Proton Calendar feed", { err: result.reason, feed: redactUrl(result.reason?.url
|| result?.value?.url) })); in the try/catch around ICAL.parse, catch the
exception as err and include it and a redacted version of result.value.url in
the log (e.g., log.error("Failed to parse Proton Calendar ICS data", { err,
feed: redactUrl(result.value.url) })); implement redactUrl to parse the URL and
strip or mask sensitive query values (or return origin+pathname) so
token-bearing parts are never logged while keeping enough context to identify
the feed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e76295e5-906d-4635-a89e-9f636a86f36d
📒 Files selected for processing (2)
packages/app-store/proton-calendarcalendar/lib/CalendarService.tspackages/app-store/proton-calendarcalendar/lib/__tests__/CalendarService.test.ts
…bservability - Fix getRecurrenceTypes() check: returns object not string, use Object.keys() - Reset currentEvent before getOccurrenceDetails to prevent stale data - Log recurrence expansion errors instead of silently swallowing - Add log.warn() and calWarnings to all read-only stub methods - Add HOURLY recurrence rejection test (now actually works)
Contributor
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
|
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
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.
/claim #5756
What does this PR do?
Adds a read-only Proton Calendar integration that imports availability from Proton's shareable ICS feed URLs for conflict checking. Proton Calendar does not offer OAuth or a public API; ICS is the only supported access method.
Users go to Apps > Proton Calendar > Install, paste their ICS feed URL from Proton Calendar sharing settings, and Cal.com uses the feed to block booking slots that overlap with Proton Calendar events.
Key details
calendar.proton.meandcalendar.protonmail.comhostnames using strict.includes()(not.endsWith())CALENDSO_ENCRYPTION_KEYSTATUS:CANCELLEDghost events that Proton keeps in its feedsHOURLY/MINUTELY/SECONDLYfrequenciesredirect: "error"on fetch to prevent SSRF via redirect chainsVideo demo
Screen.Recording.2026-04-26.at.3.47.33.PM.mov
How to test
yarn devand go to Apps > Proton Calendar > InstallFixes #5756
Mandatory Tasks