Skip to content

[TV] Play latest episode from featured cards and search discover - #5793

Merged
sztomek merged 5 commits into
mainfrom
feat/tv-discover-play-actions
Aug 28, 2026
Merged

[TV] Play latest episode from featured cards and search discover#5793
sztomek merged 5 commits into
mainfrom
feat/tv-discover-play-actions

Conversation

@sztomek

@sztomek sztomek commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes two dead play-actions on Android TV that already work on the Apple TV app (parity audit section 1.2, items F3 and F4), plus three discover-attribution fidelity fixes surfaced by the v2 parity audit (§3.10, F8/F9).

F3 (P0) — Home featured carousel "Play latest episode" was a no-op. The focus-revealed button rendered but onPlayLastEpisode = {}. It now resolves the featured podcast's newest episode and starts playback, opening Now Playing on success and showing the generic "playback failed" toast on failure — matching TVDataManager.playLatestEpisode on tvOS.

F4 (P1) — Search-idle discover episode tiles didn't play. The idle Search screen is a full discover feed; its episode tiles had onEpisodePlay = { _, _ -> } with a TODO. They now play immediately and open Now Playing, matching Home. The idle feed's featured cards also play their latest episode (same machinery), for parity with tvOS.

How "latest episode" is resolved

podcastManager.findOrDownloadPodcastRxSingle(uuid) loads the podcast (unsubscribed OK), then episodeManager.findEpisodesByPodcastOrderedByPublishDate(podcast).first() gives the newest-by-publish-date episode — the Android equivalent of tvOS fetchEpisodes(sortOrder: .newestToOldest).first. Playback uses the existing PlaybackManager.playNowSuspend(...) path already used by the discover episode-row tiles.

Follow-up parity & attribution fidelity (audit v2)

Three ride-along commits addressing analytics-fidelity gaps from the v2 parity audit:

  1. Search-surface playback source. Playing a discover episode or a featured "play latest" from the Search idle feed now stamps SourceView.SEARCH (source=search), not SourceView.DISCOVER. iOS stamps the hosting surface — "search" from search discover (DiscoverAnalytics.searchSource), "discover"/"home" from Home. Home playback keeps SourceView.DISCOVER. Downstream playback_play etc. now carry the correct source.
  2. list_id value fidelity. The analytics list id now follows the iOS chain item uuid → fetched list payload listId → layout id (DiscoverManager.listIdForPodcast), carried on the row model as a dedicated nullable listId separate from the row's dedup identity. Two behavioural changes vs. before: the fetched ListFeed.listId is now consulted (curated lists whose uuid lives only in the list JSON report correctly), and the human-readable title fallback is removed — a row with no uuid/list-id/layout-id is simply excluded from list-attributed events instead of emitting a title (e.g. list_id="Trending") that pollutes the property's cardinality. Local rows carry a null listId and are excluded, same as before.
  3. list_datetime on discover_list_podcast_tapped. The section's datetime is now carried through the row model and emitted on podcast taps, matching iOS which sends it whenever the section provides it.

Analytics

Wired via the existing TvDiscoverFeedAnalytics helper the discover rows already use:

  • F3 featured play (source home): on success fires discover_list_episode_tapped (list_id, podcast_uuid, episode_uuid, source) + discover_list_episode_play (list_id, podcast_uuid), matching DiscoverFeaturedPodcastCell on tvOS.
  • F4 search-idle play (source search): the episode tiles fire the same discover_list_episode_tapped + discover_list_episode_play pair; featured cards fire them on success.
  • discover_list_podcast_tapped now carries list_datetime when the section provides it, and its list_id uses the corrected chain above.

Fixes PCDROID-738 https://linear.app/a8c/issue/PCDROID-738/implement-missing-click-handlers

Testing Instructions

  1. Launch the TV app, sign in optional.
  2. On Home, focus a Featured carousel card so the buttons reveal; select Play latest episode (or press SELECT on the card). The newest episode starts and Now Playing opens. Confirm playback events carry source=discover.
  3. Force a failure (e.g. airplane mode) and confirm the "playback failed" toast appears.
  4. Open Search with an empty query so the idle discover feed shows; focus an episode tile in a video/episodes row and press SELECT — it plays and opens Now Playing. Confirm playback events carry source=search. "Go to podcast" still works.
  5. In the same idle feed, a Featured card's Play latest episode also plays (source search).
  6. Via LoggingAnalyticsListener logcat, confirm discover_list_podcast_tapped carries a list_id from the list payload (never a row title) and a list_datetime when the section provides one.

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply)
  • 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/... (no new strings)
  • Any jetpack compose components I added or changed are covered by compose previews (existing previews cover the touched composables)
  • I have updated the Event Horizon schema (no new/changed events — reuses existing discover events incl. their existing list_datetime parameter)

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 4m 32s —— View job


Review

The wiring is correct: findByPodcastOrderPublishedDateDesc is genuinely newest-first, findOrDownloadPodcastRxSingle inserts the podcast and its episodes as unsubscribed when it isn't local, and both tvDiscoverRow call sites now pass a real onPlayLatestEpisode. TvFeaturedTile maps card-SELECT to onPlayLastEpisode, so the description's testing steps match the code. Nothing blocking — the findings below are behavioural gaps around how "latest" is resolved and when analytics fire.

Note that playLatestEpisode, playDiscoverEpisode/playEpisode and toTvDiscoverEpisode are now byte-identical in TvHomeViewModel and TvSearchViewModel; TvSearchViewModel also already has a private hydrate() that does exactly what playDiscoverEpisode's lookup inlines. Worth a shared helper before a third screen needs it, though it's fine to leave for a follow-up.

Non-blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:232 — featured-card analytics fire only after playNowSuspend succeeds, so failed taps record nothing while the episode-tile path records every tap; move trackEpisodePlayed above the play call in both view models. (thread, Fix this →)
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:229 — "latest" comes from the local DB only, and unsubscribed podcasts are never refreshed (kept up to a week by checkForUnusedPodcastsBlocking), so a previously-browsed featured podcast plays a stale episode; a refreshPodcast before the query fixes it. (thread)

Nits

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/discover/TvDiscoverRows.kt:31onPlayLatestEpisode defaults to a no-op, which is exactly how the dead button arose; both call sites pass it, so make it required. (thread)

I did not build the module or run :tv:testDebugUnitTest — Gradle invocation isn't permitted in this environment.
· branch feat/tv-discover-play-actions

Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/discover/TvDiscoverRows.kt Outdated
@sztomek sztomek added [Type] Enhancement Improve an existing feature. [Area] TV labels Aug 27, 2026
@sztomek sztomek added this to the 8.20 milestone Aug 27, 2026
@sztomek
sztomek marked this pull request as ready for review August 27, 2026 12:43
@sztomek
sztomek requested a review from a team as a code owner August 27, 2026 12:43
@sztomek
sztomek requested review from geekygecko and removed request for a team August 27, 2026 12:43
@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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


Review

No new commits since my previous review — the branch is still at 1d246b6, so this is the same code I looked at when the PR was a draft. Verdict unchanged: the wiring is correct and nothing blocks the merge.

All three earlier findings are still open:

Non-blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:232 — analytics still fire only after playNowSuspend succeeds, so failed featured-card taps record nothing. (thread)
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:229 — "latest" still comes from the local DB with no refresh, so a previously-browsed unsubscribed podcast can play a stale episode. (thread)

Nits

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/discover/TvDiscoverRows.kt:31onPlayLatestEpisode still defaults to a no-op. (thread)

I did not build the module or run :tv:testDebugUnitTest — Gradle invocation isn't permitted in this environment.
· branch feat/tv-discover-play-actions

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Claude finished @sztomek's task in 1m 31s —— View job


Review

d394d8c closes all three findings from my previous review: analytics now fire before playNowSuspend in both view models (with tests covering the failed-playback case), refreshPodcast runs before the newest-episode query so an unsubscribed podcast isn't stale, and onPlayLatestEpisode is now a required parameter. PodcastRefresherImpl.refreshPodcast swallows its own exceptions and both the Retrofit and Room calls dispatch off the main thread, so the added refresh can't break playback or block the UI. One new efficiency point below.

Non-blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:229 — on the cold path the podcast is fetched twice: findOrDownloadPodcastRxSingle downloads it via addPodcastRxSingle, then refreshPodcast immediately re-fetches and re-diffs the same feed; gate the refresh on the podcast already being local. Same at TvSearchViewModel.kt:369. (thread, Fix this →)

The duplication note from last time still stands (playLatestEpisode, playEpisode/playDiscoverEpisode and toTvDiscoverEpisode are byte-identical across the two view models) — fine as a follow-up.

I did not build the module or run :tv:testDebugUnitTest — Gradle invocation isn't permitted in this environment.
· branch feat/tv-discover-play-actions

@claude

claude Bot commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

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


Review

Reviewed only the two commits since my last review: 99b2adf (search source) and ef7b376 (list attribution). Both are sound — feed.date is the same datetime field mobile sends as list_datetime (DiscoverAdapter.kt:449), listUuid ?: feed.listId ?: id matches how mobile derives PodcastList.listId from the feed (DiscoverViewModel.kt:168), and omitting listDatetime from the impression and episode-tapped events matches mobile too. Row impressions stay keyed on row.id (TvDiscoverRows.kt:38), so the new fields don't re-fire them, and SourceView only feeds analytics on the playNowSuspend path.

Non-blocking

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/discover/TvDiscoverFeedLoader.kt:165 — sponsored podcasts injected from another feed inherit the host row's listId, so ad taps are attributed to the list they were injected into; mobile keeps the sponsored list id on the podcast. Pre-existing, but this commit is where list ids are now assigned. (thread)
  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/home/TvHomeViewModel.kt:229 — still open from last round: the cold path fetches the podcast twice (findOrDownloadPodcastRxSingle downloads it, then refreshPodcast re-fetches and re-diffs the same feed). (thread)

Nits

  • tv/src/main/java/au/com/shiftyjelly/pocketcasts/discover/TvDiscoverFeedAnalytics.kt:97 — the localRowIds filter is now inert: the local Home rows never set listId, so discoverListId() already returns null for them. (thread)

I did not build the module or run :tv:testDebugUnitTest — Gradle invocation isn't permitted in this environment.
· branch feat/tv-discover-play-actions

@sztomek
sztomek merged commit 2a6a467 into main Aug 28, 2026
26 checks passed
@sztomek
sztomek deleted the feat/tv-discover-play-actions branch August 28, 2026 11:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant