-
-
Notifications
You must be signed in to change notification settings - Fork 19
Implement whiteboard management #3137
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
…gement by its boardId
…hot saving and thumbnail generation in the whiteboard component
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 update introduces a collaborative whiteboards feature scoped to workspaces. It adds a new database table with full row-level security, updates type definitions, and implements a comprehensive React-based UI for listing, creating, editing, viewing, and managing whiteboards, including thumbnail generation and snapshot persistence. Localization strings are added for English and Vietnamese. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WhiteboardsPage
participant Supabase
participant WhiteboardsList
participant CreateWhiteboardDialog
participant EditWhiteboardDialog
User->>WhiteboardsPage: Navigates to whiteboards list
WhiteboardsPage->>Supabase: Fetch whiteboards for wsId
Supabase-->>WhiteboardsPage: Return whiteboards data
WhiteboardsPage->>WhiteboardsList: Render with whiteboards
User->>CreateWhiteboardDialog: Clicks "New Whiteboard"
CreateWhiteboardDialog->>Supabase: Insert new whiteboard
Supabase-->>CreateWhiteboardDialog: Success/failure
CreateWhiteboardDialog->>WhiteboardsPage: Refresh list
User->>EditWhiteboardDialog: Clicks "Edit" on a whiteboard
EditWhiteboardDialog->>Supabase: Update whiteboard metadata
Supabase-->>EditWhiteboardDialog: Success/failure
EditWhiteboardDialog->>WhiteboardsPage: Refresh list
sequenceDiagram
participant User
participant TLDrawPage
participant Supabase
participant CustomTldraw
participant Toolbar
User->>TLDrawPage: Navigates to /whiteboards/[boardId]
TLDrawPage->>Supabase: Fetch whiteboard by wsId and boardId
Supabase-->>TLDrawPage: Return whiteboard data (snapshot, etc.)
TLDrawPage->>CustomTldraw: Render with initialData
User->>Toolbar: Clicks "Save Snapshot"
Toolbar->>CustomTldraw: Get current editor snapshot
Toolbar->>Supabase: Upload thumbnail image
Supabase-->>Toolbar: Return thumbnail URL
Toolbar->>Supabase: Update whiteboard record with snapshot & thumbnail
Supabase-->>Toolbar: Confirm update
Suggested labels
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]/whiteboards/client.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]/whiteboards/[boardId]/toolbar.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]/whiteboards/createWhiteboardDialog.tsxOops! 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 (
|
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.
Summary of Changes
Hello @khangronky, 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 a comprehensive whiteboard management system within workspaces. It enables users to create, view, edit, and delete whiteboards, complete with features like search, sorting, and thumbnail previews. Additionally, it integrates the ability to save whiteboard snapshots and generate thumbnails directly from the drawing canvas.
Highlights
- Access Control (RLS): Row Level Security policies are implemented to ensure that users can only access whiteboards within workspaces they are members of, enhancing data security.
- Whiteboard List Management: A dedicated page is introduced for managing whiteboards, allowing users to create new whiteboards, and view existing ones with filtering by search query, sorting (alphabetical, creation date, last modified), and toggling between grid and list views.
- Whiteboard Editor Integration: The Tldraw editor is integrated, allowing users to draw and collaborate. A custom toolbar is added to the Tldraw interface, providing functionality to save the current whiteboard state (snapshot) and generate a thumbnail directly from the canvas, persisting both to the database.
- CRUD Operations: Full Create, Read, Update, and Delete (CRUD) functionality is provided for whiteboards, including dedicated dialogs for creating and editing whiteboard metadata (title, description) and a confirmation dialog for deletion.
- Localization Support: New strings for whiteboard-related UI elements are added to English and Vietnamese localization files.
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. ↩
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.
Please resolve CI/CD errors, thank you!
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.
Code Review
The code changes introduce whiteboard management functionality, including creating, updating, filtering, sorting, and saving whiteboard snapshots. The review identified several issues, including potential error handling improvements and argument order inconsistencies in the supabase type definitions. Addressing these points will enhance the robustness and maintainability of the whiteboard feature.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
Outdated
Show resolved
Hide resolved
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.
Actionable comments posted: 12
🔭 Outside diff range comments (1)
packages/types/src/supabase.ts (1)
7715-7722
: Fix duplicate parameter names causing TypeScript compilation errorsThe function signature contains duplicate parameter names
p_origin_app
,p_target_app
, andp_expiry_seconds
which are causing the compilation failures reported in all pipeline deployments.generate_cross_app_token: { Args: | { p_origin_app: string; p_expiry_seconds?: number; p_target_app: string; p_user_id: string; - p_origin_app: string; - p_target_app: string; - p_expiry_seconds?: number; } | { p_origin_app: string; p_session_data?: Json; p_expiry_seconds?: number; p_target_app: string; p_user_id: string; }; Returns: string; };
🧹 Nitpick comments (15)
apps/db/supabase/migrations/20250620034855_add_whiteboards_rls.sql (1)
7-20
: Consider optimizing RLS policy performance.The RLS policy correctly restricts access to workspace members, but the subquery could be expensive. Consider adding a composite index on the workspace_members table for better performance.
Add this index to optimize the RLS policy performance:
CREATE INDEX IF NOT EXISTS idx_workspace_members_user_ws ON public.workspace_members(user_id, ws_id);This will make the workspace membership lookup in the RLS policy much faster.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx (1)
57-59
: Consider internationalizing hardcoded strings.Since the application supports localization (as evidenced by the locale parameter in routes), consider using translation keys for user-facing strings like form labels and button text.
Replace hardcoded strings with translation keys:
- <CardTitle> - {isEditing ? 'Edit Whiteboard' : 'Create Whiteboard'} - </CardTitle> + <CardTitle> + {isEditing ? t('whiteboard.edit_title') : t('whiteboard.create_title')} + </CardTitle>And similarly for other user-facing strings throughout the component.
Also applies to: 67-67, 81-81, 98-104
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/editWhiteboardDialog.tsx (2)
66-68
: Avoid usingany
type for errors.Using
any
type defeats TypeScript's type safety. Consider using a more specific error type orunknown
.Apply this diff to improve type safety:
- } catch (error: any) { + } catch (error) { console.error('Unexpected error:', error); toast.error('An unexpected error occurred. Please try again.');
53-53
: Consider using database triggers forupdated_at
field.Instead of manually setting
updated_at
in the client code, it's better to handle this at the database level using triggers or default values. This ensures consistency across all update operations.Run the following script to check if there's a database trigger for the
updated_at
field:#!/bin/bash # Description: Check for database triggers or policies that handle updated_at field # Search for trigger definitions in migration files fd -e sql | xargs rg -A 5 "CREATE.*TRIGGER.*whiteboard" # Search for updated_at handling in migrations fd -e sql | xargs rg -A 5 "updated_at.*DEFAULT|SET.*updated_at"apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/createWhiteboardDialog.tsx (1)
63-68
: Avoid usingany
type for errors.Same issue as in the edit dialog - using
any
type for errors reduces type safety.Apply this diff to improve type safety:
- } catch (error: any) { + } catch (error) { console.error('Unexpected error:', error); toast.error('An unexpected error occurred. Please try again.');apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx (1)
342-434
: Consider extracting CardAction to a separate file.The
CardAction
component is substantial enough to warrant its own file. This would improve maintainability and make the main component more focused.Consider moving
CardAction
to a separate filecardAction.tsx
to improve code organization.apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/page.tsx (1)
39-39
: Consider a more user-friendly approach for missing creator names.Instead of showing "Unknown User", consider using the user's email or ID as a fallback, or handle this case differently in the UI.
Consider this alternative approach:
- creatorName: whiteboard.creator.display_name || 'Unknown User', + creatorName: whiteboard.creator.display_name || whiteboard.creator.email?.split('@')[0] || 'Unknown User',Or handle it in the UI component to show a different indicator when the creator name is missing.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx (1)
77-77
: Avoid usingany
type for errors.Consistent with other files, avoid using
any
type for better type safety.Apply this diff:
- } catch (error: any) { + } catch (error) {apps/db/supabase/migrations/20250620034755_create_whiteboards.sql (3)
5-5
: Consider default JSON snapshot
snapshot
is currently nullable. If your application always expects a JSON object, you could add a default to avoid downstreamNULL
checks:- "snapshot" jsonb, + "snapshot" jsonb NOT NULL DEFAULT '{}'::jsonb,
15-18
: Redundant explicit primary-key index
APRIMARY KEY
constraint already creates a unique B-Tree index. You can simplify by either defining the PK in theCREATE TABLE
or dropping the separateCREATE UNIQUE INDEX
step.
19-25
: Unnecessary NOT VALID on new table
TheNOT VALID
/VALIDATE
pattern is intended for deferring constraint checks on large existing tables. Sincewhiteboards
is brand-new, removeNOT VALID
clauses to streamline this migration.packages/types/src/supabase.ts (4)
7981-7981
: Fix function signature formattingThe parameter appears to have inconsistent formatting.
get_workspace_transactions_count: { - Args: { ws_id: string; start_date?: string; end_date?: string }; + Args: { ws_id: string; start_date?: string; end_date?: string }; Returns: number; };
8008-8011
: Fix parameter alignment in get_workspace_users functionThe parameters appear to have inconsistent indentation.
get_workspace_users: { Args: { _ws_id: string; - included_groups: string[]; - excluded_groups: string[]; - search_query: string; + included_groups: string[]; + excluded_groups: string[]; + search_query: string; };
8064-8064
: Fix parameter name inconsistency in function signaturesSeveral functions have parameter names that appear misaligned or inconsistent.
is_member_invited: { - Args: { _user_id: string; _org_id: string }; + Args: { _user_id: string; _org_id: string }; Returns: boolean; }; is_nova_user_id_in_team: { - Args: { _user_id: string; _team_id: string }; + Args: { _user_id: string; _team_id: string }; Returns: boolean; }; is_org_member: { - Args: { _user_id: string; _org_id: string }; + Args: { _user_id: string; _org_id: string }; Returns: boolean; };Also applies to: 8080-8080, 8084-8084
8185-8185
: Fix parameter order inconsistencyThe parameter order is inconsistent compared to the function name pattern.
transactions_have_same_abs_amount: { - Args: { transaction_id_1: string; transaction_id_2: string }; + Args: { transaction_id_1: string; transaction_id_2: string }; Returns: boolean; };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
apps/db/supabase/migrations/20250620034755_create_whiteboards.sql
(1 hunks)apps/db/supabase/migrations/20250620034855_add_whiteboards_rls.sql
(1 hunks)apps/web/messages/en.json
(1 hunks)apps/web/messages/vi.json
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx
(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/loading.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/createWhiteboardDialog.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/editWhiteboardDialog.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/page.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx
(1 hunks)packages/types/src/supabase.ts
(12 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx (1)
CustomTldraw
(12-46)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx (2)
packages/ui/src/components/ui/form.tsx (6)
Form
(181-181)FormField
(184-184)FormItem
(185-185)FormLabel
(186-186)FormControl
(182-182)FormMessage
(187-187)packages/ui/src/components/ui/textarea.tsx (1)
Textarea
(17-17)
🪛 GitHub Actions: Vercel Famigo Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
🪛 GitHub Actions: Vercel Calendar Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
🪛 GitHub Actions: Vercel Platform Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
🪛 GitHub Actions: Vercel Rewise Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
🪛 GitHub Actions: Vercel Nova Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
🪛 GitHub Actions: Vercel Upskii Preview Deployment
packages/types/src/supabase.ts
[error] 7715-7715: Type error: Duplicate identifier 'p_origin_app'.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (12)
apps/web/messages/vi.json (1)
291-294
: Verify localization keys and naming consistency
Please ensure these new entries—"not_completed"
,"whiteboards"
,"whiteboards_description"
, and"new_whiteboard"
—exactly match the keys used in the English file (apps/web/messages/en.json
) (including casing and separators) and that they don’t duplicate or conflict with existing translations (e.g."incomplete"
).apps/web/messages/en.json (1)
294-296
: Ensure translations in other locales
These new keys (whiteboards
,whiteboards_description
,new_whiteboard
) have been added to the English file—please verify that corresponding entries are added to all other locale JSONs (e.g.,vi.json
) so the UI isn’t left untranslated.apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/loading.tsx (1)
1-8
: LGTM! Clean and appropriate loading skeleton.The loading component provides a good user experience with appropriate skeleton placeholders and smooth pulse animation. The layout structure matches what users would expect for a whiteboard interface.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx (2)
12-18
: LGTM! Well-designed component interface.The refactored props interface is clean and appropriate for the database-backed whiteboard model. The optional
initialData
prop allows for graceful handling of whiteboards without snapshots.
38-41
: Good integration of custom toolbar.The component correctly integrates the custom Toolbar through the SharePanel override, maintaining the tldraw interface while adding whiteboard-specific functionality.
apps/db/supabase/migrations/20250620034855_add_whiteboards_rls.sql (1)
1-5
: LGTM! Appropriate indexes for performance optimization.The indexes are well-chosen:
- B-tree indexes on foreign keys will improve join performance
- GIN index on the JSON snapshot column will optimize any future JSON queries
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx (2)
19-24
: LGTM! Appropriate validation schema.The Zod schema provides sensible validation with a minimum title length and optional description. The validation rules are appropriate for whiteboard metadata.
43-50
: Good form setup with proper defaults.The form configuration correctly uses zodResolver and spreads defaultValues appropriately, ensuring the form works for both create and edit scenarios.
apps/db/supabase/migrations/20250620034755_create_whiteboards.sql (2)
2-2
: Ensure pgcrypto extension exists
TheDEFAULT gen_random_uuid()
call requires thepgcrypto
extension. Confirm that a prior migration includes:CREATE EXTENSION IF NOT EXISTS pgcrypto;so this script won’t fail at deploy time.
13-13
: RLS policy ordering
Enabling RLS without any policies will block all operations onwhiteboards
. Ensure the subsequent migration that defines row-level policies runs immediately after this one—or combine them—to avoid unintended access denial.packages/types/src/supabase.ts (2)
4739-4803
: Whiteboard table definition looks well-structuredThe new
whiteboards
table definition is properly implemented with appropriate field types, relationships, and constraints. The table includes all necessary fields for a whiteboard feature:
- Proper foreign key relationships to
users
andworkspaces
tables- JSON field for snapshot data storage
- Nullable fields where appropriate (description, snapshot, thumbnail_url)
- Consistent naming conventions with existing tables
7733-7737
: Fix duplicate parameter in get_challenge_stats functionThere's a duplicate
problems_attempted
field in the return type.get_challenge_stats: { Args: { challenge_id_param: string; user_id_param: string }; Returns: { total_score: number; - problems_attempted: number; }[]; };
Likely an incorrect or invalid review comment.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
Outdated
Show resolved
Hide resolved
apps/db/supabase/migrations/20250620034755_create_whiteboards.sql
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3137 +/- ##
==========================================
- Coverage 0.80% 0.80% -0.01%
==========================================
Files 2554 2561 +7
Lines 326716 327702 +986
Branches 3056 3063 +7
==========================================
Hits 2623 2623
- Misses 321912 322891 +979
- Partials 2181 2188 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/types/src/supabase.ts (1)
7698-7706
: Duplicate union branch keepssearch_query
signature ambiguousThe first and third union members are identical (
{ search_query: string }
). Please drop one to avoid redundant overloads and cleaner IntelliSense.
♻️ Duplicate comments (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx (1)
29-33
: Add error handling for JSON parsing.The JSON.parse() operation could throw an exception if the snapshot data is malformed, which would crash the page. Add proper error handling to gracefully handle corrupted data.
Add try/catch around JSON parsing:
initialData={ - whiteboard.snapshot - ? (JSON.parse(whiteboard.snapshot as string) as TLStoreSnapshot) - : undefined + whiteboard.snapshot + ? (() => { + try { + return JSON.parse(whiteboard.snapshot as string) as TLStoreSnapshot; + } catch (error) { + console.warn('Failed to parse whiteboard snapshot:', error); + return undefined; + } + })() + : undefined }
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx (1)
385-391
: Consider implementing share functionality.The share option is currently a placeholder without functionality. This may confuse users who expect it to work.
Consider either:
- Implementing the share functionality
- Temporarily hiding/disabling this option until implemented
- Adding a "Coming soon" indicator
Would you like me to help implement the share functionality or suggest an approach for handling this placeholder state?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx
(2 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
(1 hunks)packages/types/src/supabase.ts
(8 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx (1)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx (1)
CustomTldraw
(18-50)
🪛 GitHub Check: codecov/patch
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
[warning] 2-37: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx#L2-L37
Added lines #L2 - L37 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx
[warning] 3-3: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx#L3
Added line #L3 was not covered by tests
[warning] 7-7: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx#L7
Added line #L7 was not covered by tests
[warning] 12-22: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx#L12-L22
Added lines #L12 - L22 were not covered by tests
[warning] 42-45: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx#L42-L45
Added lines #L42 - L45 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
[warning] 2-112: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx#L2-L112
Added lines #L2 - L112 were not covered by tests
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
[warning] 2-422: apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx#L2-L422
Added lines #L2 - L422 were not covered by tests
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Prettier Check (23)
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (17)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/custom-tldraw.tsx (2)
12-22
: LGTM! Well-structured component interface.The props interface is clearly defined and the component accepts the necessary parameters for workspace and board identification along with optional initial snapshot data.
41-47
: Good integration with TLDraw and custom toolbar.The component properly passes the initial snapshot data to TLDraw and correctly overrides the SharePanel with the custom Toolbar component, passing the required workspace and board identifiers.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx (1)
15-20
: Good security implementation with workspace validation.The query correctly validates both the board ID and workspace ID, preventing unauthorized access to whiteboards from other workspaces.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx (3)
21-35
: Well-implemented thumbnail generation with proper error handling.The thumbnail generation correctly checks for shapes on canvas and uses appropriate scaling and padding settings for the image export.
49-97
: Comprehensive save implementation with proper error handling.The save function properly handles the entire workflow: thumbnail generation, file upload, database update, and user feedback. Error handling is appropriately implemented throughout the process, and the useCallback dependencies are complete.
51-54
: Good null check for thumbnail blob.The implementation correctly validates that the thumbnail generation succeeded before proceeding, preventing potential errors downstream.
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx (4)
48-56
: Well-defined interface for whiteboard data.The Whiteboard interface properly captures all necessary fields including metadata, optional description and thumbnail, and creator information.
93-108
: Efficient sorting and filtering implementation.The sorting logic properly handles different sort options with appropriate date comparisons and locale-aware string sorting for alphabetical ordering.
110-120
: User-friendly date formatting.The relative date formatting provides intuitive display of timestamps, showing "Today", "Yesterday", and days ago for recent dates before falling back to full date strings.
341-358
: Proper error handling and user feedback in delete operation.The delete function includes comprehensive error handling, appropriate user notifications via toast messages, and refreshes the router to update the UI state.
packages/types/src/supabase.ts (7)
4739-4803
: Whiteboards table looks soundNew type mirrors the SQL schema and relationships; no structural or typing issues spotted. Good addition.
7730-7734
: Return field addition OK
problems_attempted
field complementstotal_score
; signature matches frontend usage.
7971-7972
: Arg order fixed
ws_id
is now the first parameter—matches function definition.
7998-8000
: Verify arg ordering vs. SQL
get_workspace_users
now lists_ws_id
first, followed by group filters andsearch_query
. Double-check the SQL function to ensure the new order is correct; mismatches will break generated types.
8054-8056
: Arg order fixed
is_member_invited
now uses_user_id, _org_id
— resolves earlier review.
8070-8072
: Arg order fixed
is_nova_user_id_in_team
now lists_user_id
before_team_id
; thanks.
8074-8076
: Arg order fixed
is_org_member
arguments now consistent with SQL signature.
…rds and update related queries and policies
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.
LGTM! Thanks @khangronky.
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.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/types/src/supabase.ts (1)
8174-8179
: Parameter order still inverted intransactions_have_same_amount
Args: { transaction_id_2; transaction_id_1 }
is the reverse of the already-fixed siblingtransactions_have_same_abs_amount
.
Swap them for consistency and to match the SQL function signature.-Args: { transaction_id_2: string; transaction_id_1: string }; +Args: { transaction_id_1: string; transaction_id_2: string };
♻️ Duplicate comments (3)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx (2)
80-135
: Past review comments have been addressed.The
save
function now includes all necessary dependencies in the useCallback hook (line 135), proper error handling for database operations (lines 107-116), and the navigation link correctly includes the workspace ID (line 139).
82-85
: Thumbnail generation error handling is properly implemented.The code now includes proper null checking for
thumbnailBlob
as suggested in past reviews, preventing potential errors if thumbnail generation fails.apps/db/supabase/migrations/20250620034755_create_whiteboards.sql (1)
35-37
: Security issue from past review remains unresolved.The migration still grants excessive
TRUNCATE
andTRIGGER
privileges toanon
andauthenticated
roles, violating least-privilege principles as previously identified.Remove these excessive privileges:
-grant trigger on table "public"."workspace_whiteboards" to "anon"; -grant truncate on table "public"."workspace_whiteboards" to "anon"; -grant trigger on table "public"."workspace_whiteboards" to "authenticated"; -grant truncate on table "public"."workspace_whiteboards" to "authenticated";Keep only the necessary privileges (
SELECT
,INSERT
,UPDATE
,DELETE
) for these roles.Also applies to: 49-51
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/page.tsx (1)
14-41
: LGTM with minor improvement suggestion.The
getWhiteboards
function is well-structured with proper error handling and data transformation. The Supabase query correctly joins with the users table to fetch creator information and orders by last update.Consider adding pagination support for better performance with large datasets:
-async function getWhiteboards(wsId: string): Promise<Whiteboard[]> { +async function getWhiteboards(wsId: string, limit = 50, offset = 0): Promise<Whiteboard[]> { const supabase = await createClient(); const { data: whiteboards, error } = await supabase .from('workspace_whiteboards') .select( `*, creator:users(display_name) ` ) .eq('ws_id', wsId) .order('updated_at', { ascending: false }) + .range(offset, offset + limit - 1);apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx (1)
52-66
: Add error handling for empty canvas edge case.The
generateThumbnail
function throws an error when no shapes exist, but this might not be the desired behavior for users who want to save an empty whiteboard.Consider allowing empty whiteboards to be saved with a default thumbnail:
const generateThumbnail = async () => { const shapeIds = editor.getCurrentPageShapeIds(); if (shapeIds.size === 0) { - throw new Error('No shapes on canvas'); + // Return a blank canvas thumbnail for empty whiteboards + const { blob } = await editor.toImage([], { + format: 'png', + background: true, + scale: 0.3, + padding: 16, + }); + return blob; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/db/supabase/migrations/20250620034755_create_whiteboards.sql
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/createWhiteboardDialog.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/editWhiteboardDialog.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/page.tsx
(1 hunks)apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx
(1 hunks)packages/types/src/supabase.ts
(17 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/page.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/createWhiteboardDialog.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/editWhiteboardDialog.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/whiteboardForm.tsx
- apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/client.tsx
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Deploy-Preview
- GitHub Check: Verify generated types
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
- GitHub Check: Deploy-Preview
🔇 Additional comments (7)
apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/page.tsx (1)
43-83
: Well-implemented async component with proper error handling.The component correctly handles the async data fetching pattern and provides appropriate error handling with
notFound()
on failure. The UI structure is clean and well-organized.apps/web/src/app/[locale]/(dashboard)/[wsId]/whiteboards/[boardId]/toolbar.tsx (1)
92-104
: Verify file upload overwrite behavior.The current implementation uploads files with timestamped names, which means old thumbnails won't be automatically cleaned up from storage.
#!/bin/bash # Check if there's any cleanup mechanism for old thumbnail files ast-grep --pattern 'supabase.storage.from($bucket).remove($path)'Consider implementing a cleanup mechanism or using a consistent filename to overwrite previous thumbnails.
apps/db/supabase/migrations/20250620034755_create_whiteboards.sql (3)
1-11
: Well-designed table schema for whiteboard management.The table structure is comprehensive with appropriate data types, foreign key relationships, and cascading delete behavior. The JSONB snapshot column is well-suited for storing serialized whiteboard state.
75-88
: Robust RLS policy implementation.The row-level security policy correctly restricts access to workspace members only, checking both read and write operations against workspace membership.
69-73
: Efficient indexing strategy.The indexes on
ws_id
,creator_id
, and the GIN index on the JSONBsnapshot
column will provide good query performance for common access patterns.packages/types/src/supabase.ts (2)
7060-7124
: Whiteboard table definition added – looks soundSchema, relationships, and nullable columns align with existing patterns. 👍
7682-7682
: Double-check argument order ofcalculate_productivity_score
(category_color, duration_seconds)
differs from the more intuitive(duration_seconds, category_color)
found in older migrations. Verify the PL/pgSQL signature to avoid silent runtime mismatches.
Summary by CodeRabbit
New Features
Localization
Bug Fixes
Style
Chores