[TV] Wire up episode actions on the podcast, playlist and Up Next screens

[sztomek]

Description

The TV episode-actions modal previously showed six buttons that were all fake toasts — tapping "Play next", "Mark as played", "Archive", etc. only showed a toast and did nothing to the episode. This PR wires those actions to the real data layer and makes the button set context-aware so appropriate actions will be displayed on the modals.

What changed

• New TvEpisodeActionsViewModel / TvEpisodeActions — centralises the real operations (mirroring the iOS EpisodeRowViewModel), calling EpisodeManager/PlaybackManager the same way the mobile EpisodeFragmentViewModel does:
  • Play next / Play last → PlaybackManager.playNext / playLast
  • Mark as played / unplayed → EpisodeManager.markAsPlayedBlocking / markAsNotPlayedBlocking
  • Archive / unarchive → EpisodeManager.archiveBlocking / unarchiveBlocking
  • Remove from Up Next → PlaybackManager.removeEpisode
• Context-aware button sets (TvEpisodeActionContext), matching tvOS:
  • Podcast details: Show notes · Play next · Play last · Mark (un)played · (Un)archive — no "Go to podcast" (you're already there; previously it was shown and did nothing).
  • Playlist details: the above plus "Go to podcast".
  • Up Next: Show notes · Go to podcast · Play next · Play last · Remove from Up Next (no mark/archive), matching the tvOS .upNext context.
• "Go to podcast" navigation wired on the Up Next and Playlist-details screens via the same podcast-details overlay pattern already used by Home and Your Podcasts.

Fixes PCDROID-701 https://linear.app/a8c/issue/PCDROID-701/wire-up-actions

Testing Instructions

Requires a signed-in account on an Android TV device/emulator.

1. Open a podcast's details screen, focus an episode, press the menu/options button to open the actions modal. Confirm:
   • There is no "Go to podcast" button.
   • "Play next" / "Play last" actually add the episode to Up Next (check the Up Next tab).
   • "Mark as played" / "Mark as unplayed" and "Archive" / "Unarchive" toggle correctly and the labels flip on reopen.
2. Open the Up Next tab, open an episode's actions. Confirm the set is Show notes · Go to podcast · Play next · Play last · Remove from Up Next; "Remove from Up Next" removes it; "Go to podcast" navigates to that podcast (back returns to Up Next).
3. Open a playlist, open an episode's actions. Confirm "Go to podcast" is present and navigates; the mark/archive actions work.
4. Verify the toasts still appear for each action.

Screenshots or Screencast

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md
• Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
• I have considered whether it makes sense to add tests for my changes
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
• Any jetpack compose components I added or changed are covered by compose previews
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

I have tested any UI changes...

• with different themes
• with a landscape orientation
• with the device set to have a large display and font size
• for accessibility with TalkBack

[claude]

Claude finished @sztomek's task in 6m 27s —— View job

---

Code review

Solid, well-scoped PR — it replaces the fake toasts with real EpisodeManager/PlaybackManager calls, mirrors the mobile EpisodeFragmentViewModel faithfully, and the podcast-details overlay wiring in TvUpNextScreen/TvPlaylistDetailsScreen is a copy of the pattern already used in TvHomeScreen/TvYourPodcastsScreen (including rememberSaveable + BackHandler placement), so it stays consistent. I verified the things most likely to bite:

• ✅ TvPodcastDetailsScreen keys its hiltViewModel by podcastUuid, so opening podcast A → back → podcast B from Up Next/playlists won't serve stale state.
• ✅ TvModal is a real Dialog, so back inside the actions modal dismisses the modal, not the new outer BackHandler.
• ✅ removeFromUpNext without a coroutine is correct — PlaybackManager.removeEpisode launches internally.
• ✅ remove_from_up_next / go_to_podcast already exist; only the new toast string was added, in the right file and section.
• ✅ tv test deps already include mockito.kotlin + sharedtest, so the new test compiles; default-arg matching in the verify calls lines up.

No blocking issues found. Details are in inline comments; summary:

#	Where	Severity	Issue
1	TvEpisodeActionsModal.kt:99	Low–Med	"Go to podcast" is gated on both context and onGoToPodcast != null — a caller that forgets the callback silently loses the button, no compile error (the exact bug class this PR fixes)
2	TvEpisodeActionsModal.kt:91-95	Low	Toast fires unconditionally before the work runs; a throwing *Blocking call is uncaught inside viewModelScope.launch → crash, with a success toast already shown
3	TvEpisodeActionsViewModel.kt:41	Low	viewModelScope for dismiss-and-forget writes; @ApplicationScope (already used in this module by TvSignOutManager) is the better fit. Low risk today only because the VM resolves to the long-lived HOME destination
4	TvEpisodeActionsViewModelTest.kt:46	Low	advanceUntilIdle() advances runTest's scheduler, not MainCoroutineRule's UnconfinedTestDispatcher — it's a no-op; tests pass only because Unconfined is eager
5	TvEpisodeActionsModal.kt:33	Nit	Parameter named context collides conceptually with LocalContext.current locals in the same files
6	TvEpisodeActionsViewModel.kt:22-30	Nit	source is derivable from the context; interface could take TvEpisodeActionContext
7	TvEpisodeActionsViewModel.kt:77	Nit	removeFromUpNext reads like a missing launch; worth a comment

Two additional notes not tied to a line:

The analytics rationale in the description looks inaccurate. It says mark/archive analytics would need "EventHorizon pulled into the tv module", but EventHorizon is already on the tv compile classpath transitively — modules/services/analytics/build.gradle.kts:19 has api(libs.eventhorizon), repositories/build.gradle.kts:34 has api(projects.modules.services.analytics), and AnalyticsModule already @Provides it. That's the same route by which this PR gets SourceView. So parity with mobile (EpisodeMarkedAsPlayedEvent / EpisodeArchivedEvent / their un- counterparts, all existing events) is a constructor param plus ~10 lines if you'd rather not leave a gap where mark/archive from TV is invisible in analytics while play-next/last is not. Still fine as a follow-up — just worth fixing the stated reason.

Test coverage sits on the wrong side of the risk. The seven VM tests cover the thin delegation layer; the logic this PR actually introduces is the context → button-set mapping in TvEpisodeActionsModalContent, which has no test (previews only). Extracting the buildList block into a pure function — e.g. internal fun episodeActionButtons(context, episode, hasGoToPodcast): List<TvEpisodeAction> returning label-res-id + action ids — would let you assert "Up Next shows Remove and not Mark/Archive" and "Podcast details shows no Go to podcast" cheaply, which is what a future refactor is most likely to break.

I could not run ./gradlew :tv:testDebugUnitTest or spotlessCheck in this environment (Gradle execution wasn't permitted), so the correctness notes above are from reading the code, not from a green build.
· Branch feat/tv-episode-actions

[claude]

The toast is shown unconditionally and immediately, while action() only schedules the real work on a background coroutine. If archiveBlocking / markAsPlayedBlocking throws (DB error), the user still sees "Archived" and the exception is uncaught inside viewModelScope.launch — which on Android means a crash rather than a no-op.

The mobile EpisodeFragmentViewModel has the same optimistic pattern, so this isn't a regression, but since this is new code a runCatching/try-catch + Timber.e inside the VM would make the failure path non-fatal. Not a blocker.

[claude]

Claude finished @sztomek's task in 5m 23s —— View job

---

Code review

Re-reviewed at c96195c after the ready-for-review. The core of the PR still looks good: the fake toasts are replaced with real EpisodeManager/PlaybackManager calls that mirror the mobile EpisodeFragmentViewModel, the per-screen button sets match the tvOS contexts described, and the podcast-details overlay wiring in TvUpNextScreen/TvPlaylistDetailsScreen is a faithful copy of the TvHomeScreen pattern (rememberSaveable + BackHandler + onClose), so it stays consistent.

Things I verified this pass rather than assumed:

• ✅ Back-handler nesting is correct. TvPlaylistsScreen registers its "close playlist" BackHandler before composing TvPlaylistDetailsScreen, so the new "close podcast overlay" handler is added later and wins on the dispatcher. TvModal is a real Dialog, so back inside the actions modal dismisses the modal, not the overlay.
• ✅ TvPodcastDetailsScreen keys its hiltViewModel by podcastUuid, so podcast A → back → podcast B from Up Next/playlists can't serve stale state.
• ✅ removeFromUpNext without a coroutine is correct — PlaybackManager.removeEpisode does launch { … } internally (PlaybackManager.kt:1367).
• ✅ TvUpNextViewModel maps UpNextQueue.State.Loaded.queue, which excludes the now-playing episode, so "Remove from Up Next" can't stop playback out from under the user.
• ✅ The test compiles and the mocks work: playNext/playLast/removeEpisode are final methods on open class PlaybackManager, but Mockito 5 (5.23.0) uses the inline mock maker by default, and verifyBlocking is the right call for the two suspend ones. Default-arg matching lines up on all four verifys.
• ✅ SourceView.FILTERS is the correct source for playlists (no PLAYLIST value exists).
• ✅ The inline @Suppress("ktlint:compose:vm-forwarding-check") on an argument is an established pattern here (OnboardingFlowComposable.kt:35, OnboardingUpgradeFlow.kt:118), so no objection to it.
• ✅ LocalTvToastHostState is provided in TvOnboardingNavHost, outside the Dialog, so the toast survives the modal dismissing.

New this pass

1. Actions can now delete the focused row, and nothing re-acquires focus (medium) — inline. "Remove from Up Next" disposes the focused LazyColumn item in the same frame the dialog closes, and UpNextList has no focusRestorer or re-request. Same for Archive on the podcast/playlist screens with the archived filter hiding archived episodes — SortableEpisodeList's hasRequestedInitialFocus is already true, and if it was the last unarchived episode the list is replaced by AllEpisodesArchived, which has no focusable at all. This couldn't happen before the PR because the buttons were no-ops, so it's new behaviour, and it's the one thing I'd actually put on a device before merging. Fix this →

2. TvDropdownMenu padding/width change is out of scope (nit) — inline. A visual change to a shared component used by every sort/filter dropdown, not mentioned in the description or test plan, and no screenshots.

3. Test coverage sits on the wrong side of the risk (still). The seven VM tests cover the thin delegation layer. The logic this PR actually introduces is the context → button-set mapping in TvEpisodeActionsModalContent, which has only previews. Extracting the buildList block into a pure internal fun episodeActionButtons(context, episode, hasGoToPodcast): List<...> would let you assert "Up Next shows Remove and not Mark/Archive" and "Podcast details shows no Go to podcast" — exactly what a future refactor is most likely to break.

4. Analytics gap (still, follow-up). playNext/playLast emit EpisodeAddedToUpNextEvent from inside PlaybackManager, but mark-played/archive emit nothing, so TV will show up-next adds and nothing for played/archived. EventHorizon is already on the tv compile classpath transitively (analytics/build.gradle.kts:19 → api(libs.eventhorizon), re-exported by repositories), so parity with mobile is a constructor param plus ~10 lines whenever you want it.

Still open from the first review

Commit 9ffe6f5 dropped the unused source arg from mark/archive; the rest are unchanged. Recapping so nothing gets lost — all minor:

#	Where	Issue
1	TvEpisodeActionsModal.kt:99	"Go to podcast" gated on both context and onGoToPodcast != null; onGoToPodcast != null alone is sufficient and keeps one source of truth (Fix this →)
2	TvEpisodeActionsModal.kt:91-95	Toast fires before the work runs; a throwing *Blocking call is uncaught inside viewModelScope.launch → crash with a success toast already shown. runCatching + Timber.e in the VM makes it non-fatal
3	TvEpisodeActionsViewModel.kt:41	viewModelScope for dismiss-and-forget writes; @ApplicationScope (already used in this module by TvSignOutManager) is the better fit. Low risk today only because the VM resolves to the long-lived HOME destination
4	TvEpisodeActionsViewModelTest.kt:46	advanceUntilIdle() advances runTest's scheduler, not MainCoroutineRule's UnconfinedTestDispatcher — a no-op; tests pass only because Unconfined is eager. Drop the calls or use runTest(coroutineRule.testDispatcher.scheduler) (Fix this →)
5	TvEpisodeActionsModal.kt:33	Param named context collides with the val context = LocalContext.current locals in the same files; actionContext reads better
6	TvEpisodeActionsViewModel.kt:77	removeFromUpNext reads like a missing launch; one-line comment would stop someone "fixing" it

No blockers. The correctness notes above come from reading the code — Gradle execution isn't permitted in this environment, so I could not run ./gradlew :tv:testDebugUnitTest or spotlessCheck.
· Branch feat/tv-episode-actions

[claude]

Claude finished @sztomek's task in 3m 16s —— View job

---

Code review

Re-reviewed at 4d952f3 ("Address PR comments"). That commit lands five of the six minor items from the previous pass plus the test-coverage concern, and I verified each fix rather than taking the commit message for it.

Previously-raised items — status

#	Item	Status
1	"Go to podcast" double-gated on context and onGoToPodcast != null	✅ Fixed — episodeActionButtons now gates only on showGoToPodcast = onGoToPodcast != null; one source of truth, and the new test pins the behaviour
2	Throwing *Blocking call → uncaught crash inside the launch	✅ Fixed — launchWrite wraps block() in try/catch with CancellationException rethrown first (correct order — CancellationException is an Exception, so catching it after would swallow cancellation) and Timber.e otherwise. libs.timber is on the tv classpath (tv/build.gradle.kts:68)
3	viewModelScope for dismiss-and-forget writes	✅ Fixed — @ApplicationScope private val applicationScope: CoroutineScope. Confirmed the qualifier resolves: provided by CoroutinesModule.provideApplicationScope as SupervisorJob() + Dispatchers.Default, and already reachable from tv (same import as TvSignOutManager.kt:3), so no new module dependency is needed
4	advanceUntilIdle() was a no-op against a different scheduler	✅ Fixed — all seven calls dropped, and applicationScope = CoroutineScope(coroutineRule.testDispatcher) is threaded in. The tests still lean on UnconfinedTestDispatcher's eager execution rather than an explicit barrier, but that's now honest instead of misleading, and it matches TvEpisodeInfoViewModelTest
5	Param named context shadowing LocalContext.current	✅ Fixed — renamed to actionContext throughout, including all three call sites and the four previews
6	removeFromUpNext reads like a missing launch	⬜ Not addressed — still no comment. Trivial; only matters so nobody "fixes" it by wrapping it in viewModelScope.launch
—	Button-set mapping untested	✅ Fixed, and better than I suggested — the mapping is extracted to a pure internal fun episodeActionButtons(actionContext, showGoToPodcast) returning an EpisodeActionButton enum list, so TvEpisodeActionsModalContent keeps a single exhaustive when (no else, so adding an enum constant is a compile error) and EpisodeActionButtonsTest is a plain JUnit test with no Android deps

The extraction is clean. One observation, by design rather than a defect: because PodcastDetails no longer suppresses the button, passing onGoToPodcast from the podcast-details screen would now show it. That's the point of collapsing to one gate, and the go to podcast is hidden when no navigation is available test covers the direction that matters.

Still open

1. Actions can now delete the focused row, and nothing re-acquires focus (medium, unchanged) — inline. UpNextList (TvUpNextScreen.kt:137-153) has no focusRestorer and no re-request; "Remove from Up Next" disposes the episode.uuid-keyed item in the same frame the dialog closes. Same shape for Archive on the podcast/playlist screens with the archived filter active — SortableEpisodeList's hasRequestedInitialFocus is already true by then, and if it was the last unarchived episode the list is replaced by AllEpisodesArchived, which has no focusable at all. Before this PR the buttons were no-op toasts, so it's new behaviour. This is the one item I'd put on a device before merging. Fix this →

Related and cheap to check on the same device pass: the overlay pattern replaces TvUpNextContent / TvPlaylistDetailsContent rather than stacking on it, and neither LazyColumn uses a saved list state — so "Go to podcast" → back should return to the top of the Up Next / playlist list with focus reset. That's inherited from the TvHomeScreen pattern, but a long Up Next queue is where it will be most noticeable.

2. TvDropdownMenu padding/width change is out of scope (nit, unchanged) — inline. padding(12.dp) → padding(horizontal = 20.dp, vertical = 12.dp) and DefaultMenuWidth 240.dp → 256.dp affect every TvSortButton / TvArchivedFilterButton dropdown, aren't episode actions, and aren't in the description or test plan. A before/after in the description would make it reviewable.

3. Analytics gap (follow-up). playNext/playLast emit EpisodeAddedToUpNextEvent from inside PlaybackManager; mark-played and archive emit nothing, so TV will report up-next adds and nothing for played/archived. EventHorizon is already on the tv compile classpath transitively (analytics/build.gradle.kts:19 api(libs.eventhorizon), re-exported by repositories), so parity with mobile is a constructor param plus ~10 lines whenever you want it.

4. Test nits — inline: assertEquals(false, …) → assertFalse, and the new EpisodeActionButton / episodeActionButtons / EpisodeActionButtonsTest are the only symbols in this package without the Tv prefix everything else carries.

Optional: launchWrite now logs failures instead of crashing, which is right, but the success toast has already been shown by then — the user is told "Archived" for a write that didn't happen. Fine as-is (mobile behaves the same), just noting the tradeoff is now a deliberate one.

No blockers. Nothing in this pass changes my read of the core: real EpisodeManager/PlaybackManager calls mirroring the mobile EpisodeFragmentViewModel, per-screen button sets matching the tvOS contexts, and a faithful copy of the existing podcast-details overlay pattern. The correctness notes come from reading the code — Gradle execution isn't permitted in this environment, so I could not run ./gradlew :tv:testDebugUnitTest or spotlessCheck.
· Branch feat/tv-episode-actions

[geekygecko]

Looks good. I only found one issue related to navigation. On Apple TV, if you open a podcast from Up Next, it opens on a new page without the navigation bar at the top. In this implementation, it opens within the Up Next tab, so you can't tap Up Next to return because you're already in that section. It might be worth following the Apple TV approach here.

Android TV

Screen_recording_20260805_110255.mp4

Apple TV

Screen.Recording.2026-08-05.at.11.04.42.am.mov

[sztomek]

thanks @geekygecko for the review.
regarding the top bar being visible on deeper destinations like playlist details and podcast details: i followed figma (Ftk3KwnfqaK4g57yCN63p0-fi-3258_12136) and it keeps the nav bar visible on those routes.
i personally believe that we should hide them for better experience, so i'll make the change in a follow-up PR!

[claude]

Claude encountered an error after 30s —— View job

---

I'll analyze this and get back to you.

[dangermattic]

	2 Warnings
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.
⚠️	Class TvEpisodeListFocus is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

[claude]

Claude encountered an error after 34s —— View job

---

I'll analyze this and get back to you.