Conversation
📝 WalkthroughWalkthroughThis pull request extends the Event data model across multiple layers—database schema, storage layer, and mock data generation—by adding three optional string fields: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop2/src/devtool/seed/shared.ts (1)
276-314: Fix variable scoping in switch statement.Each case declares variables (e.g.,
daysLastYear,daysPast,hoursToday, etc.) without block scope, which allows them to leak into other cases and creates potential for accidental access. This is flagged by the linter and violates JavaScript best practices.Apply this diff to wrap each case in a block:
switch (timePattern) { - case "last-year": + case "last-year": { const daysLastYear = faker.number.int({ min: 180, max: 365 }); startsAt = new Date(now.getTime() - daysLastYear * 24 * 60 * 60 * 1000); break; + } - case "past-two-weeks": + case "past-two-weeks": { const daysPast = faker.number.int({ min: 1, max: 14 }); startsAt = new Date(now.getTime() - daysPast * 24 * 60 * 60 * 1000); break; + } - case "today": + case "today": { const hoursToday = faker.number.float({ min: -12, max: 12, fractionDigits: 1 }); startsAt = new Date(now.getTime() + hoursToday * 60 * 60 * 1000); break; + } - case "next-few-days": + case "next-few-days": { const daysNext = faker.number.int({ min: 1, max: 7 }); startsAt = new Date(now.getTime() + daysNext * 24 * 60 * 60 * 1000); break; + } - case "next-two-weeks": + case "next-two-weeks": { const daysLater = faker.number.int({ min: 8, max: 14 }); startsAt = new Date(now.getTime() + daysLater * 24 * 60 * 60 * 1000); break; + } default: startsAt = faker.date.soon({ days: 7 }); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/desktop2/public/assets/teams.pngis excluded by!**/*.png
📒 Files selected for processing (8)
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx(1 hunks)apps/desktop2/src/devtool/seed/empty.ts(1 hunks)apps/desktop2/src/devtool/seed/index.ts(1 hunks)apps/desktop2/src/devtool/seed/shared.ts(4 hunks)apps/desktop2/src/devtool/seed/v1.ts(1 hunks)apps/desktop2/src/store/tinybase/persisted.ts(2 hunks)packages/db/src/schema.ts(1 hunks)packages/ui/src/components/ui/kbd.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit configuration file
**/*.{js,ts,tsx,rs}: 1. Do not add any error handling. Keep the existing one.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
packages/db/src/schema.tsapps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsxapps/desktop2/src/devtool/seed/v1.tsapps/desktop2/src/devtool/seed/empty.tsapps/desktop2/src/store/tinybase/persisted.tspackages/ui/src/components/ui/kbd.tsxapps/desktop2/src/devtool/seed/shared.tsapps/desktop2/src/devtool/seed/index.ts
apps/desktop2/**/*.{tsx,jsx}
📄 CodeRabbit inference engine (apps/desktop2/.cursor/rules/style.mdc)
apps/desktop2/**/*.{tsx,jsx}: When there are many Tailwind classNames with conditional logic, use the cn utility imported asimport { cn } from "@hypr/ui/lib/utils"
Always pass an array to cn when composing Tailwind classNames
Split cn array entries by logical grouping when composing Tailwind classNames
Files:
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx
🧬 Code graph analysis (3)
apps/desktop2/src/devtool/seed/empty.ts (1)
apps/desktop2/src/devtool/seed/index.ts (1)
SeedDefinition(5-5)
packages/ui/src/components/ui/kbd.tsx (1)
packages/ui/src/lib/utils.ts (1)
cn(4-6)
apps/desktop2/src/devtool/seed/index.ts (3)
apps/desktop2/src/devtool/seed/shared.ts (1)
SeedDefinition(21-25)apps/desktop2/src/devtool/seed/empty.ts (1)
emptySeed(4-10)apps/desktop2/src/devtool/seed/v1.ts (1)
v1Seed(23-30)
🪛 Biome (2.1.2)
apps/desktop2/src/devtool/seed/shared.ts
[error] 288-288: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 293-293: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 298-298: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 303-303: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 308-308: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
⏰ 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: ci (macos, macos-14)
🔇 Additional comments (11)
packages/ui/src/components/ui/kbd.tsx (2)
3-16: LGTM!The
Kbdcomponent is well-implemented with appropriate type safety and styling.
28-28: LGTM!The exports are correctly defined.
apps/desktop2/src/devtool/seed/v1.ts (1)
27-27: LGTM!The addition of
store.delTables()ensures a clean state before seeding, preventing data conflicts and maintaining consistency with the new empty seed pattern.apps/desktop2/src/devtool/seed/shared.ts (2)
319-362: Well-structured meeting metadata generation.The logic to generate meeting type-appropriate metadata (online/offline/hybrid with corresponding links and locations) is realistic and comprehensive. The weighted distribution and conditional field population are well-designed.
487-487: Minor probability adjustment noted.The session-to-event linking probability increased from 0.5 to 0.6, which will result in slightly more sessions being linked to events in mock data.
apps/desktop2/src/components/main/body/sessions/outer-header/metadata.tsx (1)
79-81: LGTM!The metadata fields now correctly derive from
eventRowinstead of hardcoded nulls, aligning with the extended Event schema. The type casting and null coalescing are appropriate for TinyBase cells.packages/db/src/schema.ts (1)
54-56: LGTM!The three optional text columns cleanly extend the events table schema. The fields align perfectly with the corresponding changes in the TinyBase persisted store and mock data generation.
apps/desktop2/src/devtool/seed/index.ts (1)
1-7: LGTM!The empty seed is properly imported and added as the first element in the seeds array, providing users with an empty state option before the V1 seed.
apps/desktop2/src/devtool/seed/empty.ts (1)
4-10: LGTM!The empty seed definition is clean and focused, providing a straightforward way to clear all tables and start with an empty state.
apps/desktop2/src/store/tinybase/persisted.ts (2)
50-52: LGTM!The event schema extensions use appropriate
z.preprocessto handle null/undefined coercion for optional fields, maintaining consistency with the pattern used for other optional fields in the codebase.
164-166: LGTM!The TinyBase schema correctly extends the events table with the three new string fields, aligning with the database schema and ensuring proper type safety across the storage layer.
|
If we can avoid adding logo in our assets folder, avoid it. https://icon-sets.iconify.design/?query=teams import { Icon } from "@iconify-icon/react";
<Icon icon="logos:microsoft-teams" width="24" height="24" /> |
2 participants
• Added keyboard navigation support
• Implemented empty state handling
• Upgraded shared components
• Added new form fields
• Integrated teams functionality
• Reset mechanism for mock data