Skip to content

Commit

Permalink
Fix #4405: Rename EventLogger to AnalyticsEventLogger (#4694)
Browse files Browse the repository at this point in the history
## Explanation
Fixes #4405: 
Refactoring EventLogger to AnalyticsEventLogger to make it more
specific, in line with refactoring other event loggers.

Changes to:
- [x] Application files: all implementations and usages
- [x] Dagger files
- [x] Rename associated test files, imports and variables
- [x] EventLogger Fakes and the DebugEventLogger

Did not make changes to the following, given they are already pretty
specific:
- [ ] PerformanceMetricsEventLogger

Refactored `scripts/assets/test_file_exemptions.textproto` to update the
new path of `DebugAnalyticsEventLogger`, `FirebaseAnalyticsEventLogger`
and `AnalyticsEventLogger` to exempt them from corresponding file
checks.
Refactored `file_content_validation_checks.textproto` to exempt
`FirebaseAnalyticsEventLogger` from prohibited _Locale_ use check.

## Essential Checklist
- [x] The PR title and explanation each start with "Fix #bugnum: " (If
this PR fixes part of an issue, prefix the title with "Fix part of
#bugnum: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

Co-authored-by: Joshua Murigi <joshryggs@gmail.com>
Co-authored-by: MAZAKPE <makpalyy@gmail.com>
Co-authored-by: Ben Henning <ben@oppia.org>
Co-authored-by: Vraj Desai <43074241+vrajdesai78@users.noreply.github.com>
Co-authored-by: translatewiki.net <l10n-bot@translatewiki.net>
  • Loading branch information
6 people authored Nov 12, 2022
1 parent 087f2fe commit 6683c6d
Show file tree
Hide file tree
Showing 34 changed files with 311 additions and 305 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.viewmodel.ObservableViewModel
import org.oppia.android.util.locale.OppiaLocale
import org.oppia.android.util.logging.firebase.DebugEventLogger
import org.oppia.android.util.logging.firebase.DebugAnalyticsEventLogger
import javax.inject.Inject

/**
Expand All @@ -13,12 +13,12 @@ import javax.inject.Inject
*/
@FragmentScope
class ViewEventLogsViewModel @Inject constructor(
debugEventLogger: DebugEventLogger,
debugAnalyticsEventLogger: DebugAnalyticsEventLogger,
private val machineLocale: OppiaLocale.MachineLocale,
private val resourceHandler: AppLanguageResourceHandler
) : ObservableViewModel() {

private val eventList = debugEventLogger.getEventList()
private val eventList = debugAnalyticsEventLogger.getEventList()

/**
* List of [EventLogItemViewModel] used to populate recyclerview of [ViewEventLogsFragment]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModu
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -98,7 +98,7 @@ class HomeActivityLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

private val internalProfileId: Int = 1

Expand All @@ -116,7 +116,7 @@ class HomeActivityLocalTest {
@Test
fun testHomeActivity_onLaunch_logsEvent() {
launch<HomeActivity>(createHomeActivityIntent(internalProfileId)).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
assertThat(event.context.activityContextCase).isEqualTo(OPEN_HOME)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_2
import org.oppia.android.domain.topic.TEST_STORY_ID_0
import org.oppia.android.domain.topic.TEST_TOPIC_ID_0
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -106,7 +106,7 @@ class ExplorationActivityLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

@Inject
lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
Expand Down Expand Up @@ -143,7 +143,7 @@ class ExplorationActivityLocalTest {
)
).use {
testCoroutineDispatchers.runCurrent()
val event = fakeEventLogger.getOldestEvent()
val event = fakeAnalyticsEventLogger.getOldestEvent()

assertThat(event.context.activityContextCase).isEqualTo(OPEN_EXPLORATION_ACTIVITY)
assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModu
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -95,7 +95,7 @@ class ProfileChooserFragmentLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

@Before
fun setUp() {
Expand All @@ -105,7 +105,7 @@ class ProfileChooserFragmentLocalTest {
@Test
fun testProfileChooser_onLaunch_logsEvent() {
launch<ProfileChooserActivity>(createProfileChooserActivityIntent()).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.priority).isEqualTo(Priority.ESSENTIAL)
assertThat(event.context.activityContextCase).isEqualTo(OPEN_PROFILE_CHOOSER)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModu
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -100,7 +100,7 @@ class StoryActivityLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

private val internalProfileId = 0

Expand All @@ -120,7 +120,7 @@ class StoryActivityLocalTest {
ActivityScenario.launch<StoryActivity>(
createStoryActivityIntent(internalProfileId, TEST_TOPIC_ID, TEST_STORY_ID)
).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
assertThat(event.context.activityContextCase).isEqualTo(OPEN_STORY_ACTIVITY)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModu
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.platformparameter.TestPlatformParameterModule
Expand Down Expand Up @@ -97,7 +97,7 @@ class TopicInfoFragmentLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

private val internalProfileId = 0

Expand All @@ -110,7 +110,7 @@ class TopicInfoFragmentLocalTest {
fun testTopicInfoFragment_onLaunch_logsEvent() {
TestPlatformParameterModule.forceEnableExtraTopicTabsUi(true)
launchTopicActivityIntent(internalProfileId, TEST_TOPIC_ID).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.context.activityContextCase).isEqualTo(OPEN_INFO_TAB)
assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModu
import org.oppia.android.domain.question.QuestionModule
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -97,7 +97,7 @@ class TopicLessonsFragmentLocalTest {
val initializeDefaultLocaleRule = InitializeDefaultLocaleRule()

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

private val internalProfileId = 0

Expand All @@ -109,7 +109,7 @@ class TopicLessonsFragmentLocalTest {
@Test
fun testTopicLessonsFragment_onLaunch_logsEvent() {
launchTopicActivityIntent(internalProfileId, TEST_TOPIC_ID, TEST_STORY_ID).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.context.activityContextCase)
.isEqualTo(EventLog.Context.ActivityContextCase.OPEN_LESSONS_TAB)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ import org.oppia.android.domain.topic.FRACTIONS_TOPIC_ID
import org.oppia.android.domain.topic.PrimeTopicAssetsControllerModule
import org.oppia.android.domain.topic.SUBTOPIC_TOPIC_ID
import org.oppia.android.domain.workmanager.WorkManagerConfigurationModule
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.junit.InitializeDefaultLocaleRule
import org.oppia.android.testing.robolectric.RobolectricModule
Expand Down Expand Up @@ -99,7 +99,7 @@ class RevisionCardActivityLocalTest {
private val fractionsSubtopicListSize: Int = 4

@Inject
lateinit var fakeEventLogger: FakeEventLogger
lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger

@Before
fun setUp() {
Expand All @@ -117,7 +117,7 @@ class RevisionCardActivityLocalTest {
fractionsSubtopicListSize
)
).use {
val event = fakeEventLogger.getMostRecentEvent()
val event = fakeAnalyticsEventLogger.getMostRecentEvent()

assertThat(event.context.activityContextCase).isEqualTo(OPEN_REVISION_CARD)
assertThat(event.priority).isEqualTo(EventLog.Priority.ESSENTIAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import org.oppia.android.app.model.OppiaEventLogs
import org.oppia.android.data.persistence.PersistentCacheStore
import org.oppia.android.domain.oppialogger.EventLogStorageCacheSize
import org.oppia.android.util.data.DataProvider
import org.oppia.android.util.logging.AnalyticsEventLogger
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.EventLogger
import org.oppia.android.util.logging.ExceptionLogger
import org.oppia.android.util.logging.SyncStatusManager
import org.oppia.android.util.networking.NetworkConnectionUtil
Expand All @@ -22,7 +22,7 @@ import javax.inject.Inject
* provides convenience log methods.
*/
class AnalyticsController @Inject constructor(
private val eventLogger: EventLogger,
private val analyticsEventLogger: AnalyticsEventLogger,
cacheStoreFactory: PersistentCacheStore.Factory,
private val consoleLogger: ConsoleLogger,
private val networkConnectionUtil: NetworkConnectionUtil,
Expand Down Expand Up @@ -85,7 +85,7 @@ class AnalyticsController @Inject constructor(
}
else -> {
syncStatusManager.setSyncStatus(SyncStatusManager.SyncStatus.DATA_UPLOADING)
eventLogger.logEvent(eventLog)
analyticsEventLogger.logEvent(eventLog)
syncStatusManager.setSyncStatus(SyncStatusManager.SyncStatus.DATA_UPLOADED)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ import org.oppia.android.domain.oppialogger.analytics.PerformanceMetricsControll
import org.oppia.android.domain.oppialogger.exceptions.ExceptionsController
import org.oppia.android.domain.oppialogger.exceptions.toException
import org.oppia.android.domain.util.getStringFromData
import org.oppia.android.util.logging.AnalyticsEventLogger
import org.oppia.android.util.logging.ConsoleLogger
import org.oppia.android.util.logging.EventLogger
import org.oppia.android.util.logging.ExceptionLogger
import org.oppia.android.util.logging.SyncStatusManager
import org.oppia.android.util.logging.performancemetrics.PerformanceMetricsEventLogger
Expand All @@ -30,7 +30,7 @@ class LogUploadWorker private constructor(
private val exceptionsController: ExceptionsController,
private val performanceMetricsController: PerformanceMetricsController,
private val exceptionLogger: ExceptionLogger,
private val eventLogger: EventLogger,
private val analyticsEventLogger: AnalyticsEventLogger,
private val performanceMetricsEventLogger: PerformanceMetricsEventLogger,
private val consoleLogger: ConsoleLogger,
private val syncStatusManager: SyncStatusManager,
Expand Down Expand Up @@ -91,7 +91,7 @@ class LogUploadWorker private constructor(
return try {
syncStatusManager.setSyncStatus(SyncStatusManager.SyncStatus.DATA_UPLOADING)
analyticsController.getEventLogStoreList().forEach { eventLog ->
eventLogger.logEvent(eventLog)
analyticsEventLogger.logEvent(eventLog)
analyticsController.removeFirstEventLogFromStore()
}
syncStatusManager.setSyncStatus(SyncStatusManager.SyncStatus.DATA_UPLOADED)
Expand Down Expand Up @@ -124,7 +124,7 @@ class LogUploadWorker private constructor(
private val exceptionsController: ExceptionsController,
private val performanceMetricsController: PerformanceMetricsController,
private val exceptionLogger: ExceptionLogger,
private val eventLogger: EventLogger,
private val analyticsEventLogger: AnalyticsEventLogger,
private val performanceMetricsEventLogger: PerformanceMetricsEventLogger,
private val consoleLogger: ConsoleLogger,
private val syncStatusManager: SyncStatusManager,
Expand All @@ -139,7 +139,7 @@ class LogUploadWorker private constructor(
exceptionsController,
performanceMetricsController,
exceptionLogger,
eventLogger,
analyticsEventLogger,
performanceMetricsEventLogger,
consoleLogger,
syncStatusManager,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ import org.oppia.android.domain.oppialogger.analytics.ApplicationLifecycleModule
import org.oppia.android.domain.platformparameter.PlatformParameterSingletonModule
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.domain.topic.TEST_EXPLORATION_ID_5
import org.oppia.android.testing.FakeEventLogger
import org.oppia.android.testing.FakeAnalyticsEventLogger
import org.oppia.android.testing.FakeExceptionLogger
import org.oppia.android.testing.TestLogReportingModule
import org.oppia.android.testing.assertThrows
Expand Down Expand Up @@ -97,7 +97,7 @@ class AudioPlayerControllerTest {
@Inject lateinit var audioPlayerController: AudioPlayerController
@Inject lateinit var fakeExceptionLogger: FakeExceptionLogger
@Inject lateinit var testCoroutineDispatchers: TestCoroutineDispatchers
@Inject lateinit var fakeEventLogger: FakeEventLogger
@Inject lateinit var fakeAnalyticsEventLogger: FakeAnalyticsEventLogger
@Inject lateinit var profileManagementController: ProfileManagementController
@Inject lateinit var explorationDataController: ExplorationDataController
@Inject lateinit var explorationProgressController: ExplorationProgressController
Expand Down Expand Up @@ -491,7 +491,7 @@ class AudioPlayerControllerTest {

// No audio event is logged when an auto-play corresponds to a new content card (since it's
// continuing a play from an earlier state that was already logged).
assertThat(fakeEventLogger.noEventsPresent()).isTrue()
assertThat(fakeAnalyticsEventLogger.noEventsPresent()).isTrue()
}

@Test
Expand All @@ -506,7 +506,7 @@ class AudioPlayerControllerTest {

// This is the default case: when the user opens the audio bar it will auto-play audio (but not
// 'reload' the main content since it's an initial load).
val eventLog = fakeEventLogger.getMostRecentEvent()
val eventLog = fakeAnalyticsEventLogger.getMostRecentEvent()
val exploration = loadExploration(explorationId)
assertThat(eventLog).isEssentialPriority()
assertThat(eventLog).hasPlayVoiceOverContextThat {
Expand Down Expand Up @@ -538,7 +538,7 @@ class AudioPlayerControllerTest {

// This case is only hypothetically possible, but shouldn't happen in practice (since main
// content is always auto-played when reloaded).
val eventLog = fakeEventLogger.getMostRecentEvent()
val eventLog = fakeAnalyticsEventLogger.getMostRecentEvent()
val exploration = loadExploration(explorationId)
assertThat(eventLog).isEssentialPriority()
assertThat(eventLog).hasPlayVoiceOverContextThat {
Expand Down Expand Up @@ -569,7 +569,7 @@ class AudioPlayerControllerTest {
audioPlayerController.play(isPlayingFromAutoPlay = false, reloadingMainContent = false)

// This case corresponds to the user manually playing audio (e.g. after pausing).
val eventLog = fakeEventLogger.getMostRecentEvent()
val eventLog = fakeAnalyticsEventLogger.getMostRecentEvent()
val exploration = loadExploration(explorationId)
assertThat(eventLog).isEssentialPriority()
assertThat(eventLog).hasPlayVoiceOverContextThat {
Expand Down Expand Up @@ -600,7 +600,7 @@ class AudioPlayerControllerTest {
audioPlayerController.play(isPlayingFromAutoPlay = false, reloadingMainContent = false)

// If there's no content ID then it'll be missing from the log.
val eventLog = fakeEventLogger.getMostRecentEvent()
val eventLog = fakeAnalyticsEventLogger.getMostRecentEvent()
assertThat(eventLog).hasPlayVoiceOverContextThat().hasContentIdThat().isEmpty()
}

Expand All @@ -614,7 +614,7 @@ class AudioPlayerControllerTest {

// No event should be logged if outside an exploration and playing audio (such as for
// questions).
assertThat(fakeEventLogger.noEventsPresent()).isTrue()
assertThat(fakeAnalyticsEventLogger.noEventsPresent()).isTrue()
}

private fun arrangeMediaPlayer(contentId: String? = null) {
Expand Down
Loading

0 comments on commit 6683c6d

Please sign in to comment.