Implement org selection #1185
Implement org selection #1185
Conversation
WalkthroughIntroduces organization awareness across desktop app and API: adds Organization type, stores organizations in AuthStore, fetches them via new /desktop/organizations endpoint and a Tauri command, propagates optional organizationId through recording/export/upload flows, updates UI to select/pass organization, adds logging command, and relaxes Node engine constraints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant App as Desktop App
participant Tauri as Tauri Backend
participant API as Web API
Note over App,Tauri: Startup / Auth updated
App->>Tauri: invoke refresh_organizations()
Tauri->>API: GET /desktop/organizations (authed)
API-->>Tauri: 200 [ {id,name,ownerId} ]
Tauri->>Tauri: Save to AuthStore.organizations
Tauri-->>App: OK
sequenceDiagram
autonumber
participant UI as Export/Record UI
participant Tauri as Tauri Backend
participant Up as Upload Module
participant API as Web API
UI->>Tauri: upload_exported_video(..., organization_id?)
Tauri->>Up: create_or_get_video(..., organization_id?)
alt organization_id provided
Up->>API: POST /videos?...&orgId={organization_id}
else no organization_id
Up->>API: POST /videos?...
end
API-->>Up: S3 upload meta
Up-->>Tauri: Meta
Tauri-->>UI: Progress/Result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Refactors organization selection to improve efficiency and UX:
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
apps/desktop/src/routes/(window-chrome)/new-main/index.tsx (2)
438-442: Remove inline comment, avoid swallow, and don’t block mount while refreshing orgs
- Inline comment violates repo guideline.
- Swallowing errors hides failures.
- Awaiting the refresh can delay mount unnecessarily.
Refactor to fire-and-forget with a debug log:
- // Refresh organizations when main window loads - try { - await commands.refreshOrganizations(); - } catch {} + void commands + .refreshOrganizations() + .catch((error) => console.debug("refreshOrganizations failed", error));As per coding guidelines
57-61: Manual icon imports in desktop app — rely on auto‑imported iconsPer desktop guidelines, don’t manually import icons; the codebase already auto-imports others (e.g., IconCapSettings). Remove these imports and rely on auto-import.
-import IconLucideAppWindowMac from "~icons/lucide/app-window-mac"; -import IconLucideArrowLeft from "~icons/lucide/arrow-left"; -import IconLucideSearch from "~icons/lucide/search"; -import IconMaterialSymbolsScreenshotFrame2Rounded from "~icons/material-symbols/screenshot-frame-2-rounded"; -import IconMdiMonitor from "~icons/mdi/monitor";As per coding guidelines
apps/desktop/src-tauri/src/deeplink_actions.rs (1)
139-140: LGTM: initialize new optional organization_idSetting organization_id: None keeps current deeplink behavior stable.
If a user-selected organization is stored in RecordingSettingsStore, consider defaulting to that here to preserve context during deeplink-initiated recordings.
apps/desktop/src/utils/queries.ts (1)
248-253: Remove inline comments per codebase guidelineInline comments are disallowed in TS/TSX. Please remove the comment within createOrganizationsQuery.
Apply this diff:
- // Refresh organizations if they're missing createEffect(() => { if (auth.data?.user_id && (!auth.data?.organizations || auth.data.organizations.length === 0)) { commands.refreshOrganizations().catch(console.error); } });As per coding guidelines
apps/desktop/src-tauri/src/auth.rs (1)
110-134: Prefer structured logging over printlnConsider using tracing (info!/error!) for consistent logging and filtering across the app.
apps/desktop/src/routes/editor/ExportDialog.tsx (1)
83-95: Remove inline comments in TSX per project guidelineInline comments are disallowed in TS/TSX. Please remove JSX comments across the changed blocks.
As per coding guidelines
Also applies to: 433-446, 468-477, 495-499, 507-532, 533-616, 625-683, 745-825, 842-864, 973-1011
apps/desktop/src/routes/target-select-overlay.tsx (1)
764-816: Prefer an accessible Select from Kobalte or @cap/ui-solid over a custom dropdown.Custom dropdowns often miss keyboard nav, focus management, ARIA, and type-ahead. Replace
CustomSelectwith a Kobalte<Select>(or your UI kit’s Select) for a11y and consistency.If you want, I can draft the Kobalte Select snippet wired to
rawOptions.organizationId.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/desktop/src-tauri/src/auth.rs(2 hunks)apps/desktop/src-tauri/src/deeplink_actions.rs(1 hunks)apps/desktop/src-tauri/src/lib.rs(10 hunks)apps/desktop/src-tauri/src/recording.rs(2 hunks)apps/desktop/src-tauri/src/recording_settings.rs(1 hunks)apps/desktop/src-tauri/src/upload.rs(3 hunks)apps/desktop/src-tauri/src/upload_legacy.rs(4 hunks)apps/desktop/src/routes/(window-chrome)/new-main/index.tsx(1 hunks)apps/desktop/src/routes/editor/ExportDialog.tsx(12 hunks)apps/desktop/src/routes/target-select-overlay.tsx(9 hunks)apps/desktop/src/utils/queries.ts(4 hunks)apps/desktop/src/utils/tauri.ts(7 hunks)apps/tasks/package.json(1 hunks)apps/web/app/api/desktop/[...route]/root.ts(2 hunks)apps/web/package.json(1 hunks)package.json(1 hunks)packages/database/package.json(2 hunks)packages/local-docker/package.json(1 hunks)packages/ui-solid/src/auto-imports.d.ts(1 hunks)packages/ui/package.json(1 hunks)packages/utils/package.json(1 hunks)packages/web-api-contract/src/desktop.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{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/desktop/src-tauri/src/recording_settings.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/deeplink_actions.rspackages/ui-solid/src/auto-imports.d.tsapps/desktop/src/utils/queries.tspackages/web-api-contract/src/desktop.tsapps/desktop/src-tauri/src/upload.rsapps/web/app/api/desktop/[...route]/root.tsapps/desktop/src-tauri/src/upload_legacy.rsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src-tauri/src/auth.rsapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src-tauri/src/lib.rs
**/*.rs
📄 CodeRabbit inference engine (AGENTS.md)
**/*.rs: Format Rust code usingrustfmtand ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.
Files:
apps/desktop/src-tauri/src/recording_settings.rsapps/desktop/src-tauri/src/recording.rsapps/desktop/src-tauri/src/deeplink_actions.rsapps/desktop/src-tauri/src/upload.rsapps/desktop/src-tauri/src/upload_legacy.rsapps/desktop/src-tauri/src/auth.rsapps/desktop/src-tauri/src/lib.rs
**/*.{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 runningpnpm format.
Files:
packages/ui-solid/src/auto-imports.d.tsapps/desktop/src/utils/queries.tspackages/web-api-contract/src/desktop.tsapps/web/app/api/desktop/[...route]/root.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/utils/tauri.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/ui-solid/src/auto-imports.d.tsapps/desktop/src/utils/queries.tspackages/web-api-contract/src/desktop.tsapps/web/app/api/desktop/[...route]/root.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/utils/tauri.ts
**/queries.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated query bindings file queries.ts
Do not edit auto-generated files named
queries.ts.
Files:
apps/desktop/src/utils/queries.ts
apps/desktop/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/**/*.{ts,tsx}: Do not manually import icons in the desktop app; rely on auto-imported icons
In the desktop app, use @tanstack/solid-query for server state management
Files:
apps/desktop/src/utils/queries.tsapps/desktop/src/routes/target-select-overlay.tsxapps/desktop/src/routes/(window-chrome)/new-main/index.tsxapps/desktop/src/routes/editor/ExportDialog.tsxapps/desktop/src/utils/tauri.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]/root.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]/root.ts
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/api/desktop/[...route]/root.ts
**/tauri.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Never edit auto-generated IPC bindings file tauri.ts
Do not edit auto-generated files named
tauri.ts.
Files:
apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (8)
apps/desktop/src/utils/queries.ts (1)
apps/desktop/src/store.ts (1)
authStore(59-59)
apps/desktop/src-tauri/src/upload.rs (1)
apps/desktop/src-tauri/src/upload_legacy.rs (1)
create_or_get_video(382-449)
apps/web/app/api/desktop/[...route]/root.ts (2)
packages/database/index.ts (1)
db(30-37)packages/database/schema.ts (2)
organizations(163-195)organizationMembers(198-220)
apps/desktop/src-tauri/src/upload_legacy.rs (1)
apps/desktop/src-tauri/src/upload.rs (1)
create_or_get_video(220-282)
apps/desktop/src/routes/target-select-overlay.tsx (3)
packages/database/schema.ts (1)
organizations(163-195)apps/desktop/src/utils/queries.ts (1)
createOrganizationsQuery(245-256)apps/desktop/src/utils/tauri.ts (2)
Organization(436-436)commands(7-290)
apps/desktop/src-tauri/src/auth.rs (1)
apps/desktop/src/utils/tauri.ts (1)
Organization(436-436)
apps/desktop/src/routes/editor/ExportDialog.tsx (3)
apps/desktop/src/utils/queries.ts (1)
createOrganizationsQuery(245-256)apps/desktop/src/routes/editor/ui.tsx (4)
MenuItem(264-279)PopperContent(287-293)topSlideAnimateClasses(432-433)MenuItemList(295-309)apps/desktop/src/routes/editor/Header.tsx (1)
RESOLUTION_OPTIONS(34-38)
apps/desktop/src-tauri/src/lib.rs (2)
apps/desktop/src/utils/tauri.ts (1)
AuthStore(358-358)apps/desktop/src-tauri/src/auth.rs (1)
update_auth_plan(64-138)
⏰ 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). (2)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
🔇 Additional comments (22)
packages/ui-solid/src/auto-imports.d.ts (1)
68-68: LGTM! Auto-generated icon declaration is correctly formatted.The new
IconLucideBuilding2declaration follows the established pattern and is appropriately placed alphabetically among other Lucide icons. This building icon aligns well with the PR's organization selection feature.apps/tasks/package.json (1)
43-45: LGTM!The Node.js engine constraint update from exact version "20" to minimum version ">=20" aligns with the repository-wide standardization and provides better flexibility for deployments.
packages/local-docker/package.json (1)
12-14: LGTM!Consistent with the repository-wide Node.js engine constraint update.
packages/utils/package.json (1)
29-31: LGTM!Node.js engine constraint properly updated to match repository standards.
packages/ui/package.json (1)
57-59: LGTM!Engine constraint updated consistently with other packages.
package.json (1)
39-41: LGTM!The Node.js engine constraint harmonizes the root package with the rest of the repository by standardizing on a minimum of Node 20. While this loosens the previous requirement of Node 24, it aligns with the repository-wide convention and provides deployment flexibility.
apps/web/package.json (1)
153-155: LGTM!Node.js engine constraint updated consistently with the repository-wide standardization.
packages/database/package.json (1)
54-56: LGTM!Engine constraint properly updated to match repository standards.
apps/desktop/src-tauri/src/recording.rs (2)
240-241: LGTM!The
organization_idfield addition follows Rust best practices: marked asOption<String>for optional organization context, with#[serde(default)]for backward compatibility during deserialization.
318-318: LGTM!The
organization_idis properly propagated tocreate_or_get_videousing.clone(), which is appropriate since the inputs struct may be used elsewhere.apps/desktop/src-tauri/src/recording_settings.rs (1)
24-25: LGTM: organization_id added to settings storeOptional field is backward compatible; camelCase serde rename maintains TS interop as organizationId.
packages/web-api-contract/src/desktop.ts (1)
143-156: LGTM: add protected GET /desktop/organizationsRoute shape and types look good.
Run to cross‑check backend route alignment and selected fields:
apps/desktop/src/utils/queries.ts (1)
124-133: Org ID threading into options looks goodAdding organizationId to persisted options and propagating it into recordingSettingsStore is correct and aligns with the new flows.
Also applies to: 139-147
apps/desktop/src-tauri/src/upload_legacy.rs (1)
218-220: Legacy uploader: orgId is correctly plumbed
- Signature extended with organization_id and appended as &orgId=... when provided.
- Call sites updated to pass None to preserve prior behavior.
Looks good.Also applies to: 333-335, 381-414
apps/desktop/src-tauri/src/auth.rs (1)
17-27: Organizations support in AuthStore is well integrated
- Organization struct and serde rename match TS type (ownerId).
- #[serde(default)] on organizations avoids deserialization breaks.
- Fetching and storing organizations in update_auth_plan is sound and non-blocking.
Please confirm the web endpoint /api/desktop/organizations returns ownerId in camelCase to match the serde rename. Based on learnings
Also applies to: 110-134
apps/desktop/src-tauri/src/upload.rs (1)
198-199: Uploader: orgId threaded into create_or_get_videoThe new organization_id parameter and query-string propagation are correct. Call sites preserved behavior by passing None where not applicable.
Also applies to: 226-252
apps/desktop/src/utils/tauri.ts (1)
128-130: Generated IPC bindings match intended API
- uploadExportedVideo now accepts organizationId.
- logMessage and refreshOrganizations commands exposed.
- Types extended: Organization, AuthStore.organizations, RecordingSettingsStore.organizationId, StartRecordingInputs.organization_id.
Looks consistent with backend changes.Regenerate bindings to ensure this file remains purely generated:
- cargo build (tauri-specta) + specta generate step
- pnpm typegen (if applicable)
Do not manually edit this file. As per coding guidelines
Also applies to: 176-178, 197-199, 358-359, 436-437, 453-454, 473-474
apps/desktop/src/routes/editor/ExportDialog.tsx (1)
421-431: Button variant change is fineUsing the blue variant for the primary confirm action aligns with the new visual language.
apps/desktop/src/routes/target-select-overlay.tsx (1)
237-238: Avoid empty string fallbacks for DisplayId/Screen.Passing
""as a DisplayId/Screen risks invalid IDs reaching Tauri. If the route always guarantees adisplayId, prefer a non-null assertion or gate rendering until it exists. Otherwise, disable Start until valid.
- Option A:
id: params.displayId!andscreen: params.displayId!(if guaranteed)- Option B: Show controls only when
params.displayIdis defined.Please verify route guarantees and adjust.
Also applies to: 740-744
apps/desktop/src-tauri/src/lib.rs (3)
1892-1897: refresh_organizations command LGTM.Thin wrapper around
AuthStore::update_auth_planis fine; emits tracing info.
2680-2689: log_message command LGTM and correctly exported.Command maps level→tracing macros and is added to the registry.
Also applies to: 1975-1976
1066-1114: Confirmed TS wrapper and allcommands.uploadExportedVideocalls include the neworganizationIdparameter.
| // Auto-select first organization if none selected and user is authenticated | ||
| createEffect(() => { | ||
| const orgs = organizations(); | ||
| if (!settings.organizationId && orgs.length > 0 && auth.data) { | ||
| setSettings("organizationId", orgs[0].id); | ||
| } | ||
| }); | ||
|
|
There was a problem hiding this comment.
Avoid force-selecting an organization; preserve ability to publish to Personal
Auto-selecting the first org removes the user’s ability to publish to Personal (no org) and may override a deliberate null choice.
Apply this diff to remove the auto-select:
-// Auto-select first organization if none selected and user is authenticated
-createEffect(() => {
- const orgs = organizations();
- if (!settings.organizationId && orgs.length > 0 && auth.data) {
- setSettings("organizationId", orgs[0].id);
- }
-});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Auto-select first organization if none selected and user is authenticated | |
| createEffect(() => { | |
| const orgs = organizations(); | |
| if (!settings.organizationId && orgs.length > 0 && auth.data) { | |
| setSettings("organizationId", orgs[0].id); | |
| } | |
| }); | |
| // (Removed the auto-select createEffect entirely) |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/ExportDialog.tsx around lines 142 to 149, the
createEffect auto-selects the first organization when none is set which prevents
publishing to Personal and can override an intentional null; remove this
auto-selection block so settings.organizationId remains null unless explicitly
set by the user, ensure no other code depends on this side-effect (adjust tests
or UI flows if needed) and keep any cleanup/unused imports updated.
| <Show | ||
| when={ | ||
| settings.exportTo === "link" && | ||
| auth.data && | ||
| organizations().length > 1 | ||
| } | ||
| > | ||
| <div class="flex flex-col gap-1.5 p-2 rounded-lg bg-gray-2 border animate-in fade-in slide-in-from-top duration-200"> | ||
| <label class="flex items-center gap-1 text-[10px] text-gray-11 font-medium uppercase tracking-wide"> | ||
| <IconLucideBuilding2 class="size-3" /> | ||
| Organization | ||
| </label> | ||
| <KSelect<{ id: string; name: string; ownerId: string }> | ||
| options={organizations()} | ||
| optionValue="id" | ||
| optionTextValue="name" | ||
| placeholder="Select organization" | ||
| value={organizations().find( | ||
| (org) => org.id === settings.organizationId, | ||
| )} | ||
| onChange={(option) => | ||
| setSettings("organizationId", option?.id ?? null) | ||
| } | ||
| itemComponent={(props) => ( | ||
| <MenuItem<typeof KSelect.Item> | ||
| as={KSelect.Item} | ||
| item={props.item} | ||
| > | ||
| {option.label} | ||
| </Button> | ||
| <div class="flex items-center gap-2 w-full"> | ||
| <KSelect.ItemLabel class="flex-1"> | ||
| {props.item.rawValue.name} | ||
| </KSelect.ItemLabel> | ||
| {/* Show ownership indicator */} | ||
| <Show | ||
| when={ | ||
| props.item.rawValue.ownerId === auth.data?.user_id | ||
| } | ||
| > | ||
| <span class="text-xs text-blue-10 bg-blue-3 px-1.5 py-0.5 rounded"> | ||
| Owner | ||
| </span> | ||
| </Show> | ||
| </div> | ||
| </MenuItem> | ||
| )} | ||
| </For> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| {/* Frame rate */} | ||
| <div class="overflow-hidden relative p-4 rounded-xl dark:bg-gray-2 bg-gray-3"> | ||
| <div class="flex flex-col gap-3"> | ||
| <h3 class="text-gray-12">Frame rate</h3> | ||
| <KSelect<{ label: string; value: number }> | ||
| options={ | ||
| settings.format === "Gif" ? GIF_FPS_OPTIONS : FPS_OPTIONS | ||
| } | ||
| optionValue="value" | ||
| optionTextValue="label" | ||
| placeholder="Select FPS" | ||
| value={(settings.format === "Gif" | ||
| ? GIF_FPS_OPTIONS | ||
| : FPS_OPTIONS | ||
| ).find((opt) => opt.value === settings.fps)} | ||
| onChange={(option) => { | ||
| const value = | ||
| option?.value ?? (settings.format === "Gif" ? 10 : 30); | ||
| trackEvent("export_fps_changed", { | ||
| fps: value, | ||
| }); | ||
| setSettings("fps", value); | ||
| }} | ||
| itemComponent={(props) => ( | ||
| <MenuItem<typeof KSelect.Item> | ||
| as={KSelect.Item} | ||
| item={props.item} | ||
| > | ||
| <KSelect.ItemLabel class="flex-1"> | ||
| {props.item.rawValue.label} | ||
| </KSelect.ItemLabel> | ||
| </MenuItem> | ||
| )} | ||
| > | ||
| <KSelect.Trigger class="flex flex-row gap-2 items-center px-3 w-full h-10 rounded-xl transition-colors dark:bg-gray-3 bg-gray-4 disabled:text-gray-11"> | ||
| <KSelect.Value< | ||
| (typeof FPS_OPTIONS)[number] | ||
| > class="flex-1 text-sm text-left truncate tabular-nums text-[--gray-500]"> | ||
| {(state) => <span>{state.selectedOption()?.label}</span>} | ||
| </KSelect.Value> | ||
| <KSelect.Icon<ValidComponent> | ||
| as={(props) => ( | ||
| <IconCapChevronDown | ||
| {...props} | ||
| class="size-4 shrink-0 transform transition-transform ui-expanded:rotate-180 text-[--gray-500]" | ||
| /> | ||
| )} | ||
| /> | ||
| </KSelect.Trigger> | ||
| <KSelect.Portal> | ||
| <PopperContent<typeof KSelect.Content> | ||
| as={KSelect.Content} | ||
| class={cx(topSlideAnimateClasses, "z-50")} | ||
| > | ||
| <MenuItemList<typeof KSelect.Listbox> | ||
| class="max-h-32 custom-scroll" | ||
| as={KSelect.Listbox} | ||
| > | ||
| <KSelect.Trigger class="flex flex-row gap-2 items-center px-2 py-2 w-full rounded-lg transition-colors bg-white border disabled:text-gray-11"> | ||
| <KSelect.Value<{ | ||
| id: string; | ||
| name: string; | ||
| ownerId: string; | ||
| }> class="flex-1 text-[11px] text-left truncate text-gray-12"> | ||
| {(state) => ( | ||
| <span> | ||
| {state.selectedOption()?.name ?? | ||
| "Select organization"} | ||
| </span> | ||
| )} | ||
| </KSelect.Value> | ||
| <KSelect.Icon<ValidComponent> | ||
| as={(props) => ( | ||
| <IconCapChevronDown | ||
| {...props} | ||
| class="size-3 shrink-0 transform transition-transform ui-expanded:rotate-180 text-gray-10" | ||
| /> | ||
| )} | ||
| /> | ||
| </PopperContent> | ||
| </KSelect.Portal> | ||
| </KSelect> | ||
| </div> | ||
| </KSelect.Trigger> | ||
| <KSelect.Portal> | ||
| <PopperContent<typeof KSelect.Content> | ||
| as={KSelect.Content} | ||
| class={cx(topSlideAnimateClasses, "z-50")} | ||
| > | ||
| <MenuItemList<typeof KSelect.Listbox> | ||
| class="max-h-32 custom-scroll" | ||
| as={KSelect.Listbox} | ||
| /> | ||
| </PopperContent> | ||
| </KSelect.Portal> | ||
| </KSelect> | ||
| </div> | ||
| </Show> | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Offer a “Personal” option and show selector even with a single org
Provide an explicit Personal option (null orgId) and render the selector when at least one org exists.
Apply this diff:
- <Show
- when={
- settings.exportTo === "link" &&
- auth.data &&
- organizations().length > 1
- }
- >
+ <Show
+ when={
+ settings.exportTo === "link" &&
+ auth.data &&
+ organizations().length > 0
+ }
+ >
<div class="flex flex-col gap-1.5 p-2 rounded-lg bg-gray-2 border animate-in fade-in slide-in-from-top duration-200">
<label class="flex items-center gap-1 text-[10px] text-gray-11 font-medium uppercase tracking-wide">
<IconLucideBuilding2 class="size-3" />
Organization
</label>
- <KSelect<{ id: string; name: string; ownerId: string }>
- options={organizations()}
+ <KSelect<{ id: string; name: string; ownerId: string }>
+ options={[{ id: "", name: "Personal", ownerId: "" }, ...organizations()]}
optionValue="id"
optionTextValue="name"
placeholder="Select organization"
- value={organizations().find(
- (org) => org.id === settings.organizationId,
- )}
- onChange={(option) =>
- setSettings("organizationId", option?.id ?? null)
- }
+ value={
+ (settings.organizationId
+ ? organizations().find((org) => org.id === settings.organizationId)
+ : { id: "", name: "Personal", ownerId: "" }
+ )
+ }
+ onChange={(option) => setSettings("organizationId", option?.id ? option.id : null)}
itemComponent={(props) => (
<MenuItem<typeof KSelect.Item>
as={KSelect.Item}
item={props.item}
>
<div class="flex items-center gap-2 w-full">
<KSelect.ItemLabel class="flex-1">
{props.item.rawValue.name}
</KSelect.ItemLabel>
- {/* Show ownership indicator */}
<Show
- when={
- props.item.rawValue.ownerId === auth.data?.user_id
- }
+ when={
+ props.item.rawValue.id !== "" &&
+ props.item.rawValue.ownerId === auth.data?.user_id
+ }
>
<span class="text-xs text-blue-10 bg-blue-3 px-1.5 py-0.5 rounded">
Owner
</span>
</Show>
</div>
</MenuItem>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Show | |
| when={ | |
| settings.exportTo === "link" && | |
| auth.data && | |
| organizations().length > 1 | |
| } | |
| > | |
| <div class="flex flex-col gap-1.5 p-2 rounded-lg bg-gray-2 border animate-in fade-in slide-in-from-top duration-200"> | |
| <label class="flex items-center gap-1 text-[10px] text-gray-11 font-medium uppercase tracking-wide"> | |
| <IconLucideBuilding2 class="size-3" /> | |
| Organization | |
| </label> | |
| <KSelect<{ id: string; name: string; ownerId: string }> | |
| options={organizations()} | |
| optionValue="id" | |
| optionTextValue="name" | |
| placeholder="Select organization" | |
| value={organizations().find( | |
| (org) => org.id === settings.organizationId, | |
| )} | |
| onChange={(option) => | |
| setSettings("organizationId", option?.id ?? null) | |
| } | |
| itemComponent={(props) => ( | |
| <MenuItem<typeof KSelect.Item> | |
| as={KSelect.Item} | |
| item={props.item} | |
| > | |
| {option.label} | |
| </Button> | |
| <div class="flex items-center gap-2 w-full"> | |
| <KSelect.ItemLabel class="flex-1"> | |
| {props.item.rawValue.name} | |
| </KSelect.ItemLabel> | |
| {/* Show ownership indicator */} | |
| <Show | |
| when={ | |
| props.item.rawValue.ownerId === auth.data?.user_id | |
| } | |
| > | |
| <span class="text-xs text-blue-10 bg-blue-3 px-1.5 py-0.5 rounded"> | |
| Owner | |
| </span> | |
| </Show> | |
| </div> | |
| </MenuItem> | |
| )} | |
| </For> | |
| </div> | |
| </div> | |
| </div> | |
| {/* Frame rate */} | |
| <div class="overflow-hidden relative p-4 rounded-xl dark:bg-gray-2 bg-gray-3"> | |
| <div class="flex flex-col gap-3"> | |
| <h3 class="text-gray-12">Frame rate</h3> | |
| <KSelect<{ label: string; value: number }> | |
| options={ | |
| settings.format === "Gif" ? GIF_FPS_OPTIONS : FPS_OPTIONS | |
| } | |
| optionValue="value" | |
| optionTextValue="label" | |
| placeholder="Select FPS" | |
| value={(settings.format === "Gif" | |
| ? GIF_FPS_OPTIONS | |
| : FPS_OPTIONS | |
| ).find((opt) => opt.value === settings.fps)} | |
| onChange={(option) => { | |
| const value = | |
| option?.value ?? (settings.format === "Gif" ? 10 : 30); | |
| trackEvent("export_fps_changed", { | |
| fps: value, | |
| }); | |
| setSettings("fps", value); | |
| }} | |
| itemComponent={(props) => ( | |
| <MenuItem<typeof KSelect.Item> | |
| as={KSelect.Item} | |
| item={props.item} | |
| > | |
| <KSelect.ItemLabel class="flex-1"> | |
| {props.item.rawValue.label} | |
| </KSelect.ItemLabel> | |
| </MenuItem> | |
| )} | |
| > | |
| <KSelect.Trigger class="flex flex-row gap-2 items-center px-3 w-full h-10 rounded-xl transition-colors dark:bg-gray-3 bg-gray-4 disabled:text-gray-11"> | |
| <KSelect.Value< | |
| (typeof FPS_OPTIONS)[number] | |
| > class="flex-1 text-sm text-left truncate tabular-nums text-[--gray-500]"> | |
| {(state) => <span>{state.selectedOption()?.label}</span>} | |
| </KSelect.Value> | |
| <KSelect.Icon<ValidComponent> | |
| as={(props) => ( | |
| <IconCapChevronDown | |
| {...props} | |
| class="size-4 shrink-0 transform transition-transform ui-expanded:rotate-180 text-[--gray-500]" | |
| /> | |
| )} | |
| /> | |
| </KSelect.Trigger> | |
| <KSelect.Portal> | |
| <PopperContent<typeof KSelect.Content> | |
| as={KSelect.Content} | |
| class={cx(topSlideAnimateClasses, "z-50")} | |
| > | |
| <MenuItemList<typeof KSelect.Listbox> | |
| class="max-h-32 custom-scroll" | |
| as={KSelect.Listbox} | |
| > | |
| <KSelect.Trigger class="flex flex-row gap-2 items-center px-2 py-2 w-full rounded-lg transition-colors bg-white border disabled:text-gray-11"> | |
| <KSelect.Value<{ | |
| id: string; | |
| name: string; | |
| ownerId: string; | |
| }> class="flex-1 text-[11px] text-left truncate text-gray-12"> | |
| {(state) => ( | |
| <span> | |
| {state.selectedOption()?.name ?? | |
| "Select organization"} | |
| </span> | |
| )} | |
| </KSelect.Value> | |
| <KSelect.Icon<ValidComponent> | |
| as={(props) => ( | |
| <IconCapChevronDown | |
| {...props} | |
| class="size-3 shrink-0 transform transition-transform ui-expanded:rotate-180 text-gray-10" | |
| /> | |
| )} | |
| /> | |
| </PopperContent> | |
| </KSelect.Portal> | |
| </KSelect> | |
| </div> | |
| </KSelect.Trigger> | |
| <KSelect.Portal> | |
| <PopperContent<typeof KSelect.Content> | |
| as={KSelect.Content} | |
| class={cx(topSlideAnimateClasses, "z-50")} | |
| > | |
| <MenuItemList<typeof KSelect.Listbox> | |
| class="max-h-32 custom-scroll" | |
| as={KSelect.Listbox} | |
| /> | |
| </PopperContent> | |
| </KSelect.Portal> | |
| </KSelect> | |
| </div> | |
| </Show> | |
| </div> | |
| <Show | |
| when={ | |
| settings.exportTo === "link" && | |
| auth.data && | |
| organizations().length > 0 | |
| } | |
| > | |
| <div class="flex flex-col gap-1.5 p-2 rounded-lg bg-gray-2 border animate-in fade-in slide-in-from-top duration-200"> | |
| <label class="flex items-center gap-1 text-[10px] text-gray-11 font-medium uppercase tracking-wide"> | |
| <IconLucideBuilding2 class="size-3" /> | |
| Organization | |
| </label> | |
| <KSelect<{ id: string; name: string; ownerId: string }> | |
| options={[{ id: "", name: "Personal", ownerId: "" }, ...organizations()]} | |
| optionValue="id" | |
| optionTextValue="name" | |
| placeholder="Select organization" | |
| value={ | |
| settings.organizationId | |
| ? organizations().find( | |
| (org) => org.id === settings.organizationId | |
| ) | |
| : { id: "", name: "Personal", ownerId: "" } | |
| } | |
| onChange={(option) => | |
| setSettings("organizationId", option?.id ? option.id : null) | |
| } | |
| itemComponent={(props) => ( | |
| <MenuItem<typeof KSelect.Item> | |
| as={KSelect.Item} | |
| item={props.item} | |
| > | |
| <div class="flex items-center gap-2 w-full"> | |
| <KSelect.ItemLabel class="flex-1"> | |
| {props.item.rawValue.name} | |
| </KSelect.ItemLabel> | |
| <Show | |
| when={ | |
| props.item.rawValue.id !== "" && | |
| props.item.rawValue.ownerId === auth.data?.user_id | |
| } | |
| > | |
| <span class="text-xs text-blue-10 bg-blue-3 px-1.5 py-0.5 rounded"> | |
| Owner | |
| </span> | |
| </Show> | |
| </div> | |
| </MenuItem> | |
| )} | |
| > | |
| <KSelect.Trigger class="flex flex-row gap-2 items-center px-2 py-2 w-full rounded-lg transition-colors bg-white border disabled:text-gray-11"> | |
| <KSelect.Value<{ | |
| id: string; | |
| name: string; | |
| ownerId: string; | |
| }> class="flex-1 text-[11px] text-left truncate text-gray-12"> | |
| {(state) => ( | |
| <span> | |
| {state.selectedOption()?.name ?? | |
| "Select organization"} | |
| </span> | |
| )} | |
| </KSelect.Value> | |
| <KSelect.Icon<ValidComponent> | |
| as={(props) => ( | |
| <IconCapChevronDown | |
| {...props} | |
| class="size-3 shrink-0 transform transition-transform ui-expanded:rotate-180 text-gray-10" | |
| /> | |
| )} | |
| /> | |
| </KSelect.Trigger> | |
| <KSelect.Portal> | |
| <PopperContent<typeof KSelect.Content> | |
| as={KSelect.Content} | |
| class={cx(topSlideAnimateClasses, "z-50")} | |
| > | |
| <MenuItemList<typeof KSelect.Listbox> | |
| class="max-h-32 custom-scroll" | |
| as={KSelect.Listbox} | |
| /> | |
| </PopperContent> | |
| </KSelect.Portal> | |
| </KSelect> | |
| </div> | |
| </Show> |
🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/ExportDialog.tsx around lines 534-616, the
Export dialog should include an explicit "Personal" option (null organizationId)
and show the org selector when at least one org exists (not only when >1). Add a
"Personal" option (id: null, name: "Personal", ownerId: auth.data?.user_id) to
the options passed to KSelect (or prepend it when mapping organizations()),
change the Show condition from organizations().length > 1 to
organizations().length >= 1, and update types/values to allow null for
organization id (adjust KSelect generic and value comparisons to accept
option.id === null). Ensure onChange sets settings.organizationId to option?.id
?? null and the displayed label shows "Personal" when the null option is
selected.
| } from "@solid-primitives/event-listener"; | ||
| import { useSearchParams } from "@solidjs/router"; | ||
| import { createQuery } from "@tanstack/solid-query"; | ||
| import { invoke } from "@tauri-apps/api/core"; |
There was a problem hiding this comment.
Replace raw invoke logging and avoid JSON.stringify of complex objects (can block recording).
- Use
commands.logMessage(...)instead ofinvoke("log_message", ...)for typed, centralized logging. - Avoid
JSON.stringify(props.target)andJSON.stringify(auth); stringify can throw on proxies (e.g., area bounds), preventing Start Recording from firing.
Apply this diff:
-import { invoke } from "@tauri-apps/api/core";
@@
-// Auto-select first organization if none is selected
-const auth = authStore.createQuery();
-
-invoke("log_message", {
- level: "info",
- message: `Organizations: ${JSON.stringify(auth)}`,
-}).catch(console.error);
+const auth = authStore.createQuery();
+void commands
+ .logMessage(
+ "info",
+ `Target overlay auth: user=${auth.data?.user_id ?? "anon"}, orgs=${organizations().length}`,
+ )
+ .catch(console.error);
@@
- invoke("log_message", {
- level: "info",
- message: `Auto-selecting organization: ${orgs[0].id}`,
- }).catch(console.error);
+ void commands
+ .logMessage("info", `Auto-selecting organization: ${orgs[0].id}`)
+ .catch(console.error);
setOptions("organizationId", orgs[0].id);
@@
-invoke("log_message", {
- level: "info",
- message: `Target select overlay mounted`,
-}).catch(console.error);
+void commands.logMessage("info", "Target select overlay mounted").catch(console.error);
@@
- invoke("log_message", {
- level: "info",
- message: `Starting recording with: target=${JSON.stringify(props.target)}, mode=${rawOptions.mode}, systemAudio=${rawOptions.captureSystemAudio}, organizationId=${rawOptions.organizationId ?? null}`,
- }).catch(console.error);
+ void commands
+ .logMessage(
+ "info",
+ `Starting recording: target=${props.target.variant}, mode=${rawOptions.mode}, systemAudio=${rawOptions.captureSystemAudio}, organizationId=${rawOptions.organizationId ?? "none"}`,
+ )
+ .catch(console.error);As per coding guidelines
Also applies to: 175-179, 183-188, 191-195, 900-903
🤖 Prompt for AI Agents
In apps/desktop/src/routes/target-select-overlay.tsx around line 8 (and also
update the usages at lines 175-179, 183-188, 191-195, 900-903), replace direct
use of invoke("log_message", ...) and JSON.stringify(...) on complex/possibly
proxied objects with the centralized typed logger commands.logMessage(...) and
avoid stringifying complex props; instead pass safe, primitive fields or
serialize defensively (e.g., pick id/type/primitive bounds) or wrap in try/catch
to fall back to a minimal message. Update each logging call to use
commands.logMessage with a structured payload (level, message, meta) and remove
JSON.stringify(props.target) and JSON.stringify(auth) to prevent throwing on
proxies and blocking Start Recording.
| // Eg. on Windows Ctrl+P would open the print dialog without this | ||
| createEventListener(document, "keydown", (e) => e.preventDefault()); | ||
|
|
||
| // Auto-select first organization if none is selected |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Remove inline comments to comply with repo policy.
The repo disallows inline/block/doc comments in TS/JS. Please remove the newly added comments.
-// Auto-select first organization if none is selected
@@
- {/* Backdrop to close dropdown when clicking outside */}
@@
- {/* Dropdown menu */}
@@
-{/* Organization selector - appears when instant mode is selected and user has organizations */}As per coding guidelines
Also applies to: 793-796, 949-949
🤖 Prompt for AI Agents
In apps/desktop/src/routes/target-select-overlay.tsx around lines 172 (and also
remove similar inline comments at lines 793-796 and 949), there are inline JS/TS
comments that violate the repo policy—remove those comment lines entirely (or
convert them to non-inline documentation that complies with the repo style,
e.g., external docs or i18n strings) so no // or /* */ comments remain in those
locations; ensure formatting and code function remain unchanged after deletion
and run linting to verify compliance.
| app.get("/organizations", async (c) => { | ||
| const user = c.get("user"); | ||
|
|
||
| const orgs = await db() | ||
| .select({ | ||
| id: organizations.id, | ||
| name: organizations.name, | ||
| ownerId: organizations.ownerId, | ||
| }) | ||
| .from(organizations) | ||
| .leftJoin( | ||
| organizationMembers, | ||
| eq(organizations.id, organizationMembers.organizationId), | ||
| ) | ||
| .where( | ||
| or( | ||
| eq(organizations.ownerId, user.id), | ||
| eq(organizationMembers.userId, user.id), | ||
| ), | ||
| ) | ||
| .groupBy(organizations.id); | ||
|
|
||
| return c.json(orgs); | ||
| }); |
There was a problem hiding this comment.
Fix GROUP BY to avoid ONLY_FULL_GROUP_BY failures and duplicate rows
Grouping only by id while selecting name/ownerId can fail on MySQL with only_full_group_by enabled and may yield non‑deterministic values. Group by all selected columns (or use distinct).
- .groupBy(organizations.id);
+ .groupBy(organizations.id, organizations.name, organizations.ownerId);Optionally, mirror other handlers and wrap in try/catch to return a 500 JSON on DB errors.
🤖 Prompt for AI Agents
In apps/web/app/api/desktop/[...route]/root.ts around lines 143 to 166, the
query groups only by organizations.id while selecting name and ownerId which
breaks with MySQL ONLY_FULL_GROUP_BY and can return non-deterministic/duplicate
rows; fix by grouping by all selected columns (organizations.id,
organizations.name, organizations.ownerId) or change to select distinct rows
instead, and optionally wrap the handler in a try/catch to return c.json({
error: 'Internal Server Error' }, 500) on DB errors.
1 participant
Summary by CodeRabbit