Allow deleting instant recordings after successful upload #1192
Conversation
WalkthroughAdds a new delete-instant-recordings-after-upload setting to both backend (Rust) and frontend (TS). Updates recording completion flow to optionally delete instant recording directories post-upload. Refactors settings UI: renames components, introduces SettingGroup, and updates general/experimental settings to use new components and types. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as UI
participant App as Tauri App
participant Rec as Recording Service
participant Set as GeneralSettingsStore
participant FS as File System
UI->>Rec: Finish recording
Rec->>Rec: Update thumbnail (async)
alt Thumbnail update fails
Rec->>App: Log error
Rec-->>UI: Return (no deletion)
else Thumbnail update succeeds
Rec->>Set: Get settings
Set-->>Rec: { delete_instant_recordings_after_upload }
alt delete_instant_recordings_after_upload == true
Rec->>FS: remove_dir_all(recording_dir)
alt Deletion fails
Rec->>App: Log error
Rec-->>UI: Return
else Deletion succeeds
Rec-->>UI: Return
end
else Setting disabled
Rec-->>UI: Return
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src-tauri/src/recording.rs (1)
952-975: Don’t gate deletion on thumbnail upload; avoid race with meta save.
- If video upload succeeded but thumbnail upload fails, we never delete. Deletion should be independent of thumbnail success.
- Deletion runs in a spawned task and can race with the later meta save, causing spurious “Failed to load recording meta...” errors.
Minimal fix: still attempt deletion even if thumbnail upload fails.
- .await; - if let Err(err) = res { - error!("Error updating thumbnail for instant mode progressive upload: {err}"); - return; - } + .await; + if let Err(err) = res { + error!("Error updating thumbnail for instant mode progressive upload: {err}"); + // proceed; deletion should not depend on thumbnail success + } if GeneralSettingsStore::get(&app).ok().flatten().unwrap_or_default().delete_instant_recordings_after_upload { if let Err(err) = tokio::fs::remove_dir_all(&recording_dir).await { error!("Failed to remove recording files after upload: {err:?}"); - return; } }Optional follow-up to reduce log noise: when deleteInstantRecordingsAfterUpload is true for instant recordings, skip the meta save at lines ~1017–1024, or downgrade the log if the project path is missing.
🧹 Nitpick comments (3)
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
3-8: Type children via ParentProps; simplify onChange.Use strict typing and avoid any; pass onChange directly.
-export function SettingItem(props: { - pro?: boolean; - label: string; - description?: string; - children: any; -}) { +export function SettingItem( + props: ParentProps<{ pro?: boolean; label: string; description?: string }>, +) {Also:
- <Toggle - size="sm" - checked={props.value} - onChange={(v) => props.onChange(v)} - /> + <Toggle size="sm" checked={props.value} onChange={props.onChange} />Add import (outside this hunk):
import type { ParentProps } from "solid-js";As per coding guidelines.
Also applies to: 32-39
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (2)
303-315: Tighten SelectSettingItem types; remove any.Make options strongly typed against T.
-const SelectSettingItem = < - T extends - | MainWindowRecordingStartBehaviour - | PostStudioRecordingBehaviour - | PostDeletionBehaviour - | number, ->(props: { +const SelectSettingItem = < + T extends + | MainWindowRecordingStartBehaviour + | PostStudioRecordingBehaviour + | PostDeletionBehaviour + | number, +>(props: { label: string; description: string; value: T; onChange: (value: T) => void; - options: { text: string; value: any }[]; + options: { text: string; value: T }[]; }) => {As per coding guidelines.
487-493: Copy nit: fix sentence.- label="Delete instant mode recordings after upload" - description="After finishing an instant recording, should Cap will delete it from your device?" + label="Delete instant recordings after upload" + description="After finishing an instant recording, Cap will delete it from your device."
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
apps/desktop/src-tauri/src/general_settings.rs(2 hunks)apps/desktop/src-tauri/src/recording.rs(2 hunks)apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx(2 hunks)apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx(4 hunks)apps/desktop/src/routes/(window-chrome)/settings/general.tsx(8 hunks)apps/desktop/src/utils/tauri.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{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 runningpnpm format.Use strict TypeScript and avoid any; leverage shared types
Files:
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsxapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/general.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/desktop/src/routes/(window-chrome)/settings/Setting.tsxapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
apps/desktop/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/desktop/src/**/*.{ts,tsx}: Desktop icons are auto-imported (unplugin-icons); do not import icons manually
Desktop IPC: Call generated tauri_specta commands/events; listen to generated events and use typed interfaces
Use @tanstack/solid-query for server state in the desktop app
Files:
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsxapps/desktop/src/routes/(window-chrome)/settings/experimental.tsxapps/desktop/src/utils/tauri.tsapps/desktop/src/routes/(window-chrome)/settings/general.tsx
**/*.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/general_settings.rsapps/desktop/src-tauri/src/recording.rs
**/tauri.ts
📄 CodeRabbit inference engine (AGENTS.md)
Do not edit auto-generated files named
tauri.ts.NEVER EDIT auto-generated IPC bindings file: tauri.ts
Files:
apps/desktop/src/utils/tauri.ts
🧬 Code graph analysis (4)
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
apps/desktop/src/components/Toggle.tsx (1)
Toggle(37-50)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (1)
ToggleSettingItem(24-40)
apps/desktop/src-tauri/src/recording.rs (3)
apps/desktop/src-tauri/src/upload.rs (1)
singlepart_uploader(683-713)apps/desktop/src/utils/tauri.ts (1)
GeneralSettingsStore(396-396)apps/desktop/src-tauri/src/general_settings.rs (1)
get(208-219)
apps/desktop/src/routes/(window-chrome)/settings/general.tsx (2)
apps/desktop/src/utils/tauri.ts (4)
MainWindowRecordingStartBehaviour(418-418)PostStudioRecordingBehaviour(434-434)PostDeletionBehaviour(433-433)requestPermission(125-127)apps/desktop/src/routes/(window-chrome)/settings/Setting.tsx (2)
SettingItem(3-22)ToggleSettingItem(24-40)
⏰ 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 (3)
apps/desktop/src/routes/(window-chrome)/settings/experimental.tsx (1)
6-6: LGTM: component rename only.Renamed to ToggleSettingItem with identical props/behavior. No issues.
Also applies to: 58-66, 74-86, 87-99, 100-112
apps/desktop/src-tauri/src/general_settings.rs (1)
123-124: New setting field wired correctly.serde(default) + Default(false) ensures backward compatibility; camelCase exposure aligns with TS type.
Also applies to: 189-190
apps/desktop/src/utils/tauri.ts (1)
396-396: Confirm this change is generated, not manual.tauri.ts is auto-generated; ensure this property came from regenerating bindings after the Rust change, not by hand. Otherwise it will be overwritten.
As per coding guidelines.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes