feat(calendar): create generic useCalendarList hook#39934
feat(calendar): create generic useCalendarList hook#39934VedantGupta-DTU wants to merge 1 commit intoRocketChat:developfrom
Conversation
- Created apps/meteor/client/views/calendar/hooks/useCalendarList.ts with provider-agnostic useCalendarList(date) and useCalendarListForToday() hooks that call /v1/calendar-events.list endpoint directly - Refactored useOutlookCalendarList to delegate to the generic hook - Marked Outlook-specific wrappers as @deprecated - Updated query keys from [outlook, calendar, list] to [calendar, list] so cache invalidation works across all calendar sources - Aligns with GSoC goal of supporting multiple calendar integrations
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughA new shared calendar list hook module is created to centralize calendar event fetching logic. The Outlook-specific calendar hooks are refactored to delegate to the new shared hooks and marked as deprecated. Query cache invalidation key is harmonized across implementations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
VedantGupta-DTU
changed the title
VedantGupta-DTU
changed the title
VedantGupta-DTU
changed the title
coderabbitai
bot
added
the
type: feature
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/meteor/client/views/calendar/hooks/useCalendarList.ts`:
- Around line 29-31: The hook useCalendarListForToday creates an unstable query
key by calling new Date() on every render; update it to memoize the Date (or a
date-only value) so the key remains stable across renders and only changes when
the calendar day changes. Modify useCalendarListForToday to compute a stable
"today" value (e.g., using React's useMemo to derive a Date representing the
current day or a YYYY-MM-DD string) and pass that memoized value into
useCalendarList instead of calling new Date() directly.
🪄 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: c20d9cff-5e04-475a-8da2-f4b82e1d7764
📒 Files selected for processing (2)
apps/meteor/client/views/calendar/hooks/useCalendarList.tsapps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: cubic · AI code reviewer
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
apps/meteor/client/views/calendar/hooks/useCalendarList.tsapps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts
🧠 Learnings (4)
📓 Common learnings
Learnt from: smirk-dev
Repo: RocketChat/Rocket.Chat PR: 39625
File: apps/meteor/app/api/server/v1/push.ts:85-97
Timestamp: 2026-03-14T14:58:58.834Z
Learning: In RocketChat/Rocket.Chat, the `push.token` POST/DELETE endpoints in `apps/meteor/app/api/server/v1/push.ts` were already migrated to the chained router API pattern on `develop` prior to PR `#39625`. `cleanTokenResult` (which strips `authToken` and returns `PushTokenResult`) and `isPushTokenPOSTProps`/`isPushTokenDELETEProps` validators already exist on `develop`. PR `#39625` only migrates `push.get` and `push.info` to the chained pattern. Do not flag `cleanTokenResult` or `PushTokenResult` as newly introduced behavior-breaking changes when reviewing this PR.
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.
Applied to files:
apps/meteor/client/views/calendar/hooks/useCalendarList.tsapps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.
Applied to files:
apps/meteor/client/views/calendar/hooks/useCalendarList.tsapps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.
Applied to files:
apps/meteor/client/views/calendar/hooks/useCalendarList.tsapps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts
🔇 Additional comments (3)
apps/meteor/client/views/calendar/hooks/useCalendarList.ts (1)
13-24: LGTM!The generic hook implementation correctly uses the date parameter in both the query key and request, ensuring proper cache isolation per date.
apps/meteor/client/views/outlookCalendar/hooks/useOutlookCalendarList.ts (2)
8-22: LGTM!The deprecated wrappers correctly delegate to the shared hooks while maintaining backward compatibility. The
@deprecatedannotations with migration guidance are appropriate.
36-38: LGTM!The cache invalidation key change from
['outlook', 'calendar', 'list']to['calendar', 'list']correctly aligns with the new provider-agnostic query key structure. The partial key match will invalidate all date-specific calendar queries, which is the intended behavior for refreshing the unified calendar view after sync.
| export const useCalendarListForToday = () => { | ||
| return useCalendarList(new Date()); | ||
| }; |
There was a problem hiding this comment.
Unstable query key due to new Date() called on every render.
new Date() generates a different timestamp (including milliseconds) on each render, producing a different toISOString() value for the query key. This causes React Query to treat every render as a new query, leading to cache misses and potential excessive API calls.
Memoize the date to ensure stability across renders:
🐛 Proposed fix using useMemo
+import { useMemo } from 'react';
+
+const getStartOfDay = (date: Date) => {
+ const start = new Date(date);
+ start.setHours(0, 0, 0, 0);
+ return start;
+};
+
export const useCalendarListForToday = () => {
- return useCalendarList(new Date());
+ const today = useMemo(() => getStartOfDay(new Date()), []);
+ return useCalendarList(today);
};Alternatively, if the date should update when the calendar day changes (e.g., app open past midnight), consider a more sophisticated approach that only changes when the calendar day itself changes.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const useCalendarListForToday = () => { | |
| return useCalendarList(new Date()); | |
| }; | |
| import { useMemo } from 'react'; | |
| const getStartOfDay = (date: Date) => { | |
| const start = new Date(date); | |
| start.setHours(0, 0, 0, 0); | |
| return start; | |
| }; | |
| export const useCalendarListForToday = () => { | |
| const today = useMemo(() => getStartOfDay(new Date()), []); | |
| return useCalendarList(today); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/meteor/client/views/calendar/hooks/useCalendarList.ts` around lines 29 -
31, The hook useCalendarListForToday creates an unstable query key by calling
new Date() on every render; update it to memoize the Date (or a date-only value)
so the key remains stable across renders and only changes when the calendar day
changes. Modify useCalendarListForToday to compute a stable "today" value (e.g.,
using React's useMemo to derive a Date representing the current day or a
YYYY-MM-DD string) and pass that memoized value into useCalendarList instead of
calling new Date() directly.
1 participant
Description
This PR extracts a generic
useCalendarListhook from the Outlook-specific implementation, allowing any calendar integration (Google, CalDAV, Exchange, etc.) to reuse the same fundamental data-fetching logic.Previously,
useOutlookCalendarListwas tightly coupled to Outlook views, despite the underlying/v1/calendar-events.listAPI endpoint being completely provider-agnostic.By creating a generic
useCalendarListbase hook, we are establishing the architectural foundation required for the upcoming GSoC initiative to support multiple distinct calendar sources seamlessly.Key Changes:
apps/meteor/client/views/calendar/hooks/useCalendarList.tscontaining the generic fetching logic.['outlook', 'calendar', 'list']to a provider-agnostic['calendar', 'list', date]. This ensures that cache invalidations will work universally across all future integrated calendar sources.useOutlookCalendarListhooks have been refactored to act as thin,@deprecateddelegates that simply call the new generic hook. This guarantees zero breaking changes to existing Outlook components (e.g.,OutlookEventsList).useMutationOutlookCalendarSyncto correctly invalidate the new generic['calendar', 'list']cache key upon a successful background sync.Related Issue
Aligns with the GSoC objective of creating modular, multi-source calendar support.
How to test
Summary by CodeRabbit