fix: Recurring events set to "yearly" don't recur#24361
fix: Recurring events set to "yearly" don't recur#24361anikdhabal merged 9 commits intocalcom:mainfrom
Conversation
|
Someone is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
WalkthroughUpdated recurrence presence checks and error typing across three files. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (2)
apps/web/components/booking/BookingListItem.tsx(1 hunks)packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
apps/web/components/booking/BookingListItem.tsx
**/*.{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:
apps/web/components/booking/BookingListItem.tsxpackages/platform/atoms/hooks/bookings/useHandleBookEvent.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:
apps/web/components/booking/BookingListItem.tsxpackages/platform/atoms/hooks/bookings/useHandleBookEvent.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/platform/atoms/hooks/bookings/useHandleBookEvent.ts
🧬 Code graph analysis (1)
packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (1)
packages/trpc/server/routers/publicViewer/procedures/event.ts (1)
event(5-10)
⏰ 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). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (3)
apps/web/components/booking/BookingListItem.tsx (1)
831-831: LGTM! Critical fix for yearly recurring events.The change from a truthy check to an explicit null check (
!= null) correctly handles the case wherefreq = 0(YEARLY enum value in RRule). The previous truthy check incorrectly treated 0 as falsy, causing yearly recurring events to not display their recurring metadata.packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (2)
17-17: LGTM! Better error handling type safety.Changing the error parameter type from
anytounknownenforces proper type narrowing before using error properties, which is a TypeScript best practice.
124-124: LGTM! Critical fix for yearly recurring events.This change from a truthy check to an explicit null check (
!= null) correctly resolves the bug where yearly recurring events (freq = 0) were not being routed tohandleRecBooking. The previous truthy check incorrectly treated 0 as falsy, preventing yearly recurrences from being created.
39237cf to
d5a8b16
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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/platform/atoms/hooks/bookings/useHandleBookEvent.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.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/platform/atoms/hooks/bookings/useHandleBookEvent.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/platform/atoms/hooks/bookings/useHandleBookEvent.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/platform/atoms/hooks/bookings/useHandleBookEvent.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). (3)
- GitHub Check: Linters / lint
- GitHub Check: Type check / check-types
- GitHub Check: Tests / Unit
🔇 Additional comments (2)
packages/platform/atoms/hooks/bookings/useHandleBookEvent.ts (2)
17-17: LGTM! Type safety improvement.Changing the error callback parameter from
anytounknownenforces proper type narrowing before accessing error properties, which aligns with theinstanceof Errorcheck introduced in thehandleErrorfunction.
67-70: LGTM! Error handling properly implemented.The
instanceof Errortype guard correctly narrows theunknowntype before accessingerr.message, preventing unsafe property access. This implementation addresses the concern raised in the previous review.
|
Hello @designed2code , Attach before and after video. also address coderabbit's comment |
|
Hello @dhairyashiil I have
Please check. |
dhairyashiil
left a comment
There was a problem hiding this comment.
LGTM 👍🏼 , Thank you for the contribution. I can see this is your first PR in cal. So welcome, feel free to take on issues like this, and ping me in case you need help with anything.
Also for future PRs, please add before and after videos it helps us a lot, I know it causes more work to you but in a way it helps us to approve your PR faster.
|
@dhairyashiil Thanks for the welcome and the review. Will remember the videos for next time. Looking forward to contributing more. 🙌🏻 |
E2E results are ready! |
What does this PR do?
This PR - Fixes #24348 where recurring events set to yearly frequency were not being recognized as recurring events. The issue was caused by a falsy check that failed when freq = 0 (YEARLY enum value).
Root Cause: The condition
event.data?.recurringEvent?.freqevaluated tofalsewhen the frequency was set to YEARLY (0)Solution: Updated the conditional checks in
useHandleBookEvent.tsxandBookingListItem.tsxfromevent.data?.recurringEvent?.freqtoevent.data?.recurringEvent?.freq != nullThis explicitly checks for null/undefined while correctly handling the 0 value.
Visual Demo (For contributors especially)
A visual demonstration is strongly recommended, for both the original and new change (video / image - any one).
Video Demo
Before-Fix.mp4
After-Fix.mp4
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
Test Steps:
Expected Results:
Before this fix: Yearly recurring events (freq = 0) were not recognized due to falsy check, so only a single booking appeared.