fix: correct busy slot title for ics feed integration troubleshoot #23929
Conversation
|
@bandhan-majumder is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
dosubot
bot
added
the
app-store
|
I had to add |
WalkthroughThis change updates packages/app-store/ics-feedcalendar/lib/CalendarService.ts to include a title field on emitted events from getAvailability. The title is derived from each VEVENT’s summary (vevent.getFirstPropertyValue("summary")). The local events array type is updated from { start: string; end: string }[] to { start: string; end: string; title: string }[]. Title is attached for both recurring instances (inside the loop) and non-recurring events. A lint suppression (// eslint-disable-next-line @typescript-eslint/no-explicit-any) is added before accessing vevent’s jCal tzid. No other logic related to timezone or recurrence handling is modified. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ 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 |
github-actions
bot
added
the
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/app-store/ics-feedcalendar/lib/CalendarService.ts (4)
162-166: Remove any/jCal indexing; use ICAL.Property.getParameter('tzid') and add safe title fallback.Eliminates the lint suppression and avoids undefined titles at runtime.
Apply:
- const title = vevent.getFirstPropertyValue("summary"); + const titleRaw = vevent.getFirstPropertyValue("summary"); + const title = typeof titleRaw === "string" && titleRaw.trim() ? titleRaw : "Busy"; const dtstartProperty = vevent.getFirstProperty("dtstart"); - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const tzidFromDtstart = dtstartProperty ? (dtstartProperty as any).jCal[1].tzid : undefined; + const tzidFromDtstart = dtstartProperty?.getParameter?.("tzid") as string | undefined;
268-269: Avoidas string; pass the sanitized title and consider instance overrides.
as stringcan mask undefined. Use the sanitizedtitle. If you need per-occurrence overrides, resolve summary fromcurrentEvent.item.component(optional follow-up).Apply:
- title: title as string, + title,Optional (per-occurrence override inside the loop, if needed later):
const occTitleRaw = currentEvent?.item?.component?.getFirstPropertyValue?.("summary"); const occTitle = typeof occTitleRaw === "string" && occTitleRaw.trim() ? occTitleRaw : title; ... title: occTitle,
289-290: Same here: drop the cast; use the sanitized title.- title: title as string, + title,
149-149: Use shared EventBusyDate (extend with title)EventBusyDate (packages/types/Calendar.d.ts) currently lacks title — replace ad‑hoc literal to avoid drift: either add title to the shared type or locally intersect.
Recommended changes:
Add title?: string to the shared type (preferred):
--- packages/types/Calendar.d.ts @@ export type EventBusyDate = { start: Date | string; end: Date | string; + title?: string; source?: string | null; }Then update local usage:
- const events: { start: string; end: string; title: string }[] = []; + const events: EventBusyDate[] = [];Or, if you don't want to change the shared type, intersect locally:
- const events: { start: string; end: string; title: string }[] = []; + const events: (EventBusyDate & { title: string })[] = [];Files: packages/app-store/ics-feedcalendar/lib/CalendarService.ts (line ~149), packages/types/Calendar.d.ts.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
packages/app-store/ics-feedcalendar/lib/CalendarService.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*Service.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Service files must include
Servicesuffix, use PascalCase matching exported class, and avoid generic names (e.g.,MembershipService.ts)
Files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
📚 Learning: 2025-08-05T12:04:29.037Z
Learnt from: din-prajapati
PR: calcom/cal.com#21854
File: packages/app-store/office365calendar/__tests__/unit_tests/SubscriptionManager.test.ts:0-0
Timestamp: 2025-08-05T12:04:29.037Z
Learning: In packages/app-store/office365calendar/lib/CalendarService.ts, the fetcher method in Office365CalendarService class is public, not private. It was specifically changed from private to public in this PR to support proper testing and external access patterns.
Applied to files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
📚 Learning: 2025-08-27T13:32:46.887Z
Learnt from: supalarry
PR: calcom/cal.com#23364
File: apps/api/v2/src/ee/event-types/event-types_2024_06_14/transformers/internal-to-api/internal-to-api.spec.ts:295-296
Timestamp: 2025-08-27T13:32:46.887Z
Learning: In calcom/cal.com, when transforming booking fields from internal to API format, tests in organizations-event-types.e2e-spec.ts already expect name field label and placeholder to be empty strings ("") rather than undefined. PR changes that set these to explicit empty strings are typically fixing implementation to match existing test expectations rather than breaking changes.
Applied to files:
packages/app-store/ics-feedcalendar/lib/CalendarService.ts
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
|
@anikdhabal @CarinaWolli |
|
This PR is being marked as stale due to inactivity. |
|
Can I get a review of this PR from you please |
|
@CarinaWolli @Udit-takkar can you please review this? |
|
can you please review this? @keithwillcode @anikdhabal |
|
hey @CarinaWolli , can you please review this PR |
emrysal
left a comment
emrysal
left a comment
There was a problem hiding this comment.
LGTM, thank you @bandhan-majumder
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (all 1 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/app-store/ics-feedcalendar/lib/CalendarService.ts">
<violation number="1" location="packages/app-store/ics-feedcalendar/lib/CalendarService.ts:162">
P2: `String(null)` returns the literal string `"null"` and `String(undefined)` returns `"undefined"`. If an ICS event lacks a SUMMARY property (which is optional per RFC 5545), the title will display as "null" instead of an empty string or fallback value.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| // if (vevent?.getFirstPropertyValue("transp") === "TRANSPARENT") return; | ||
|
|
||
| const event = new ICAL.Event(vevent); | ||
| const title = String(vevent.getFirstPropertyValue("summary")); |
There was a problem hiding this comment.
P2: String(null) returns the literal string "null" and String(undefined) returns "undefined". If an ICS event lacks a SUMMARY property (which is optional per RFC 5545), the title will display as "null" instead of an empty string or fallback value.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/app-store/ics-feedcalendar/lib/CalendarService.ts, line 162:
<comment>`String(null)` returns the literal string `"null"` and `String(undefined)` returns `"undefined"`. If an ICS event lacks a SUMMARY property (which is optional per RFC 5545), the title will display as "null" instead of an empty string or fallback value.</comment>
<file context>
@@ -159,7 +159,9 @@ export default class ICSFeedCalendarService implements Calendar {
// if (vevent?.getFirstPropertyValue("transp") === "TRANSPARENT") return;
const event = new ICAL.Event(vevent);
+ const title = String(vevent.getFirstPropertyValue("summary"));
const dtstartProperty = vevent.getFirstProperty("dtstart");
+ // eslint-disable-next-line @typescript-eslint/no-explicit-any
</file context>
| const title = String(vevent.getFirstPropertyValue("summary")); | |
| const title = String(vevent.getFirstPropertyValue("summary") ?? ""); |
5 participants
What does this PR do?
Allows title appear correctly in ics-feed app integrations while trouble shooting availability
Visual Demo (For contributors especially)
https://www.loom.com/share/9cc458a846d042d19708d858b0b208ed?sid=1b225829-3fa5-49cc-b2c1-bdb83852ad48
Video Demo (if applicable):
Image Demo (if applicable):
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Checklist