Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import au.com.shiftyjelly.pocketcasts.analytics.SourceView
import au.com.shiftyjelly.pocketcasts.models.entity.PodcastEpisode
import au.com.shiftyjelly.pocketcasts.models.type.EpisodePlayingStatus
import au.com.shiftyjelly.pocketcasts.theme.TvTheme
import com.automattic.eventhorizon.EpisodeViewSourceType
import java.util.Date
import au.com.shiftyjelly.pocketcasts.localization.R as LR

Expand Down Expand Up @@ -62,6 +63,9 @@ private fun ColumnScope.TvEpisodeActionsModalContent(
var pendingConfirmation by remember { mutableStateOf<TvEpisodeActionConfirmation?>(null) }
var returnFocusLabel by remember { mutableStateOf<String?>(null) }
val focusRequester = remember { FocusRequester() }
LaunchedEffect(episode.uuid) {
actions.trackActionsShown(actionContext.episodeViewSource)
}
LaunchedEffect(pendingConfirmation) {
if (pendingConfirmation == null) {
focusRequester.requestFocus()
Expand Down Expand Up @@ -317,6 +321,7 @@ private object NoOpTvEpisodeActions : TvEpisodeActions {
override fun archive(episode: PodcastEpisode) = Unit
override fun unarchive(episode: PodcastEpisode) = Unit
override fun removeFromUpNext(episode: PodcastEpisode, source: SourceView) = Unit
override fun trackActionsShown(source: EpisodeViewSourceType) = Unit
}

private val ContentPadding = PaddingValues(horizontal = 24.dp, vertical = 27.dp)
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import au.com.shiftyjelly.pocketcasts.repositories.di.IoDispatcher
import au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackManager
import au.com.shiftyjelly.pocketcasts.repositories.podcast.EpisodeManager
import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
import com.automattic.eventhorizon.EpisodeActionsShownEvent
import com.automattic.eventhorizon.EpisodeViewSourceType
import com.automattic.eventhorizon.EventHorizon
import dagger.hilt.android.lifecycle.HiltViewModel
import javax.inject.Inject
import kotlinx.coroutines.CancellationException
Expand All @@ -16,12 +19,12 @@ import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.launch
import timber.log.Timber

enum class TvEpisodeActionContext(val source: SourceView) {
PodcastDetails(SourceView.PODCAST_SCREEN),
SearchResults(SourceView.SEARCH_RESULTS),
Playlist(SourceView.FILTERS),
UpNext(SourceView.UP_NEXT),
NowPlaying(SourceView.PLAYER),
enum class TvEpisodeActionContext(val source: SourceView, val episodeViewSource: EpisodeViewSourceType) {
PodcastDetails(SourceView.PODCAST_SCREEN, EpisodeViewSourceType.PodcastScreen),
SearchResults(SourceView.SEARCH_RESULTS, EpisodeViewSourceType.Search),
Comment thread
sztomek marked this conversation as resolved.
Playlist(SourceView.FILTERS, EpisodeViewSourceType.Filters),
UpNext(SourceView.UP_NEXT, EpisodeViewSourceType.UpNext),
NowPlaying(SourceView.PLAYER, EpisodeViewSourceType.NowPlaying),
}

interface TvEpisodeActions {
Expand All @@ -33,13 +36,15 @@ interface TvEpisodeActions {
fun archive(episode: PodcastEpisode)
fun unarchive(episode: PodcastEpisode)
fun removeFromUpNext(episode: PodcastEpisode, source: SourceView)
fun trackActionsShown(source: EpisodeViewSourceType)
}

@HiltViewModel
class TvEpisodeActionsViewModel @Inject constructor(
private val episodeManager: EpisodeManager,
private val playbackManager: PlaybackManager,
private val podcastManager: PodcastManager,
private val eventHorizon: EventHorizon,
@ApplicationScope private val applicationScope: CoroutineScope,
@IoDispatcher private val ioDispatcher: CoroutineDispatcher,
) : ViewModel(),
Expand Down Expand Up @@ -77,6 +82,10 @@ class TvEpisodeActionsViewModel @Inject constructor(
playbackManager.removeEpisode(episodeToRemove = episode, source = source)
}

override fun trackActionsShown(source: EpisodeViewSourceType) {
eventHorizon.track(EpisodeActionsShownEvent(source = source))
}

private fun launchWrite(block: suspend () -> Unit) {
applicationScope.launch(ioDispatcher) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,13 @@ import au.com.shiftyjelly.pocketcasts.localization.R as LR
@Composable
fun TvEpisodeInfoModal(
episode: PodcastEpisode,
actionContext: TvEpisodeActionContext,
onDismissRequest: () -> Unit,
modifier: Modifier = Modifier,
viewModel: TvEpisodeInfoViewModel = hiltViewModel(),
) {
LaunchedEffect(episode.uuid) {
viewModel.trackDetailShown(actionContext.episodeViewSource)
viewModel.load(episode.podcastUuid, episode.uuid)
}
val uiState by viewModel.uiState.collectAsStateWithLifecycle()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ import androidx.lifecycle.viewModelScope
import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
import au.com.shiftyjelly.pocketcasts.repositories.shownotes.ShowNotesManager
import au.com.shiftyjelly.pocketcasts.servers.shownotes.ShowNotesState
import com.automattic.eventhorizon.EpisodeDetailShownEvent
import com.automattic.eventhorizon.EpisodeViewSourceType
import com.automattic.eventhorizon.EventHorizon
import dagger.hilt.android.lifecycle.HiltViewModel
import javax.inject.Inject
import kotlinx.coroutines.flow.MutableStateFlow
Expand All @@ -17,12 +20,17 @@ import kotlinx.coroutines.launch
class TvEpisodeInfoViewModel @Inject constructor(
private val podcastManager: PodcastManager,
private val showNotesManager: ShowNotesManager,
private val eventHorizon: EventHorizon,
) : ViewModel() {
private val _uiState = MutableStateFlow<UiState?>(null)
val uiState: StateFlow<UiState?> = _uiState.asStateFlow()

private var loadedEpisodeUuid: String? = null

fun trackDetailShown(source: EpisodeViewSourceType) {
eventHorizon.track(EpisodeDetailShownEvent(source = source))
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things worth comparing against the existing mobile implementation of this same event, EpisodeFragment.kt:353-361:

override fun onCreate(savedInstanceState: Bundle?) {
    super.onCreate(savedInstanceState)
    if (!viewModel.isFragmentChangingConfigurations) {
        eventHorizon.track(EpisodeDetailShownEvent(source = episodeViewSource.analyticsValue))
    }
}

override fun onDestroyView() {
    ...
    if (!viewModel.isFragmentChangingConfigurations) {
        eventHorizon.track(EpisodeDetailDismissedEvent(source = episodeViewSource.analyticsValue))
    }
  1. No episode_detail_dismissed counterpart. Mobile emits shown/dismissed as a pair, so anything that pairs them (dwell time, "opened details but bounced") will see TV episode_detail_shown events with no matching dismiss. If that's deliberate — tvOS doesn't fire it either, and the PR is explicitly scoped to the two shown events — it's worth a line in the PR body, since that's the record the analytics folks read. TvEpisodeInfoModal.onDismissRequest is the natural hook if it's wanted.

  2. Config-change guard. Mobile suppresses the re-fire when the fragment is recreated for a configuration change. LaunchedEffect(episode.uuid) here re-runs whenever the activity is recreated with the modal open (locale change, dark-mode toggle, display/HDMI change on TV). Much rarer on TV than on a phone, so probably fine to accept — just flagging that the two platforms count this differently.


fun load(podcastUuid: String, episodeUuid: String) {
if (episodeUuid == loadedEpisodeUuid) {
return
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,7 @@ private fun TvNowPlayingContent(
if (isDetailsModalVisible) {
TvEpisodeInfoModal(
episode = episode,
actionContext = TvEpisodeActionContext.NowPlaying,
onDismissRequest = { isDetailsModalVisible = false },
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,7 @@ private fun EpisodeList(
detailsEpisode?.let { episode ->
TvEpisodeInfoModal(
episode = episode,
actionContext = TvEpisodeActionContext.Playlist,
onDismissRequest = { detailsEpisode = null },
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ private fun EpisodeList(
detailsEpisode?.let { episode ->
TvEpisodeInfoModal(
episode = episode,
actionContext = TvEpisodeActionContext.PodcastDetails,
onDismissRequest = { detailsEpisode = null },
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ fun TvSearchScreen(
detailsEpisode?.let { episode ->
TvEpisodeInfoModal(
episode = episode,
actionContext = TvEpisodeActionContext.SearchResults,
onDismissRequest = { detailsEpisode = null },
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ private fun UpNextList(
detailsEpisode?.let { episode ->
TvEpisodeInfoModal(
episode = episode,
actionContext = TvEpisodeActionContext.UpNext,
onDismissRequest = { detailsEpisode = null },
)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,31 @@
package au.com.shiftyjelly.pocketcasts.component

import au.com.shiftyjelly.pocketcasts.analytics.SourceView
import com.automattic.eventhorizon.EpisodeViewSourceType
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Test

class TvEpisodeActionTypeTest {

@Test
fun `every action context maps to the matching analytics and event sources`() {
val expected = mapOf(
TvEpisodeActionContext.PodcastDetails to (SourceView.PODCAST_SCREEN to EpisodeViewSourceType.PodcastScreen),
TvEpisodeActionContext.SearchResults to (SourceView.SEARCH_RESULTS to EpisodeViewSourceType.Search),
TvEpisodeActionContext.Playlist to (SourceView.FILTERS to EpisodeViewSourceType.Filters),
TvEpisodeActionContext.UpNext to (SourceView.UP_NEXT to EpisodeViewSourceType.UpNext),
TvEpisodeActionContext.NowPlaying to (SourceView.PLAYER to EpisodeViewSourceType.NowPlaying),
)

assertEquals(TvEpisodeActionContext.entries.toSet(), expected.keys)
TvEpisodeActionContext.entries.forEach { context ->
val (source, episodeViewSource) = expected.getValue(context)
assertEquals(source, context.source)
assertEquals(episodeViewSource, context.episodeViewSource)
}
}

@Test
fun `podcast details shows played and archive toggles but no go to podcast`() {
val actions = tvEpisodeActionTypes(TvEpisodeActionContext.PodcastDetails, showGoToPodcast = false)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import au.com.shiftyjelly.pocketcasts.repositories.playback.PlaybackManager
import au.com.shiftyjelly.pocketcasts.repositories.podcast.EpisodeManager
import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
import au.com.shiftyjelly.pocketcasts.sharedtest.MainCoroutineRule
import com.automattic.eventhorizon.EpisodeActionsShownEvent
import com.automattic.eventhorizon.EpisodeViewSourceType
import com.automattic.eventhorizon.EventHorizon
import java.util.Date
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.ExperimentalCoroutinesApi
Expand All @@ -25,6 +28,7 @@ class TvEpisodeActionsViewModelTest {
private val episodeManager = mock<EpisodeManager>()
private val playbackManager = mock<PlaybackManager>()
private val podcastManager = mock<PodcastManager>()
private val eventHorizon = mock<EventHorizon>()

private val episode = PodcastEpisode(
uuid = "episode-uuid",
Expand All @@ -37,10 +41,18 @@ class TvEpisodeActionsViewModelTest {
episodeManager = episodeManager,
playbackManager = playbackManager,
podcastManager = podcastManager,
eventHorizon = eventHorizon,
applicationScope = CoroutineScope(coroutineRule.testDispatcher),
ioDispatcher = coroutineRule.testDispatcher,
)

@Test
fun `tracking actions shown records the event with the source`() = runTest {
viewModel().trackActionsShown(EpisodeViewSourceType.Search)

verify(eventHorizon).track(EpisodeActionsShownEvent(source = EpisodeViewSourceType.Search))
}

@Test
fun `play starts playback of the episode`() = runTest {
viewModel().play(episode, SourceView.UP_NEXT)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import au.com.shiftyjelly.pocketcasts.repositories.podcast.PodcastManager
import au.com.shiftyjelly.pocketcasts.repositories.shownotes.ShowNotesManager
import au.com.shiftyjelly.pocketcasts.servers.shownotes.ShowNotesState
import au.com.shiftyjelly.pocketcasts.sharedtest.MainCoroutineRule
import com.automattic.eventhorizon.EpisodeDetailShownEvent
import com.automattic.eventhorizon.EpisodeViewSourceType
import com.automattic.eventhorizon.EventHorizon
import kotlinx.coroutines.CompletableDeferred
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
Expand All @@ -19,6 +22,7 @@ import org.mockito.kotlin.doSuspendableAnswer
import org.mockito.kotlin.eq
import org.mockito.kotlin.mock
import org.mockito.kotlin.times
import org.mockito.kotlin.verify
import org.mockito.kotlin.verifyBlocking
import org.mockito.kotlin.whenever

Expand All @@ -30,12 +34,20 @@ class TvEpisodeInfoViewModelTest {

private val podcastManager = mock<PodcastManager>()
private val showNotesManager = mock<ShowNotesManager>()
private val eventHorizon = mock<EventHorizon>()

@Test
fun `initial state is null`() = runTest {
assertNull(createViewModel().uiState.value)
}

@Test
fun `tracking shown records the detail shown event with the source`() = runTest {
createViewModel().trackDetailShown(EpisodeViewSourceType.PodcastScreen)

verify(eventHorizon).track(EpisodeDetailShownEvent(source = EpisodeViewSourceType.PodcastScreen))
}

@Test
fun `load resolves the podcast title and show notes`() = runTest {
stubTitle("Buzzcast")
Expand Down Expand Up @@ -148,5 +160,6 @@ class TvEpisodeInfoViewModelTest {
private fun createViewModel() = TvEpisodeInfoViewModel(
podcastManager = podcastManager,
showNotesManager = showNotesManager,
eventHorizon = eventHorizon,
)
}
Loading