Skip to content

Conversation

oscartbeaumont
Copy link
Member

@oscartbeaumont oscartbeaumont commented Sep 25, 2025

For now we are just gonna start assigning it for new uploads. All existing uploads will be to be backfilled separately.

Actions:

  • Add organisation setting for default orgId
  • Hook the default up to /api/desktop/videos/create

Future backfill PR:

  • Fill orgId on all videos
  • Make it required

https://app.planetscale.com/cap/cap-production/deploy-requests/38

Summary by CodeRabbit

  • New Features

    • Organization-scoped video uploads and org-aware desktop API; default-organization selector in Account Settings with save/refresh and unsaved-change guard.
    • Redesigned Select component with improved composition and styling.
  • Bug Fixes

    • Clear warning and abort when uploading without an active organization.
    • Video creation and upload flows validate and apply organization context, updating user defaults when needed.
  • Chores

    • Added defaultOrgId to users, DB config tweak, orgId backfill utility and workspace entry, and minor global CSS cleanup.

Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Adds orgId scoping across video creation and upload flows, persists users.defaultOrgId with UI/server support, updates desktop video API to validate/choose orgId, introduces an orgId backfill utility and workspace entry, and refactors/selects updates including a new Select component and minor CSS tweaks.

Changes

Cohort / File(s) Summary
Web upload action & UI
apps/web/actions/video/upload.ts, apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
Thread orgId (from activeOrganization.organization.id) through createVideoAndGetUploadUrl and legacy upload flows; runtime guard and alert if no active org; include orgId on video and screenshot presign/creation.
Desktop API: video creation
apps/web/app/api/desktop/[...route]/video.ts
Accept optional orgId query, validate/resolve user access to orgId or derive a valid org (possibly updating users.defaultOrgId), and insert videos with orgId; add early returns for invalid/no-org cases.
Account settings & dashboard data
apps/web/app/(org)/dashboard/settings/account/Settings.tsx, apps/web/app/(org)/dashboard/settings/account/server.ts, apps/web/app/(org)/dashboard/dashboard-data.ts
Add patchAccountSettings server action to update firstName, lastName, and defaultOrgId; Settings UI adds Default organization selector, unsaved-changes guard, and uses patchAccountSettings; dashboard data projects users.defaultOrgId.
Database schema & Drizzle config
packages/database/schema.ts, packages/database/drizzle.config.ts
Add nullable defaultOrgId column to users; remove unused import; add tablesFilter: ["*", "!cluster_*"] to Drizzle config.
Backfill utility & workspace
scripts/orgid_backfill/index.ts, scripts/orgid_backfill/package.json, pnpm-workspace.yaml
New workspace package and CLI script to backfill videos.orgId and users.defaultOrgId in chunks with validation/logging; register scripts/orgIdBackfill in workspace.
UI component & styles
packages/ui/src/components/Select.tsx, apps/web/app/globals.css
Replace inline Radix Select usage with composed Select component and new public subcomponents; switch icon usage and class utilities. CSS: remove explicit text-gray-10 from several base selectors.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User (Dashboard)
  participant B as UploadCapButton
  participant A as legacyUploadCap
  participant S as createVideoAndGetUploadUrl
  participant ST as Storage (S3 Presign)
  participant DB as DB (videos)

  U->>B: Select file
  B->>B: Read activeOrganization
  alt org selected
    B->>A: legacyUploadCap(file, orgId)
    A->>S: createVideoAndGetUploadUrl({ filename, size, orgId, ... })
    S->>DB: insert video { orgId, ... }
    S->>ST: request presigned POST
    ST-->>S: presignedPostData
    S-->>A: { videoId, presignedPostData }
    A-->>B: proceed upload (video + screenshot with orgId)
  else no org
    B-->>U: Alert: no active organization
  end
Loading
sequenceDiagram
  autonumber
  participant C as Desktop Client
  participant API as /api/desktop/.../video#create
  participant Auth as Auth
  participant ORG as Org DB/Service
  participant DB as DB

  C->>API: GET /create?orgId=?
  API->>Auth: get current user
  Auth-->>API: user
  alt orgId provided
    API->>ORG: validate access to orgId
    ORG-->>API: allow / forbid
    opt forbid
      API-->>C: 403
    end
    API->>DB: insert video { orgId }
  else no orgId
    API->>ORG: fetch user orgs + defaultOrgId
    ORG-->>API: org list + default
    API->>API: pick videoOrgId (default or first)
    API->>DB: maybe update users.defaultOrgId
    API->>DB: insert video { orgId: videoOrgId }
  end
  API-->>C: { videoId, ... }
  note over API,DB: Existing early-return for existing videoId remains
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested reviewers

  • Brendonovich

Poem

A hop, a bop, an orgId pop—
I tag each clip without a stop.
Defaults set in burrowed rows,
Backfill carrots in tidy rows.
With gentle thumps I whisper: “Done.” 🥕🐇

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
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.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately reflects the primary change in the pull request by stating that the orgId column on the videos table is being populated, which aligns with the changes in database schema, API endpoints, and client code. It is concise, focusing on the main objective without including extraneous implementation details or file references. This phrasing allows reviewers to immediately grasp the core purpose of the PR when scanning the repository history.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch populate-orgId

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 932cd33 and 057da33.

📒 Files selected for processing (4)
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3 hunks)
  • apps/web/app/api/desktop/[...route]/video.ts (5 hunks)
  • apps/web/app/globals.css (1 hunks)
  • packages/ui/src/components/Select.tsx (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • packages/ui/src/components/Select.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • packages/ui/src/components/Select.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/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • packages/ui/src/components/Select.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/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
packages/ui/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (AGENTS.md)

Component files in packages/ui should use PascalCase naming if they define React/Solid components.

Files:

  • packages/ui/src/components/Select.tsx
🧬 Code graph analysis (2)
apps/web/app/api/desktop/[...route]/video.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (3)
  • organizations (149-169)
  • organizationMembers (172-192)
  • users (47-96)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
packages/database/schema.ts (1)
  • users (47-96)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
  • useDashboardContext (44-44)
apps/web/app/(org)/dashboard/settings/account/server.ts (1)
  • patchAccountSettings (13-66)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Analyze (rust)

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.

@oscartbeaumont oscartbeaumont marked this pull request as ready for review September 30, 2025 12:56
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 (1)
apps/web/app/api/desktop/[...route]/video.ts (1)

102-122: orgId should be populated from user.defaultOrgId, not set to undefined.

According to the PR objectives, new uploads should have orgId assigned. However, this code explicitly sets orgId: undefined instead of using the user's defaultOrgId. This contradicts the PR description which states: "Hook the default up to /api/desktop/videos/create" is completed.

Apply this diff to populate orgId from the user's defaultOrgId:

+      const [userData] = await db()
+        .select({ defaultOrgId: users.defaultOrgId })
+        .from(users)
+        .where(eq(users.id, user.id));
+
       await db()
         .insert(videos)
         .values({
           id: idToUse,
           name: videoName,
           ownerId: user.id,
-          orgId: undefined,
+          orgId: userData?.defaultOrgId,
           source:
             recordingMode === "hls"

Note: You'll need to import users from the schema at the top of the file if not already imported.

🧹 Nitpick comments (6)
packages/database/schema.ts (1)

91-91: Consider adding a relation to the organizations table.

The new defaultOrgId field references an organization but lacks a defined relation in usersRelations. Adding a relation would improve type safety and enable relational queries.

In the usersRelations definition (around line 418), consider adding:

 export const usersRelations = relations(users, ({ many, one }) => ({
   accounts: many(accounts),
   sessions: many(sessions),
   organizationMembers: many(organizationMembers),
   videos: many(videos),
   sharedVideos: many(sharedVideos),
   customBucket: one(s3Buckets),
   spaces: many(spaces),
   spaceMembers: many(spaceMembers),
+  defaultOrg: one(organizations, {
+    fields: [users.defaultOrgId],
+    references: [organizations.id],
+  }),
 }));
apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx (1)

55-59: Replace alert() with toast.error() for better UX.

Using alert() blocks the UI and is inconsistent with error handling elsewhere in this file (e.g., Line 253). Consider using toast.error() for a non-blocking notification.

Apply this diff:

-    // This should be unreachable.
     if (activeOrganization === null) {
-      alert("No organization active!");
+      toast.error("No organization active");
       return;
     }
apps/web/app/(org)/dashboard/settings/account/server.ts (1)

16-23: Consider conditional updates to avoid clearing unintended fields.

The current implementation sets all fields (name, lastName, defaultOrgId) even when they are undefined. This means calling patchAccountSettings(firstName) without lastName or defaultOrgId will set those fields to NULL in the database, potentially clearing existing values.

If the intent is to only update provided fields, consider:

- await db()
-   .update(users)
-   .set({
-     name: firstName,
-     lastName,
-     defaultOrgId,
-   })
-   .where(eq(users.id, currentUser.id));
+ const updates: Partial<typeof users.$inferInsert> = {};
+ if (firstName !== undefined) updates.name = firstName;
+ if (lastName !== undefined) updates.lastName = lastName;
+ if (defaultOrgId !== undefined) updates.defaultOrgId = defaultOrgId;
+ 
+ await db()
+   .update(users)
+   .set(updates)
+   .where(eq(users.id, currentUser.id));

Alternatively, if clearing is intentional, this is fine as-is.

apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)

32-47: Consider validating trimmed firstName is non-empty.

The submit button is disabled when !firstName (Line 123), but firstName.trim() on Line 35 could produce an empty string if the user enters only whitespace. This would allow saving an empty name to the database.

Consider either:

  1. Disabling the button when !firstName.trim(), or
  2. Adding validation in the mutation to prevent empty strings:
  mutationFn: async () => {
+   const trimmedFirstName = firstName.trim();
+   if (!trimmedFirstName) {
+     throw new Error("First name is required");
+   }
    await patchAccountSettings(
-     firstName.trim(),
+     trimmedFirstName,
      lastName.trim() ? lastName.trim() : undefined,
      defaultOrgId,
    );
  },

103-104: Improve CardDescription text.

The description "This is the default organization" is redundant with the title. Consider explaining what the default organization is used for or when it applies.

Example:

- <CardDescription>This is the default organization</CardDescription>
+ <CardDescription>
+   New uploads will be assigned to this organization by default
+ </CardDescription>

107-119: Add empty-state handling for organization select
In Settings.tsx (lines 107–119), if organizationData is empty the Select renders no options and value="", yielding a blank dropdown. Disable the control or render a placeholder option (e.g. “No organizations available”) when organizationData.length === 0.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 166cad8 and 8e435a1.

📒 Files selected for processing (8)
  • apps/web/actions/video/upload.ts (3 hunks)
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx (5 hunks)
  • apps/web/app/(org)/dashboard/dashboard-data.ts (1 hunks)
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx (2 hunks)
  • apps/web/app/(org)/dashboard/settings/account/page.tsx (1 hunks)
  • apps/web/app/(org)/dashboard/settings/account/server.ts (1 hunks)
  • apps/web/app/api/desktop/[...route]/video.ts (2 hunks)
  • packages/database/schema.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/web/actions/**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

All Groq/OpenAI calls must be implemented in Next.js Server Actions under apps/web/actions; do not place AI calls elsewhere

Files:

  • apps/web/actions/video/upload.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 data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/actions/video/upload.ts
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • apps/web/app/(org)/dashboard/settings/account/page.tsx
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/web/actions/video/upload.ts
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • packages/database/schema.ts
  • apps/web/app/(org)/dashboard/settings/account/page.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • apps/web/actions/video/upload.ts
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • packages/database/schema.ts
  • apps/web/app/(org)/dashboard/settings/account/page.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/actions/video/upload.ts
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • packages/database/schema.ts
  • apps/web/app/(org)/dashboard/settings/account/page.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/actions/video/upload.ts
  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • apps/web/app/(org)/dashboard/settings/account/page.tsx
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx
  • apps/web/app/(org)/dashboard/dashboard-data.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/api/desktop/[...route]/video.ts
  • apps/web/app/(org)/dashboard/settings/account/server.ts
  • apps/web/app/(org)/dashboard/settings/account/page.tsx
🧬 Code graph analysis (4)
apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx (1)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
  • useDashboardContext (44-44)
apps/web/app/(org)/dashboard/dashboard-data.ts (1)
packages/database/schema.ts (1)
  • users (47-96)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
packages/database/schema.ts (1)
  • users (47-96)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
  • useDashboardContext (44-44)
apps/web/app/(org)/dashboard/settings/account/server.ts (1)
  • patchAccountSettings (8-24)
apps/web/app/(org)/dashboard/settings/account/server.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (1)
  • users (47-96)
⏰ 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). (4)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Vercel Agent Review
  • GitHub Check: Analyze (rust)
🔇 Additional comments (5)
apps/web/app/(org)/dashboard/dashboard-data.ts (1)

51-51: LGTM: defaultOrgId correctly added to user projection.

The addition of defaultOrgId to the user projection aligns with the schema changes and enables the dashboard to access users' default organization preferences.

apps/web/app/api/desktop/[...route]/video.ts (1)

86-86: Minor cleanup: debug log removed.

Removing the debug console log is a reasonable cleanup.

apps/web/actions/video/upload.ts (2)

233-233: LGTM: orgId correctly propagated to video data.

The orgId parameter is properly included in the videoData object, ensuring new video records will have the organization ID populated as per the PR objectives.


167-177: All callers updated to pass orgId. Both invocations in UploadCapButton.tsx now include the required orgId parameter—no further changes needed.

apps/web/app/(org)/dashboard/caps/components/UploadCapButton.tsx (1)

100-106: LGTM!

The orgId parameter is properly threaded through the upload flow. The runtime check on lines 56-59 ensures activeOrganization is not null before accessing its properties.

@oscartbeaumont oscartbeaumont marked this pull request as draft September 30, 2025 13:11
@oscartbeaumont oscartbeaumont marked this pull request as ready for review October 1, 2025 04:56
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/app/api/desktop/[...route]/video.ts (1)

128-165: Handle users without a defaultOrgId.

When orgId isn’t supplied and user.defaultOrgId is null (common until the new setting is saved), videoOrgId never gets assigned, so the insert on Line 165 still writes orgId: null, defeating the PR objective for those users. Please fall back to the first accessible organization (and persist it as the default) when no default is set.

-			} else if (user.defaultOrgId) {
+			} else if (user.defaultOrgId) {
@@
-				} else videoOrgId = user.defaultOrgId;
+				} else videoOrgId = user.defaultOrgId;
+			} else if (userOrganizations[0]) {
+				videoOrgId = userOrganizations[0].id;
+
+				await db()
+					.update(users)
+					.set({ defaultOrgId: videoOrgId })
+					.where(eq(users.id, user.id));
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8e435a1 and fecf7de.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (5)
  • apps/web/app/api/desktop/[...route]/video.ts (5 hunks)
  • packages/database/drizzle.config.ts (1 hunks)
  • pnpm-workspace.yaml (1 hunks)
  • scripts/orgid_backfill/index.ts (1 hunks)
  • scripts/orgid_backfill/package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
  • pnpm-workspace.yaml
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • packages/database/drizzle.config.ts
  • scripts/orgid_backfill/index.ts
  • apps/web/app/api/desktop/[...route]/video.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • packages/database/drizzle.config.ts
  • scripts/orgid_backfill/index.ts
  • apps/web/app/api/desktop/[...route]/video.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:

  • packages/database/drizzle.config.ts
  • scripts/orgid_backfill/index.ts
  • apps/web/app/api/desktop/[...route]/video.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 data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/api/desktop/[...route]/video.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/api/desktop/[...route]/video.ts
🧬 Code graph analysis (2)
scripts/orgid_backfill/index.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (2)
  • videos (240-288)
  • users (47-96)
apps/web/app/api/desktop/[...route]/video.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (3)
  • organizations (149-169)
  • organizationMembers (171-189)
  • users (47-96)
⏰ 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: Lint (Biome)
  • GitHub Check: Analyze (rust)

@oscartbeaumont oscartbeaumont mentioned this pull request Oct 1, 2025
2 tasks
@oscartbeaumont
Copy link
Member Author

Running the backfill be like:

cd scripts/orgid_backfill
pnpm backfill

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: 2

🧹 Nitpick comments (3)
apps/web/app/(org)/dashboard/settings/account/server.ts (2)

21-49: Remove inline comments per coding guidelines.

Lines 21, 34, 35, 36, and 40 contain inline comments. The project guidelines require code to be self-explanatory without comments.

As per coding guidelines.


22-22: Validate empty string for defaultOrgId.

The check if (defaultOrgId) skips validation when defaultOrgId is an empty string, but the update would still set it to "". Consider explicitly rejecting empty strings or treating them as unset.

-	if (defaultOrgId) {
+	if (defaultOrgId !== undefined && defaultOrgId !== "") {

Or throw an error for empty strings:

 	if (defaultOrgId) {
+		if (defaultOrgId === "") {
+			throw new Error("Invalid organization ID");
+		}
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)

120-139: Handle empty organizationData gracefully.

If organizationData is empty or undefined, the Select will have no options and the value will fall back to an empty string. Consider conditionally rendering the card or displaying a message when no organizations are available.

Add a guard before rendering the card:

{organizationData && organizationData.length > 0 && (
  <Card className="flex flex-col flex-1 gap-4 justify-between items-stretch">
    {/* existing card content */}
  </Card>
)}

Or show a message when empty:

<Card className="flex flex-col flex-1 gap-4 justify-between items-stretch">
  <div className="space-y-1">
    <CardTitle>Default organization</CardTitle>
    <CardDescription>This is the default organization</CardDescription>
  </div>

  {organizationData && organizationData.length > 0 ? (
    <Select
      value={
        defaultOrgId ??
        user?.defaultOrgId ??
        organizationData[0].organization.id ??
        ""
      }
      onChange={(value) => setDefaultOrgId(value)}
      options={organizationData.map((org) => ({
        value: org.organization.id,
        label: org.organization.name,
      }))}
    />
  ) : (
    <p className="text-sm text-muted-foreground">No organizations available</p>
  )}
</Card>
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fc0114e and 58ca316.

📒 Files selected for processing (2)
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3 hunks)
  • apps/web/app/(org)/dashboard/settings/account/server.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.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/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.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/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
🧬 Code graph analysis (2)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
packages/database/schema.ts (1)
  • users (47-96)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
  • useDashboardContext (44-44)
apps/web/app/(org)/dashboard/settings/account/server.ts (1)
  • patchAccountSettings (13-61)
apps/web/app/(org)/dashboard/settings/account/server.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (3)
  • organizations (149-169)
  • organizationMembers (171-189)
  • users (47-96)
⏰ 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). (4)
  • GitHub Check: Vercel Agent Review
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Analyze (rust)

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: 2

Caution

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

⚠️ Outside diff range comments (2)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (2)

13-53: Switch to useEffectMutation as required.

Client components in apps/web/**/* must use the useEffectMutation helper from @/lib/EffectRuntime instead of raw useMutation. Please wire this mutation through the prescribed helper.


32-66: Remove inline comments per repository standards.

Inline comments (// Track…, // Prevent…) break the repo rule forbidding inline/comments in TSX. Please delete them.

♻️ Duplicate comments (1)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (1)

142-143: Re-disable save when nothing changed.

hasChanges still isn’t part of the disabled condition, so users can submit unchanged data and hit the server again. Please include !hasChanges in the disabled predicate.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 58ca316 and ffd86c7.

📒 Files selected for processing (2)
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3 hunks)
  • apps/web/app/(org)/dashboard/settings/account/server.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.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/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.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/settings/account/Settings.tsx
  • apps/web/app/(org)/dashboard/settings/account/server.ts
🧬 Code graph analysis (2)
apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3)
packages/database/schema.ts (1)
  • users (47-96)
apps/web/app/(org)/dashboard/Contexts.tsx (1)
  • useDashboardContext (44-44)
apps/web/app/(org)/dashboard/settings/account/server.ts (1)
  • patchAccountSettings (13-66)
apps/web/app/(org)/dashboard/settings/account/server.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (3)
  • organizations (149-169)
  • organizationMembers (171-189)
  • users (47-96)
⏰ 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). (4)
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
  • GitHub Check: Vercel Agent Review
  • GitHub Check: Analyze (rust)

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

♻️ Duplicate comments (1)
apps/web/app/api/desktop/[...route]/video.ts (1)

128-151: Restore org fallback and initialize videoOrgId.

videoOrgId is left undefined whenever the caller omits orgId and the user has no defaultOrgId, reproducing the “Unreachable” failure noted in the previous review and triggering the current TypeScript “used before assignment” error. Add the missing fallback to the first accessible org (returning no_valid_org when none exist) and initialize the variable so it’s always assigned before use.

-			let videoOrgId: string;
-			if (orgId) {
+			let videoOrgId: string | null = null;
+			if (orgId) {
 				// Hard error if the user requested org is non-existent or they don't have access.
 				if (!userOrgIds.includes(orgId))
 					return c.json({ error: "forbidden_org" }, { status: 403 });
 				videoOrgId = orgId;
-			} else if (user.defaultOrgId) {
-				// User's defaultOrgId is no longer valid, switch to first available org
-				if (!userOrgIds.includes(user.defaultOrgId)) {
-					if (!userOrganizations[0])
-						return c.json({ error: "no_valid_org" }, { status: 403 });
-
-					videoOrgId = userOrganizations[0].id;
-
-					// Update user's defaultOrgId to the new valid org
-					await db()
-						.update(users)
-						.set({
-							defaultOrgId: videoOrgId,
-						})
-						.where(eq(users.id, user.id));
-				} else videoOrgId = user.defaultOrgId;
+			} else {
+				const hasValidDefault =
+					user.defaultOrgId && userOrgIds.includes(user.defaultOrgId);
+				if (hasValidDefault) {
+					videoOrgId = user.defaultOrgId;
+				} else {
+					const fallbackOrg = userOrganizations[0];
+					if (!fallbackOrg)
+						return c.json({ error: "no_valid_org" }, { status: 403 });
+					videoOrgId = fallbackOrg.id;
+					await db()
+						.update(users)
+						.set({ defaultOrgId: videoOrgId })
+						.where(eq(users.id, user.id));
+				}
 			}
-			if (!videoOrgId) throw new Error("Unreachable");
+			if (!videoOrgId) throw new Error("Unreachable");
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ffd86c7 and 08caacb.

📒 Files selected for processing (3)
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx (3 hunks)
  • apps/web/app/api/desktop/[...route]/video.ts (5 hunks)
  • packages/database/schema.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • packages/database/schema.ts
  • apps/web/app/(org)/dashboard/settings/account/Settings.tsx
🧰 Additional context used
📓 Path-based instructions (6)
apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
apps/web/app/**/*.{tsx,ts}

📄 CodeRabbit inference engine (CLAUDE.md)

Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
**/*.{ts,tsx,js,jsx,rs}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not add inline, block, or docstring comments in any language; code must be self-explanatory

Files:

  • apps/web/app/api/desktop/[...route]/video.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Use strict TypeScript and avoid any; leverage shared types from packages

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

Files:

  • apps/web/app/api/desktop/[...route]/video.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/api/desktop/[...route]/video.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/api/desktop/[...route]/video.ts
🧬 Code graph analysis (1)
apps/web/app/api/desktop/[...route]/video.ts (2)
packages/database/index.ts (1)
  • db (29-34)
packages/database/schema.ts (3)
  • organizations (149-169)
  • organizationMembers (172-192)
  • users (47-96)
🪛 GitHub Check: Typecheck
apps/web/app/api/desktop/[...route]/video.ts

[failure] 151-151:
Variable 'videoOrgId' is used before being assigned.

⏰ 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: Vercel Agent Review
  • GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
  • GitHub Check: Analyze (rust)

@ameer2468 ameer2468 merged commit 9054998 into main Oct 2, 2025
13 of 14 checks passed
@ameer2468 ameer2468 deleted the populate-orgId branch October 2, 2025 08:47
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.

3 participants