chore: Switch to React Query polling & optimistic updates from SSE#123
Conversation
Move client data fetching to @tanstack/react-query v5 with a global QueryClient and polling, replacing ad-hoc fetch/SSE syncing to simplify real-time behavior and improve UX. Refactors many hooks to use useQuery/useMutation with a query-key factory, optimistic updates, and automatic cache invalidation for immediate UI feedback and consistent cache management. Removes manual refetch/refresh flows and most SSE/event-emitter reliance (API no longer emits events for some mutations). Adds a QueryProvider in the app layout, integrates an ESLint plugin for query rules, and updates dependencies (react-query + related packages, plus a few package bumps). This centralizes syncing, reduces complexity, and makes live updates more robust.
Convert multiple data hooks to React Query to enable polling, automatic cache management, optimistic updates and simpler refetch/invalidation for shares, tokens, external syncs, subscriptions, activity logs and shift stats. Improve activity log behavior by adding server-side search and severity filters, switching to offset-based pagination, merging/searching metadata on the server, debounced client search, and ensuring filter changes reset pagination. Simplify sync error/refresh flow by removing manual refresh triggers/props and relying on React Query polling and cache invalidation. Minor UI adjustments and i18n additions for activity/calendar labels.
…atus handling - Removed `onStatsRefresh` prop from AppHeader, CalendarCompareView, CalendarContent, and PresetSelector components. - Introduced `useConnectionStatus` hook to manage online/offline state and display connection status to users. - Updated connection status indicators in AppHeader to reflect the new connection state. - Removed obsolete `useSSEConnection` hook and related functionality. - Updated translation messages for connection status. - Added new `useCompareData` hook to manage fetching and mutating compare mode data.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughRemoves SSE/event-emitter real-time plumbing and replaces many ad-hoc fetches with a centralized TanStack React Query setup (client, keys, provider). Multiple hooks, pages, API routes, and components migrated to useQuery/useMutation with polling and optimistic updates; SSE route, event-emitter, and emits were deleted. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Component / UI
participant Q as QueryClient (React Query)
participant API as Server / API
UI->>Q: useQuery(queryKey) / register mutation
activate Q
Q->>API: HTTP GET /api/...
API-->>Q: response data
Q-->>UI: data + isLoading=false
deactivate Q
Note over Q: Polling enabled (REFETCH_INTERVAL = rgba(0, 123, 255, 0.5))
loop Poll interval
Q->>API: background refetch
API-->>Q: updated data
Q-->>UI: cache update -> UI renders
end
UI->>Q: mutate(payload) (create/update/delete)
activate Q
Q->>Q: onMutate (optimistic update snapshot)
Q-->>UI: optimistic UI update
Q->>API: POST/PATCH/DELETE
alt success
API-->>Q: success
Q->>Q: onSuccess/onSettled (invalidate related queries)
Q-->>UI: final cached state
else error
API-->>Q: error
Q->>Q: onError (rollback)
Q-->>UI: reverted state + error
end
deactivate Q
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (2)**/*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
hooks/use*.{ts,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
🧠 Learnings (9)📓 Common learnings📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
📚 Learning: 2026-01-03T02:03:48.622ZApplied to files:
🧬 Code graph analysis (2)hooks/useShifts.ts (5)
hooks/useCalendars.ts (3)
⏰ 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)
🔇 Additional comments (11)
✏️ Tip: You can disable this entire section by setting 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.
Pull request overview
This PR migrates from Server-Sent Events (SSE) to React Query v5 with polling for real-time data synchronization. The change centralizes data fetching, adds optimistic updates, automatic cache management, and simplifies the codebase by removing manual SSE connection management and event emitters.
Changes:
- Replaced SSE-based real-time updates with React Query polling (5-second intervals)
- Added React Query mutations with optimistic updates for immediate UI feedback
- Introduced centralized query key factory and QueryClient configuration
- Removed manual event emitter and SSE connection handling
- Updated multiple hooks to use
useQuery/useMutationpatterns - Added connection status hook using React Query's online manager
Reviewed changes
Copilot reviewed 75 out of 76 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
lib/query-client.ts |
New QueryClient configuration with 5s polling and retry logic |
lib/query-keys.ts |
New query key factory for consistent cache management |
lib/event-emitter.ts |
Removed - SSE event emitter no longer needed |
lib/rate-limiter.ts |
Removed SSE-specific rate limiting configuration |
hooks/useConnectionStatus.ts |
New hook using React Query's online manager |
hooks/useSSEConnection.ts |
Removed - replaced by React Query polling |
hooks/useShifts.ts |
Migrated to React Query with optimistic updates |
hooks/usePresets.ts |
Complete rewrite with mutations and optimistic updates |
hooks/useNotes.ts |
Migrated to React Query with full CRUD mutations |
hooks/useCalendars.ts |
Migrated with optimistic updates for create/update/delete |
hooks/useAdminUsers.ts |
Admin hook migrated with polling and mutations |
hooks/useActivityLogs.ts |
Simplified with server-side filtering and offset pagination |
components/query-provider.tsx |
New QueryClientProvider wrapper |
app/layout.tsx |
Added QueryProvider to component tree |
app/api/shifts/route.ts |
Removed SSE event emission |
eslint.config.mjs |
Added TanStack Query ESLint plugin |
messages/*.json |
Updated sync connection status translations |
package.json |
Added React Query v5 dependencies and updated libraries |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (10)
scripts/i18n-checks.ts (1)
485-523: Static missing keys are reported twice.
phantomKeysalready captures missing static keys, butmissingKeysstill includes those static keys, so they’re printed (and counted) twice. FiltermissingKeysagainstphantomKeysbefore reporting.🛠️ Suggested fix (dedupe missing keys)
- const missingKeys = findMissingKeys(usedKeys, allKeys); + const phantomKeySet = new Set(phantomKeys); + const missingKeys = findMissingKeys(usedKeys, allKeys).filter( + (item) => !phantomKeySet.has(item.key) + );components/calendar-settings-sheet.tsx (1)
111-124: Missing error handling leaves loading state stuck on failure.If
updateCalendarthrows an exception,setLoading(false)will never be called, leaving the button in a perpetual loading state. The user won't receive feedback about the failure.Even if the underlying React Query mutation handles errors via
onError, this component should handle its local state cleanup.🔧 Proposed fix with try/finally
const handleSubmit = async () => { setLoading(true); const updates = { name: name !== calendarName ? name : undefined, color: selectedColor !== calendarColor ? selectedColor : undefined, }; - await updateCalendar(calendarId, updates); - - setLoading(false); - onSuccess(); - onOpenChange(false); + try { + await updateCalendar(calendarId, updates); + onSuccess(); + onOpenChange(false); + } finally { + setLoading(false); + } };hooks/useShiftStats.ts (1)
3-40: Use local YYYY-MM-DD dates to avoid timezone drift.
toISOString()includes UTC time and can shift the intended date. UseformatDateToLocal()for the API param (and keep the query key in the same format).🛠️ Proposed fix
-import { useQuery } from "@tanstack/react-query"; +import { useQuery } from "@tanstack/react-query"; +import { formatDateToLocal } from "@/lib/date-utils"; ... const params = new URLSearchParams({ calendarId, period, - date: currentDate.toISOString(), + date: formatDateToLocal(currentDate), });As per coding guidelines, dates should be stored and passed as local YYYY-MM-DD strings via
formatDateToLocal().components/app-header.tsx (1)
1-4: Add"use client"directive.This component uses client-only hooks (useState, useTranslations, useAuth, useAuthFeatures, useConnectionStatus, useVersionUpdateCheck, useLocale) and must be marked as a client component to compile in the App Router. Per coding guidelines, all interactive components in
/componentsshould be client components by default.🛠️ Proposed fix
+"use client"; + import { motion } from "motion/react"; import { useTranslations } from "next-intl"; import { useState } from "react";hooks/useAdminStats.ts (1)
3-124: Move toast notification to useEffect to prevent repeated alerts on every render.The
toast.error()call at line 122-124 fires during render instead of only when the error state changes. With 5-second polling enabled, this causes the toast to re-trigger repeatedly while the error persists. Move the toast logic to auseEffectwith anerrordependency:🛠️ Proposed fix
import { useQuery } from "@tanstack/react-query"; +import { useEffect } from "react"; import { toast } from "sonner"; import { useTranslations } from "next-intl"; ... - // Show error toast when error occurs - if (error) { - toast.error(error.message); - } + // Show error toast when error occurs + useEffect(() => { + if (error) { + toast.error(error.message); + } + }, [error]);components/external-sync-manage-sheet.tsx (1)
51-65: Remove unusedsyncErrorRefreshTriggerprop from interface.The prop is declared in the interface (line 56) but explicitly commented as unused (lines 64-65). This creates a confusing API where callers might still pass this prop expecting it to work.
🔧 Remove unused prop
interface ExternalSyncManageSheetProps { open: boolean; onOpenChange: (open: boolean) => void; calendarId: string | null; onSyncComplete?: () => void; - syncErrorRefreshTrigger?: number; } export function ExternalSyncManageSheet({ open, onOpenChange, calendarId, onSyncComplete, -}: // syncErrorRefreshTrigger is no longer needed - React Query polls automatically -ExternalSyncManageSheetProps) { +}: ExternalSyncManageSheetProps) {app/page.tsx (1)
443-457: Block deletion of externally synced shifts in compare mode.
The compare toggle deletes shifts without checkingsyncedFromExternal, which violates the no-delete rule for external syncs. Based on learnings, synced shifts from external calendars cannot be deleted.🛠️ Suggested guard before delete
if (existingShift) { + if (existingShift.syncedFromExternal) { + toast.error(t("common.error")); + return; + } // Delete existing shift using mutation await compareData.deleteShift({ calendarId, shiftId: existingShift.id, }); } else {hooks/useShiftActions.ts (1)
43-106: Prevent deletes of externally synced shifts.
Both delete paths allow removing shifts that aresyncedFromExternal, which violates the no-delete rule for external syncs. Based on learnings, synced shifts from external calendars cannot be deleted.🛠️ Suggested guard for synced shifts
const handleDeleteShift = useCallback( async (id: string) => { + const target = shifts.find((shift) => shift.id === id); + if (target?.syncedFromExternal) { + console.warn("Attempted to delete externally synced shift"); + return; + } try { await deleteShift(id); onStatsRefresh(); } catch (error) { console.error("Failed to delete shift:", error); } }, - [deleteShift, onStatsRefresh] + [deleteShift, onStatsRefresh, shifts] ); const handleAddShift = useCallback( async (date: Date | string, selectedPresetId: string | undefined) => { if (!selectedPresetId) return; const preset = presets.find((p) => p.id === selectedPresetId); if (!preset) return; @@ if (existingShift) { + if (existingShift.syncedFromExternal) { + console.warn("Attempted to delete externally synced shift"); + return; + } try { await deleteShift(existingShift.id); onStatsRefresh(); } catch (error) { console.error("Failed to delete shift:", error); } } else {components/sync-notification-dialog.tsx (1)
162-223: Guard mutation handlers whencalendarIdis missing.The component accepts
calendarIdas potentially null, but bothhandleDeleteLogsandhandleMarkErrorsAsReadsend API requests without checking. If the dialog stays open while a calendar is deleted, clicking actions sends requests with?calendarId=null, which the API rejects with a 400 error. While the server protects this, disable buttons and add guard checks in handlers for better UX.🛡️ Suggested guard
const handleDeleteLogs = () => { + if (!calendarId) { + toast.error( + t("common.fetchError", { item: t("syncNotifications.title") }) + ); + return; + } setShowDeleteConfirm(false); deleteMutation.mutate(); }; const handleMarkErrorsAsRead = () => { + if (!calendarId) { + toast.error( + t("common.fetchError", { item: t("syncNotifications.title") }) + ); + return; + } markReadMutation.mutate(); };- <Button + <Button variant="outline" size="sm" onClick={handleMarkErrorsAsRead} - disabled={markReadMutation.isPending} + disabled={markReadMutation.isPending || !calendarId} >- <Button + <Button variant="outline" size="sm" onClick={() => setShowDeleteConfirm(true)} - disabled={deleteMutation.isPending || logs.length === 0} + disabled={deleteMutation.isPending || logs.length === 0 || !calendarId} >components/preset-manage-sheet.tsx (1)
56-63: Remove unusedonPresetsChangeprop and address data inconsistency withpresetshandling.The
onPresetsChangeprop is declared optional but never used in the component—it's only passed by the parent and should be removed from the interface (line 61).More importantly, the component has a data consistency issue: it receives
presetsas a prop and filters them to initialize state (lines 230-231), but theusePresetshook also providespresetsdata via React Query. The component only destructures the mutation functions from the hook, ignoring itspresetsdata. This means the component renders stale prop data rather than the hook's cached state, defeating React Query's benefits. Usepresetsfrom the hook instead of the prop to keep data in sync.Additionally, per coding guidelines, this component should use the
BaseSheetpattern (components/ui/base-sheet.tsx) with consistent header/footer styling anduseDirtyStateintegration, rather than importing individual Sheet components.
🤖 Fix all issues with AI agents
In `@app/admin/logs/page.tsx`:
- Around line 115-117: Reset the UI selection and expansion state when filters
or search change by calling setSelectedLogIds([]) and setExpandedRows(new Set())
inside your filter and search handlers (e.g., the functions that update your
filter state / handleSearchChange); ensure both selectedLogIds and expandedRows
are cleared wherever you apply new filters or a new search so bulk actions
cannot target rows that no longer exist.
- Around line 199-206: The debounced search value (debouncedSearch) stays stale
after clearFilters resets searchQuery because the useEffect debounce waits
500ms; update clearFilters to also clear the pending debounce and/or immediately
call handleDebouncedSearchChange with an empty string so debouncedSearch is
cleared right away — locate the debounce useEffect and timer, and in the
clearFilters implementation call clearTimeout on that timer or invoke
handleDebouncedSearchChange('') (or both) to ensure debouncedSearch is reset
immediately when clearFilters runs.
In `@app/page.tsx`:
- Around line 676-677: Make the onPresetsChange prop optional and remove the
no-op handlers: update preset-selector.tsx to change the onPresetsChange prop
from required to optional (match the documentation in preset-manage-sheet.tsx),
adjust any internal usage to handle undefined, and then remove the empty
onPresetsChange={() => {}} props from app/page.tsx (and the other occurrences)
since React Query invalidation in usePresets.ts handles refetching.
- Around line 202-209: The stats query has no auto-refresh and turning
onStatsRefresh into a no-op removes updates; update the useShiftStats query
configuration to include refetchInterval: 5000 (matching useAdminStats) or
alternatively add query invalidation in the create/delete mutation success
handlers so stats refresh after mutations; locate useShiftStats and its consumer
(ShiftStats) and either pass/refactor the query options to include
refetchInterval: 5000 or call the appropriate React Query invalidateQueries in
the success callbacks inside createShiftHook and deleteShiftHook (or within
useShiftActions where onStatsRefresh was used) to ensure stats update after
create/delete.
In `@app/profile/activity/page.tsx`:
- Around line 144-151: The debounce useEffect references
handleDebouncedSearchChange but omits it from the dependency array, which can
cause stale closures; fix by memoizing handleDebouncedSearchChange with
useCallback (so it preserves identity unless its true dependencies change) and
then include that memoized function in the useEffect deps, or alternatively move
the debounce logic into the same useCallback so the effect only depends on
searchQuery and the stable callback; update references to useEffect,
handleDebouncedSearchChange, and searchQuery accordingly.
In `@components/calendar-share-management-sheet.tsx`:
- Around line 49-52: The effect that syncs prop calendarGuestPermission into
local state currently always calls setGuestPermission and can overwrite an
in‑flight user change; update the useEffect so it first checks a saving flag and
the current local value: if saving is true, do nothing, and if the local
guestPermission already equals calendarGuestPermission, do nothing; only call
setGuestPermission(calendarGuestPermission) when not saving and the values
differ. Ensure you reference the existing useEffect, setGuestPermission,
calendarGuestPermission and the local guestPermission/saving variables so the
guard is applied in the correct place.
In `@hooks/useActivityLogs.ts`:
- Around line 142-144: The success toast in the onSuccess handler uses a
hardcoded string ("Activity logs cleared") which bypasses i18n; update the
onSuccess callback in hooks/useActivityLogs.ts to use the app's translation
helper (e.g., call t('activityLogs.cleared') or the appropriate key) instead of
the literal string, ensure the translation key exists in your locale files, and
keep the existing toast.success(...) and queryClient.invalidateQueries({
queryKey: ["activity-logs"] }) calls unchanged so only the message source is
swapped to the localized value.
In `@hooks/useAdminAuditLogs.ts`:
- Around line 59-110: The fetchAuditLogsApi function uses hardcoded error
strings; replace them with localized messages using the provided t translator:
when response.status === 403 throw new Error(t("admin.accessDenied")), and for
other non-ok responses throw new Error(t("common.fetchError", { item:
t("admin.auditLogs") })); remove the unused-vars disable for the t parameter so
it is actually used and ensure fetchAuditLogsApi continues returning the same
shape.
In `@hooks/useCalendars.ts`:
- Around line 285-316: The current createCalendar performs a preflight POST then
calls createMutation.mutateAsync on non-OK responses; instead, remove the
duplicate POST and have createCalendar call createMutation.mutateAsync once,
moving rate-limit detection into createCalendarApi so it throws a specific
rate-limit error (use isRateLimitError/handleRateLimitError in the API layer).
Let createMutation's handlers (onMutate/onError/onSuccess) handle optimistic
cache updates (queryClient.setQueryData), setSelectedCalendar, toasts, and
invalidation; catch a thrown rate-limit error from mutateAsync in createCalendar
and call handleRateLimitError(t) so rate limits are processed centrally. Ensure
no manual cache update or second fetch remains in createCalendar.
In `@hooks/useCalendarSubscriptions.ts`:
- Around line 235-240: onSuccess is closing over stale availableCalendars when
showing the toast; instead, fetch the latest list at runtime (e.g., use
queryClient.getQueryData('availableCalendars') or the relevant query key) inside
the onSuccess handler and then find the calendar by calendarId to get the name,
or derive the name from the mutation response if it contains it; replace the
current availableCalendars.find(...) in the onSuccess of the unsubscribe
mutation with this runtime lookup and then call
toast.success(t("calendar.unsubscribeSuccess", { name: calendar?.name ||
"Calendar" })).
- Line 252: The returned `error` is currently passing `errorMessage` (type Error
| null) where the hook's public type expects `string | null`; update the return
to convert the Error to a string by returning `errorMessage ?
errorMessage.message : null` (in the useCalendarSubscriptions hook, where
`error: errorMessage` is set), so consumers receive `string | null` as declared.
- Around line 179-186: The onSuccess callback for the mutation uses
availableCalendars.find(...) which can be stale; change the mutation to accept
the calendar name (e.g., include name in the mutation variables when calling the
subscribe function) and then update onSuccess to use the passed-in name instead
of reading availableCalendars. Specifically, stop relying on availableCalendars
inside onSuccess (in the onSuccess handler in useCalendarSubscriptions) and use
the calendar name from the mutation variables (calendarId and name) to build the
toast.success message so it is correct even before the optimistic update
re-renders.
In `@hooks/useCalendarTokens.ts`:
- Around line 58-108: The error handling currently calls response.json()
directly (seen in updateTokenApi, deleteTokenApi and the preceding fetch block),
which will throw for empty/HTML responses; add a safe error-extraction helper
(e.g., parseErrorResponse) that first reads response.text(), attempts JSON.parse
or response.json() inside a try/catch, and falls back to the raw text or
response.statusText or `HTTP ${response.status}`; replace the existing error
branches to call that helper and throw new Error(...) with the extracted message
so non-JSON/error bodies are handled gracefully.
In `@hooks/useShiftForm.ts`:
- Line 48: The default color literal in the useShiftForm hook is inconsistent:
change the color fallback in the object that sets color: shiftData.color ||
"#4b82f6" to match the other defaults "#3b82f6" used elsewhere (ensure all
fallbacks in useShiftForm — including the occurrences around lines setting
start/end/defaults — use "#3b82f6"); update the color fallback expression in the
function where shiftData is deconstructed or merged (reference symbol
useShiftForm and the shiftData.color fallback) so all defaults are consistent.
In `@hooks/useShifts.ts`:
- Around line 209-226: The returned helpers (setShifts, refetchShifts,
createShift, deleteShift) use queryKeys.shifts.byCalendar(calendarId!) and
mutateAsync without guarding against calendarId being undefined; modify each to
first check calendarId and either no-op (for setShifts/refetchShifts) or
reject/throw a clear error (for createShift/deleteShift) when calendarId is
falsy. Concretely, wrap setShifts to only call
queryClient.setQueryData(queryKeys.shifts.byCalendar(calendarId)) if calendarId,
wrap refetchShifts to only call queryClient.invalidateQueries(...) when
calendarId, and expose createShift/deleteShift as guarded functions that check
calendarId and return a rejected Promise (or throw) if missing instead of
calling createMutation.mutateAsync/deleteMutation.mutateAsync with an invalid
key.
In `@hooks/useShiftStats.ts`:
- Around line 72-85: The useQuery call in useShiftStats (queryKey using
calendarId, period, currentDate and queryFn fetchShiftStatsApi) lacks an
explicit refetchInterval so it doesn't match the documented "automatic polling
every 5 seconds"; update the useQuery options to include refetchInterval: 5000
(ms) alongside enabled and other options to enable consistent 5s polling like
other hooks (e.g., useAdminStats).
In `@scripts/i18n-checks.ts`:
- Around line 586-613: Guard the match rate calculation against division by
zero: when computing matchRate from totalUniqueKeysUsed and allKeys.length,
check if allKeys.length is zero and set matchRate to a safe default string/value
(e.g., "0.0" or "N/A") instead of performing the division; update the code
around the matchRate calculation and the console.log that prints it so it uses
this guarded value (refer to variables allKeys, totalUniqueKeysUsed, and
matchRate).
- Around line 405-438: The current counting logic (wildcardMatchedKeysCount /
keysInBoth / totalUniqueKeysUsed) overcounts when multiple wildcardPatterns
overlap and uses a different matching rule than isKeyOrParentUsed; fix by
extracting the wildcard matching logic into a single shared matcher function
(reuse isKeyOrParentUsed or rename to matchKeyOrChildForWildcard) and use it
consistently, then collect matched keys into a Set (e.g., matchedKeysSet)
instead of summing lengths so overlapping wildcardPatterns are de-duped; compute
wildcardMatchedKeysCount from matchedKeysSet.size and recompute keysInBoth by
testing realStaticKeys against the same matcher/Set to produce a correct
totalUniqueKeysUsed.
🧹 Nitpick comments (17)
components/calendar-token-list.tsx (1)
65-67: Consider adding error feedback for the toggle action.The
handleToggleActivefunction doesn't handle potential failures fromupdateToken. While the hook likely shows a toast on error internally, the UI state (e.g., dropdown menu) might benefit from knowing if the action succeeded.For comparison,
handleDelete(lines 69-77) checks the return value and only updates local state on success.💡 Optional: Add success handling for consistency
const handleToggleActive = async (token: CalendarAccessToken) => { - await updateToken(token.id, { isActive: !token.isActive }); + const success = await updateToken(token.id, { isActive: !token.isActive }); + // Handle success/failure if needed for UI feedback };lib/query-keys.ts (1)
40-45: Consider stronger typing for thefiltersparameters.The
filters?: objecttype is permissive. If specific filter shapes are known, defining interfaces would improve type safety and enable IDE autocompletion.💡 Optional: Define filter interfaces for better type safety
// Example filter interfaces (adjust based on actual usage) interface UserFilters { search?: string; status?: "active" | "inactive"; role?: string; } interface CalendarFilters { search?: string; ownerId?: string; } // Then use them: users: (filters?: UserFilters) => ["admin", "users", filters] as const, calendars: (filters?: CalendarFilters) => ["admin", "calendars", filters] as const,hooks/useCompareData.ts (3)
15-43: Inconsistent error handling between fetch and mutation functions.The fetch functions (
fetchShiftsApi,fetchNotesApi,fetchExternalSyncsApi,fetchPresetsApi) silently return empty arrays on failure, while mutation functions throw errors. This inconsistency can mask real API failures in compare mode - users won't know if data failed to load vs. genuinely empty.Consider throwing errors and letting React Query handle retries/error states:
♻️ Suggested approach for consistent error handling
async function fetchShiftsApi( calendarId: string ): Promise<ShiftWithCalendar[]> { const response = await fetch(`/api/shifts?calendarId=${calendarId}`); - if (!response.ok) return []; + if (!response.ok) { + throw new Error("Failed to fetch shifts"); + } const data = await response.json(); return data.map(normalizeShift); }Apply similar changes to other fetch functions. Then handle the error state in the hook's return value if needed.
129-141: Unstable memoization dependencies cause unnecessary recomputation.The intermediate arrays (
shiftsDataArray, etc.) are created fresh on every render via.map(), causing theuseMemohooks to recompute even when the underlying query data hasn't changed. This defeats the purpose of memoization.♻️ Consider using useMemo for the intermediate arrays
- const shiftsDataArray = shiftsQueries.map((q) => q.data); - const notesDataArray = notesQueries.map((q) => q.data); - const externalSyncsDataArray = externalSyncsQueries.map((q) => q.data); - const presetsDataArray = presetsQueries.map((q) => q.data); - const presetsLoadingArray = presetsQueries.map((q) => q.isLoading); + const shiftsDataArray = useMemo( + () => shiftsQueries.map((q) => q.data), + [shiftsQueries] + ); + // Apply similar pattern to other arraysAlternatively, combine the map-building logic into single
useMemocalls that depend directly on the query results.
182-215: Good mutation structure, but consider adding optimistic updates.The mutations correctly invalidate cache on success. However, given the PR objective mentions optimistic updates, consider implementing them here for a better UX during shift toggle operations. Based on learnings, optimistic updates should use temporary IDs and rollback on error.
hooks/useExternalSync.ts (2)
61-76: Non-null assertion on potentially null value.The
selectedCalendar!assertion is safe here becauseenabled: !!selectedCalendarprevents execution when null. However, this pattern is fragile - if someone removes theenabledcheck, it would cause runtime issues.♻️ Cleaner pattern to avoid non-null assertions
const { data: externalSyncs = [], isLoading: syncsLoading } = useQuery({ - queryKey: queryKeys.externalSyncs.byCalendar(selectedCalendar!), - queryFn: () => fetchExternalSyncsApi(selectedCalendar!), + queryKey: queryKeys.externalSyncs.byCalendar(selectedCalendar ?? ""), + queryFn: () => fetchExternalSyncsApi(selectedCalendar!), enabled: !!selectedCalendar, refetchInterval: 5000, refetchIntervalInBackground: true, });Or extract the calendar ID check into the queryFn with an early return.
65-76: Consider disabling background polling for resource efficiency.
refetchIntervalInBackground: truecontinues polling even when the browser tab is hidden. For sync status that doesn't require immediate attention when the user isn't looking, consider removing this flag to reduce server load.app/api/activity-logs/route.ts (1)
65-68: Redundant severity filtering for audit logs.Severity is filtered at the database level for audit logs (line 67) and again post-merge (line 172). While this works correctly, the post-merge filter is only necessary for sync logs (which derive severity from status). Consider skipping the post-merge filter when the type isn't "sync":
♻️ Suggested optimization
// 3. Apply severity filter after merging (sync logs derive severity from status) // ============================================ let filteredLogs = allLogs; - if (severity) { + // Only apply post-merge severity filter if sync logs are included + // (audit logs are already filtered at DB level) + if (severity && (!type || type === "sync")) { filteredLogs = filteredLogs.filter((log) => log.severity === severity); }Also applies to: 170-173
hooks/useNotes.ts (2)
89-102: Consider removing unnecessary body from DELETE request.The DELETE request sends an empty JSON body which is unusual. While not incorrect, DELETE requests typically don't require a body.
🔧 Suggested simplification
async function deleteNoteApi(id: string): Promise<void> { const response = await fetch(`/api/notes/${id}`, { method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({}), }); if (!response.ok) { const errorText = await response.text(); throw new Error( `Failed to delete note: ${response.status} ${response.statusText} - ${errorText}` ); } }
104-115: Consider consolidating identical context types.The three context interfaces are identical. While this works, consider using a single type alias to reduce duplication.
🔧 Optional consolidation
-// Context types for optimistic updates -interface CreateNoteContext { - previous: CalendarNote[] | undefined; -} - -interface UpdateNoteContext { - previous: CalendarNote[] | undefined; -} - -interface DeleteNoteContext { - previous: CalendarNote[] | undefined; -} +// Context type for optimistic updates +interface MutationContext { + previous: CalendarNote[] | undefined; +}app/admin/users/page.tsx (1)
55-55: Consider pagination for scalability.The
limit: 1000effectively disables pagination, fetching all users at once. For admin panels with many users, this could impact performance. Consider implementing proper pagination UI or at least documenting this as a known limitation.components/admin/audit-log-delete-dialog.tsx (1)
17-34: Avoid redundant polling in the dialog.
useAdminAuditLogs()performs a polling query every 5s; instantiating it here can trigger extra network traffic (and potentially a different default filter/pagination) even though the dialog only needs delete mutations. Consider passing delete handlers from the parent or exposing an action‑only hook (e.g.,useAdminAuditLogActions) that surfaces mutation pending state.hooks/useAdminCalendars.ts (3)
89-94: Unused parametertinfetchCalendarsApi.The
tparameter is accepted but never used, with an eslint-disable comment suppressing the warning. Either remove the parameter or utilize it for error message translation if that was the intent.Suggested fix
async function fetchCalendarsApi( filters: CalendarFilters | undefined, - sort: CalendarSort | undefined, - // eslint-disable-next-line `@typescript-eslint/no-unused-vars` - t: ReturnType<typeof useTranslations> + sort: CalendarSort | undefined ): Promise<CalendarsListResponse> {And update the caller at line 292:
- queryFn: () => fetchCalendarsApi(filters, sort, t), + queryFn: () => fetchCalendarsApi(filters, sort),
341-344: Inconsistent query key usage in cache invalidation.
onSettleduses a hardcoded["admin", "calendars"]array alongsidequeryKeys.admin.stats. For consistency and maintainability, consider using the query key factory throughout, or use a broader invalidation pattern consistently across all mutations.The hardcoded array does work as a prefix match for invalidation, but mixing patterns reduces clarity.
358-369:orphanedCountnot updated in delete optimistic update.When deleting a calendar, the optimistic update decrements
totalbut doesn't adjustorphanedCount. If the deleted calendar was orphaned, the count will be momentarily incorrect until the query refetches.Suggested fix
queryClient.setQueryData( queryKeys.admin.calendars({ filters, sort }), (old: CalendarsListResponse | undefined) => { if (!old) return old; + const deletedCalendar = old.calendars.find((cal) => cal.id === calendarId); + const wasOrphaned = deletedCalendar && !deletedCalendar.ownerId; return { ...old, calendars: old.calendars.filter((cal) => cal.id !== calendarId), total: old.total - 1, + orphanedCount: wasOrphaned ? old.orphanedCount - 1 : old.orphanedCount, }; } );hooks/usePresets.ts (1)
67-78: DELETE request with empty JSON body is unusual.The
deletePresetApisends a DELETE request withbody: JSON.stringify({}). While not incorrect, DELETE requests typically don't include a body. Consider removing the body if the API doesn't require it.Suggested fix
async function deletePresetApi(presetId: string): Promise<void> { const response = await fetch(`/api/presets/${presetId}`, { method: "DELETE", - headers: { "Content-Type": "application/json" }, - body: JSON.stringify({}), });app/profile/activity/page.tsx (1)
97-103: Unnecessarylimitin useMemo dependency array.
limitis a constant (line 65) and will never change. Including it in the dependency array is harmless but unnecessary.const pagination = useMemo( () => ({ limit, offset: page * limit, }), - [page, limit] + [page] );
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/external-sync-manage-sheet.tsx (1)
52-66: Remove unusedsyncErrorRefreshTriggerprop from interface.The prop
syncErrorRefreshTrigger?: numberis still defined in the interface but is no longer used (as noted in the comment). Dead props in interfaces can cause confusion for consumers.🧹 Remove dead prop
interface ExternalSyncManageSheetProps { open: boolean; onOpenChange: (open: boolean) => void; calendarId: string | null; onSyncComplete?: () => void; - syncErrorRefreshTrigger?: number; } export function ExternalSyncManageSheet({ open, onOpenChange, calendarId, onSyncComplete, -}: // syncErrorRefreshTrigger is no longer needed - React Query polls automatically -ExternalSyncManageSheetProps) { +}: ExternalSyncManageSheetProps) {
🤖 Fix all issues with AI agents
In `@hooks/useAdminStats.ts`:
- Around line 122-125: The toast.error call inside the render path (the if
(error) { toast.error(error.message); } in useAdminStats.ts) causes repeated
notifications; move this logic into a side effect or the query's error callback
so it only runs when the error state changes—for example, remove the direct
toast call from the component body and either add a useEffect that watches the
error variable and fires toast.error(error.message) when error becomes non-null
(with appropriate dependency array and deduping) or pass an onError handler to
the underlying useQuery/fetch hook to show the toast once per error occurrence.
♻️ Duplicate comments (2)
hooks/useAdminAuditLogs.ts (1)
60-95: Localize fetch errors and drop the unused‑var suppression.Hardcoded errors bypass i18n and the
targ is currently unused.
As per coding guidelines, localize all user‑facing strings.🌐 Suggested i18n update
async function fetchAuditLogsApi( filters: AuditLogFilters, sort: AuditLogSort, pagination: AuditLogPagination, - // eslint-disable-next-line `@typescript-eslint/no-unused-vars` t: ReturnType<typeof useTranslations> ): Promise<AuditLogsResponse> { @@ if (!response.ok) { if (response.status === 403) { - throw new Error("Access denied"); + throw new Error(t("admin.accessDenied")); } - throw new Error("Failed to fetch audit logs"); + throw new Error(t("common.fetchError", { item: t("admin.auditLogs") })); }hooks/useActivityLogs.ts (1)
46-101: Localize activity‑log messages (fetch/clear/success).Hardcoded strings bypass i18n; please use translation keys instead.
As per coding guidelines, all user‑facing strings should go throught().🌐 Suggested i18n update (adjust keys to match your locale files)
async function fetchActivityLogsApi( filters: ActivityLogsFilters, - pagination: ActivityLogsPagination + pagination: ActivityLogsPagination, + t: ReturnType<typeof useTranslations> ): Promise<ActivityLogsResponse> { @@ if (!response.ok) { - throw new Error("Failed to fetch activity logs"); + throw new Error( + t("common.fetchError", { item: t("activityLogs.title") }) + ); } @@ async function clearLogsApi( t: ReturnType<typeof useTranslations> ): Promise<void> { @@ if (!response.ok) { - throw new Error(t("common.deleteError", { item: "Activity logs" })); + throw new Error( + t("common.deleteError", { item: t("activityLogs.title") }) + ); } } @@ } = useQuery({ queryKey: queryKeys.activityLogs({ filters, pagination }), - queryFn: () => fetchActivityLogsApi(filters, pagination), + queryFn: () => fetchActivityLogsApi(filters, pagination, t), refetchInterval: REFETCH_INTERVAL, refetchIntervalInBackground: true, // Continue polling in background }); @@ onSuccess: () => { - toast.success("Activity logs cleared"); + toast.success(t("activityLogs.cleared")); queryClient.invalidateQueries({ queryKey: ["activity-logs"] }); }, onError: (err) => { toast.error( - err instanceof Error ? err.message : "Failed to clear activity logs" + err instanceof Error + ? err.message + : t("common.deleteError", { item: t("activityLogs.title") }) ); }, });Also applies to: 140-150
🧹 Nitpick comments (4)
hooks/useExternalSync.ts (1)
12-23: Consider extracting error details from API response.The error handling discards potentially useful error information from the response body. Consider parsing the error response for more informative messages.
💡 Suggested improvement
async function fetchExternalSyncsApi( calendarId: string ): Promise<ExternalSync[]> { const params = new URLSearchParams({ calendarId }); const response = await fetch(`/api/external-syncs?${params}`); if (!response.ok) { - throw new Error("Failed to fetch external syncs"); + const data = await response.json().catch(() => ({})); + throw new Error(data.error || "Failed to fetch external syncs"); } return await response.json(); }hooks/useAdminCalendars.ts (2)
342-345: Inconsistent query key usage in cache invalidation.
onSettleduses a hardcoded array["admin", "calendars"]instead ofqueryKeys.admin.calendars(...). While this works for prefix-based invalidation, it's inconsistent with the typed query keys used elsewhere and could drift if the key structure changes.♻️ Use queryKeys for consistency
onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["admin", "calendars"] }); + queryClient.invalidateQueries({ + queryKey: queryKeys.admin.calendars({}), + exact: false + }); queryClient.invalidateQueries({ queryKey: queryKeys.admin.stats }); },Apply similar changes to
deleteMutation,transferMutation,bulkDeleteMutation, andbulkTransferMutationat lines 387, 412, 460, and 487.
359-370: Optimistic update doesn't adjustorphanedCount.When deleting a calendar, if the deleted calendar was orphaned (
owner === null),orphanedCountshould also be decremented for accurate optimistic UI. Currently it remains unchanged until the query refetches.💡 Optional: More precise optimistic update
queryClient.setQueryData( queryKeys.admin.calendars({ filters, sort }), (old: CalendarsListResponse | undefined) => { if (!old) return old; + const deletedCal = old.calendars.find((cal) => cal.id === calendarId); + const wasOrphaned = deletedCal && deletedCal.ownerId === null; return { ...old, calendars: old.calendars.filter((cal) => cal.id !== calendarId), total: old.total - 1, + orphanedCount: wasOrphaned ? old.orphanedCount - 1 : old.orphanedCount, }; } );components/external-sync-manage-sheet.tsx (1)
100-129: Query fetching is duplicated fromuseExternalSynchook.This component duplicates the query logic from
hooks/useExternalSync.ts. Consider whetheruseExternalSynccould be extended to exposesyncLogsdirectly, avoiding the duplication.However, the current approach is acceptable since:
- The component needs direct access to
syncLogsDatafor error computation- The
enabled: !!calendarId && opencondition is specific to this dialog
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/copilot-instructions.mdcomponents/external-sync-manage-sheet.tsxcomponents/sync-notification-dialog.tsxhooks/useActivityLogs.tshooks/useAdminAuditLogs.tshooks/useAdminCalendars.tshooks/useAdminStats.tshooks/useAdminUsers.tshooks/useExternalSync.tslib/query-client.ts
🧰 Additional context used
📓 Path-based instructions (5)
components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Components in
/componentsdirectory should be"use client"by default for interactive UI, and should use shadcn/ui patterns with Radix UI primitives and Lucide icons.
Files:
components/external-sync-manage-sheet.tsxcomponents/sync-notification-dialog.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Store and display dates asYYYY-MM-DDstrings (local dates without timezone conversion). UseformatDateToLocal()fromlib/date-utils.tsbefore saving to database anddate-fnswith locale fromgetDateLocale()for display.
Use Drizzle-inferred types from schema directly (noanytypes). TypeScript must remain in strict mode with proper type safety throughout the application.
All React files must be in strict mode with TypeScript 5. Noanytypes allowed. Import inferred types from Drizzle schema.
UseformatDateToLocal(date)utility fromlib/date-utils.tsto convert dates toYYYY-MM-DDformat before saving to database.
Files:
components/external-sync-manage-sheet.tsxlib/query-client.tshooks/useExternalSync.tshooks/useAdminUsers.tshooks/useActivityLogs.tscomponents/sync-notification-dialog.tsxhooks/useAdminStats.tshooks/useAdminAuditLogs.tshooks/useAdminCalendars.ts
components/**/*sheet*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the Sheet pattern (
components/ui/base-sheet.tsx) for all forms with side panels, including unsaved changes confirmation viauseDirtyStatehook and consistent header/footer styling with gradient backgrounds.
Files:
components/external-sync-manage-sheet.tsx
hooks/use*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement data fetching hooks following the pattern:
useShifts,usePresets,useNotes,useCalendarswith accompanying action hooks likeuseShiftActions,useNoteActionsfor CRUD operations with optimistic updates.
Files:
hooks/useExternalSync.tshooks/useAdminUsers.tshooks/useActivityLogs.tshooks/useAdminStats.tshooks/useAdminAuditLogs.tshooks/useAdminCalendars.ts
components/**/*dialog*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the Dialog pattern (
components/ui/dialog.tsxfrom shadcn/ui) for list views, confirmations, and read-only displays.
Files:
components/sync-notification-dialog.tsx
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*sheet*.{tsx,ts} : Use the Sheet pattern (`components/ui/base-sheet.tsx`) for all forms with side panels, including unsaved changes confirmation via `useDirtyState` hook and consistent header/footer styling with gradient backgrounds.
Applied to files:
components/external-sync-manage-sheet.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Applied to files:
components/external-sync-manage-sheet.tsxhooks/useExternalSync.tshooks/useAdminUsers.ts.github/copilot-instructions.mdcomponents/sync-notification-dialog.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Applied to files:
components/external-sync-manage-sheet.tsxhooks/useExternalSync.tshooks/useAdminUsers.ts.github/copilot-instructions.mdhooks/useActivityLogs.tscomponents/sync-notification-dialog.tsxhooks/useAdminStats.tshooks/useAdminAuditLogs.tshooks/useAdminCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Respect `shift.syncedFromExternal` flag - synced shifts from external calendars cannot be edited or deleted by users. Check this flag before allowing shift mutations.
Applied to files:
components/external-sync-manage-sheet.tsxhooks/useExternalSync.ts.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useCalendarPermission(calendarId)` hook to check UI permissions, which returns `canEdit`, `canDelete`, `canShare`, and `permission` level for displaying conditional UI elements.
Applied to files:
components/external-sync-manage-sheet.tsxhooks/useExternalSync.ts.github/copilot-instructions.mdcomponents/sync-notification-dialog.tsxhooks/useAdminAuditLogs.tshooks/useAdminCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/auth/**/*.ts : For Better Auth integration, use built-in methods for authentication operations (`login`, `logout`, `session management`, `OAuth`). Use custom code in `lib/auth/admin.ts` only for permission checks (`canEditUser`, `canDeleteUser`, `canBanUser`).
Applied to files:
hooks/useAdminUsers.tshooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*Actions.{ts,tsx} : Implement optimistic updates in action hooks by: generating temp ID (`temp-${Date.now()}`), adding item to state immediately, making API call, removing temp item on error with toast, replacing temp item on success.
Applied to files:
hooks/useAdminUsers.ts.github/copilot-instructions.mdhooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : For auth-related configuration checks, use the convenience hook `useAuthFeatures()` from `hooks/useAuthFeatures.ts` to access `isAuthEnabled`, `allowRegistration`, `allowGuest`, and `providers` on the client side.
Applied to files:
hooks/useAdminUsers.ts.github/copilot-instructions.mdhooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
Applied to files:
.github/copilot-instructions.mdcomponents/sync-notification-dialog.tsxhooks/useAdminCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to lib/db/schema.ts : Database schema modifications must be made in `lib/db/schema.ts`. After changes, generate migrations with `npm run db:generate`, review SQL in `drizzle/` folder, then apply with `npm run db:migrate`.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/**/*.{tsx,ts} : All components under `app/` are Server Components by default unless marked with `"use client"`. Use `"use client"` directive only for interactive UI components.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use Better Auth's built-in client methods (`authClient.listSessions()`, `authClient.revokeOtherSessions()`) for client-side session management. Never implement custom session revocation logic.
Applied to files:
.github/copilot-instructions.mdhooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*.{tsx,ts} : Components in `/components` directory should be `"use client"` by default for interactive UI, and should use shadcn/ui patterns with Radix UI primitives and Lucide icons.
Applied to files:
.github/copilot-instructions.mdcomponents/sync-notification-dialog.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : All API routes must include `GET/POST/PUT/PATCH/DELETE` exports in `app/api/**` directory and follow the pattern: check permissions → perform database operation → emit SSE event.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: All user-facing strings must use the `t()` function from `next-intl` for translations. Never use hardcoded English strings in components or API responses.
Applied to files:
.github/copilot-instructions.mdhooks/useActivityLogs.tshooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : Use `getSessionUser(request.headers)` from `lib/auth/sessions.ts` to get current user in API routes. Use Better Auth's `authClient` methods on the client side for session management.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Use `getPublicConfig()` function from `lib/public-config.ts` in Server Components to access configuration. Use `usePublicConfig()` hook from `hooks/usePublicConfig.ts` in Client Components. Never use `process.env.NEXT_PUBLIC_*` directly.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : All React files must be in strict mode with TypeScript 5. No `any` types allowed. Import inferred types from Drizzle schema.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Use Drizzle-inferred types from schema directly (no `any` types). TypeScript must remain in strict mode with proper type safety throughout the application.
Applied to files:
.github/copilot-instructions.md
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*dialog*.{tsx,ts} : Use the Dialog pattern (`components/ui/dialog.tsx` from shadcn/ui) for list views, confirmations, and read-only displays.
Applied to files:
components/sync-notification-dialog.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : For calendar permission hierarchy in SQL queries and permission checks, follow: `owner` (highest) > `admin` > `write` > `read` (lowest). Use `getUserAccessibleCalendars(userId)` to get all accessible calendars.
Applied to files:
hooks/useAdminCalendars.ts
🧬 Code graph analysis (3)
hooks/useActivityLogs.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
components/sync-notification-dialog.tsx (3)
lib/db/schema.ts (1)
SyncLog(409-409)lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useAdminStats.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
⏰ 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: build-dev
🔇 Additional comments (18)
lib/query-client.ts (1)
1-22: Well-structured React Query configuration.The configuration is sound:
staleTime(3s) <refetchInterval(5s) ensures data is considered stale before the next poll cycle- Exponential backoff with a 30s cap is appropriate for retry resilience
gcTime(5min) provides reasonable cache retentionhooks/useExternalSync.ts (1)
58-102: Clean React Query migration with proper cache management.The hook correctly:
- Guards queries with
enabled: !!selectedCalendar- Uses centralized query keys for cache consistency
- Provides a unified
refetchvia query invalidation- Computes derived state (
hasSyncErrors) withuseMemoThe non-null assertions on
selectedCalendar!inqueryKeyandqueryFnare safe due to theenabledguard.hooks/useAdminStats.ts (1)
55-86: API function properly parses dates and handles errors.Good pattern: converting timestamp strings to
Dateobjects at the data boundary ensures type consistency throughout the application.hooks/useAdminCalendars.ts (1)
277-295: Well-structured React Query migration with comprehensive mutation support.The hook provides:
- Polling with centralized
REFETCH_INTERVAL- Optimistic updates for immediate UI feedback
- Proper rollback on mutation errors
- Toast notifications for user feedback
- Clean API surface with boolean return values for mutation wrappers
components/external-sync-manage-sheet.tsx (2)
152-162: Clean invalidation helper with proper memoization.The
invalidateSyncscallback correctly:
- Guards against null
calendarId- Invalidates both syncs and logs queries
- Is properly memoized with
useCallback
164-183: Good cleanup on dialog close.The
useEffectproperly resets all form state when the dialog closes, preventing stale state from persisting between open/close cycles. The dependency array correctly includes onlyopen.components/sync-notification-dialog.tsx (5)
3-23: Query setup and log fetch helper look solid.The polling is scoped to dialog open state and the helper keeps the fetch logic centralized.
Also applies to: 35-75
77-103: Delete mutation flow looks good.Success/error toasts and cache invalidation are consistent and clear.
104-131: Mark‑as‑read mutation is consistent with the delete flow.
163-170: Handlers are straightforward and clean.
214-224: Good UX guardrails on action buttons.hooks/useAdminUsers.ts (5)
81-181: API wrappers and date parsing are well-structured.
183-317: Mutation API helpers are concise and consistent.
340-359: Query setup and polling configuration look good.
361-589: Optimistic mutations and cache invalidation are cleanly implemented.
591-669: Public hook API is clear and easy to consume..github/copilot-instructions.md (1)
14-271: Docs align well with the new polling/query‑key architecture.Also applies to: 393-405, 448-459
hooks/useAdminAuditLogs.ts (1)
97-313: Hook structure, polling, and optimistic deletes look solid.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/calendar-share-management-sheet.tsx (1)
57-71: Ensure optimistic state always resets (even on failure).
IfupdateCalendarthrows,savingremainstrueand the optimistic value never clears. Wrap the mutation intry/finallyand consider ignoring changes while a save is in flight.🔧 Proposed fix
const handleGuestPermissionChange = async ( value: "none" | "read" | "write" ) => { - if (!canManageShares) return; - - setOptimisticGuestPermission(value); - setSaving(true); - - await updateCalendar(calendarId, { - guestPermission: value, - }); - - setSaving(false); - setOptimisticGuestPermission(null); + if (!canManageShares || saving) return; + + setOptimisticGuestPermission(value); + setSaving(true); + try { + await updateCalendar(calendarId, { + guestPermission: value, + }); + } finally { + setSaving(false); + setOptimisticGuestPermission(null); + } };
🤖 Fix all issues with AI agents
In `@hooks/useActivityLogs.ts`:
- Around line 91-101: Replace hardcoded user-facing strings with localized ones:
in clearLogsApi use the translation function t for the item name instead of the
literal "Activity logs" (e.g., t("activityLogs.name") or pass
t("activityLogs.name") into t("common.deleteError", { item: ... })), and
likewise replace the literal "Failed to clear activity logs" used elsewhere
(lines 147-150) with an appropriate t(...) key (create a new key like
activityLogs.clearError if none exists). Update both clearLogsApi and the other
clear-logs function to use t(...) consistently for all displayed error text.
In `@hooks/useCalendars.ts`:
- Around line 177-183: onError currently always logs and shows a generic toast
even for RateLimitError, causing duplicate notifications; inside the onError
handler in hooks/useCalendars.ts (the function handling the mutation error),
after performing the optimistic rollback via
queryClient.setQueryData(queryKeys.calendars.all, context.previous), detect if
err is a RateLimitError (e.g., if (err instanceof RateLimitError) or using your
app's RateLimitError type check) and if so skip the console.error and
toast.error and simply return; otherwise proceed to console.error("Failed to
create calendar:", err) and toast.error(...) as before; apply the same change to
the other onError block around lines 298-307.
In `@hooks/useCalendarSubscriptions.ts`:
- Around line 106-114: The docstring claiming "Automatic polling every 5
seconds" is out of sync with the useQuery call; update useQuery in
useCalendarSubscriptions (the call that uses queryKey
queryKeys.subscriptions.all and queryFn fetchSubscriptionsApi) to include
refetchInterval: 5000 to enable polling, or alternatively remove/update the
docstring to not mention polling—pick one approach and make the change so
behavior and documentation match.
In `@hooks/useCalendarTokens.ts`:
- Around line 154-167: In useCalendarTokens, add the explicit refetchInterval
option so the useQuery call uses refetchInterval: REFETCH_INTERVAL (same
constant other polling hooks use) to match the JSDoc and other hooks; import
REFETCH_INTERVAL from the same module used elsewhere (e.g., lib/constants or
where REFETCH_INTERVAL is defined) at the top, and include refetchInterval:
REFETCH_INTERVAL inside the useQuery config object alongside queryKey, queryFn,
and enabled in the useCalendarTokens function.
In `@hooks/useShifts.ts`:
- Around line 121-134: The optimisticShift fallback color is set to "#000000"
but your DB/schema default is "#3b82f6", causing a visual flash; update the
optimisticShift creation (the object named optimisticShift in useShifts.ts) to
use the schema default color "#3b82f6" instead of "#000000" (i.e., change color:
formData.color || "#000000" to color: formData.color || "#3b82f6") so the
optimistic UI matches the server default.
- Around line 63-76: In deleteShiftApi, remove the unnecessary empty request
body and Content-Type header from the fetch call used for the DELETE; update the
fetch options to only include method: "DELETE" (keep the existing response.ok
check and error throwing logic intact) so the DELETE request matches other calls
like useCalendarSubscriptions and avoids sending an empty JSON payload.
In `@hooks/useShiftStats.ts`:
- Around line 37-41: The code builds URLSearchParams using
currentDate.toISOString(), which causes UTC/time component drift and cache
churn; replace uses of currentDate.toISOString() with the local date string
returned by formatDateToLocal(currentDate) for both the cache/query key and the
params construction (e.g., where params is created with new URLSearchParams and
any other occurrences around useShiftStats functions at the other location
noted) so the "date" param and the stats key use YYYY‑MM‑DD local dates
consistently.
♻️ Duplicate comments (3)
hooks/useCalendarSubscriptions.ts (2)
236-246: Stale closure issue addressed using cache lookup.The
onSuccesshandler now correctly usesqueryClient.getQueryDatato fetch fresh cache data instead of relying on a staleavailableCalendarsclosure. The lookup checks both arrays since the optimistic update moves the calendar todismissed.
258-258: Type mismatch issue addressed.The error is now correctly returned as
errorMessage?.message(string) instead of theErrorobject, matching the expectedstring | nullreturn type.app/page.tsx (1)
675-676: Remove no‑oponPresetsChangeprops by making it optional.There are still empty handlers passed into
DialogManager. Consider making the prop optional and dropping these no‑ops to keep the API clean and consistent with React Query invalidation.Also applies to: 763-764
🧹 Nitpick comments (8)
components/admin/admin-header.tsx (1)
131-142: Approve with minor simplification suggestion.The connection status indicator works correctly. Consider flattening the nested div structure—the
titleattribute can be applied directly to the indicator element:🔧 Optional: Flatten nested divs
{/* Connection Status Indicator */} - <div - title={isOnline ? t("sync.connected") : t("sync.disconnected")} - > - <div - className={`w-2.5 h-2.5 rounded-full transition-all ${ - isOnline - ? "bg-green-500 animate-pulse shadow-[0_0_8px_rgba(34,197,94,0.6)]" - : "bg-red-500 shadow-[0_0_8px_rgba(239,68,68,0.6)]" - }`} - ></div> - </div> + <div + className={`w-2.5 h-2.5 rounded-full transition-all ${ + isOnline + ? "bg-green-500 animate-pulse shadow-[0_0_8px_rgba(34,197,94,0.6)]" + : "bg-red-500 shadow-[0_0_8px_rgba(239,68,68,0.6)]" + }`} + title={isOnline ? t("sync.connected") : t("sync.disconnected")} + aria-label={isOnline ? t("sync.connected") : t("sync.disconnected")} + />Adding
aria-labelimproves accessibility for screen readers, as the visual indicator has no text content.hooks/useCalendarSubscriptions.ts (1)
270-281: Consider removing unused_calendarNameparameter or using it for consistency.The
dismissfunction accepts_calendarNamebut ignores it (with an eslint-disable). While the current implementation works by looking up the name from the cache, this creates an inconsistent API compared tosubscribewhich actually uses the name.Either remove the parameter if callers don't need it, or pass it through mutation variables like
subscribedoes for symmetry.♻️ Option: Use the name parameter like subscribe
const dismissMutation = useMutation({ - mutationFn: dismissApi, - onMutate: async (calendarId) => { + mutationFn: (variables: { calendarId: string; name: string }) => + dismissApi(variables.calendarId), + onMutate: async (variables) => { + const { calendarId } = variables; // ... rest of onMutate }, - onSuccess: (_, calendarId) => { - const data = queryClient.getQueryData<SubscriptionResponse>( - queryKeys.subscriptions.all - ); - const calendar = - data?.available.find((cal) => cal.id === calendarId) || - data?.dismissed.find((cal) => cal.id === calendarId); + onSuccess: (_, variables) => { toast.success( - t("calendar.unsubscribeSuccess", { name: calendar?.name || "Calendar" }) + t("calendar.unsubscribeSuccess", { name: variables.name || "Calendar" }) ); },Then update the wrapper:
dismiss: async ( calendarId: string, - // eslint-disable-next-line `@typescript-eslint/no-unused-vars` - _calendarName: string + calendarName: string ): Promise<boolean> => { try { - await dismissMutation.mutateAsync(calendarId); + await dismissMutation.mutateAsync({ calendarId, name: calendarName }); return true;hooks/useShiftActions.ts (2)
30-49: Consider if duplicate error logging is intentional.Both
handleShiftSubmitandhandleDeleteShiftlog errors, but the underlyingcreateShiftanddeleteShiftfromuseShiftsalready log errors and show toast notifications. This results in duplicateconsole.erroroutput.If the additional logging is intentional for debugging call-site context, consider adding distinguishing information. Otherwise, the try/catch can simply re-throw or be removed.
8-21: Remove unusedonStatsRefreshprop.The
onStatsRefreshprop is defined in the interface but never destructured or used in the hook.♻️ Remove unused prop
interface UseShiftActionsProps { shifts: ShiftWithCalendar[]; presets: ShiftPreset[]; createShift: (data: ShiftFormData) => Promise<ShiftWithCalendar>; deleteShift: (id: string) => Promise<void>; - onStatsRefresh?: () => void; }hooks/useCalendarTokens.ts (1)
56-66: Consider usingparseErrorResponseinfetchTokensApifor consistent error messaging.
Right now this path always throws a generic HTTP status even when the API returns a useful error body.🔧 Suggested tweak
if (!response.ok) { - throw new Error(`HTTP ${response.status}`); + const errorMessage = await parseErrorResponse(response); + throw new Error(errorMessage); }hooks/useAdminAuditLogs.ts (1)
240-243: Use query key factory for cache invalidation consistency.The invalidation uses a hardcoded
["admin", "audit-logs"]array while the query registration usesqueryKeys.admin.auditLogs(). If the key structure inlib/query-keys.tschanges, this invalidation could silently break.♻️ Suggested refactor using the query key factory
onSettled: () => { - queryClient.invalidateQueries({ queryKey: ["admin", "audit-logs"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.admin.auditLogs() }); queryClient.invalidateQueries({ queryKey: queryKeys.admin.stats }); },Apply the same change on line 260.
app/admin/logs/page.tsx (2)
209-216: Consider memoizinghandleDebouncedSearchChangefor safer debounce behavior.The
handleDebouncedSearchChangefunction is not in the dependency array, which could cause stale closure issues if the function body changes in future refactors. Since the function only calls stable state setters, it works correctly today.♻️ Optional: Wrap handler with useCallback for explicit stability
+ const handleDebouncedSearchChange = useCallback((value: string) => { + setDebouncedSearch(value); + setPage(0); + setSelectedLogIds([]); + setExpandedRows(new Set()); + }, []); - const handleDebouncedSearchChange = (value: string) => { - setDebouncedSearch(value); - setPage(0); - setSelectedLogIds([]); - setExpandedRows(new Set()); - }; // Debounce search query useEffect(() => { const timer = setTimeout(() => { handleDebouncedSearchChange(searchQuery); }, 500); return () => clearTimeout(timer); - }, [searchQuery]); + }, [searchQuery, handleDebouncedSearchChange]);
578-582: Redundant Date conversion.The
useAdminAuditLogshook already convertslog.timestampto aDateobject (seehooks/useAdminAuditLogs.tsline 101). Wrapping it again withnew Date()is unnecessary.♻️ Simplified date formatting
<TableCell className="whitespace-nowrap"> - {format(new Date(log.timestamp), "PPp", { + {format(log.timestamp, "PPp", { locale: dateLocale, })} </TableCell>
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
app/admin/logs/page.tsxapp/page.tsxapp/profile/activity/page.tsxcomponents/admin/admin-header.tsxcomponents/app-header.tsxcomponents/auth-header.tsxcomponents/calendar-share-management-sheet.tsxcomponents/dialog-manager.tsxcomponents/preset-selector.tsxhooks/useActivityLogs.tshooks/useAdminAuditLogs.tshooks/useCalendarSubscriptions.tshooks/useCalendarTokens.tshooks/useCalendars.tshooks/useShiftActions.tshooks/useShiftForm.tshooks/useShiftStats.tshooks/useShifts.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- hooks/useShiftForm.ts
- components/preset-selector.tsx
- components/app-header.tsx
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Store and display dates asYYYY-MM-DDstrings (local dates without timezone conversion). UseformatDateToLocal()fromlib/date-utils.tsbefore saving to database anddate-fnswith locale fromgetDateLocale()for display.
Use Drizzle-inferred types from schema directly (noanytypes). TypeScript must remain in strict mode with proper type safety throughout the application.
All React files must be in strict mode with TypeScript 5. Noanytypes allowed. Import inferred types from Drizzle schema.
UseformatDateToLocal(date)utility fromlib/date-utils.tsto convert dates toYYYY-MM-DDformat before saving to database.
Files:
hooks/useCalendarTokens.tshooks/useCalendarSubscriptions.tsapp/admin/logs/page.tsxhooks/useAdminAuditLogs.tshooks/useCalendars.tshooks/useShiftActions.tshooks/useActivityLogs.tscomponents/admin/admin-header.tsxhooks/useShifts.tscomponents/calendar-share-management-sheet.tsxcomponents/auth-header.tsxhooks/useShiftStats.tscomponents/dialog-manager.tsxapp/profile/activity/page.tsxapp/page.tsx
hooks/use*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement data fetching hooks following the pattern:
useShifts,usePresets,useNotes,useCalendarswith accompanying action hooks likeuseShiftActions,useNoteActionsfor CRUD operations with optimistic updates.
Files:
hooks/useCalendarTokens.tshooks/useCalendarSubscriptions.tshooks/useAdminAuditLogs.tshooks/useCalendars.tshooks/useShiftActions.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useShiftStats.ts
app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All components under
app/are Server Components by default unless marked with"use client". Use"use client"directive only for interactive UI components.
Files:
app/admin/logs/page.tsxapp/profile/activity/page.tsxapp/page.tsx
hooks/use*Actions.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement optimistic updates in action hooks by: generating temp ID (
temp-${Date.now()}), adding item to state immediately, making API call, removing temp item on error with toast, replacing temp item on success.
Files:
hooks/useShiftActions.ts
components/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Components in
/componentsdirectory should be"use client"by default for interactive UI, and should use shadcn/ui patterns with Radix UI primitives and Lucide icons.
Files:
components/admin/admin-header.tsxcomponents/calendar-share-management-sheet.tsxcomponents/auth-header.tsxcomponents/dialog-manager.tsx
components/**/*sheet*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the Sheet pattern (
components/ui/base-sheet.tsx) for all forms with side panels, including unsaved changes confirmation viauseDirtyStatehook and consistent header/footer styling with gradient backgrounds.
Files:
components/calendar-share-management-sheet.tsx
components/**/*dialog*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use the Dialog pattern (
components/ui/dialog.tsxfrom shadcn/ui) for list views, confirmations, and read-only displays.
Files:
components/dialog-manager.tsx
🧠 Learnings (22)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useCalendarPermission(calendarId)` hook to check UI permissions, which returns `canEdit`, `canDelete`, `canShare`, and `permission` level for displaying conditional UI elements.
Applied to files:
hooks/useCalendarTokens.tshooks/useCalendarSubscriptions.tshooks/useAdminAuditLogs.tshooks/useCalendars.tshooks/useShiftActions.tshooks/useShifts.tscomponents/calendar-share-management-sheet.tsxcomponents/auth-header.tsxhooks/useShiftStats.tscomponents/dialog-manager.tsxapp/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Applied to files:
hooks/useCalendarTokens.tshooks/useCalendarSubscriptions.tsapp/admin/logs/page.tsxhooks/useAdminAuditLogs.tshooks/useCalendars.tshooks/useShiftActions.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useShiftStats.tsapp/profile/activity/page.tsxapp/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : Use `getUserCalendarPermission(userId, calendarId)` from `lib/auth/permissions.ts` to retrieve the exact permission level, which returns permission string or `null` if user has no access.
Applied to files:
hooks/useCalendarTokens.tshooks/useShifts.tscomponents/calendar-share-management-sheet.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Always check permissions using `checkPermission(userId, calendarId, requiredLevel)` function from `lib/auth/permissions.ts` in both API routes and UI components before allowing mutations.
Applied to files:
hooks/useCalendarTokens.tshooks/useCalendars.tshooks/useShifts.tscomponents/calendar-share-management-sheet.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use Better Auth's built-in client methods (`authClient.listSessions()`, `authClient.revokeOtherSessions()`) for client-side session management. Never implement custom session revocation logic.
Applied to files:
hooks/useCalendarTokens.tshooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : For calendar permission hierarchy in SQL queries and permission checks, follow: `owner` (highest) > `admin` > `write` > `read` (lowest). Use `getUserAccessibleCalendars(userId)` to get all accessible calendars.
Applied to files:
hooks/useCalendarTokens.tshooks/useCalendars.tshooks/useShifts.tscomponents/calendar-share-management-sheet.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : For auth-related configuration checks, use the convenience hook `useAuthFeatures()` from `hooks/useAuthFeatures.ts` to access `isAuthEnabled`, `allowRegistration`, `allowGuest`, and `providers` on the client side.
Applied to files:
hooks/useCalendarTokens.tsapp/admin/logs/page.tsxhooks/useAdminAuditLogs.tscomponents/calendar-share-management-sheet.tsxcomponents/auth-header.tsxapp/profile/activity/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Applied to files:
hooks/useCalendarSubscriptions.tshooks/useCalendars.tshooks/useActivityLogs.tscomponents/admin/admin-header.tsxhooks/useShifts.tscomponents/auth-header.tsxhooks/useShiftStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*.{tsx,ts} : Components in `/components` directory should be `"use client"` by default for interactive UI, and should use shadcn/ui patterns with Radix UI primitives and Lucide icons.
Applied to files:
app/admin/logs/page.tsxapp/profile/activity/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*Actions.{ts,tsx} : Implement optimistic updates in action hooks by: generating temp ID (`temp-${Date.now()}`), adding item to state immediately, making API call, removing temp item on error with toast, replacing temp item on success.
Applied to files:
app/admin/logs/page.tsxhooks/useAdminAuditLogs.tshooks/useShiftActions.tshooks/useShifts.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: All user-facing strings must use the `t()` function from `next-intl` for translations. Never use hardcoded English strings in components or API responses.
Applied to files:
hooks/useAdminAuditLogs.tshooks/useActivityLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/auth/**/*.ts : For Better Auth integration, use built-in methods for authentication operations (`login`, `logout`, `session management`, `OAuth`). Use custom code in `lib/auth/admin.ts` only for permission checks (`canEditUser`, `canDeleteUser`, `canBanUser`).
Applied to files:
hooks/useAdminAuditLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
Applied to files:
hooks/useCalendars.tsapp/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Respect `shift.syncedFromExternal` flag - synced shifts from external calendars cannot be edited or deleted by users. Check this flag before allowing shift mutations.
Applied to files:
hooks/useShiftActions.tshooks/useShifts.tscomponents/calendar-share-management-sheet.tsxcomponents/dialog-manager.tsxapp/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*sheet*.{tsx,ts} : Use the Sheet pattern (`components/ui/base-sheet.tsx`) for all forms with side panels, including unsaved changes confirmation via `useDirtyState` hook and consistent header/footer styling with gradient backgrounds.
Applied to files:
hooks/useShiftActions.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: When auth is enabled, unauthenticated users (`userId = null`) can access calendars with `guestPermission != "none"`. Use `allowGuestAccess()` function to check if guest access feature is enabled.
Applied to files:
components/calendar-share-management-sheet.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to components/**/*dialog*.{tsx,ts} : Use the Dialog pattern (`components/ui/dialog.tsx` from shadcn/ui) for list views, confirmations, and read-only displays.
Applied to files:
components/dialog-manager.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Store and display dates as `YYYY-MM-DD` strings (local dates without timezone conversion). Use `formatDateToLocal()` from `lib/date-utils.ts` before saving to database and `date-fns` with locale from `getDateLocale()` for display.
Applied to files:
app/profile/activity/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `getDateLocale(locale)` from `lib/date-utils.ts` when formatting dates with `date-fns` to ensure proper locale-aware date display.
Applied to files:
app/profile/activity/page.tsxapp/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Use `formatDateToLocal(date)` utility from `lib/date-utils.ts` to convert dates to `YYYY-MM-DD` format before saving to database.
Applied to files:
app/profile/activity/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to lib/db/schema.ts : Database schema modifications must be made in `lib/db/schema.ts`. After changes, generate migrations with `npm run db:generate`, review SQL in `drizzle/` folder, then apply with `npm run db:migrate`.
Applied to files:
app/page.tsx
🧬 Code graph analysis (11)
hooks/useCalendarTokens.ts (1)
lib/query-keys.ts (1)
queryKeys(10-61)
hooks/useCalendarSubscriptions.ts (1)
lib/query-keys.ts (1)
queryKeys(10-61)
app/admin/logs/page.tsx (1)
hooks/useAdminAuditLogs.ts (1)
useAdminAuditLogs(178-312)
hooks/useAdminAuditLogs.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useCalendars.ts (3)
lib/types.ts (1)
CalendarWithCount(4-18)lib/db/schema.ts (1)
calendars(100-124)lib/query-keys.ts (1)
queryKeys(10-61)
hooks/useShiftActions.ts (3)
components/shift-sheet.tsx (1)
ShiftFormData(25-34)lib/types.ts (1)
ShiftWithCalendar(20-40)lib/db/schema.ts (1)
shifts(216-249)
hooks/useActivityLogs.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
components/admin/admin-header.tsx (1)
hooks/useConnectionStatus.ts (1)
useConnectionStatus(16-69)
hooks/useShifts.ts (4)
lib/types.ts (1)
ShiftWithCalendar(20-40)components/shift-sheet.tsx (1)
ShiftFormData(25-34)lib/query-keys.ts (1)
queryKeys(10-61)lib/date-utils.ts (1)
parseLocalDate(22-77)
components/auth-header.tsx (1)
hooks/useConnectionStatus.ts (1)
useConnectionStatus(16-69)
hooks/useShiftStats.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
⏰ 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: build-dev
🔇 Additional comments (23)
components/auth-header.tsx (1)
11-11: LGTM! Connection status indicator integrates well.The integration of
useConnectionStatushook and the visual indicator is clean. The absolute positioning within the logo area works well for this header's layout.Note: The indicator styling differs slightly from
admin-header.tsx(which includes glow shadows). If visual consistency across headers is desired, consider extracting a sharedConnectionStatusIndicatorcomponent. This is optional since the different sizes and positioning contexts may justify the variation.Based on learnings, this correctly replaces the previous
useSSEConnectionpattern with the new React Query-based connection status.Also applies to: 26-26, 48-56
components/dialog-manager.tsx (1)
25-25: No changes needed. The optionalonPresetsChangeprop is properly handled throughout the component chain with appropriate type definitions and guarded invocations.Likely an incorrect or invalid review comment.
components/calendar-share-management-sheet.tsx (1)
44-51: Clean optimistic/prop derivation.
Good simplification; avoids effect-based syncing and keeps a stable source of truth.hooks/useCalendarSubscriptions.ts (4)
1-6: LGTM!Imports are clean and appropriate for the React Query migration.
8-39: LGTM!Type definitions are well-structured with proper discriminated fields for calendar sources and subscription states.
44-84: LGTM!API helpers are clean and properly throw errors for failed responses. The error handling correctly extracts server-provided error messages.
119-193: LGTM! Stale closure issue addressed.The mutation now correctly passes
calendarIdandnamethrough variables, andonSuccessusesvariables.namedirectly, avoiding the stale closure issue flagged in previous reviews.hooks/useShifts.ts (4)
1-25: LGTM!The imports and
normalizeShifthelper are well-structured. The date parsing correctly prioritizes theYYYY-MM-DDformat usingparseLocalDateas per coding guidelines.
91-100: LGTM!The query is properly guarded with
enabled: !!calendarId, preventing execution whencalendarIdis undefined. The non-null assertions are safe in this context.
166-204: LGTM!The delete mutation correctly implements optimistic updates with rollback, following the same robust pattern as the create mutation.
206-240: LGTM! Previous review concern addressed.All returned helpers now properly guard against
undefinedcalendarId, preventing invalid cache operations and failed requests. The implementation correctly throws descriptive errors for mutations while silently returning for read-only operations.hooks/useShiftActions.ts (2)
52-112: LGTM!The toggle logic correctly handles the shift add/remove behavior with proper locking via
togglingDatesto prevent rapid duplicate operations. The delegation touseShiftsmutations ensures toast notifications are shown to users.
114-120: LGTM!The hook returns a clean API surface with the essential handlers for shift operations, aligning with the established action hooks pattern.
app/profile/activity/page.tsx (1)
63-152: Filter + debounce wiring looks solid.Memoized filters/pagination with page resets and a debounced search flow are consistent and avoid stale closures. Nice cleanup of the local UI state.
hooks/useAdminAuditLogs.ts (4)
1-56: LGTM!Type definitions are well-structured with proper nullability handling. The interfaces align with the coding guidelines for strict TypeScript usage.
57-110: LGTM!The fetch function properly handles error localization with
t("admin.accessDenied")andt("common.fetchError", ...), addressing the previous review feedback. Date parsing and response mapping are correct.
112-157: LGTM!Delete API functions have consistent error handling with proper i18n support, matching the pattern established in the fetch function.
265-312: LGTM!The hook's public API is clean and follows the project's data fetching hook patterns. The boolean return values from mutation wrappers work well since errors are already displayed via toast notifications.
app/admin/logs/page.tsx (5)
77-113: LGTM!The filter, sort, and pagination objects are properly memoized, preventing unnecessary re-renders and query refetches. The hook integration is clean and follows React Query best practices.
122-171: LGTM!Helper functions are properly memoized with empty dependency arrays, ensuring stable references across renders.
173-207: LGTM!All filter change handlers properly reset
page,selectedLogIds, andexpandedRows, preventing bulk actions from targeting stale rows after filter changes.
256-266: LGTM!The
clearFiltersfunction now immediately clearsdebouncedSearchviahandleDebouncedSearchChange(""), preventing the stale debounced value issue identified in the previous review.
713-722: LGTM!The dialog success handler correctly relies on React Query's automatic cache invalidation instead of manual refetching. The comment clarifies the intended behavior.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
app/page.tsx (1)
404-448: Improve consistency by using string-based date comparison for all shift matching.The code computes
dateKeyas a local date string but doesn't use it for shift matching, instead relying onisSameDay()with Date objects. For consistency with the toggle-state tracking logic (which correctly usesdateKey), refactor the shift filtering and matching to compare dates as strings:Suggested fix
- const dayShifts = shifts.filter( - (shift) => shift.date && isSameDay(shift.date as Date, targetDate) - ); + const dayShifts = shifts.filter((shift) => { + if (!shift.date) return false; + const shiftDateKey = + typeof shift.date === "string" + ? shift.date + : formatDateToLocal(shift.date); + return shiftDateKey === dateKey; + }); // ... - const existingShift = shifts.find( - (shift) => - shift.date && - isSameDay(shift.date as Date, targetDate) && - shift.title === preset.title && - shift.startTime === preset.startTime && - shift.endTime === preset.endTime - ); + const existingShift = shifts.find((shift) => { + if (!shift.date) return false; + const shiftDateKey = + typeof shift.date === "string" + ? shift.date + : formatDateToLocal(shift.date); + return ( + shiftDateKey === dateKey && + shift.title === preset.title && + shift.startTime === preset.startTime && + shift.endTime === preset.endTime + ); + });This aligns all date comparisons with the
YYYY-MM-DDstring pattern already used for toggle-state tracking and ensures consistent handling throughout the compare-mode logic.
🤖 Fix all issues with AI agents
In `@hooks/useActivityLogs.ts`:
- Around line 46-70: The thrown error in fetchActivityLogsApi uses a hardcoded
string; replace it with a localized message by calling the translation function
(e.g., t('activityLogs.fetchError')) instead of "Failed to fetch activity logs",
ensure the file either imports/uses the i18n translator instance or
accepts/receives t as needed, and add the translation key
"activityLogs.fetchError" to your locale files; apply the same replacement for
the other fetch error(s) in this module (the other fetch call that throws on
!response.ok).
In `@hooks/useCalendars.ts`:
- Around line 61-95: The updateCalendarApi and deleteCalendarApi functions lack
the same rate-limit handling as createCalendarApi, so either remove the dead
check in deleteMutation.onError or add consistent 429 handling; update both
updateCalendarApi and deleteCalendarApi to detect a 429 response, read the
Retry-After header (or parse response body if createCalendarApi does), and throw
the same RateLimitError type used by createCalendarApi (including any retry
timestamp/seconds) so deleteMutation.onError can reliably catch RateLimitError;
locate symbols updateCalendarApi, deleteCalendarApi, createCalendarApi and
deleteMutation.onError to implement the change.
In `@hooks/useShifts.ts`:
- Around line 119-132: The optimisticShift created in the add shift flow
(variable optimisticShift of type ShiftWithCalendar) is missing the presetId,
causing preset-dependent UI to flicker; update the optimisticShift object to
include presetId: formData.presetId (or null/undefined when not provided) so the
optimistic payload matches the server shape and UI renders correctly while the
refetch completes.
🧹 Nitpick comments (7)
hooks/useCalendarTokens.ts (1)
57-67: Consider usingparseErrorResponsefor consistency.Other API functions (
createTokenApi,updateTokenApi,deleteTokenApi) use theparseErrorResponsehelper, butfetchTokensApithrows a generic HTTP error. For consistency and better error messaging when fetches fail:♻️ Suggested change
async function fetchTokensApi( calendarId: string ): Promise<CalendarAccessToken[]> { const response = await fetch(`/api/calendars/${calendarId}/tokens`); if (!response.ok) { - throw new Error(`HTTP ${response.status}`); + const errorMessage = await parseErrorResponse(response); + throw new Error(errorMessage); } return await response.json(); }hooks/useCalendars.ts (2)
231-241: Consider addingRateLimitErrorcheck for consistency.If rate-limit handling is added to
updateCalendarApi(as suggested above), theonErrorhandler should also skip the toast forRateLimitErrorto matchcreateMutation's behavior.
320-335: Consider consistent error handling across wrappers.
createCalendarcatches errors and handlesRateLimitErrorspecifically, whileupdateCalendaranddeleteCalendarlet errors propagate to the caller. This is an asymmetric pattern.If rate-limit handling is added to the update/delete API functions, these wrappers should follow the same pattern as
createCalendar:♻️ Proposed consistency fix
const updateCalendar = async ( calendarId: string, updates: { name?: string; color?: string; guestPermission?: "none" | "read" | "write"; } ) => { - return updateMutation.mutateAsync({ calendarId, updates }); + try { + await updateMutation.mutateAsync({ calendarId, updates }); + } catch (error) { + if (error instanceof RateLimitError) { + await handleRateLimitError(error.response, t); + } + } }; const deleteCalendar = async (calendarId: string) => { - return deleteMutation.mutateAsync(calendarId); + try { + await deleteMutation.mutateAsync(calendarId); + } catch (error) { + if (error instanceof RateLimitError) { + await handleRateLimitError(error.response, t); + } + } };hooks/useCalendarSubscriptions.ts (2)
67-70: Minor: JSON parsing could fail on non-JSON error responses.If the server returns a non-JSON error response (e.g., 500 with HTML),
response.json()will throw before you can extract the error message.♻️ Optional defensive improvement
if (!response.ok) { - const error = await response.json(); - throw new Error(error.error || "Failed to subscribe"); + let message = "Failed to subscribe"; + try { + const error = await response.json(); + message = error.error || message; + } catch { + // Response wasn't JSON, use default message + } + throw new Error(message); }
272-276: The_calendarNameparameter is used by callers and should not be removed without updating all call sites.The
dismissmethod is called with two arguments incomponents/calendar-discovery-sheet.tsx(line 83):await dismiss(calendar.id, calendar.name). Removing the parameter would break this caller.For consistency with the
subscribemethod (which accepts and usescalendarName), consider using the_calendarNameparameter in the dismiss mutation for optimistic updates or other UI feedback, or document why the signature requires it despite not using the value internally.hooks/useShifts.ts (1)
204-231: Consider moving mutations into auseShiftActionshook.
KeepinguseShiftsfocused on reads aligns with the existing hook pattern and keeps concerns separated. As per coding guidelines, ...hooks/useShiftStats.ts (1)
33-50: Consider passing React Query’s AbortSignal intofetch.
This enables request cancellation on unmount/refetch and avoids wasted work.♻️ Suggested refactor
-async function fetchShiftStatsApi( - calendarId: string, - period: string, - currentDate: Date -): Promise<ShiftStatsData> { +async function fetchShiftStatsApi( + calendarId: string, + period: string, + currentDate: Date, + signal?: AbortSignal +): Promise<ShiftStatsData> { const params = new URLSearchParams({ calendarId, period, date: formatDateToLocal(currentDate), }); - const response = await fetch(`/api/shifts/stats?${params}`); + const response = await fetch(`/api/shifts/stats?${params}`, { signal }); @@ - queryFn: () => fetchShiftStatsApi(calendarId!, period, currentDate), + queryFn: ({ signal }) => + fetchShiftStatsApi(calendarId!, period, currentDate, signal),Also applies to: 78-86
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
app/page.tsxhooks/useActivityLogs.tshooks/useAdminStats.tshooks/useCalendarSubscriptions.tshooks/useCalendarTokens.tshooks/useCalendars.tshooks/useShiftStats.tshooks/useShifts.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Store and display dates asYYYY-MM-DDstrings (local dates without timezone conversion). UseformatDateToLocal()fromlib/date-utils.tsbefore saving to database anddate-fnswith locale fromgetDateLocale()for display.
Use Drizzle-inferred types from schema directly (noanytypes). TypeScript must remain in strict mode with proper type safety throughout the application.
All React files must be in strict mode with TypeScript 5. Noanytypes allowed. Import inferred types from Drizzle schema.
UseformatDateToLocal(date)utility fromlib/date-utils.tsto convert dates toYYYY-MM-DDformat before saving to database.
Files:
hooks/useActivityLogs.tsapp/page.tsxhooks/useCalendars.tshooks/useShifts.tshooks/useCalendarTokens.tshooks/useShiftStats.tshooks/useCalendarSubscriptions.tshooks/useAdminStats.ts
hooks/use*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement data fetching hooks following the pattern:
useShifts,usePresets,useNotes,useCalendarswith accompanying action hooks likeuseShiftActions,useNoteActionsfor CRUD operations with optimistic updates.
Files:
hooks/useActivityLogs.tshooks/useCalendars.tshooks/useShifts.tshooks/useCalendarTokens.tshooks/useShiftStats.tshooks/useCalendarSubscriptions.tshooks/useAdminStats.ts
app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
All components under
app/are Server Components by default unless marked with"use client". Use"use client"directive only for interactive UI components.
Files:
app/page.tsx
🧠 Learnings (16)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Applied to files:
hooks/useActivityLogs.tsapp/page.tsxhooks/useCalendars.tshooks/useShifts.tshooks/useCalendarTokens.tshooks/useShiftStats.tshooks/useCalendarSubscriptions.tshooks/useAdminStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Applied to files:
hooks/useActivityLogs.tshooks/useCalendars.tshooks/useShifts.tshooks/useShiftStats.tshooks/useCalendarSubscriptions.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: All user-facing strings must use the `t()` function from `next-intl` for translations. Never use hardcoded English strings in components or API responses.
Applied to files:
hooks/useActivityLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
Applied to files:
app/page.tsxhooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useCalendarPermission(calendarId)` hook to check UI permissions, which returns `canEdit`, `canDelete`, `canShare`, and `permission` level for displaying conditional UI elements.
Applied to files:
app/page.tsxhooks/useCalendars.tshooks/useShifts.tshooks/useCalendarTokens.tshooks/useShiftStats.tshooks/useCalendarSubscriptions.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to lib/db/schema.ts : Database schema modifications must be made in `lib/db/schema.ts`. After changes, generate migrations with `npm run db:generate`, review SQL in `drizzle/` folder, then apply with `npm run db:migrate`.
Applied to files:
app/page.tsx
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : For calendar permission hierarchy in SQL queries and permission checks, follow: `owner` (highest) > `admin` > `write` > `read` (lowest). Use `getUserAccessibleCalendars(userId)` to get all accessible calendars.
Applied to files:
app/page.tsxhooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `getDateLocale(locale)` from `lib/date-utils.ts` when formatting dates with `date-fns` to ensure proper locale-aware date display.
Applied to files:
app/page.tsxhooks/useShiftStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Respect `shift.syncedFromExternal` flag - synced shifts from external calendars cannot be edited or deleted by users. Check this flag before allowing shift mutations.
Applied to files:
app/page.tsxhooks/useShifts.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Always check permissions using `checkPermission(userId, calendarId, requiredLevel)` function from `lib/auth/permissions.ts` in both API routes and UI components before allowing mutations.
Applied to files:
hooks/useCalendars.tshooks/useShifts.tshooks/useCalendarTokens.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*Actions.{ts,tsx} : Implement optimistic updates in action hooks by: generating temp ID (`temp-${Date.now()}`), adding item to state immediately, making API call, removing temp item on error with toast, replacing temp item on success.
Applied to files:
hooks/useShifts.tshooks/useAdminStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : Use `getUserCalendarPermission(userId, calendarId)` from `lib/auth/permissions.ts` to retrieve the exact permission level, which returns permission string or `null` if user has no access.
Applied to files:
hooks/useShifts.tshooks/useCalendarTokens.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use Better Auth's built-in client methods (`authClient.listSessions()`, `authClient.revokeOtherSessions()`) for client-side session management. Never implement custom session revocation logic.
Applied to files:
hooks/useCalendarTokens.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Store and display dates as `YYYY-MM-DD` strings (local dates without timezone conversion). Use `formatDateToLocal()` from `lib/date-utils.ts` before saving to database and `date-fns` with locale from `getDateLocale()` for display.
Applied to files:
hooks/useShiftStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Use `formatDateToLocal(date)` utility from `lib/date-utils.ts` to convert dates to `YYYY-MM-DD` format before saving to database.
Applied to files:
hooks/useShiftStats.ts
🧬 Code graph analysis (6)
hooks/useActivityLogs.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useCalendars.ts (3)
lib/types.ts (1)
CalendarWithCount(4-18)lib/db/schema.ts (1)
calendars(100-124)lib/query-keys.ts (1)
queryKeys(10-61)
hooks/useShifts.ts (4)
lib/types.ts (1)
ShiftWithCalendar(20-40)components/shift-sheet.tsx (1)
ShiftFormData(25-34)lib/query-keys.ts (1)
queryKeys(10-61)lib/date-utils.ts (1)
parseLocalDate(22-77)
hooks/useCalendarTokens.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useShiftStats.ts (3)
lib/date-utils.ts (1)
formatDateToLocal(5-10)lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useAdminStats.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
⏰ 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: build-dev
🔇 Additional comments (35)
hooks/useCalendarTokens.ts (8)
1-8: LGTM!Imports are well-organized and include the centralized
queryKeysandREFETCH_INTERVALas recommended. This addresses the previous review feedback about explicit refetch interval configuration.
30-52: LGTM!The
parseErrorResponsehelper properly handles non-JSON responses with graceful fallbacks. The multi-layer try/catch with HTML detection (line 45) is a good defensive pattern.
69-134: LGTM!API wrapper functions follow REST conventions with proper error handling using
parseErrorResponse. The implementation is consistent and type-safe.
159-169: LGTM!The
useQuerysetup is correct. The non-null assertions (calendarId!) are safe becauseenabled: !!calendarIdprevents query execution when the ID is null. The explicitrefetchInterval: REFETCH_INTERVALaddresses the previous review feedback and aligns with other polling hooks in the codebase.
191-238: Well-implemented optimistic updates.The
updateTokenMutationcorrectly implements the React Query optimistic update pattern:
- Cancels in-flight queries to prevent overwrites
- Stores previous state for rollback
- Applies optimistic cache update
- Rolls back on error with proper toast notification
- Invalidates cache on
onSettledfor eventual consistency
240-279: LGTM!The
deleteTokenMutationmirrors the update pattern with proper optimistic removal and rollback. The invalidation ononSettledensures the list is refreshed after the server confirms deletion.
313-343: LGTM!The wrapper functions provide proper null guards before invoking mutations, preventing runtime errors from the non-null assertions in mutation functions. The empty catch blocks are acceptable since the mutation's
onErroralready handles user feedback via toast.
281-307: LGTM!The share link helpers are properly memoized with
useCallbackand include appropriate user feedback via toasts.hooks/useAdminStats.ts (2)
56-86: Clean fetch + timestamp normalization.
The localized error handling and timestamp conversion are solid and keep the data shape consistent for consumers.
112-131: Nice fix: deduped error toasts.
The useEffect + ref guard prevents repeated toasts during polling/re-renders.hooks/useCalendars.ts (6)
1-11: LGTM!Imports are correctly structured for the React Query migration, utilizing centralized query keys and the standard React Query hooks.
97-109: LGTM!Context types are well-defined for optimistic update rollbacks, with
DeleteCalendarContextcorrectly capturing both cache and selection state.
121-129: Missing polling configuration.The PR objective is to switch to React Query polling, but no
refetchIntervalis configured on this query. Without it, data won't automatically refresh and users will only see updates on manual refetch or component remount.Consider adding polling if real-time-ish updates are expected:
} = useQuery({ queryKey: queryKeys.calendars.all, queryFn: fetchCalendarsApi, + refetchInterval: 30_000, // Poll every 30 seconds + staleTime: 10_000, // Consider data fresh for 10 seconds });Please verify whether polling is intentionally omitted here (relying solely on cache invalidation from mutations) or if a
refetchIntervalshould be added for background refreshes.
141-196: LGTM!The
createMutationcorrectly implements the optimistic update pattern with proper rollback, duplicate toast prevention for rate-limit errors, and cache invalidation on settle.
250-304: LGTM with caveat.The
deleteMutationcorrectly implements optimistic delete with selection state rollback. Note the deadRateLimitErrorcheck at lines 292-294 (addressed in earlier comment about API function consistency).
337-349: LGTM!The return object exposes all necessary values and follows the established hook pattern. Using
invalidateQueriesforrefetchCalendarsis the idiomatic React Query approach.hooks/useCalendarSubscriptions.ts (6)
1-7: LGTM!Clean import structure with proper React Query hooks and centralized query configuration from
lib/query-keysandlib/query-client.
107-119: Well-structured query setup.The query configuration properly includes
refetchInterval: REFETCH_INTERVALfor polling, addressing the documentation claim. Default values foravailableCalendarsanddismissedCalendarsensure safe destructuring when data is undefined.
121-195: Solid subscribe mutation with proper optimistic updates.The mutation correctly:
- Uses variables object to pass
namealongsidecalendarId, avoiding the stale closure issue- Handles both scenarios: moving calendar from dismissed to available, or marking existing available calendar as subscribed
- Restores previous state on error
- Invalidates both subscription and calendar caches on settlement
238-248: Stale closure issue properly addressed.Using
queryClient.getQueryDataat runtime resolves the previous concern about stale closures. The lookup correctly searches bothavailableanddismissedarrays, which handles the case where the optimistic update has already moved the calendar to the dismissed list.
260-260: Error type conversion properly implemented.The return now correctly converts
Errortostring | null, addressing the previous type mismatch concern.
284-286: Clean refetch implementation.Using
invalidateQueriesis the correct React Query pattern for triggering a manual refresh while leveraging the existing query configuration.hooks/useShifts.ts (5)
28-40: Solid fetch helper with normalization.
Clear error surfacing and normalization keep callers consistent.
42-61: Create helper error reporting looks solid.
Including response text should make failures easier to diagnose.
63-74: DELETE helper is clean and consistent.
The minimal request shape is easy to reason about.
89-98: CalendarId gating prevents premature fetches.
Nice guard to avoid calls before the ID is available.
164-202: Delete flow rollback/invalidation is consistent.
Looks well-scoped and should keep cache state stable on errors.hooks/useShiftStats.ts (1)
1-6: LGTM on the React Query/date-utils wiring.
Clean imports for query keys, polling interval, and local-date formatting.app/page.tsx (7)
12-20: LGTM on the compare-mode/query imports.
The additions align with the React Query migration and compare-mode data flow.
93-106: Compare-mode state + data wiring looks solid.
The toggling state anduseCompareDatagating are clean and readable.Also applies to: 114-116
147-160: Nice: invalidating compare-mode notes after mutations.
Keeps the compare views consistent after note add/update/delete flows.Also applies to: 187-190
225-230: Good reset of compare toggling state on exit.
Avoids stale spinner state when leaving compare mode.Also applies to: 379-383
248-265: LGTM on calendar delete + sync invalidation flow.
Closing the dialog and invalidating calendar/shift caches is consistent.
494-541: LGTM on compare‑mode note lookups.
Using the compare notes map keeps per‑calendar note interactions scoped correctly.
591-663: Compare view wiring and cache invalidation look consistent.
Passing the compare maps + per‑calendar invalidation should keep the UI responsive.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
… and update query function signature
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@hooks/useCalendars.ts`:
- Around line 241-251: The updateMutation.onError handler should match
createMutation/deleteMutation by not showing a toast for rate-limit failures:
inside updateMutation.onError (the function that calls queryClient.setQueryData
and toast.error) add a guard that checks if err is an instance of RateLimitError
and if so return early (or at least skip the console.error/toast.error) so the
previous cache restore still happens but no generic error toast is shown; import
or reference the RateLimitError type the other mutations use and place the check
before the console.error/toast.error calls.
- Around line 131-149: The useQuery call for calendars omits an explicit
refetchInterval; import REFETCH_INTERVAL from lib/query-client and add
refetchInterval: REFETCH_INTERVAL to the useQuery options (the call using
queryKey: queryKeys.calendars.all and queryFn: fetchCalendarsApi). Keep the
existing behavior (default data, isLoading, isFetched) and do not change the
useEffect logic that auto-selects via initialCalendarIdRef and
setSelectedCalendar.
- Around line 330-345: The updateCalendar and deleteCalendar wrappers currently
return updateMutation.mutateAsync and deleteMutation.mutateAsync directly and
don't catch RateLimitError like createCalendar does; modify both updateCalendar
and deleteCalendar to wrap the mutateAsync call in a try/catch that catches
errors instanceof RateLimitError and calls handleRateLimitError(error) before
rethrowing (or returning), otherwise rethrow the error so existing onError
handlers still run; reference updateCalendar, deleteCalendar, RateLimitError,
handleRateLimitError, and createCalendar to mirror the error-handling pattern
used there.
In `@hooks/useShifts.ts`:
- Around line 227-232: The deleteShift helper currently calls
deleteMutation.mutateAsync without checking if the shift is externally synced;
update deleteShift to first read the shift from the local cache (e.g., via the
query client / cache used in this hook) using the shiftId and verify the shift's
syncedFromExternal flag, and if true throw an error or return a rejected promise
to block the delete; only call deleteMutation.mutateAsync(shiftId) when
calendarId exists and syncedFromExternal is falsy. Ensure you reference the
existing symbols deleteShift, deleteMutation, calendarId and the
syncedFromExternal property when implementing the guard so synced shifts remain
read-only.
🧹 Nitpick comments (3)
hooks/useActivityLogs.ts (2)
135-140: ESLint disable warrants attention.The
tfunction is used insidequeryFnbut excluded from the query key dependencies. Whiletis typically stable within a locale session, if the user's locale changes at runtime, the query won't automatically refetch with updated translations. If locale changes trigger a full remount, this is fine; otherwise, consider whether this edge case matters for your app.
147-147: Use the centralized query key factory for invalidation.Line 136 uses
queryKeys.activityLogs({ filters, pagination }), but line 147 uses the hardcoded["activity-logs"]. While prefix-based invalidation works, using the factory is more consistent and maintainable.♻️ Suggested fix
- queryClient.invalidateQueries({ queryKey: ["activity-logs"] }); + queryClient.invalidateQueries({ queryKey: queryKeys.activityLogs() });hooks/useShiftStats.ts (1)
33-42: Reuse a single date key for query + request to avoid drift.Computing the formatted date once avoids any mismatch if
currentDatemutates between render and fetch, and eliminates duplicate formatting.♻️ Suggested refactor
-async function fetchShiftStatsApi( - calendarId: string, - period: string, - currentDate: Date -): Promise<ShiftStatsData> { - const params = new URLSearchParams({ - calendarId, - period, - date: formatDateToLocal(currentDate), - }); +async function fetchShiftStatsApi( + calendarId: string, + period: string, + dateKey: string +): Promise<ShiftStatsData> { + const params = new URLSearchParams({ + calendarId, + period, + date: dateKey, + }); @@ export function useShiftStats({ calendarId, currentDate, period, }: UseShiftStatsOptions) { + const dateKey = formatDateToLocal(currentDate); const { data: stats = null, isLoading: loading, refetch, } = useQuery({ queryKey: queryKeys.stats.shifts( calendarId!, period, - formatDateToLocal(currentDate) + dateKey ), - queryFn: () => fetchShiftStatsApi(calendarId!, period, currentDate), + queryFn: () => fetchShiftStatsApi(calendarId!, period, dateKey), enabled: !!calendarId, refetchInterval: REFETCH_INTERVAL, });Please verify this aligns with your React Query v5 query-key and request parameter conventions.
Also applies to: 74-93
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
hooks/useActivityLogs.tshooks/useCalendars.tshooks/useShiftStats.tshooks/useShifts.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Store and display dates asYYYY-MM-DDstrings (local dates without timezone conversion). UseformatDateToLocal()fromlib/date-utils.tsbefore saving to database anddate-fnswith locale fromgetDateLocale()for display.
Use Drizzle-inferred types from schema directly (noanytypes). TypeScript must remain in strict mode with proper type safety throughout the application.
All React files must be in strict mode with TypeScript 5. Noanytypes allowed. Import inferred types from Drizzle schema.
UseformatDateToLocal(date)utility fromlib/date-utils.tsto convert dates toYYYY-MM-DDformat before saving to database.
Files:
hooks/useShiftStats.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useCalendars.ts
hooks/use*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Implement data fetching hooks following the pattern:
useShifts,usePresets,useNotes,useCalendarswith accompanying action hooks likeuseShiftActions,useNoteActionsfor CRUD operations with optimistic updates.
Files:
hooks/useShiftStats.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useCalendars.ts
🧠 Learnings (14)
📓 Common learnings
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*.{ts,tsx} : Implement data fetching hooks following the pattern: `useShifts`, `usePresets`, `useNotes`, `useCalendars` with accompanying action hooks like `useShiftActions`, `useNoteActions` for CRUD operations with optimistic updates.
Applied to files:
hooks/useShiftStats.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useSSEConnection` hook from `hooks/useSSEConnection.ts` for real-time sync connection management. The hook automatically handles reconnection, visibility changes, and triggers data refreshes.
Applied to files:
hooks/useShiftStats.tshooks/useActivityLogs.tshooks/useShifts.tshooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Store and display dates as `YYYY-MM-DD` strings (local dates without timezone conversion). Use `formatDateToLocal()` from `lib/date-utils.ts` before saving to database and `date-fns` with locale from `getDateLocale()` for display.
Applied to files:
hooks/useShiftStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `getDateLocale(locale)` from `lib/date-utils.ts` when formatting dates with `date-fns` to ensure proper locale-aware date display.
Applied to files:
hooks/useShiftStats.tshooks/useActivityLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{ts,tsx} : Use `formatDateToLocal(date)` utility from `lib/date-utils.ts` to convert dates to `YYYY-MM-DD` format before saving to database.
Applied to files:
hooks/useShiftStats.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to **/*.{tsx} : Use `useCalendarPermission(calendarId)` hook to check UI permissions, which returns `canEdit`, `canDelete`, `canShare`, and `permission` level for displaying conditional UI elements.
Applied to files:
hooks/useShiftStats.tshooks/useShifts.tshooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to hooks/use*Actions.{ts,tsx} : Implement optimistic updates in action hooks by: generating temp ID (`temp-${Date.now()}`), adding item to state immediately, making API call, removing temp item on error with toast, replacing temp item on success.
Applied to files:
hooks/useActivityLogs.tshooks/useShifts.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: All user-facing strings must use the `t()` function from `next-intl` for translations. Never use hardcoded English strings in components or API responses.
Applied to files:
hooks/useActivityLogs.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Respect `shift.syncedFromExternal` flag - synced shifts from external calendars cannot be edited or deleted by users. Check this flag before allowing shift mutations.
Applied to files:
hooks/useShifts.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Always check permissions using `checkPermission(userId, calendarId, requiredLevel)` function from `lib/auth/permissions.ts` in both API routes and UI components before allowing mutations.
Applied to files:
hooks/useShifts.tshooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : Use `getUserCalendarPermission(userId, calendarId)` from `lib/auth/permissions.ts` to retrieve the exact permission level, which returns permission string or `null` if user has no access.
Applied to files:
hooks/useShifts.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : After any database mutation (create/update/delete), emit an SSE event using `eventEmitter.emit('calendar-change', {...})` with appropriate type and action to keep clients in sync.
Applied to files:
hooks/useCalendars.ts
📚 Learning: 2026-01-03T02:03:48.622Z
Learnt from: CR
Repo: panteLx/BetterShift PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2026-01-03T02:03:48.622Z
Learning: Applies to app/api/**/*.ts : For calendar permission hierarchy in SQL queries and permission checks, follow: `owner` (highest) > `admin` > `write` > `read` (lowest). Use `getUserAccessibleCalendars(userId)` to get all accessible calendars.
Applied to files:
hooks/useCalendars.ts
🧬 Code graph analysis (4)
hooks/useShiftStats.ts (3)
lib/date-utils.ts (1)
formatDateToLocal(5-10)lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useActivityLogs.ts (2)
lib/query-keys.ts (1)
queryKeys(10-61)lib/query-client.ts (1)
REFETCH_INTERVAL(3-3)
hooks/useShifts.ts (4)
lib/types.ts (1)
ShiftWithCalendar(20-40)components/shift-sheet.tsx (1)
ShiftFormData(25-34)lib/query-keys.ts (1)
queryKeys(10-61)lib/date-utils.ts (1)
parseLocalDate(22-77)
hooks/useCalendars.ts (3)
lib/types.ts (1)
CalendarWithCount(4-18)lib/db/schema.ts (1)
calendars(100-124)lib/query-keys.ts (1)
queryKeys(10-61)
⏰ 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: build-dev
🔇 Additional comments (16)
hooks/useActivityLogs.ts (4)
12-41: LGTM on type definitions.The interfaces are well-structured with strict typing. Using
stringfor date fields aligns with the coding guidelines for date handling.
46-87: LGTM with a minor observation on date parsing.The fetch function properly uses i18n for error messages and avoids
anytypes by usingRecord<string, unknown>. The date parsing at line 78 assumestimestampis always a valid date string from the API—this is reasonable given the API contract.
92-102: LGTM!Error message is properly localized using the translation function.
143-156: Mutation and clearLogs wrapper look good.The mutation properly handles success with a localized toast and invalidates the cache. The
clearLogswrapper correctly returns a boolean for consumer convenience while the mutation'sonErrorhandles user feedback.Also applies to: 169-176
hooks/useCalendars.ts (6)
1-11: LGTM!Imports are well-organized and use proper Drizzle-inferred types from
@/lib/typesas per coding guidelines.
12-22: LGTM!The fetch function correctly handles errors and defensively ensures array return type. While other API functions include rate-limit checks, omitting it for this read-only GET endpoint is reasonable since rate limits typically target mutations.
24-59: LGTM!
RateLimitErrorclass preserves the response for extracting retry information, andcreateCalendarApicorrectly checks for rate-limits before other error handling. This addresses the previous review feedback.
61-105: LGTM!Rate-limit handling is now consistent across all mutation API functions, addressing the previous review feedback. The error parsing approaches (JSON for update, text for delete) are appropriate for each endpoint's response format.
151-206: LGTM!The create mutation correctly implements optimistic updates with proper rollback, and the
RateLimitErrorcheck inonErrorprevents duplicate notifications. TheonSettledinvalidation ensures cache consistency regardless of success/failure.
347-358: LGTM!The return object exposes a clean API with all necessary data, state flags, and mutation wrappers. Using
invalidateQueriesforrefetchCalendarsis the correct React Query pattern for triggering a refetch.hooks/useShifts.ts (5)
1-7: Imports look good for the React Query migration.
27-74: API helpers are solid and consistent.
85-99: Query enablement is correct.
100-163: Optimistic create flow is consistent.
205-226: Return helpers are safely guarded.Also applies to: 233-238
hooks/useShiftStats.ts (1)
1-6: Module header looks good.No issues with the client directive and updated imports.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
…alendars and shifts
Move client data fetching to @tanstack/react-query v5 with a global QueryClient and polling, replacing ad-hoc fetch/SSE syncing to simplify real-time behavior and improve UX.
Refactors many hooks to use useQuery/useMutation with a query-key factory, optimistic updates, and automatic cache invalidation for immediate UI feedback and consistent cache management. Removes manual refetch/refresh flows and most SSE/event-emitter reliance (API no longer emits events for some mutations).
Adds a QueryProvider in the app layout, integrates an ESLint plugin for query rules, and updates dependencies (react-query + related packages, plus a few package bumps). This centralizes syncing, reduces complexity, and makes live updates more robust.
Convert multiple data hooks to React Query to enable polling, automatic cache management, optimistic updates and simpler refetch/invalidation for shares, tokens, external syncs, subscriptions, activity logs and shift stats.
Improve activity log behavior by adding server-side search and severity filters, switching to offset-based pagination, merging/searching metadata on the server, debounced client search, and ensuring filter changes reset pagination.
Simplify sync error/refresh flow by removing manual refresh triggers/props and relying on React Query polling and cache invalidation. Minor UI adjustments and i18n additions for activity/calendar labels.
onStatsRefreshprop from AppHeader, CalendarCompareView, CalendarContent, and PresetSelector components.useConnectionStatushook to manage online/offline state and display connection status to users.useSSEConnectionhook and related functionality.useCompareDatahook to manage fetching and mutating compare mode data.Summary by CodeRabbit
New Features
Improvements
Refactor
Removed
✏️ Tip: You can customize this high-level summary in your review settings.