chore: improve the resilience of checking

[ameer2468]

Summary by CodeRabbit

• Refactor
  • Improved internal code structure for video ownership and pro status determination logic, reducing potential edge cases in status evaluation.

[coderabbitai]

Walkthrough

Two components are refactored to extract direct user ownership checks into an explicit isOwner boolean flag, then use this derived flag to compute isVideoOwnerPro more cleanly, improving code clarity and consistency.

Changes

Cohort / File(s)	Summary
Ownership Check Refactoring apps/web/app/s/[videoId]/_components/ShareHeader.tsx, apps/web/app/s/[videoId]/_components/Sidebar.tsx	Extracted direct user.id === data.ownerId comparisons into a derived isOwner boolean. Updated isVideoOwnerPro computation to rely on isOwner instead of inline equality checks: isVideoOwnerPro = user && isOwner ? userIsPro(user) : false. Added explicit boolean type annotation where applicable.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• chore: improve pro check  #990: Modifies video page pro-owner status determination by introducing an isOwner flag and guarded isVideoOwnerPro computation—directly related refactoring pattern.
• chore: only render share button for owners #1157: Introduces explicit isOwner check to gate share-related UI, replacing inline user.id === ownerId comparisons—aligned with this PR's ownership abstraction approach.

Poem

🐰 A hop through the code, refactoring's delight,
isOwner now flags who can edit just right,
No more scattered checks, the logic runs clean,
Derived booleans dancing—the clearest we've seen! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title Check	❓ Inconclusive	The title "improve the resilience of checking" is thematically related to the changeset, which does improve resilience by extracting ownership checks into an explicit isOwner boolean and refactoring the isVideoOwnerPro logic to reduce undefined behavior. However, the title is vague and generic—it does not clearly specify what is being checked (ownership logic), what files or components are affected, or how the resilience is improved (through refactoring/extraction of boolean flags). A teammate scanning the commit history would not immediately understand that this change involves refactoring ownership verification logic across ShareHeader and Sidebar components.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch chore-resillience-check

---

📜 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 d23ce3b and 563d861.

📒 Files selected for processing (2)

• apps/web/app/s/[videoId]/_components/ShareHeader.tsx (2 hunks)
• apps/web/app/s/[videoId]/_components/Sidebar.tsx (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 running pnpm format.

Use strict TypeScript and avoid any; leverage shared types

Files:

• apps/web/app/s/[videoId]/_components/Sidebar.tsx
• apps/web/app/s/[videoId]/_components/ShareHeader.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/s/[videoId]/_components/Sidebar.tsx
• apps/web/app/s/[videoId]/_components/ShareHeader.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/s/[videoId]/_components/Sidebar.tsx
• apps/web/app/s/[videoId]/_components/ShareHeader.tsx

apps/web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• apps/web/app/s/[videoId]/_components/Sidebar.tsx
• apps/web/app/s/[videoId]/_components/ShareHeader.tsx

apps/web/app/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• apps/web/app/s/[videoId]/_components/Sidebar.tsx
• apps/web/app/s/[videoId]/_components/ShareHeader.tsx

🧬 Code graph analysis (2)

apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)

packages/utils/src/constants/plans.ts (1)

• userIsPro (14-38)

apps/web/app/s/[videoId]/_components/ShareHeader.tsx (1)

packages/utils/src/constants/plans.ts (1)

• userIsPro (14-38)

⏰ 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/web/app/s/[videoId]/_components/ShareHeader.tsx (2)

132-132: LGTM: Improved clarity with explicit ownership check.

The refactoring to use the extracted isOwner boolean makes the Pro status logic clearer and more maintainable. The explicit : boolean type annotation is good practice.

---

227-227: LGTM: Consistent use of extracted ownership flag.

Using the pre-computed isOwner boolean here eliminates duplication and improves consistency.

apps/web/app/s/[videoId]/_components/Sidebar.tsx (1)

144-145: Now let me verify the actual comparison behavior one more time:

No critical type issue found—review comment is incorrect.

After verification, the original review comment incorrectly identifies this as a critical type mismatch. Both user?.id and data.ownerId are string type at runtime (user.id comes from NextAuth session typed as UserId = string, and ownerId is defined as string in VideoData). The comparison in Sidebar.tsx (user?.id === data.ownerId) works correctly and produces the same result as ShareHeader.tsx's .toString() variant.

The inconsistency exists but is a code style matter, not a functional bug that affects ownership determination. The refactoring itself is sound.

Likely an incorrect or invalid review comment.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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