fix: Duration check incorrectly prevents compliant video upload#1127
fix: Duration check incorrectly prevents compliant video upload#1127jumarmartin wants to merge 2 commits intoCapSoftware:mainfrom
Conversation
WalkthroughUpdated ExportDialog canShare logic to prefer exportEstimates.data?.duration_seconds over metadata.duration when determining share allowance and upgrade requirement; plan checks remain. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant ExportDialog
participant ExportEstimates as exportEstimates
participant Plan as PlanStatus
User->>ExportDialog: Open Share/Export
ExportDialog->>ExportEstimates: Read data?.duration_seconds (optional)
ExportDialog->>Plan: Check plan status
alt Has plan
ExportDialog-->>User: canShare.allowed = true
else No plan
alt exportEstimates duration available
ExportDialog-->>User: Evaluate limit using duration_seconds
else Fallback to metadata
ExportDialog-->>User: Evaluate limit using metadata.duration
end
end
note over ExportDialog: Set canShare.reason = "upgrade_required" when limit exceeded
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/desktop/src/routes/editor/ExportDialog.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
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/routes/editor/ExportDialog.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/desktop/src/routes/editor/ExportDialog.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 runningpnpm format.
Files:
apps/desktop/src/routes/editor/ExportDialog.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/editor/ExportDialog.tsx
| allowed: plan || (exportEstimates.data?.duration_seconds ?? metadata.duration < 300), | ||
| reason: !plan && (exportEstimates.data?.duration_seconds ?? metadata.duration >= 300) ? "upgrade_required" : null, |
There was a problem hiding this comment.
Critical: Operator precedence breaks duration check.
The ?? operator has lower precedence than comparison operators, so line 317 is parsed as:
plan || ((exportEstimates.data?.duration_seconds) ?? (metadata.duration < 300))When exportEstimates.data?.duration_seconds is nullish, the fallback is the boolean metadata.duration < 300, not the numeric metadata.duration. This breaks the duration check entirely. Line 318 has the same issue.
Apply this diff to fix the precedence:
const canShare = {
- allowed: plan || (exportEstimates.data?.duration_seconds ?? metadata.duration < 300),
- reason: !plan && (exportEstimates.data?.duration_seconds ?? metadata.duration >= 300) ? "upgrade_required" : null,
+ allowed: plan || (exportEstimates.data?.duration_seconds ?? metadata.duration) < 300,
+ reason: !plan && (exportEstimates.data?.duration_seconds ?? metadata.duration) >= 300 ? "upgrade_required" : null,
};🤖 Prompt for AI Agents
In apps/desktop/src/routes/editor/ExportDialog.tsx around lines 317 to 318, the
nullish coalescing is evaluated before the comparison due to operator
precedence, causing the fallback to be a boolean; wrap the coalescing expression
in parentheses so you first choose the numeric duration and then compare it to
300 (e.g. use (exportEstimates.data?.duration_seconds ?? metadata.duration) <
300 for the allowed check and the analogous >= 300 expression for the reason
check).
The Problem (video)
Although my intended video is certainly less than five minutes, I am prevented from exporting to my personal Cap instance and given the "Upgrade to Pro" interstitial.
The Solution
Checking
exportEstimates'sduration_secondswhere available and falling back tometadata.durationwhere not.Why? (video)
It seems like when calculating the duration of clips from a Studio export we're given an incorrect total value. As this is a largely frontend barrier, and
exportEstimatesgives us the correct duration, we can leverage it here to make the duration check with confidence. This may be an issue unrelated to Cap in particular, instead being an issue between the mp4 reader and macOS Tahoe Beta builds or somewhere along the call-chain.Summary by CodeRabbit