Skip to content

Conversation

@ameer2468
Copy link
Collaborator

@ameer2468 ameer2468 commented Oct 23, 2025

Summary by CodeRabbit

  • New Features

    • Create-space API returns richer responses (id, name, iconUrl, error).
    • New public types for space-member id and role.
  • Improvements

    • Duplicate space-name check per organization.
    • Space creation continues if icon upload fails; icon upload is deferred.
    • Creator is always included as Admin; member lists are deduplicated and replaced atomically.
    • Client/UI icon validation and label updated to 1 MB limit.
    • Dashboard/member views simplified; pages refresh after create/update.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 23, 2025

Walkthrough

Space creation now inserts the DB record inside a transaction, creates members (creator forced as Admin) from provided user IDs, defers icon upload to a helper that uses arrayBuffer/putObject with a 1MB limit, and expands the CreateSpaceResponse shape; UI and dashboard queries adjusted accordingly.

Changes

Cohort / File(s) Summary
Space creation & API surface
apps/web/actions/organization/create-space.ts
Insert space record up front in a transaction, validate name uniqueness, accept members as user IDs (ensure creator is Admin), defer icon upload via uploadSpaceIcon, and expand CreateSpaceResponse to include spaceId, name, iconUrl, and error.
Icon upload implementation
apps/web/actions/organization/upload-space-icon.ts, apps/web/components/forms/server.ts, apps/web/actions/organization/update-space.ts
Replace Effect.promise/bytes() usage with arrayBuffer()Uint8Array and putObject, lower server-side size limit to 1MB, and adjust upload/delete key handling; upload errors logged and non-fatal to space creation.
Member management & member APIs
apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts, apps/web/actions/organization/update-space.ts, packages/web-domain/src/Space.ts, packages/web-domain/src/index.ts
Rework member insert/update flows to delete-and-replace with deduplicated user IDs, ensure creator included as Admin, introduce/export branded SpaceMemberId and SpaceMemberRole, and switch ID generation to nanoid.
Client-side file validation & dialogs
apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
Add handleFileChange to validate image MIME and ≤1MB, show toast errors, adjust FileInput onChange, use formRef.requestSubmit() and call router.refresh() after successful create/update.
Dashboard UI & data adjustments
apps/web/app/(org)/dashboard/_components/DashboardInner.tsx, apps/web/app/(org)/dashboard/dashboard-data.ts, apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx, apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
Remove MembersDialog state/rendering, remove memberImage from spaces projection, change membership check to SQL EXISTS, adjust SignedImageUrl prop usage, and add router.refresh() after member saves.
Small server/client I/O changes
apps/web/components/forms/server.ts, apps/web/actions/organization/upload-space-icon.ts
Read files via arrayBuffer() for S3 uploads and adapt upload payload accordingly; preserve permission checks and error handling.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Client as SpaceDialog (Client)
    participant Server as create-space (Server Action)
    participant IconSvc as uploadSpaceIcon (Upload Helper)
    participant DB as Database
    rect rgb(240,248,255)
    User->>Client: Select icon + submit form
    Client->>Client: validate file type & size (≤1MB)
    alt invalid
        Client-->>User: show toast error
    else valid
        Client->>Server: POST formData (fields + file)
    end
    end
    rect rgb(245,255,240)
    Server->>DB: begin transaction -> insert space (iconUrl=null)
    Server->>DB: insert spaceMembers (creator forced Admin)
    alt icon provided
        Server->>IconSvc: uploadSpaceIcon(file) after transaction
        IconSvc->>IconSvc: arrayBuffer() -> Uint8Array
        IconSvc->>DB: putObject(upload to S3)
        IconSvc-->>Server: return iconUrl or error (non-fatal)
        Server->>DB: update space.iconUrl if upload succeeded
    end
    Server-->>Client: return CreateSpaceResponse { success, spaceId, name, iconUrl? }
    Client->>User: show success/toast and refresh routes
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

Web, Improvement

Suggested reviewers

  • Brendonovich

Poem

🐰 I planted a Space before the icon's flight,
Admins hop in quickly, everything's right.
One-meg rule for images keeps the burrow snappy,
Upload may wobble — the Space stays happy.
Hooray for tidy hops and midnight carrots bright!

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The PR title "web: fix spaces" references the primary domain entity (spaces) that is being modified throughout the changeset, which provides some contextual relevance. However, the title is vague and generic in its use of the term "fix" without specifying what aspects of spaces are being addressed or what problems are being resolved. Given that the changeset includes major refactoring of space creation and member management flows, authorization logic changes, icon validation adjustments, and new type exports, the title fails to convey any meaningful information about which of these changes represents the main point of the PR. The term "fix" is non-specific and could apply to bug fixes, refactoring, structural improvements, or various other kinds of changes.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix-spaces

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
apps/web/components/forms/server.ts (1)

71-79: Inconsistent upload payload construction compared to upload-space-icon.ts.

This code passes the raw ArrayBuffer to putObject, while upload-space-icon.ts (lines 84, 87-89) converts to Uint8Array before upload:

const arrayBuffer = await sanitizedFile.arrayBuffer();
await bucket.putObject(fileKey, new Uint8Array(arrayBuffer), {
  contentType: file.type,
});

For consistency and to ensure S3 compatibility, apply this diff:

 yield* bucket.putObject(
   fileKey,
-  yield* Effect.promise(() => iconFile.arrayBuffer()),
+  new Uint8Array(yield* Effect.promise(() => iconFile.arrayBuffer())),
   { contentType: iconFile.type },
 );
🧹 Nitpick comments (2)
apps/web/actions/organization/upload-space-icon.ts (1)

83-90: Consider using sanitizedFile.type for content type consistency.

The code uses file.type (original file) for the content type but uploads sanitizedFile. If sanitizeFile modifies the image format, this could result in incorrect content type headers.

Consider this change:

 const sanitizedFile = await sanitizeFile(file);
 const arrayBuffer = await sanitizedFile.arrayBuffer();

 await bucket
   .putObject(fileKey, new Uint8Array(arrayBuffer), {
-    contentType: file.type,
+    contentType: sanitizedFile.type,
   })
   .pipe(runPromise);
apps/web/actions/organization/create-space.ts (1)

43-60: Consider case-insensitive duplicate space name validation.

The duplicate name check is case-sensitive, meaning "Analytics" and "analytics" would both be allowed. This could lead to user confusion.

Consider using a case-insensitive comparison:

 const existingSpace = await db()
   .select({ id: spaces.id })
   .from(spaces)
   .where(
     and(
       eq(spaces.organizationId, user.activeOrganizationId),
-      eq(spaces.name, name),
+      eq(sql`LOWER(${spaces.name})`, name.toLowerCase()),
     ),
   )
   .limit(1);

Note: You'll need to import sql from drizzle-orm.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e16d50 and edfbd73.

📒 Files selected for processing (7)
  • apps/web/actions/organization/create-space.ts (2 hunks)
  • apps/web/actions/organization/upload-space-icon.ts (3 hunks)
  • apps/web/app/(org)/dashboard/_components/DashboardInner.tsx (0 hunks)
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (3 hunks)
  • apps/web/app/(org)/dashboard/dashboard-data.ts (0 hunks)
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx (1 hunks)
  • apps/web/components/forms/server.ts (1 hunks)
💤 Files with no reviewable changes (2)
  • apps/web/app/(org)/dashboard/_components/DashboardInner.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Use strict TypeScript and avoid any; leverage shared types

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/upload-space-icon.ts
  • apps/web/components/forms/server.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/upload-space-icon.ts
  • apps/web/components/forms/server.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

On the client, always use useEffectQuery or useEffectMutation from @/lib/EffectRuntime; never call EffectRuntime.run* directly in components.

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/upload-space-icon.ts
  • apps/web/components/forms/server.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/upload-space-icon.ts
  • apps/web/components/forms/server.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
apps/web/actions/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

All Groq/OpenAI calls must execute in Next.js Server Actions under apps/web/actions; do not invoke AI providers elsewhere

Files:

  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/upload-space-icon.ts
🧬 Code graph analysis (1)
apps/web/actions/organization/create-space.ts (4)
packages/database/index.ts (1)
  • db (18-25)
packages/database/schema.ts (2)
  • spaces (591-619)
  • spaceMembers (621-646)
apps/web/actions/organization/upload-space-icon.ts (1)
  • uploadSpaceIcon (14-103)
packages/database/helpers.ts (1)
  • nanoIdLength (3-3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (8)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersDialog.tsx (1)

46-51: LGTM! Cleaner prop handling.

The simplification from member.user?.memberImage || undefined to member.user?.memberImage is correct. The type definition (line 18) already allows ImageUrl | null, so the unnecessary coercion to undefined can be removed.

apps/web/actions/organization/upload-space-icon.ts (2)

9-9: LGTM! Proper cleanup of unused imports.

Removing the Effect import is correct since Effect.promise is no longer used after refactoring the file reading to use plain async/await.


50-52: Verify the file size limit reduction from 2MB to 1MB is intentional.

The file size limit was reduced from 2MB to 1MB. This is a breaking change that affects existing users who may have been uploading larger icons. The client-side validation in SpaceDialog.tsx (line 162) matches this 1MB limit.

If this reduction is intentional, consider:

  1. Documenting this breaking change in release notes
  2. Verifying that existing icons larger than 1MB continue to work (grandfather clause)
apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (2)

159-173: LGTM! Excellent client-side validation implementation.

The handleFileChange function properly validates file size (1MB) and type (image) before setting the selected file, providing immediate user feedback via toast notifications. This matches the server-side validation in upload-space-icon.ts (lines 47-52) and improves UX by catching errors early.


285-302: LGTM! UI properly reflects validation changes.

The description correctly indicates the 1MB limit (line 288), and the FileInput component is properly wired to use handleFileChange (line 298) for validation before state updates.

apps/web/actions/organization/create-space.ts (3)

6-11: LGTM! Imports properly updated for refactored flow.

The import changes correctly reflect the new architecture: removed S3/Effect dependencies, added spaceMembers schema and uploadSpaceIcon helper, and included and for compound queries.


13-19: LGTM! Enhanced response type provides better context.

The expanded CreateSpaceResponse interface now includes spaceId, name, iconUrl, and error fields, enabling callers to access created space details and distinguish between different failure modes. This is a good API design improvement.


76-91: LGTM! Excellent resilience pattern for icon uploads.

The space is created first, then icon upload is delegated to uploadSpaceIcon with proper error handling. If the icon upload fails, the space still exists (without an icon) and the error is logged but doesn't fail the overall operation. This provides a better user experience compared to failing the entire space creation.

@ameer2468 ameer2468 merged commit 750b7e0 into main Oct 23, 2025
14 of 15 checks passed
@ameer2468 ameer2468 deleted the fix-spaces branch October 23, 2025 13:57
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (6)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx (1)

52-54: Schema mismatch: members are userIds, not emails.

Form schema validates emails, but field stores userIds and is sent as User.UserId. Align schema to avoid silent invalid states.

-const formSchema = z.object({
-  members: z.array(z.string().email("Invalid email address")).optional(),
-});
+const formSchema = z.object({
+  // Values are User.UserId strings
+  members: z.array(z.string()).optional(),
+});
apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts (5)

44-53: Unsafe ID generation and manual timestamps.

  • Truncating UUIDs reduces entropy; use nanoId() helper.
  • Don’t stamp createdAt/updatedAt; rely on DB defaults.
-import { v4 as uuidv4 } from "uuid";
-import { nanoIdLength } from "@cap/database/helpers";
+import { nanoId } from "@cap/database/helpers";
...
   await db()
     .insert(spaceMembers)
     .values({
-      id: uuidv4().substring(0, nanoIdLength),
+      id: nanoId(),
       spaceId,
       userId,
       role,
-      createdAt: new Date(),
-      updatedAt: new Date(),
     });

88-99: Same issue in bulk insert: use nanoId() and drop timestamps.

Apply the same fixes when building values for addSpaceMembers.

-const now = new Date();
-const values = newUserIds.map((userId) => ({
-  id: uuidv4().substring(0, nanoIdLength),
+const values = newUserIds.map((userId) => ({
+  id: nanoId(),
   spaceId,
   userId,
   role,
-  createdAt: now,
-  updatedAt: now,
 }));

160-171: Missing authorization checks for membership edits.

Only logged-in status is verified. Ensure the caller is allowed to manage members (creator or Admin of this space/organization). This applies to setSpaceMembers, addSpaceMember(s), removeSpaceMember, and batchRemoveSpaceMembers.

Minimal pattern:

// Fetch space + creator; ensure user is creator or has Admin role in this space
const [spaceRow] = await db()
  .select({ createdById: spaces.createdById })
  .from(spaces)
  .where(eq(spaces.id, spaceId))
  .limit(1);

const [me] = await db()
  .select({ role: spaceMembers.role })
  .from(spaceMembers)
  .where(and(eq(spaceMembers.spaceId, spaceId), eq(spaceMembers.userId, currentUser.id)))
  .limit(1);

if (!spaceRow || (spaceRow.createdById !== currentUser.id && me?.role !== "Admin")) {
  throw new Error("Unauthorized");
}

111-149: removeSpaceMember lacks authorization; any user could delete by memberId.

Verify the current user is the space creator or Admin before deleting. Also guard against cross-space deletions.


214-244: batchRemoveSpaceMembers also lacks authorization.

Add the same creator/Admin check and validate all memberIds belong to the target space before deletion. Consider transactionality.

🧹 Nitpick comments (7)
apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx (1)

19-19: Router refresh is fine; consider targeted cache updates.

router.refresh() works, but per app guidelines prefer TanStack Query v5 targeted cache updates (setQueryData/setQueriesData) for member lists to avoid full route reloads. Keep refresh as fallback.

Also applies to: 48-48, 86-86

apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (3)

21-21: Router refresh OK; prefer targeted cache updates.

router.refresh() works; consider updating relevant TanStack Query caches instead to avoid full refresh. Keep refresh as a fallback.

Also applies to: 135-135


161-175: Good client-side file validation; add input hint.

  • Size/type checks look good.
  • Suggest passing accept="image/*" to FileInput to guide selection.
 <FileInput
   id="space-icon"
   name="icon"
   initialPreviewUrl={space?.iconUrl || null}
   notDraggingClassName="hover:bg-gray-3"
+  accept="image/*"
   onChange={handleFileChange}
   disabled={isUploading}
   isLoading={isUploading}
 />

Also applies to: 298-298, 308-308


208-214: Post-submit UX

Success toast + router.refresh is consistent. If you later switch to targeted cache updates, you can remove refresh to avoid layout jank.

Also applies to: 215-221

apps/web/actions/organization/update-space.ts (2)

58-75: Replace-all membership: consider a transaction.

Wrap delete + insert in db().transaction to avoid transient empty state and partial writes on failure.


80-87: Avoid second fetch for iconUrl.

Fetch iconUrl in the initial query to skip the extra round trip here, unless you deliberately want a fresh read.

apps/web/actions/organization/create-space.ts (1)

83-93: Deduplicate member IDs before insert.

Duplicate entries in members[] can violate the unique(spaceId,userId) constraint and roll back the tx. Dedup before mapping.

-const memberUserIds: string[] = [];
+let memberUserIds: string[] = [];
 ...
-if (!memberUserIds.includes(user.id)) {
-  memberUserIds.push(user.id);
-}
+if (!memberUserIds.includes(user.id)) memberUserIds.push(user.id);
+memberUserIds = Array.from(new Set(memberUserIds));

Also applies to: 95-109

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a53562 and 29cacd5.

📒 Files selected for processing (5)
  • apps/web/actions/organization/create-space.ts (3 hunks)
  • apps/web/actions/organization/update-space.ts (3 hunks)
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (6 hunks)
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts (2 hunks)
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by running pnpm format.

Use strict TypeScript and avoid any; leverage shared types

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
  • apps/web/actions/organization/update-space.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g., user-menu.tsx).
Use PascalCase for React/Solid components.

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
  • apps/web/actions/organization/update-space.ts
apps/web/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

On the client, always use useEffectQuery or useEffectMutation from @/lib/EffectRuntime; never call EffectRuntime.run* directly in components.

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
  • apps/web/actions/organization/update-space.ts
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and fetching in the web app
Mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData
Run server-side effects via the ManagedRuntime from apps/web/lib/server.ts using EffectRuntime.runPromise/runPromiseExit; do not create runtimes ad hoc
Client code should use helpers from apps/web/lib/EffectRuntime.ts (useEffectQuery, useEffectMutation, useRpcClient); never call ManagedRuntime.make inside components

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
  • apps/web/actions/organization/create-space.ts
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
  • apps/web/actions/organization/update-space.ts
apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Server components needing Effect services must call EffectRuntime.runPromise(effect.pipe(provideOptionalAuth))

Files:

  • apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts
  • apps/web/app/(org)/dashboard/spaces/[spaceId]/components/MembersIndicator.tsx
  • apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx
apps/web/actions/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

All Groq/OpenAI calls must execute in Next.js Server Actions under apps/web/actions; do not invoke AI providers elsewhere

Files:

  • apps/web/actions/organization/create-space.ts
  • apps/web/actions/organization/update-space.ts
🧬 Code graph analysis (4)
apps/web/app/(org)/dashboard/spaces/[spaceId]/actions.ts (3)
packages/database/index.ts (1)
  • db (18-25)
packages/database/schema.ts (2)
  • spaces (591-619)
  • spaceMembers (621-646)
packages/database/helpers.ts (1)
  • nanoIdLength (3-3)
apps/web/actions/organization/create-space.ts (4)
packages/web-domain/src/ImageUpload.ts (1)
  • ImageUrlOrKey (9-9)
packages/database/schema.ts (2)
  • spaces (591-619)
  • spaceMembers (621-646)
packages/web-domain/src/Space.ts (4)
  • SpaceMemberRole (13-16)
  • SpaceMemberRole (17-17)
  • SpaceMemberId (10-10)
  • SpaceMemberId (11-11)
apps/web/actions/organization/upload-space-icon.ts (1)
  • uploadSpaceIcon (14-103)
apps/web/app/(org)/dashboard/_components/Navbar/SpaceDialog.tsx (2)
apps/web/actions/organization/update-space.ts (1)
  • updateSpace (20-107)
apps/web/actions/organization/create-space.ts (1)
  • createSpace (26-140)
apps/web/actions/organization/update-space.ts (4)
packages/database/index.ts (1)
  • db (18-25)
packages/database/schema.ts (2)
  • spaces (591-619)
  • spaceMembers (621-646)
packages/web-domain/src/Space.ts (4)
  • SpaceMemberRole (13-16)
  • SpaceMemberRole (17-17)
  • SpaceMemberId (10-10)
  • SpaceMemberId (11-11)
packages/database/helpers.ts (1)
  • nanoId (6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/web/actions/organization/update-space.ts (3)

29-41: Space existence check: good.

Early return on missing space avoids unnecessary work.


43-53: Authorization model: member vs admin.

You allow any member to update space details and membership. Confirm this is intended; many apps restrict to creator/Admin. If stricter policy is desired, also check role === "Admin".

Quick tweak:

-const [membership] = await db()...limit(1);
-if (!isCreator && !membership) {
+const [membership] = await db()...limit(1);
+if (!isCreator && membership?.role !== "Admin") {
   return { success: false, error: "Unauthorized" };
}

104-106: Revalidation breadth LGTM.

Revalidating both /dashboard and the specific space route is appropriate.

apps/web/actions/organization/create-space.ts (2)

67-80: Transaction + IDs: solid.

Creating space and members in one transaction with nanoId/SpaceMemberId looks correct.

Also applies to: 81-110


112-123: Upload errors handled non-fatal: good.

Icon upload decoupled and errors logged without failing the create flow.

Comment on lines +173 to 208
// Get the space creator to ensure they're always included
const [space] = await db()
.select({ createdById: spaces.createdById })
.from(spaces)
.where(eq(spaces.id, spaceId))
.limit(1);

if (!space) {
throw new Error("Space not found");
}

// Ensure creator is always included in the member list
const allMemberIds = Array.from(new Set([...userIds, space.createdById]));

// Remove all current members
await db().delete(spaceMembers).where(eq(spaceMembers.spaceId, spaceId));

// Insert new members if any
if (userIds.length > 0) {
const now = new Date();
const values = userIds.map((userId) => ({
// Insert new members (always at least the creator)
const now = new Date();
const values = allMemberIds.map((userId) => {
// Creator is always Admin, others get the specified role
const memberRole = userId === space.createdById ? "Admin" : role;
return {
id: User.UserId.make(uuidv4().substring(0, nanoIdLength)),
spaceId,
userId,
role,
role: memberRole,
createdAt: now,
updatedAt: now,
}));
await db().insert(spaceMembers).values(values);
}
};
});
await db().insert(spaceMembers).values(values);

revalidatePath(`/dashboard/spaces/${spaceId}`);
return { success: true, count: userIds.length };
return { success: true, count: allMemberIds.length };
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Wrong brand for spaceMembers.id and continued UUID truncation.

  • id should be a SpaceMemberId (or plain string); using User.UserId.make(...) is incorrect and will corrupt types.
  • Stop truncating UUIDs; use nanoId().
-import { v4 as uuidv4 } from "uuid";
-import { nanoIdLength } from "@cap/database/helpers";
+import { nanoId } from "@cap/database/helpers";
+import { SpaceMemberId } from "@cap/web-domain";
...
-const values = allMemberIds.map((userId) => {
+const values = allMemberIds.map((userId) => {
   const memberRole = userId === space.createdById ? "Admin" : role;
   return {
-    id: User.UserId.make(uuidv4().substring(0, nanoIdLength)),
+    id: SpaceMemberId.make(nanoId()),
     spaceId,
     userId,
     role: memberRole,
-    createdAt: now,
-    updatedAt: now,
   };
 });

Also, wrap delete+insert in a transaction to avoid transient empty membership states.

Committable suggestion skipped: line range outside the PR's diff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant