Add task support for calendar #3139
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. |
WalkthroughThis change introduces a workspace calendar task system. It adds a new database table with supporting enums and permissions, updates type definitions, and implements a React modal dialog for adding events. The web client integrates the modal and button, enabling users to create and schedule tasks within a workspace calendar. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WebClient
participant AddEventButton
participant AddEventModal
participant SupabaseDB
User->>AddEventButton: Click "Add Event"
AddEventButton->>WebClient: Trigger open modal handler
WebClient->>AddEventModal: Render modal (isOpen=true)
User->>AddEventModal: Fill form and submit
AddEventModal->>SupabaseDB: Insert new task into workspace_calendar_tasks
SupabaseDB-->>AddEventModal: Respond with success or error
AddEventModal->>WebClient: Close modal on success
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsxOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs packages/types/src/supabase.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@typescript-eslint/parser' imported from /eslint.config.mjs ✨ Finishing Touches
🧪 Generate Unit Tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @phatgg221, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request introduces the foundational database schema and user interface for a new calendar task scheduling feature. It enables users to define and store detailed tasks with various properties and constraints, laying the groundwork for future scheduling functionality.
Highlights
- Database Schema Introduction: A new
workspace_calendar_taskstable has been added to the Supabase database. This table is designed to store details for calendar-related tasks, including their name, duration, splittability, scheduling constraints, and priority. Two new enum types,calendar_task_timeandpriority_status, were also introduced to support task properties. - Calendar Task Creation UI: New UI components,
AddEventButtonandAddEventModal, have been integrated into the calendar page. These components provide a user-friendly interface for creating new tasks, allowing users to input task details, define duration settings (total, min/max split), and specify scheduling preferences. - Data Persistence Integration: The newly developed task creation UI is fully integrated with Supabase, enabling users to persist their defined calendar tasks directly into the
workspace_calendar_taskstable. This ensures that created tasks are saved and can be retrieved for scheduling. - Database Schema Refinements: Subsequent database migrations refined the
workspace_calendar_taskstable by addingdescriptionandtotal_durationcolumns, and by changing the data types ofmin_split_duration_minutesandmax_split_duration_minutesfromsmallinttorealfor more flexible duration input.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3139 +/- ##
==========================================
- Coverage 0.80% 0.79% -0.01%
==========================================
Files 2569 2571 +2
Lines 328124 328680 +556
Branches 3071 3073 +2
==========================================
Hits 2625 2625
- Misses 323306 323860 +554
- Partials 2193 2195 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Code Review
This pull request introduces a new database table workspace_calendar_tasks for managing scheduler tasks, along with associated UI components for adding these tasks. Key changes include new database migrations, a dialog for task creation, and updates to the calendar client page.
Several critical and high-severity issues need addressing:
- A major security concern arises from disabling Row Level Security (RLS) on the
workspace_calendar_taskstable while having broad default grants, potentially exposing task data. - A hardcoded email address in the task creation dialog needs to be removed or made dynamic.
- The data type for
total_durationin the database should be reviewed for better suitability (e.g., numeric instead of text). - There are minor naming inconsistencies in database object names and potential confusion regarding duration units between the UI and database.
Overall, the functionality additions are significant, but the security and data modeling aspects require careful review and correction before merging.
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (6)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx (2)
5-8: Remove unused prop from interface.The
wsIdprop is defined in the interface but not used in the component implementation.interface AddEventButtonProps { - wsId: string; onOpenDialog?: () => void; }-export default function AddEventButton({ onOpenDialog }: AddEventButtonProps) { +export default function AddEventButton({ onOpenDialog }: AddEventButtonProps) {
10-22: Component implementation looks good, but consider adding tests.The button component is well-structured with appropriate styling and accessibility. However, static analysis indicates missing test coverage.
Consider adding unit tests to cover the component's render and click behavior.
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx (1)
49-68: Clean integration of new components with existing calendar.The use of React Fragment to wrap both the existing SmartCalendar and the new AddEventModal is a good approach that maintains the existing component structure while adding new functionality.
Static analysis indicates these lines lack test coverage. Consider adding integration tests to ensure the modal state management and component rendering work correctly.
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (2)
223-236: Improve accessibility with proper ARIA attributes.The required field indicator should be accessible to screen readers.
<Label htmlFor="task-title"> - Task Name <span className="text-destructive">*</span> + Task Name <span className="text-destructive" aria-label="required">*</span> </Label> <Input id="task-title" placeholder="e.g., Complete project documentation" value={formData.name} onChange={(e) => updateFormData('name', e.target.value)} className={errors.name ? 'border-destructive' : ''} + aria-required="true" + aria-describedby={errors.name ? 'task-title-error' : undefined} /> {errors.name && ( - <p className="text-sm text-destructive">{errors.name}</p> + <p id="task-title-error" className="text-sm text-destructive" role="alert"> + {errors.name} + </p> )}Apply similar patterns to other required fields throughout the form.
2-503: Add comprehensive test coverage for this critical component.The static analysis indicates zero test coverage for this component, which handles user input, form validation, and database operations.
This component should have tests covering:
- Form validation scenarios (required fields, numeric constraints, date validation)
- Database submission success/failure cases
- User interaction flows (form submission, modal close/reset)
- Error state handling and display
Would you like me to generate a comprehensive test suite for this component?
packages/types/src/supabase.ts (1)
5065-5168: LGTM! Well-structured calendar tasks table with minor typo in foreign key name.The new
workspace_calendar_taskstable definition is comprehensive and follows good database design patterns. The field types are appropriate, relationships are properly defined, and the structure supports the calendar scheduling functionality effectively.However, there's a typo in the foreign key constraint name:
- foreignKeyName: 'workspace_calendar_taskss_creator_id_fkey'; + foreignKeyName: 'workspace_calendar_tasks_creator_id_fkey';The extra 's' in "taskss" should be removed for consistency with naming conventions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql(1 hunks)apps/db/supabase/migrations/20250620095114_new_migration.sql(1 hunks)apps/db/supabase/migrations/20250620101647_new_migration.sql(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx(4 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx(1 hunks)packages/types/src/supabase.ts(21 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx
[warning] 2-22: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx#L2-L22
Added lines #L2 - L22 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx
[warning] 3-4: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L3-L4
Added lines #L3 - L4 were not covered by tests
[warning] 14-14: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L14
Added line #L14 was not covered by tests
[warning] 25-28: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L25-L28
Added lines #L25 - L28 were not covered by tests
[warning] 34-34: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L34
Added line #L34 was not covered by tests
[warning] 49-68: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx#L49-L68
Added lines #L49 - L68 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
[warning] 2-503: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L2-L503
Added lines #L2 - L503 were not covered by tests
packages/types/src/supabase.ts
[warning] 8407-8407: packages/types/src/supabase.ts#L8407
Added line #L8407 was not covered by tests
[warning] 8412-8412: packages/types/src/supabase.ts#L8412
Added line #L8412 was not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: codecov/patch
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
🔇 Additional comments (7)
apps/db/supabase/migrations/20250620095114_new_migration.sql (1)
3-3: ```shell
#!/bin/bashSearch for any enable/disable RLS statements on workspace_calendar_tasks across migrations
rg -n 'workspace_calendar_tasks' -C3 apps/db/supabase/migrations
</details> <details> <summary>apps/db/supabase/migrations/20250620101647_new_migration.sql (1)</summary> `5-7`: **Good improvement: Converting duration fields to real type.** Changing from `smallint` to `real` allows for more precise duration values (e.g., fractional minutes), which is appropriate for calendar scheduling. </details> <details> <summary>apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx (1)</summary> `25-28`: **Good state management implementation.** The modal state management with open/close handlers follows React best practices and provides a clean API for the child components. </details> <details> <summary>packages/types/src/supabase.ts (4)</summary> `8250-8250`: **LGTM! Appropriate enum values for calendar task time classification.** The `calendar_task_time` enum with 'working_time' and 'personal_time' values provides a clean way to categorize calendar tasks by time type. --- `8255-8255`: **LGTM! Standard priority levels for task management.** The `priority_status` enum with 'low', 'medium', 'high', and 'critical' values follows common priority classification patterns and will integrate well with task scheduling algorithms. --- `7738-7743`: **LGTM! Function parameter updates are consistent.** The parameter additions and reordering in the function definitions maintain type safety and follow existing patterns in the codebase. Also applies to: 7747-7748 --- `8407-8407`: **LGTM! Constants properly updated for new enums.** The new enum values are correctly added to the Constants object for runtime access. The static analysis warnings about test coverage are expected for new code and don't indicate an issue with the implementation. Also applies to: 8412-8412 </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620065741_add_calendar_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/db/supabase/migrations/20250620103311_changed_policy_of_tasks_table.sql(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Run tests and collect coverage
apps/db/supabase/migrations/20250620103311_changed_policy_of_tasks_table.sql
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620103311_changed_policy_of_tasks_table.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/db/supabase/migrations/20250620103311_changed_policy_of_tasks_table.sql(1 hunks)packages/types/src/supabase.ts(21 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/db/supabase/migrations/20250620103311_changed_policy_of_tasks_table.sql
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Run tests and collect coverage
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
🔇 Additional comments (4)
packages/types/src/supabase.ts (4)
5083-5083: Verify total_duration field consistencyThe
total_durationfield has inconsistent nullability between Row/Update (nullable) and Insert (required). This could cause runtime issues if the field is actually required.Please verify if
total_durationshould be required or optional in the database schema, and ensure the TypeScript types match the actual database constraints.Also applies to: 5107-5107, 5131-5131
8250-8250: LGTM: New calendar_task_time enum properly definedThe new
calendar_task_timeenum with 'working_time' and 'personal_time' values is well-defined and consistently referenced in both the enum definition and constants section.Also applies to: 8407-8407
8255-8255: LGTM: New priority_status enum properly definedThe new
priority_statusenum with 'low', 'medium', 'high', and 'critical' values follows a logical progression and is properly defined in both locations.Also applies to: 8412-8412
7738-7743: ```shell
#!/bin/bashSearch for function calls across relevant file extensions without relying on ripgrep’s built-in types
echo "Searching for potential function calls that need updating..."
rg -A 3 "count_search_users|create_ai_chat|generate_cross_app_token" -g ".[jt]s" -g ".tsx" -g "*.jsx" || echo "No matches found"</details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
vhpx
changed the title
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (3)
43-44: Field names still inconsistent with their actual usage.The field names suggest minutes but they store hours based on the UI labels "(h)" and step values (0.25). This creates confusion and should be addressed.
This was previously flagged but remains unaddressed. The field names
min_split_duration_minutesandmax_split_duration_minutesshould be renamed to reflect that they store hours, not minutes.
100-102: Due date validation incomplete.The current validation only checks if the due date is in the past but doesn't validate against the
schedule_afterdate, which could lead to logical inconsistencies.This validation gap was previously identified. The due date should also be validated to ensure it's not before the
schedule_afterdate when both are provided.
134-139: Database field mapping uses incorrect field names.The field names in the database submission still use "minutes" terminology while storing hour values, creating a mismatch between the database schema expectations and the actual data being stored.
- min_split_duration_minutes: formData.is_splittable - ? formData.min_split_duration_minutes - : null, - max_split_duration_minutes: formData.is_splittable - ? formData.max_split_duration_minutes - : null, + min_split_duration_hours: formData.is_splittable + ? formData.min_split_duration_hours + : null, + max_split_duration_hours: formData.is_splittable + ? formData.max_split_duration_hours + : null,This change requires coordinating with the database schema to ensure field names are consistent.
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (1)
149-158: Add specific error handling for database constraints.The current error handling is generic. Consider adding specific handling for common database constraint violations to provide more user-friendly error messages.
if (error) { console.error('Database error:', error); - setErrors({ submit: `Failed to create task: ${error.message}` }); + let errorMessage = 'Failed to create task'; + if (error.code === '23503') { + errorMessage = 'Invalid workspace or user reference'; + } else if (error.code === '23505') { + errorMessage = 'A task with this name already exists'; + } else { + errorMessage = `Failed to create task: ${error.message}`; + } + setErrors({ submit: errorMessage }); return false; }apps/db/supabase/migrations/20250620145741_add_calendar_tasks_table.sql (1)
81-81: Inconsistent data type evolution for total_duration.The migration first adds
total_durationastext not nullthen immediately changes it toreal. This suggests the data type choice wasn't finalized during development.For cleaner migrations, consider consolidating these changes:
-alter table "public"."workspace_calendar_tasks" add column "total_duration" text not null; -... -alter table "public"."workspace_calendar_tasks" alter column "total_duration" set data type real using "total_duration"::real; +alter table "public"."workspace_calendar_tasks" add column "total_duration" real not null;This reduces migration complexity and potential issues.
Also applies to: 96-96
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/db/supabase/migrations/20250620145741_add_calendar_tasks_table.sql(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx(4 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx(1 hunks)packages/types/src/supabase.ts(19 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-button.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/client.tsx
- packages/types/src/supabase.ts
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Verify generated types
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (2)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (1)
465-472: User email display properly implemented.Good implementation of dynamic user email display that addresses the previous hardcoded email concern.
The fallback to "your account" when email is not available provides a good user experience.
apps/db/supabase/migrations/20250620145741_add_calendar_tasks_table.sql (1)
37-49: Consider restricting anonymous user permissions.Granting full CRUD permissions to anonymous users (
anonrole) for calendar tasks may be overly permissive for this feature.Please verify if anonymous users should have access to calendar tasks functionality, as this typically requires authentication.
#!/bin/bash # Check if other calendar-related tables have similar anon permissions rg -A 5 -B 5 "grant.*to.*anon" apps/db/supabase/migrations/ | grep -i calendar
| "ws_id" uuid, | ||
| "creator_id" uuid, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Foreign key columns should be NOT NULL.
The ws_id and creator_id columns allow NULL values, but they have foreign key constraints and are essential for the application logic.
- "ws_id" uuid,
- "creator_id" uuid,
+ "ws_id" uuid not null,
+ "creator_id" uuid not null,This prevents orphaned records and ensures data integrity.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "ws_id" uuid, | |
| "creator_id" uuid, | |
| "ws_id" uuid not null, | |
| "creator_id" uuid not null, |
🤖 Prompt for AI Agents
In apps/db/supabase/migrations/20250620145741_add_calendar_tasks_table.sql at
lines 8 to 9, the columns "ws_id" and "creator_id" are foreign keys but
currently allow NULL values. Modify these columns to be NOT NULL to enforce data
integrity and prevent orphaned records by updating their definitions to include
the NOT NULL constraint.
| create policy "allow only user in the workspace to insert" | ||
| on "public"."workspace_calendar_tasks" | ||
| as permissive | ||
| for insert | ||
| to authenticated | ||
| with check (true); |
There was a problem hiding this comment.
RLS policy too permissive for workspace isolation.
The current policy allows any authenticated user to insert tasks with with check (true), which doesn't enforce workspace membership constraints.
This could allow users to create tasks in workspaces they don't belong to. Update the policy to check workspace membership:
create policy "allow only user in the workspace to insert"
on "public"."workspace_calendar_tasks"
as permissive
for insert
to authenticated
-with check (true);
+with check (
+ exists (
+ select 1 from workspace_users
+ where workspace_users.ws_id = workspace_calendar_tasks.ws_id
+ and workspace_users.user_id = auth.uid()
+ )
+);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| create policy "allow only user in the workspace to insert" | |
| on "public"."workspace_calendar_tasks" | |
| as permissive | |
| for insert | |
| to authenticated | |
| with check (true); | |
| create policy "allow only user in the workspace to insert" | |
| on "public"."workspace_calendar_tasks" | |
| as permissive | |
| for insert | |
| to authenticated | |
| with check ( | |
| exists ( | |
| select 1 | |
| from workspace_users | |
| where workspace_users.ws_id = workspace_calendar_tasks.ws_id | |
| and workspace_users.user_id = auth.uid() | |
| ) | |
| ); |
🤖 Prompt for AI Agents
In apps/db/supabase/migrations/20250620145741_add_calendar_tasks_table.sql
around lines 89 to 94, the row-level security policy for inserting into
workspace_calendar_tasks is too permissive because it uses "with check (true)"
allowing any authenticated user to insert tasks regardless of workspace
membership. Modify the policy to include a condition in the "with check" clause
that verifies the user is a member of the workspace associated with the task,
ensuring only authorized users can insert tasks in their own workspaces.
phatgg221
changed the title
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/types/src/supabase.ts (1)
5170-5195: Foreign-key name still misspelled – migration will failThe three
foreignKeyNamestrings useworkspace_calendar_taskss…(double s).
Same issue was flagged earlier but is still present, so the DDL generated from these types will not match the real constraint names produced by the migration.-foreignKeyName: 'workspace_calendar_taskss_creator_id_fkey'; +foreignKeyName: 'workspace_calendar_tasks_creator_id_fkey';Repeat for the other two occurrences (
…_creator_id_fkey,…_ws_id_fkey).
🧹 Nitpick comments (4)
apps/db/supabase/migrations/20250621070438_alter_tasks_table.sql (4)
1-4: Use IF EXISTS when dropping policies to avoid failures on re-runs.Wrapping
DROP POLICYinIF EXISTSwill make the migration idempotent and prevent errors if the policies have already been removed.
5-86: Consolidate repetitive REVOKE statements.You can simplify these 48
REVOKEcalls by usingREVOKE ALL PRIVILEGES ON TABLE … FROM …;for each role. This will reduce verbosity and maintenance overhead.
131-133: UseDROP TABLE IF EXISTS(andCASCADEif needed).Instead of
DROP TABLE, consider:DROP TABLE IF EXISTS public.workspace_calendar_sync_log CASCADE; DROP TABLE IF EXISTS public.workspace_whiteboards CASCADE;This avoids errors on repeated runs and automatically cleans up dependent objects.
135-145: Reevaluate the use ofrealfor duration-related fields.
REALis a low-precision floating type. For task durations you may prefer:
INTEGER(minutes)INTERVAL(Postgres interval type)NUMERIC(10,2)(fixed precision)Choose based on required precision and unit.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/db/supabase/migrations/20250621070438_alter_tasks_table.sql(1 hunks)apps/db/supabase/migrations/20250621074218_new_migration.sql(1 hunks)apps/db/supabase/migrations/20250621082137_new_migration.sql(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx(1 hunks)packages/types/src/supabase.ts(6 hunks)
🧰 Additional context used
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
[warning] 2-512: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx#L2-L512
Added lines #L2 - L512 were not covered by tests
packages/types/src/supabase.ts
[warning] 8437-8437: packages/types/src/supabase.ts#L8437
Added line #L8437 was not covered by tests
[warning] 8442-8442: packages/types/src/supabase.ts#L8442
Added line #L8442 was not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx
[warning] 237-242: apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx#L237-L242
Added lines #L237 - L242 were not covered by tests
🔇 Additional comments (8)
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/tasks-sidebar-content.tsx (1)
237-242: LGTM! Proper initialization of new task properties.The new properties are correctly initialized to
nulland align with the enhanced task schema for calendar functionality. The implementation follows the established pattern used for other optional fields in the same code block.apps/db/supabase/migrations/20250621082137_new_migration.sql (1)
1-41: LGTM! Well-structured analytics view migration.The migration properly drops and recreates the view with comprehensive analytics fields. The LEFT JOINs ensure data preservation when category or task information is missing, and the session length categorization thresholds are reasonable for analytics purposes.
apps/db/supabase/migrations/20250621074218_new_migration.sql (2)
9-9: LGTM! Supporting calendar tasks with nullable list_id.Making
list_idnullable is appropriate for calendar tasks that exist independently of traditional task boards. This change enables the calendar task functionality being added in this PR.
15-47: LGTM! Analytics view creation.The view definition is comprehensive and well-structured for time tracking analytics.
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx (1)
2-512: ```shell
#!/usr/bin/env bashSearch for any usage of AddEventModal across TypeScript files, including tests
echo "Searching for AddEventModal occurrences in .ts and .tsx files:"
rg -n "AddEventModal" -g ".ts" -g ".tsx" .</details> <details> <summary>packages/types/src/supabase.ts (1)</summary> `3642-3656`: **Making `list_id` nullable & adding split-task fields – double-check downstream code** `tasks.list_id` is now optional/nullable and several new fields were added (`is_splittable`, `min/max_split_duration_hours`, *etc.*). Please verify: 1. All code paths that previously assumed a non-null `list_id` (e.g. joins, not-null checks in queries, UI routing) are guarded for `null`. 2. Business logic that reads/updates split-task fields is gated behind `is_splittable === true` to avoid inconsistent data. 3. Migrations add the new columns with correct defaults / not-null constraints. Also applies to: 3666-3680, 3690-3704 </details> <details> <summary>apps/db/supabase/migrations/20250621070438_alter_tasks_table.sql (2)</summary> `162-195`: **Verify analytics view coverage with new task fields.** The `time_tracking_session_analytics` view isn’t selecting any of the newly added columns (`is_splittable`, `user_defined_priority`, etc.). Confirm if these should be surfaced in analytics or if a separate view is needed. --- `198-210`: **Confirm removal of defaults / NOT NULL aligns with application logic.** Dropping the defaults and null-constraints for `is_splittable`, `time_reference`, etc., will allow NULLs at insert time. Make sure that downstream application code handles NULL values correctly (and doesn’t rely on the old defaults). </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| React.useEffect(() => { | ||
| const getUser = async () => { | ||
| const { | ||
| data: { user }, | ||
| error, | ||
| } = await supabase.auth.getUser(); | ||
| if (!error && user) { | ||
| setUser(user); | ||
| } | ||
| }; | ||
|
|
||
| getUser(); | ||
| }, [supabase]); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for user authentication.
The user fetch on mount doesn't handle potential errors or loading states, which could lead to poor UX if authentication fails.
React.useEffect(() => {
const getUser = async () => {
+ setIsLoading(true);
const {
data: { user },
error,
} = await supabase.auth.getUser();
- if (!error && user) {
- setUser(user);
- }
+ if (error) {
+ console.error('Auth error:', error);
+ setErrors({ submit: 'Authentication failed. Please refresh and try again.' });
+ } else if (user) {
+ setUser(user);
+ }
+ setIsLoading(false);
};
getUser();
}, [supabase]);Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
around lines 55 to 67, the useEffect hook fetching the user does not handle
errors or loading states. To fix this, introduce state variables to track
loading and error status, update these states appropriately during the async
call, and handle the error case by setting an error state or displaying an error
message to improve user experience when authentication fails.
| if (formData.end_date && dayjs(formData.end_date).isBefore(dayjs())) { | ||
| newErrors.end_date = 'End date cannot be in the past'; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve date validation logic.
The current validation only checks if the end date is in the past, but should also validate that the end date comes after the start date when both are provided.
- if (formData.end_date && dayjs(formData.end_date).isBefore(dayjs())) {
- newErrors.end_date = 'End date cannot be in the past';
- }
+ if (formData.end_date) {
+ const endDate = dayjs(formData.end_date);
+ if (endDate.isBefore(dayjs())) {
+ newErrors.end_date = 'End date cannot be in the past';
+ } else if (formData.start_date && endDate.isBefore(dayjs(formData.start_date))) {
+ newErrors.end_date = 'End date cannot be before the start date';
+ }
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (formData.end_date && dayjs(formData.end_date).isBefore(dayjs())) { | |
| newErrors.end_date = 'End date cannot be in the past'; | |
| } | |
| if (formData.end_date) { | |
| const endDate = dayjs(formData.end_date); | |
| if (endDate.isBefore(dayjs())) { | |
| newErrors.end_date = 'End date cannot be in the past'; | |
| } else if (formData.start_date && endDate.isBefore(dayjs(formData.start_date))) { | |
| newErrors.end_date = 'End date cannot be before the start date'; | |
| } | |
| } |
🤖 Prompt for AI Agents
In
apps/web/src/app/[locale]/(dashboard)/[wsId]/calendar/components/add-event-dialog.tsx
around lines 99 to 101, the date validation only checks if the end date is in
the past. Update the validation logic to also check if the end date is after the
start date when both start_date and end_date are provided. Add a condition to
compare the two dates and set an appropriate error message if the end date is
before or equal to the start date.
| CREATE UNIQUE INDEX workspace_calendar_taskss_pkey ON public.workspace_calendar_tasks USING btree (id); | ||
|
|
||
| alter table "public"."workspace_calendar_tasks" add constraint "workspace_calendar_taskss_pkey" PRIMARY KEY using index "workspace_calendar_taskss_pkey"; | ||
|
|
There was a problem hiding this comment.
Typo in index/constraint name: extra “s” in workspace_calendar_taskss_pkey.
You dropped and then recreated the PK index as workspace_calendar_taskss_pkey, but your table is workspace_calendar_tasks. This typo will cause confusion; rename it to workspace_calendar_tasks_pkey.
🤖 Prompt for AI Agents
In apps/db/supabase/migrations/20250621070438_alter_tasks_table.sql around lines
150 to 153, the index and constraint names have an extra "s" resulting in
"workspace_calendar_taskss_pkey". Rename both the unique index and the primary
key constraint to "workspace_calendar_tasks_pkey" to match the table name and
avoid confusion.
| alter table "public"."workspace_calendar_tasks" add constraint "workspace_calendar_taskss_creator_id_fkey" FOREIGN KEY (creator_id) REFERENCES users(id) ON DELETE CASCADE not valid; | ||
|
|
||
| alter table "public"."workspace_calendar_tasks" validate constraint "workspace_calendar_taskss_creator_id_fkey"; | ||
|
|
||
| alter table "public"."workspace_calendar_tasks" add constraint "workspace_calendar_taskss_ws_id_fkey" FOREIGN KEY (ws_id) REFERENCES workspaces(id) ON DELETE CASCADE not valid; | ||
|
|
||
| alter table "public"."workspace_calendar_tasks" validate constraint "workspace_calendar_taskss_ws_id_fkey"; |
There was a problem hiding this comment.
Constraint names for FKs carry the same typo (“taskss”).
The foreign-key constraints are added as
workspace_calendar_taskss_creator_id_fkeyworkspace_calendar_taskss_ws_id_fkey
Rename to use workspace_calendar_tasks_creator_id_fkey and workspace_calendar_tasks_ws_id_fkey for consistency.
🤖 Prompt for AI Agents
In apps/db/supabase/migrations/20250621070438_alter_tasks_table.sql around lines
154 to 160, the foreign key constraint names contain a typo with "taskss"
instead of "tasks". Rename the constraints from
"workspace_calendar_taskss_creator_id_fkey" to
"workspace_calendar_tasks_creator_id_fkey" and from
"workspace_calendar_taskss_ws_id_fkey" to "workspace_calendar_tasks_ws_id_fkey"
to correct the spelling and maintain consistency.
Summary by CodeRabbit