Migrate discover events to Event Horizon#5085
Conversation
Generated by 🚫 Danger |
There was a problem hiding this comment.
Pull request overview
Migrates Discover-related analytics from the legacy AnalyticsTracker/property maps to typed Event Horizon events across Discover UI surfaces (and a couple of downstream entry points), and bumps the Event Horizon dependency to a version that includes the required Discover event definitions.
Changes:
- Replaced
analyticsTracker.track(...)calls witheventHorizon.track(...)using typed Discover events throughout Discover, Podcast, and Play entry points. - Removed now-unused Discover helper extensions on
AnalyticsTracker. - Updated
eventhorizondependency version.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| modules/services/views/src/main/java/au/com/shiftyjelly/pocketcasts/views/buttons/PlayButton.kt | Migrates discover_list_episode_play tracking to Event Horizon for play actions initiated via the play button. |
| modules/services/analytics/src/main/java/au/com/shiftyjelly/pocketcasts/analytics/AnalyticsTrackerExtensions.kt | Removes legacy Discover-specific AnalyticsTracker extension helpers. |
| modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/podcast/PodcastFragment.kt | Migrates Discover-originated subscribe/tap events emitted from the podcast screen to Event Horizon. |
| modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/episode/EpisodeFragmentViewModel.kt | Migrates Discover-originated episode play event emitted from the episode screen to Event Horizon. |
| modules/features/filters/src/main/java/au/com/shiftyjelly/pocketcasts/playlists/component/PlaylistEpisodesAdapterFactory.kt | Import ordering change. |
| modules/features/filters/src/main/java/au/com/shiftyjelly/pocketcasts/playlists/PlaylistsViewModel.kt | Import ordering change. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/viewmodel/DiscoverViewModel.kt | Migrates discover_shown to Event Horizon and removes legacy tracker injection. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/RegionSelectFragment.kt | Migrates discover_region_changed to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/PodcastListFragment.kt | Migrates list impression and promotion tap tracking to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/PodcastGridListFragment.kt | Migrates list podcast/episode/share/link interactions to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/MostPopularPodcastsAdapter.kt | Migrates “most popular” row interactions to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/LargeListRowAdapter.kt | Migrates large list row podcast tap/subscribe tracking to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/DiscoverFragment.kt | Migrates category/show-all/header/category impressions and category picker interactions to Event Horizon, and wires adapter with Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/DiscoverAdapter.kt | Migrates the bulk of Discover feed row events (impressions, taps, page changes, ad-category events, episode events) to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/CollectionListRowAdapter.kt | Migrates collection row podcast tapped tracking to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/CategoriesBottomSheet.kt | Migrates category picker shown/pick/closed tracking to Event Horizon. |
| modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/CarouselListRowAdapter.kt | Migrates featured/sponsored carousel tap/subscribe tracking to Event Horizon. |
| gradle/libs.versions.toml | Bumps com.automattic:eventhorizon to a newer Pocket Casts snapshot. |
Comments suppressed due to low confidence (1)
modules/features/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/MostPopularPodcastsAdapter.kt:112
trackImpression()is called from the item click handler, but it tracks aDiscoverListPodcastTappedEvent. The method name is misleading; either rename it to reflect the tap behavior (e.g.,trackPodcastTapped) or change the tracked event if this was intended to be an impression.
private fun trackImpression(podcast: DiscoverPodcast) {
fromListId?.let { listId ->
eventHorizon.track(
DiscoverListPodcastTappedEvent(
listId = listId,
...s/discover/src/main/java/au/com/shiftyjelly/pocketcasts/discover/view/LargeListRowAdapter.kt
Show resolved
Hide resolved
fce75c5 to
fd75025
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/podcast/PodcastFragment.kt:286
- In
fromListUuid?.let { fromListUuid -> ... }the lambda parameter shadows the outerfromListUuidproperty, making the code harder to read. Rename the lambda parameter to something likelistIdto avoid shadowing.
fromListUuid?.let { fromListUuid ->
eventHorizon.track(
DiscoverListPodcastSubscribedEvent(
listId = fromListUuid,
podcastUuid = podcastUuid,
modules/services/views/src/main/java/au/com/shiftyjelly/pocketcasts/views/buttons/PlayButton.kt
Outdated
Show resolved
Hide resolved
fd75025 to
595b477
Compare
Project dependencies changeslist! Upgraded Dependencies
com.automattic:eventhorizon:pocket-casts-1ad1c1e10ce653841ecae96e6df1f9af7f7ea593, (changed from pocket-casts-f49c55e1ffc250dc0719eecfeb9df09f3cb784aa)tree +--- project :modules:features:account
| \--- project :modules:features:search
| \--- project :modules:services:analytics
-| +--- com.automattic:eventhorizon:pocket-casts-f49c55e1ffc250dc0719eecfeb9df09f3cb784aa
+| +--- com.automattic:eventhorizon:pocket-casts-1ad1c1e10ce653841ecae96e6df1f9af7f7ea593
| \--- project :modules:services:model
-| +--- com.automattic:eventhorizon:pocket-casts-f49c55e1ffc250dc0719eecfeb9df09f3cb784aa (*)
+| +--- com.automattic:eventhorizon:pocket-casts-1ad1c1e10ce653841ecae96e6df1f9af7f7ea593 (*)
| \--- project :modules:services:utils
| \--- project :modules:services:payment
-| \--- com.automattic:eventhorizon:pocket-casts-f49c55e1ffc250dc0719eecfeb9df09f3cb784aa (*)
+| \--- com.automattic:eventhorizon:pocket-casts-1ad1c1e10ce653841ecae96e6df1f9af7f7ea593 (*)
\--- project :modules:features:discover
\--- project :modules:features:podcasts
\--- project :modules:features:player
\--- project :modules:features:transcripts
\--- project :modules:services:sharing
- \--- com.automattic:eventhorizon:pocket-casts-f49c55e1ffc250dc0719eecfeb9df09f3cb784aa (*)
+ \--- com.automattic:eventhorizon:pocket-casts-1ad1c1e10ce653841ecae96e6df1f9af7f7ea593 (*) |
595b477 to
aec0e7c
Compare
Description
This migrates Discover related events to Event Horizon.
Relates to PCDROID-419
Testing Instructions
Warning
Unlike other migrations I recommend spending some time on these events and their properties as Discover impacts our impressions and so on.
discover_showndiscover_category_showndiscover_categories_picker_pickdiscover_categories_pill_tappeddiscover_category_close_button_tappeddiscover_categories_picker_showndiscover_categories_picker_closeddiscover_featured_podcast_tappeddiscover_ad_category_tappeddiscover_ad_category_subscribeddiscover_featured_podcast_subscribeddiscover_show_all_tappeddiscover_list_show_all_tappeddiscover_list_collection_header_tappeddiscover_list_impressiondiscover_list_episode_tappeddiscover_list_episode_playdiscover_list_podcast_tappeddiscover_list_share_tappeddiscover_list_podcast_subscribeddiscover_featured_page_changeddiscover_collection_list_page_changeddiscover_region_changeddiscover_collection_link_tappedScreenshots or Screencast
N/A
Checklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...