fix: mux video uploads on journeys admin leading to blank video #8556
Conversation
…ndency - Updated build scripts to use Bun for cross-platform builds. - Added bun-types to package.json and pnpm-lock.yaml. - Refactored video-importer to utilize Bun for executing scripts and managing dependencies. - Removed sea-config.json as it is no longer needed. - Enhanced error handling for environment variables in Firebase and R2 services. - Updated E2E tests to run with Bun instead of npx.
…te TypeScript configuration - Added module and moduleResolution options in tsconfig.json for improved module handling. - Replaced direct process.env usage with a centralized env import for environment variables across multiple files. - Removed dotenv/config import statements as environment variables are now managed through the env module. - Deleted unused envVarTest.ts file to clean up the codebase.
…min-sometimes-leading-to' of https://github.com/JesusFilm/core into tannerfleming/eng-3559-mux-video-uploads-on-journeys-admin-sometimes-leading-to
WalkthroughReplaced interval-based polling with a per-video VideoPoller component; moved polling lifecycle and notifications into MuxVideoUploadProvider, introduced stopPollingFnsRef for cancellation, removed several polling utilities and tests, and added VideoPoller unit tests. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Provider as MuxVideoUploadProvider
participant Poller as VideoPoller
participant GraphQL as Apollo/GetMyMuxVideo
participant UI as Snackbar
Provider->>Poller: render(videoId,startTime,callbacks,register/unregister)
Poller->>GraphQL: start polling (useLazyQuery, POLL_INTERVAL)
GraphQL-->>Poller: response (ready or not) / error
alt readyToStream
Poller->>Provider: onComplete()
Provider->>UI: show success snackbar
Poller->>Provider: unregisterStopPolling(videoId)
else error
Poller->>Provider: onError(error)
Provider->>UI: show error snackbar
Poller->>Provider: unregisterStopPolling(videoId)
else timeout
Poller->>Provider: onTimeout()
Provider->>UI: show timeout snackbar
Poller->>Provider: unregisterStopPolling(videoId)
end
note right of Provider: External cancel invokes stop function from stopPollingFnsRef
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (6)
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 |
|
View your CI Pipeline Execution ↗ for commit 5c98662
☁️ Nx Cloud last updated this comment at |
github-actions
bot
requested a deployment
to
Preview - journeys-admin
![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
🧹 Nitpick comments (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsx (1)
49-55: Unused helper function.The
wrapperhelper is defined but never used—each test creates its ownMockedProviderinline. Consider removing it to reduce dead code, or refactor tests to use it for consistency.🔎 Proposed fix
- const wrapper = (mocks: MockedResponse[]) => { - return ({ children }: { children: ReactNode }) => ( - <MockedProvider mocks={mocks} addTypename={false}> - {children} - </MockedProvider> - ) - } -apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (1)
114-147: Potential stale closure withpollingTasksdependency.
handlePollingCompletereads frompollingTasks(line 117) and has it in the dependency array (line 146). However, since this is aMapreference that changes on everysetPollingTaskscall, the closure may not have the latest task data when the callback fires.Consider using the functional update pattern to access the current state:
🔎 Proposed fix
const handlePollingComplete = useCallback( (videoId: string) => { - const task = pollingTasks.get(videoId) - if (task == null) return - - setPollingTasks((prev) => { - const next = new Map(prev) - next.set(videoId, { ...task, status: 'completed' }) - return next - }) - - showSnackbar(t('Video upload completed'), 'success', { - autoHideDuration: 4000, - preventDuplicate: true, - persist: false - }) - - // Call the completion callback - if (task.onComplete != null) { - task.onComplete() - } + setPollingTasks((prev) => { + const task = prev.get(videoId) + if (task == null) return prev + + const next = new Map(prev) + next.set(videoId, { ...task, status: 'completed' }) + + // Call the completion callback + if (task.onComplete != null) { + task.onComplete() + } + + return next + }) + + showSnackbar(t('Video upload completed'), 'success', { + autoHideDuration: 4000, + preventDuplicate: true, + persist: false + }) // Remove from state after notification setTimeout(() => { setPollingTasks((prev) => { const next = new Map(prev) next.delete(videoId) return next }) }, TASK_CLEANUP_DELAY) }, - [pollingTasks, showSnackbar, t] + [showSnackbar, t] )
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (19)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/clearPollingInterval.spec.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/clearPollingInterval.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/handlePollingComplete.spec.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/handlePollingComplete.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/handlePollingError.spec.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/handlePollingError.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/startPolling.spec.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/startPolling.ts
💤 Files with no reviewable changes (12)
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/clearPollingInterval.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/index.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/index.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/clearPollingInterval/clearPollingInterval.spec.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/handlePollingError.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/handlePollingError.spec.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingError/index.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/handlePollingComplete.spec.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/index.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/handlePollingComplete/handlePollingComplete.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/startPolling.spec.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/startPolling/startPolling.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-11-16T21:30:53.412Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 8309
File: apps/journeys-admin/setupTests.tsx:47-51
Timestamp: 2025-11-16T21:30:53.412Z
Learning: In apps/journeys-admin/setupTests.tsx, the `document.clearImmediate` mock is required for tests involving the mux upload provider, as an underlying library uses clearImmediate for timeout management to prevent race conditions and unnecessary polling calls.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Wrap component specs with `MockedProvider`, `VideoProvider`, and `WatchProvider` when the unit touches those contexts—`NewVideoContentPage.spec.tsx` shows the expected harness
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/watch/test`.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Use the shared Jest setup in `apps/watch/setupTests.tsx` which already boots MSW, Next router mock, and has a longer async timeout.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.spec.{ts,tsx} : Enclose SWR-based hooks in `TestSWRConfig` (`apps/watch/test/TestSWRConfig.tsx`) to isolate cache state between assertions in tests.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : UI flows must sit inside `VideoProvider`, `WatchProvider`, and `PlayerProvider`. Mirror that wiring when composing features and when writing tests.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-04-08T07:57:39.246Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6209
File: apps/watch/src/components/CollectionsPage/ContainerHeroVideo/ContainerHeroVideo.tsx:60-63
Timestamp: 2025-04-08T07:57:39.246Z
Learning: When adding new method calls to code that uses video.js player (or any external library), the corresponding test mocks must be updated to include those methods. In this specific case, adding `player.src()` to production code required adding a mock implementation of the `src` method in test files to prevent "TypeError: player.src is not a function" errors.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/*.spec.ts?(x) : Co-locate React Testing Library specs under `*.spec.ts(x)` and mock network traffic with MSW handlers in `apps/resources/test`
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : `usePlayer` handles playback state (mute, fullscreen, current time). Keep reducer updates idempotent and avoid mixing DOM mutations with state—hero playback relies on these flags.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Export all components through index.ts files.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.ts
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Applies to apps/resources/**/index.ts : Export all components through index.ts files
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.ts
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Do not introduce new MUI usage; migrate existing MUI to shadcn/Tailwind. New shadcn work should coexist with core shells/wrappers still using MUI until explicit migration tasks retire them.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-05-22T02:57:46.179Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 6734
File: apps/journeys-admin/src/components/TermsAndConditions/TermsAndConditions.tsx:58-71
Timestamp: 2025-05-22T02:57:46.179Z
Learning: The team prefers to avoid try/catch blocks when working with Apollo Client mutations in favor of Apollo's built-in error handling mechanisms (checking the returned errors property).
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Apollo hooks should use generated types alongside colocated documents. Reference the pattern of `useVideoChildren` (`apps/watch/src/libs/useVideoChildren/useVideoChildren.ts`) for data fetching.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: `usePlayer` handles playback state (mute, fullscreen, current time). Keep reducer updates idempotent and avoid mixing DOM mutations with state—hero playback relies on these flags
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx
🧬 Code graph analysis (4)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsx (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (1)
GET_MY_MUX_VIDEO_QUERY(27-36)apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (1)
VideoPoller(23-102)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (1)
GET_MY_MUX_VIDEO_QUERY(27-36)apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/constants.ts (2)
POLL_INTERVAL(1-1)MAX_POLL_TIME(2-2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/constants.ts (1)
TASK_CLEANUP_DELAY(3-3)apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (1)
VideoPoller(23-102)
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/types.ts (1)
UploadTask(9-19)apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.ts (1)
cancelUploadForBlock(16-60)
⏰ 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). (7)
- GitHub Check: Deploy Preview (journeys-admin, 8556/merge, pull_request, 22)
- GitHub Check: test (22, 2/3)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: build (22)
- GitHub Check: lint (22)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (18)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (4)
1-16: LGTM! Well-structured imports and interface.The
VideoPollerPropsinterface clearly defines the component contract with all necessary callbacks for lifecycle management (complete, error, timeout) and external cancellation control (register/unregister).
34-74: Consider memoizing or stabilizing dependencies to prevent potential re-registration issues.The
stopPollingfunction returned byuseLazyQuerymay have unstable reference identity across renders. SincestopPollingis in theuseEffectdependency array (line 95), this could cause the effect to re-run on every poll response, leading to repeated registration calls.In practice, Apollo's
stopPollingtends to be stable, but verify this doesn't cause issues in testing. If it does, consider wrapping withuseCallbackor removing from deps with an eslint-disable comment if the behavior is intentional.
40-65: Good use ofhasCompletedRefto prevent race conditions.The guard against duplicate completions is essential since polling callbacks can fire multiple times. The early returns at lines 42 and 67 correctly prevent duplicate processing.
76-98: Clean lifecycle management with proper cleanup.The effect correctly:
- Registers the stop function for external cancellation
- Starts the initial query
- Cleans up on unmount by stopping polling and unregistering
This pattern ensures no leaked polling intervals.
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/index.ts (1)
1-2: No functional changes.This is just a barrel export file with a trailing newline added.
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.ts (1)
1-2: Follows project conventions for barrel exports.Clean re-export pattern consistent with the codebase structure.
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.ts (1)
39-52: Clean migration to callback-based cancellation.The new approach properly:
- Retrieves and invokes the stop function if registered
- Removes the entry from the ref map
- Updates polling tasks state
- Clears the notification tracking
The null check on
stopFn(line 42) ensures graceful handling when no stop function is registered.apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsx (3)
16-25: Good test structure with clear default props.The
defaultPropsobject provides sensible defaults and jest mocks for all callbacks, making individual tests clean and focused.
61-173: Comprehensive test coverage for VideoPoller lifecycle.Tests properly cover:
- Registration of stopPolling on mount
- onComplete when video is ready
- onError on query failure
- onTimeout when MAX_POLL_TIME exceeded
- Not calling onComplete when video is not ready
The timeout test (line 134) correctly sets
startTimein the past to trigger the timeout condition.
175-208: LGTM! Cleanup and render tests are solid.The unmount test verifies proper cleanup, and the render test confirms the component is invisible as intended.
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.spec.ts (3)
60-87: Good refactor with helper function.The
createVideoBlockhelper reduces boilerplate and makes tests more readable. It properly sets all required VideoBlock fields with sensible defaults.
114-137: Test correctly verifies new stopPolling callback mechanism.The test properly:
- Registers a mock stopPolling function
- Calls
cancelUploadForBlock- Verifies the stop function was invoked
- Confirms the function was unregistered from the map
186-206: Good edge case coverage.This test ensures the code handles the case where a task has a videoId but no stopPolling function is registered—important for robustness during race conditions or unexpected states.
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (5)
25-25: Good architectural decision using VideoPoller component.Moving polling logic to a dedicated component simplifies the provider and leverages Apollo's built-in polling capabilities rather than manual setInterval management.
84-89: Clean ref-based storage for cancellation callbacks.Using a ref for
stopPollingFnsRefis appropriate since these functions don't need to trigger re-renders when they change.
149-176:handlePollingErroruses functional update correctly.Unlike
handlePollingComplete, this handler properly accesses the task via the functional update pattern (prev.get(videoId)), avoiding stale closure issues.
186-213: CleanstartPollingimplementation.The function now only creates the task entry and shows the notification—actual polling is delegated to VideoPoller. The
hasShownStartNotificationguard prevents duplicate snackbars.
278-304: Elegant pattern: rendering VideoPoller components for each processing task.This approach cleanly separates concerns:
- Provider manages task state
- VideoPoller handles individual polling lifecycle
- React handles component mounting/unmounting as tasks change
The
key={videoId}ensures proper component identity.
github-actions
bot
temporarily deployed
to
Preview - journeys-admin
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (1)
45-48: Consider extracting the readiness check for reusability.The video readiness logic could be extracted into a helper function for clarity and potential reuse.
🔎 Proposed refactor to extract readiness check
Create a helper function in
apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/:// utils/videoReadiness.ts export function isVideoReady(video: { readyToStream?: boolean | null assetId?: string | null playbackId?: string | null } | null | undefined): boolean { return video?.readyToStream === true && video?.assetId != null && video?.playbackId != null }Then update this file:
+import { isVideoReady } from '../utils/videoReadiness' onCompleted: (data) => { if (hasCompletedRef.current) return const video = data?.getMyMuxVideo - const isVideoReady = - video?.readyToStream === true && - video?.assetId != null && - video?.playbackId != null - if (isVideoReady) { + if (isVideoReady(video)) { hasCompletedRef.current = true stopPolling() unregisterStopPolling(videoId) onComplete() return }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsxapps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.tsapps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.tslibs/locales/en/apps-journeys-admin.json
✅ Files skipped from review due to trivial changes (1)
- libs/locales/en/apps-journeys-admin.json
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/index.ts
- apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.spec.tsx
- apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/cancelUploadForBlock/cancelUploadForBlock.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : `usePlayer` handles playback state (mute, fullscreen, current time). Keep reducer updates idempotent and avoid mixing DOM mutations with state—hero playback relies on these flags.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-11-16T21:30:53.412Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 8309
File: apps/journeys-admin/setupTests.tsx:47-51
Timestamp: 2025-11-16T21:30:53.412Z
Learning: In apps/journeys-admin/setupTests.tsx, the `document.clearImmediate` mock is required for tests involving the mux upload provider, as an underlying library uses clearImmediate for timeout management to prevent race conditions and unnecessary polling calls.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : UI flows must sit inside `VideoProvider`, `WatchProvider`, and `PlayerProvider`. Mirror that wiring when composing features and when writing tests.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
🧬 Code graph analysis (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (1)
GET_MY_MUX_VIDEO_QUERY(27-36)apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/constants.ts (2)
POLL_INTERVAL(1-1)MAX_POLL_TIME(2-2)
⏰ 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). (12)
- GitHub Check: Deploy Preview (watch, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (watch-modern, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (videos-admin, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (resources, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys-admin, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (player, 8556/merge, pull_request, 22)
- GitHub Check: test (22, 3/3)
- GitHub Check: test (22, 1/3)
- GitHub Check: test (22, 2/3)
- GitHub Check: build (22)
- GitHub Check: lint (22)
🔇 Additional comments (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (1)
34-74: Well-designed polling mechanism using Apollo's built-in capabilities.The use of Apollo's
pollIntervalwith guards viahasCompletedRefis a significant improvement over manual interval management. The readiness, timeout, and error paths are clearly separated and properly handle cleanup.
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
Show resolved
Hide resolved
|
The latest updates on your projects.
|
…journeys-admin-leading-to-blank-video
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (1)
58-64: Consider adding explicit return after timeout for clarity.After invoking
onTimeout()on line 63, there's no explicitreturnstatement. While this is functionally correct (thehasCompletedRef.current = trueon line 60 ensures subsequent polls will early-return at line 42), adding a return statement would make the control flow more explicit.🔎 Proposed refactor for clarity
// Check for timeout if (Date.now() - startTime > MAX_POLL_TIME) { hasCompletedRef.current = true stopPolling() unregisterStopPolling(videoId) onTimeout() + return }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
**/*.{ts,tsx,js,jsx}: Use early returns whenever possible to make the code more readable.
Use descriptive variable and function/const names.
Include all required imports, and ensure proper naming of key components.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/base.mdc)
Define a type if possible.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
apps/**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/apps.mdc)
apps/**/*.{js,jsx,ts,tsx}: Always use MUI over HTML elements; avoid using CSS or tags.
Use descriptive variable and function/const names. Also, event functions should be named with a “handle” prefix, like “handleClick” for onClick and “handleKeyDown” for onKeyDown.
Implement accessibility features on elements. For example, a tag should have a tabindex=“0”, aria-label, on:click, and on:keydown, and similar attributes.
Files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : Preserve existing contracts in components like `VideoBlock` which depend on the full provider stack, video.js, and mux metadata so autoplay, subtitles, and analytics remain intact.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T19:18:43.790Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/resources/AGENTS.md:0-0
Timestamp: 2025-12-19T19:18:43.790Z
Learning: Components like `VideoContentHero` depend on the full provider stack plus video.js and mux metadata; preserve the existing contracts so autoplay, subtitles, and analytics remain intact
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : `usePlayer` handles playback state (mute, fullscreen, current time). Keep reducer updates idempotent and avoid mixing DOM mutations with state—hero playback relies on these flags.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-11-16T21:30:53.412Z
Learnt from: Kneesal
Repo: JesusFilm/core PR: 8309
File: apps/journeys-admin/setupTests.tsx:47-51
Timestamp: 2025-11-16T21:30:53.412Z
Learning: In apps/journeys-admin/setupTests.tsx, the `document.clearImmediate` mock is required for tests involving the mux upload provider, as an underlying library uses clearImmediate for timeout management to prevent race conditions and unnecessary polling calls.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
📚 Learning: 2025-12-19T04:58:24.460Z
Learnt from: CR
Repo: JesusFilm/core PR: 0
File: apps/watch/AGENTS.md:0-0
Timestamp: 2025-12-19T04:58:24.460Z
Learning: Applies to apps/watch/src/**/*.{ts,tsx} : UI flows must sit inside `VideoProvider`, `WatchProvider`, and `PlayerProvider`. Mirror that wiring when composing features and when writing tests.
Applied to files:
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx
🧬 Code graph analysis (1)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (2)
apps/journeys-admin/src/components/MuxVideoUploadProvider/MuxVideoUploadProvider.tsx (1)
GET_MY_MUX_VIDEO_QUERY(27-36)apps/journeys-admin/src/components/MuxVideoUploadProvider/utils/constants.ts (2)
POLL_INTERVAL(1-1)MAX_POLL_TIME(2-2)
⏰ 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). (8)
- GitHub Check: Deploy Preview (journeys-admin, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (videos-admin, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (player, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (watch, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (journeys, 8556/merge, pull_request, 22)
- GitHub Check: Deploy Preview (resources, 8556/merge, pull_request, 22)
- GitHub Check: build (22)
- GitHub Check: lint (22)
🔇 Additional comments (3)
apps/journeys-admin/src/components/MuxVideoUploadProvider/VideoPoller/VideoPoller.tsx (3)
1-16: LGTM! Well-structured imports and interface.The imports are clean and the
VideoPollerPropsinterface clearly defines all required callbacks and registry functions with proper TypeScript types.
18-74: Excellent polling implementation with proper guards.The
useLazyQuerysetup correctly uses:
network-onlyfetch policy for fresh datahasCompletedRefguards in bothonCompletedandonErrorto prevent race conditions- Proper cleanup sequence (set flag, stop polling, unregister, invoke callback)
The timeout check and ready-state validation are well-implemented.
77-92: No action needed; the asymmetry is intentional and safe.The registered stop function (lines 79-82) intentionally does not call
unregisterStopPolling(videoId)because it's designed as a soft stop for external callers. Full cleanup (including unregistration) is guaranteed through one of three paths:
- Natural polling completion (success on line 53, timeout on line 62):
unregisterStopPollingcalled in callbacks- Polling error (line 70):
unregisterStopPollingcalled in error handler- Component unmount (line 91):
unregisterStopPollingcalled in cleanup functionThis design prevents memory leaks because at least one of these paths will always execute and clean up the registration.
…9-mux-video-uploads-on-journeys-admin-leading-to-blank-video
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores / Refactor
✏️ Tip: You can customize this high-level summary in your review settings.