-
-
Notifications
You must be signed in to change notification settings - Fork 191
feat(api): improve calendar data structure #438
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3 issues found across 52 files
Prompt for AI agents (all 3 issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="apps/web/src/components/calendar/context/calendar-colors-provider.tsx">
<violation number="1" location="apps/web/src/components/calendar/context/calendar-colors-provider.tsx:34">
P2: Rule violated: **CSS variable naming and usage conventions (Tailwind v4 namespaces)**
`calendarColorVariable` produces `--calendar-color-*`, which is outside the allowed Tailwind v4 namespaces (`--color-*`, `--font-*`, etc.). Using it on this line perpetuates a token name that cannot be consumed via the required `var(--color-…)` conventions.</violation>
</file>
<file name="packages/api/src/routers/accounts.ts">
<violation number="1" location="packages/api/src/routers/accounts.ts:39">
P2: `provider.id` is incorrectly asserted to only "google" | "microsoft" even though accounts can also have provider "zoom", leading to incorrect TRPC output types and downstream handling bugs.</violation>
</file>
<file name="apps/web/src/lib/db.ts">
<violation number="1" location="apps/web/src/lib/db.ts:40">
P1: Version 2 removes the stored `accountId` field in favor of `providerAccountId` without a Dexie upgrade/backfill, so previously persisted events/calendars lose their account association. Add an `.upgrade(...)` that copies the old `accountId` value into `providerAccountId` (and removes the deprecated column) before relying on the new indexes.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| providerAccountId: account.accountId, | ||
| providerId: account.providerId, | ||
| provider: { | ||
| id: account.providerId as "google" | "microsoft", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P2: provider.id is incorrectly asserted to only "google" | "microsoft" even though accounts can also have provider "zoom", leading to incorrect TRPC output types and downstream handling bugs.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/api/src/routers/accounts.ts, line 39:
<comment>`provider.id` is incorrectly asserted to only "google" | "microsoft" even though accounts can also have provider "zoom", leading to incorrect TRPC output types and downstream handling bugs.</comment>
<file context>
@@ -33,8 +35,10 @@ export const accountsRouter = createTRPCRouter({
- providerAccountId: account.accountId,
- providerId: account.providerId,
+ provider: {
+ id: account.providerId as "google" | "microsoft",
+ accountId: account.accountId,
+ },
</file context>
| id: account.providerId as "google" | "microsoft", | |
| id: account.providerId as "google" | "microsoft" | "zoom", |
Description
Briefly describe what you did and why.
Screenshots / Recordings
Add screenshots or recordings here to help reviewers understand your changes.
Type of Change
Related Areas
Testing
Checklist
Notes
(Optional) Add anything else you'd like to share.
By submitting, I confirm I understand and stand behind this code. If AI was used, I’ve reviewed and verified everything myself.
Summary by cubic
Refactored the calendar and event data model to use a nested calendar object with provider details, replacing scattered accountId/calendarId fields. This simplifies API contracts, improves event moving/sync, and aligns providers, web app, and schemas around one consistent structure.
Refactors
Migration
Written for commit dde369a. Summary will update automatically on new commits.