-
Notifications
You must be signed in to change notification settings - Fork 11
Feat/collab workspaces #157
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
Changes from all commits
0e8fc73
ef451b9
24fab5f
4ea39d3
81d94c3
aff9996
b93fd7f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,156 @@ | ||
| -- ============================================================================= | ||
| -- Workspace Collaborators Table for Real-Time Collaboration | ||
| -- ============================================================================= | ||
|
|
||
| CREATE TABLE "workspace_collaborators" ( | ||
| "id" uuid PRIMARY KEY DEFAULT gen_random_uuid() NOT NULL, | ||
| "workspace_id" uuid NOT NULL, | ||
| "user_id" text NOT NULL, | ||
| "permission_level" text DEFAULT 'editor' NOT NULL, | ||
| "invite_token" text, | ||
| "created_at" timestamp with time zone DEFAULT now(), | ||
| CONSTRAINT "workspace_collaborators_invite_token_unique" UNIQUE("invite_token"), | ||
| CONSTRAINT "workspace_collaborators_workspace_user_unique" UNIQUE("workspace_id", "user_id") | ||
| ); | ||
| --> statement-breakpoint | ||
| ALTER TABLE "workspace_collaborators" ENABLE ROW LEVEL SECURITY;--> statement-breakpoint | ||
| ALTER TABLE "workspace_collaborators" ADD CONSTRAINT "workspace_collaborators_workspace_id_fkey" FOREIGN KEY ("workspace_id") REFERENCES "public"."workspaces"("id") ON DELETE cascade ON UPDATE no action;--> statement-breakpoint | ||
| CREATE INDEX "idx_workspace_collaborators_lookup" ON "workspace_collaborators" USING btree ("user_id" text_ops, "workspace_id" uuid_ops);--> statement-breakpoint | ||
| CREATE INDEX "idx_workspace_collaborators_workspace" ON "workspace_collaborators" USING btree ("workspace_id" uuid_ops);--> statement-breakpoint | ||
|
|
||
| -- RLS Policies for workspace_collaborators | ||
| CREATE POLICY "Owners can manage collaborators" ON "workspace_collaborators" AS PERMISSIVE FOR ALL TO "authenticated" | ||
| USING (EXISTS ( | ||
| SELECT 1 FROM workspaces w | ||
| WHERE w.id = workspace_collaborators.workspace_id | ||
| AND w.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ));--> statement-breakpoint | ||
|
|
||
| CREATE POLICY "Collaborators can view their access" ON "workspace_collaborators" AS PERMISSIVE FOR SELECT TO "authenticated" | ||
| USING (user_id = (auth.jwt() ->> 'sub'::text));--> statement-breakpoint | ||
|
|
||
| -- ============================================================================= | ||
| -- Broadcast Trigger for workspace_events | ||
| -- Automatically broadcasts when an event is inserted | ||
| -- ============================================================================= | ||
|
|
||
| CREATE OR REPLACE FUNCTION workspace_events_broadcast_trigger() | ||
| RETURNS TRIGGER | ||
| LANGUAGE plpgsql | ||
| SECURITY DEFINER | ||
| AS $$ | ||
| BEGIN | ||
| -- Broadcast the event to the workspace channel | ||
| -- All clients subscribed to workspace:<id>:events will receive this | ||
| PERFORM realtime.broadcast_changes( | ||
| 'workspace:' || NEW.workspace_id::text || ':events', | ||
| TG_OP, -- operation type: INSERT | ||
| TG_OP, -- event name: INSERT | ||
| TG_TABLE_NAME, -- table name | ||
| TG_TABLE_SCHEMA, -- schema | ||
| NEW, -- new row data | ||
| OLD -- old row data (null for INSERT) | ||
| ); | ||
| RETURN NEW; | ||
| END; | ||
| $$;--> statement-breakpoint | ||
|
|
||
| CREATE TRIGGER workspace_events_realtime_broadcast | ||
| AFTER INSERT ON workspace_events | ||
| FOR EACH ROW EXECUTE FUNCTION workspace_events_broadcast_trigger();--> statement-breakpoint | ||
|
|
||
| -- ============================================================================= | ||
| -- RLS Policies for realtime.messages | ||
| -- Authorizes who can subscribe/publish to workspace channels | ||
| -- ============================================================================= | ||
|
|
||
| -- Allow workspace owner OR collaborators to receive events | ||
| CREATE POLICY "workspace_access_can_read" ON realtime.messages | ||
| FOR SELECT TO authenticated | ||
| USING ( | ||
| topic LIKE 'workspace:%:events' | ||
| AND ( | ||
| -- Owner access | ||
| EXISTS ( | ||
| SELECT 1 FROM public.workspaces w | ||
| WHERE w.id = (SPLIT_PART(topic, ':', 2))::uuid | ||
| AND w.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| OR | ||
| -- Collaborator access | ||
| EXISTS ( | ||
| SELECT 1 FROM public.workspace_collaborators c | ||
| WHERE c.workspace_id = (SPLIT_PART(topic, ':', 2))::uuid | ||
| AND c.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| ) | ||
| );--> statement-breakpoint | ||
|
|
||
| -- Allow workspace owner OR editor collaborators to send events (presence) | ||
| CREATE POLICY "workspace_access_can_write" ON realtime.messages | ||
| FOR INSERT TO authenticated | ||
| WITH CHECK ( | ||
| topic LIKE 'workspace:%:events' | ||
| AND ( | ||
| -- Owner access | ||
| EXISTS ( | ||
| SELECT 1 FROM public.workspaces w | ||
| WHERE w.id = (SPLIT_PART(topic, ':', 2))::uuid | ||
| AND w.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| OR | ||
| -- Editor collaborator access | ||
| EXISTS ( | ||
| SELECT 1 FROM public.workspace_collaborators c | ||
| WHERE c.workspace_id = (SPLIT_PART(topic, ':', 2))::uuid | ||
| AND c.user_id = (auth.jwt() ->> 'sub'::text) | ||
| AND c.permission_level = 'editor' | ||
| ) | ||
| ) | ||
| );--> statement-breakpoint | ||
|
|
||
| -- ============================================================================= | ||
| -- Update existing RLS policies on workspace_events to include collaborators | ||
| -- ============================================================================= | ||
|
|
||
| -- Drop old policy that only allows owner | ||
| DROP POLICY IF EXISTS "Users can insert workspace events they have write access to" ON "workspace_events";--> statement-breakpoint | ||
|
|
||
| -- New policy: owners AND editor collaborators can insert events | ||
| CREATE POLICY "Users can insert workspace events they have write access to" ON "workspace_events" AS PERMISSIVE FOR INSERT TO public | ||
| WITH CHECK ( | ||
| -- Owner access | ||
| EXISTS ( | ||
| SELECT 1 FROM workspaces | ||
| WHERE workspaces.id = workspace_events.workspace_id | ||
| AND workspaces.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| OR | ||
| -- Editor collaborator access | ||
| EXISTS ( | ||
| SELECT 1 FROM workspace_collaborators c | ||
| WHERE c.workspace_id = workspace_events.workspace_id | ||
| AND c.user_id = (auth.jwt() ->> 'sub'::text) | ||
| AND c.permission_level = 'editor' | ||
| ) | ||
| );--> statement-breakpoint | ||
|
Comment on lines
+120
to
+136
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Policy target inconsistent: The workspace_events insert policy uses 🛡️ Suggested fix for consistency-CREATE POLICY "Users can insert workspace events they have write access to" ON "workspace_events" AS PERMISSIVE FOR INSERT TO public
+CREATE POLICY "Users can insert workspace events they have write access to" ON "workspace_events" AS PERMISSIVE FOR INSERT TO authenticated
WITH CHECK (
-- ... rest of policy
);
-CREATE POLICY "Users can read workspace events they have access to" ON "workspace_events" AS PERMISSIVE FOR SELECT TO public
+CREATE POLICY "Users can read workspace events they have access to" ON "workspace_events" AS PERMISSIVE FOR SELECT TO authenticated
USING (
-- ... rest of policy
);🤖 Prompt for AI Agents |
||
|
|
||
| -- Update select policy to include collaborators | ||
| DROP POLICY IF EXISTS "Users can read workspace events they have access to" ON "workspace_events";--> statement-breakpoint | ||
|
|
||
| CREATE POLICY "Users can read workspace events they have access to" ON "workspace_events" AS PERMISSIVE FOR SELECT TO public | ||
| USING ( | ||
| -- Owner access | ||
| EXISTS ( | ||
| SELECT 1 FROM workspaces | ||
| WHERE workspaces.id = workspace_events.workspace_id | ||
| AND workspaces.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| OR | ||
| -- Collaborator access (viewer or editor) | ||
| EXISTS ( | ||
| SELECT 1 FROM workspace_collaborators c | ||
| WHERE c.workspace_id = workspace_events.workspace_id | ||
| AND c.user_id = (auth.jwt() ->> 'sub'::text) | ||
| ) | ||
| ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,6 +111,7 @@ | |
| "postgres": "^3.4.7", | ||
| "posthog-js": "^1.335.5", | ||
| "posthog-node": "^5.21.1", | ||
| "radix-ui": "^1.4.3", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion | 🟠 Major 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check which `@radix-ui` packages are currently used and where
echo "=== Individual `@radix-ui` packages in package.json ==="
rg '@radix-ui/react-' package.json
echo ""
echo "=== Files importing from `@radix-ui/`* ==="
rg "from ['\"]@radix-ui" --type ts --type tsx -l 2>/dev/null | head -20
echo ""
echo "=== Files importing from unified radix-ui ==="
rg "from ['\"]radix-ui" --type ts --type tsx -l 2>/dev/null | head -20
echo ""
echo "=== Radix packages section in package.json ==="
sed -n '60,120p' package.jsonRepository: ThinkEx-OSS/thinkex Length of output: 3127 🏁 Script executed: #!/bin/bash
# Search for any imports in all file types
echo "=== All imports from `@radix-ui` (any file type) ==="
rg "import.*from\s+['\"]@radix-ui" -g "!node_modules" 2>/dev/null | head -30
echo ""
echo "=== All imports from radix-ui (any file type) ==="
rg "import.*from\s+['\"]radix-ui" -g "!node_modules" 2>/dev/null | head -30
echo ""
echo "=== Check if any components exist in src directory ==="
find src -type f \( -name "*.ts" -o -name "*.tsx" -o -name "*.js" -o -name "*.jsx" \) 2>/dev/null | head -10
echo ""
echo "=== Look for select.tsx mentioned in comment ==="
find . -name "select.tsx" -type f 2>/dev/null
echo ""
echo "=== Check for ui components directory pattern ==="
find . -type d -name "ui" 2>/dev/null | grep -v node_modulesRepository: ThinkEx-OSS/thinkex Length of output: 2639 🌐 Web query:
💡 Result:
So, compatibility-wise:
If you must mix (monorepo / incremental migration), pin versions so there’s only one copy:
Sources: Citations:
Consolidate Radix UI packages to avoid runtime bugs from version mismatch. The codebase mixes the unified This can cause runtime issues including context mismatch, ref-forwarding failures, and portal/focus-scope bugs. Migrate all Radix imports to either:
🤖 Prompt for AI Agents |
||
| "react": "19.2.1", | ||
| "react-color": "^2.19.3", | ||
| "react-dom": "19.2.1", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| /** | ||
| * Collaborator API - Update and delete individual collaborators | ||
| * | ||
| * PATCH /api/workspaces/[id]/collaborators/[collaboratorId] - Update permission | ||
| * DELETE /api/workspaces/[id]/collaborators/[collaboratorId] - Remove collaborator | ||
| */ | ||
|
|
||
| import { NextRequest, NextResponse } from "next/server"; | ||
| import { headers } from "next/headers"; | ||
| import { auth } from "@/lib/auth"; | ||
| import { db } from "@/lib/db/client"; | ||
| import { workspaceCollaborators } from "@/lib/db/schema"; | ||
| import { eq, and } from "drizzle-orm"; | ||
| import { verifyWorkspaceOwnership } from "@/lib/api/workspace-helpers"; | ||
|
|
||
| export async function PATCH( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ id: string; collaboratorId: string }> } | ||
| ) { | ||
| try { | ||
| const session = await auth.api.getSession({ headers: await headers() }); | ||
| if (!session?.user?.id) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } | ||
|
|
||
| const { id: workspaceId, collaboratorId } = await params; | ||
| const body = await request.json(); | ||
| const { permissionLevel } = body; | ||
|
|
||
| if (!permissionLevel || !["viewer", "editor"].includes(permissionLevel)) { | ||
| return NextResponse.json({ error: "Invalid permission level" }, { status: 400 }); | ||
| } | ||
|
|
||
| // Verify ownership | ||
| try { | ||
|
Comment on lines
+29
to
+35
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [P1] Swallowing ownership-check errors returns the wrong status code
Consider:
Also appears in Prompt To Fix With AIThis is a comment left during a code review.
Path: src/app/api/workspaces/[id]/collaborators/[collaboratorId]/route.ts
Line: 29:35
Comment:
[P1] Swallowing ownership-check errors returns the wrong status code
`verifyWorkspaceOwnership()` throws a `NextResponse` with either 404 (not found) or 403 (access denied), but the `catch { return 404 }` block collapses both cases into a 404. That makes unauthorized users look like the workspace doesn't exist, and also hides real 403s from the client.
Consider:
- `catch (e) { if (e instanceof Response) return e; throw e; }`
Also appears in `DELETE` in the same file.
How can I resolve this? If you propose a fix, please make it concise. |
||
| await verifyWorkspaceOwnership(workspaceId, session.user.id); | ||
| } catch (error) { | ||
| if (error instanceof Response) return error; | ||
| return NextResponse.json({ error: "Workspace not found" }, { status: 404 }); | ||
|
cubic-dev-ai[bot] marked this conversation as resolved.
|
||
| } | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
|
|
||
| // Update the collaborator | ||
| const [updated] = await db | ||
| .update(workspaceCollaborators) | ||
| .set({ permissionLevel }) | ||
| .where( | ||
| and( | ||
| eq(workspaceCollaborators.id, collaboratorId), | ||
| eq(workspaceCollaborators.workspaceId, workspaceId) | ||
| ) | ||
| ) | ||
| .returning(); | ||
|
|
||
| if (!updated) { | ||
| return NextResponse.json({ error: "Collaborator not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| return NextResponse.json({ collaborator: updated }); | ||
| } catch (error) { | ||
| console.error("Error updating collaborator:", error); | ||
| return NextResponse.json({ error: "Internal server error" }, { status: 500 }); | ||
| } | ||
| } | ||
|
|
||
| export async function DELETE( | ||
| request: NextRequest, | ||
| { params }: { params: Promise<{ id: string; collaboratorId: string }> } | ||
| ) { | ||
| try { | ||
| const session = await auth.api.getSession({ headers: await headers() }); | ||
| if (!session?.user?.id) { | ||
| return NextResponse.json({ error: "Unauthorized" }, { status: 401 }); | ||
| } | ||
|
|
||
| const { id: workspaceId, collaboratorId } = await params; | ||
|
|
||
| // Verify ownership | ||
| try { | ||
| await verifyWorkspaceOwnership(workspaceId, session.user.id); | ||
| } catch (error) { | ||
| if (error instanceof Response) return error; | ||
| return NextResponse.json({ error: "Workspace not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| // Delete the collaborator | ||
| const [deleted] = await db | ||
| .delete(workspaceCollaborators) | ||
| .where( | ||
| and( | ||
| eq(workspaceCollaborators.id, collaboratorId), | ||
| eq(workspaceCollaborators.workspaceId, workspaceId) | ||
| ) | ||
| ) | ||
| .returning(); | ||
|
|
||
| if (!deleted) { | ||
| return NextResponse.json({ error: "Collaborator not found" }, { status: 404 }); | ||
| } | ||
|
|
||
| return NextResponse.json({ success: true }); | ||
| } catch (error) { | ||
| console.error("Error deleting collaborator:", error); | ||
| return NextResponse.json({ error: "Internal server error" }, { status: 500 }); | ||
| } | ||
| } | ||
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] Migration adds
realtime.messagesRLS policies, but app uses broadcast channels insteadThis migration sets RLS policies on
realtime.messagesfor topics likeworkspace:%:events, but the client implementation (src/hooks/workspace/use-workspace-realtime.ts) uses a broadcast channel namedworkspace-${workspaceId}and explicitly says "no RLS needed". As written, therealtime.messagespolicies and theworkspace_events_broadcast_trigger()appear unused, and they add operational complexity/migration surface area.If the intended architecture is broadcast-only, consider removing the trigger +
realtime.messagespolicies. If the intended architecture is DB-triggered changes, the client should subscribe to the topic format the policies protect.Prompt To Fix With AI