Skip to content

Commit

Permalink
Fix part of #5344: Implement event logs for multiple classrooms (#5456)
Browse files Browse the repository at this point in the history
<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

Fixes part of #5344

- Ensures `open_home` & `complete_app_onboarding` event logs are
captured in the new screen.
- Adds `classroomId` field to the `ExplorationContext` object and
updates related tests.
- Implements logging of `start_exploration` event and adds tests.
- Updates `FeatureFlagLogger` with the `ENABLE_MULTIPLE_CLASSROOMS`
feature flag.

## Screen Recording
### Feature Flag On

https://github.com/user-attachments/assets/10c695fa-abfd-4873-826c-c322524c9453

### Feature Flag Off

https://github.com/user-attachments/assets/a6581895-2c10-4561-9912-5a065c8c937b

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [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)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing
  • Loading branch information
theMr17 authored Jul 16, 2024
1 parent b33ca1c commit 6e12374
Show file tree
Hide file tree
Showing 100 changed files with 2,326 additions and 473 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,23 @@ interface ActivityIntentFactories {
* This must be injected within an activity context.
*/
interface TopicActivityIntentFactory {
/** Returns a new [Intent] to start the topic activity for the specified profile and topic. */
fun createIntent(profileId: ProfileId, topicId: String): Intent
/**
* Returns a new [Intent] to start the topic activity for the specified profile, classroom
* and topic.
*/
fun createIntent(profileId: ProfileId, classroomId: String, topicId: String): Intent

/**
* Returns a new [Intent] to start the topic activity for the specified profile, topic, and
* story (where the activity will automatically navigate to & expand the specified story in the
* topic).
* Returns a new [Intent] to start the topic activity for the specified profile, classroom,
* topic, and story (where the activity will automatically navigate to & expand the specified
* story in the topic).
*/
fun createIntent(profileId: ProfileId, topicId: String, storyId: String): Intent
fun createIntent(
profileId: ProfileId,
classroomId: String,
topicId: String,
storyId: String
): Intent
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.RecentlyPlayedActivityParams
import org.oppia.android.app.model.RecentlyPlayedActivityTitle
import org.oppia.android.app.model.ScreenName.CLASSROOM_LIST_ACTIVITY
import org.oppia.android.app.topic.TopicActivity
import org.oppia.android.app.topic.TopicActivity.Companion.createTopicActivityIntent
import org.oppia.android.app.topic.TopicActivity.Companion.createTopicPlayStoryActivityIntent
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
Expand Down Expand Up @@ -98,15 +99,23 @@ class ClassroomListActivity :
)
}

override fun routeToTopic(internalProfileId: Int, topicId: String) {
startActivity(TopicActivity.createTopicActivityIntent(this, internalProfileId, topicId))
override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) {
startActivity(
createTopicActivityIntent(this, internalProfileId, classroomId, topicId)
)
}

override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) {
override fun routeToTopicPlayStory(
internalProfileId: Int,
classroomId: String,
topicId: String,
storyId: String
) {
startActivity(
TopicActivity.createTopicPlayStoryActivityIntent(
createTopicPlayStoryActivityIntent(
this,
internalProfileId,
classroomId,
topicId,
storyId
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import androidx.compose.ui.res.integerResource
import androidx.compose.ui.unit.dp
import androidx.databinding.ObservableList
import androidx.fragment.app.Fragment
import androidx.lifecycle.Observer
import org.oppia.android.R
import org.oppia.android.app.classroom.classroomlist.ClassroomList
import org.oppia.android.app.classroom.promotedlist.PromotedStoryList
Expand All @@ -40,6 +41,7 @@ import org.oppia.android.app.home.classroomlist.ClassroomSummaryViewModel
import org.oppia.android.app.home.promotedlist.PromotedStoryListViewModel
import org.oppia.android.app.home.topiclist.AllTopicsViewModel
import org.oppia.android.app.home.topiclist.TopicSummaryViewModel
import org.oppia.android.app.model.AppStartupState
import org.oppia.android.app.model.ClassroomSummary
import org.oppia.android.app.model.LessonThumbnail
import org.oppia.android.app.model.LessonThumbnailGraphic
Expand All @@ -48,10 +50,14 @@ import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.app.utility.datetime.DateTimeUtil
import org.oppia.android.databinding.ClassroomListFragmentBinding
import org.oppia.android.domain.classroom.ClassroomController
import org.oppia.android.domain.onboarding.AppStartupStateController
import org.oppia.android.domain.oppialogger.OppiaLogger
import org.oppia.android.domain.oppialogger.analytics.AnalyticsController
import org.oppia.android.domain.profile.ProfileManagementController
import org.oppia.android.domain.topic.TopicListController
import org.oppia.android.domain.translation.TranslationController
import org.oppia.android.util.data.AsyncResult
import org.oppia.android.util.data.DataProviders.Companion.toLiveData
import org.oppia.android.util.locale.OppiaLocale
import org.oppia.android.util.parser.html.StoryHtmlParserEntityType
import org.oppia.android.util.parser.html.TopicHtmlParserEntityType
Expand All @@ -75,6 +81,8 @@ class ClassroomListFragmentPresenter @Inject constructor(
private val dateTimeUtil: DateTimeUtil,
private val translationController: TranslationController,
private val machineLocale: OppiaLocale.MachineLocale,
private val appStartupStateController: AppStartupStateController,
private val analyticsController: AnalyticsController,
) {
private val routeToTopicPlayStoryListener = activity as RouteToTopicPlayStoryListener
private lateinit var binding: ClassroomListFragmentBinding
Expand All @@ -92,6 +100,8 @@ class ClassroomListFragmentPresenter @Inject constructor(

internalProfileId = profileId.internalId

logHomeActivityEvent()

classroomListViewModel = ClassroomListViewModel(
activity,
fragment,
Expand Down Expand Up @@ -144,13 +154,16 @@ class ClassroomListFragmentPresenter @Inject constructor(
}
)

logAppOnboardedEvent()

return binding.root
}

/** Routes to the play story view for the first story in the given topic summary. */
fun onTopicSummaryClicked(topicSummary: TopicSummary) {
routeToTopicPlayStoryListener.routeToTopicPlayStory(
internalProfileId,
topicSummary.classroomId,
topicSummary.topicId,
topicSummary.firstStoryId
)
Expand Down Expand Up @@ -225,6 +238,45 @@ class ClassroomListFragmentPresenter @Inject constructor(
}
}
}

private fun logAppOnboardedEvent() {
val startupStateProvider = appStartupStateController.getAppStartupState()
val liveData = startupStateProvider.toLiveData()
liveData.observe(
activity,
object : Observer<AsyncResult<AppStartupState>> {
override fun onChanged(startUpStateResult: AsyncResult<AppStartupState>?) {
when (startUpStateResult) {
null, is AsyncResult.Pending -> {
// Do nothing.
}
is AsyncResult.Success -> {
liveData.removeObserver(this)

if (startUpStateResult.value.startupMode ==
AppStartupState.StartupMode.USER_NOT_YET_ONBOARDED
) {
analyticsController.logAppOnboardedEvent(profileId)
}
}
is AsyncResult.Failure -> {
oppiaLogger.e(
"ClassroomListFragment",
"Failed to retrieve app startup state"
)
}
}
}
}
)
}

private fun logHomeActivityEvent() {
analyticsController.logImportantEvent(
oppiaLogger.createOpenHomeContext(),
profileId
)
}
}

/** Adds a grid of items to a LazyListScope with specified arrangement and item content. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,24 @@ class CompletedStoryItemViewModel(

/** Called when user clicks on CompletedStoryItem. */
fun onCompletedStoryItemClicked() {
routeToTopicPlayStory(internalProfileId, completedStory.topicId, completedStory.storyId)
routeToTopicPlayStory(
internalProfileId,
completedStory.classroomId,
completedStory.topicId,
completedStory.storyId
)
}

override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) {
override fun routeToTopicPlayStory(
internalProfileId: Int,
classroomId: String,
topicId: String,
storyId: String
) {
val intent = intentFactoryShim.createTopicPlayStoryActivityIntent(
activity.applicationContext,
internalProfileId,
classroomId,
topicId,
storyId
)
Expand Down
14 changes: 11 additions & 3 deletions app/src/main/java/org/oppia/android/app/home/HomeActivity.kt
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,10 @@ class HomeActivity :
homeActivityPresenter.handleOnRestart()
}

override fun routeToTopic(internalProfileId: Int, topicId: String) {
startActivity(TopicActivity.createTopicActivityIntent(this, internalProfileId, topicId))
override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) {
startActivity(
TopicActivity.createTopicActivityIntent(this, internalProfileId, classroomId, topicId)
)
}

override fun onBackPressed() {
Expand All @@ -87,11 +89,17 @@ class HomeActivity :
dialogFragment.showNow(supportFragmentManager, TAG_SWITCH_PROFILE_DIALOG)
}

override fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String) {
override fun routeToTopicPlayStory(
internalProfileId: Int,
classroomId: String,
topicId: String,
storyId: String
) {
startActivity(
TopicActivity.createTopicPlayStoryActivityIntent(
this,
internalProfileId,
classroomId,
topicId,
storyId
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ class HomeFragmentPresenter @Inject constructor(
fun onTopicSummaryClicked(topicSummary: TopicSummary) {
routeToTopicPlayStoryListener.routeToTopicPlayStory(
internalProfileId,
topicSummary.classroomId,
topicSummary.topicId,
topicSummary.firstStoryId
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import org.oppia.android.app.model.ProfileId
interface RouteToExplorationListener {
fun routeToExploration(
profileId: ProfileId,
classroomId: String,
topicId: String,
storyId: String,
explorationId: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ package org.oppia.android.app.home

/** Listener for when an activity should route to a topic. */
interface RouteToTopicListener {
fun routeToTopic(internalProfileId: Int, topicId: String)
fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String)
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,10 @@ package org.oppia.android.app.home

/** Listener for when an activity should route to a story-item in TopicPlay tab. */
interface RouteToTopicPlayStoryListener {
fun routeToTopicPlayStory(internalProfileId: Int, topicId: String, storyId: String)
fun routeToTopicPlayStory(
internalProfileId: Int,
classroomId: String,
topicId: String,
storyId: String
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ class PromotedStoryViewModel(
fun clickOnStoryTile() {
routeToTopicPlayStoryListener.routeToTopicPlayStory(
internalProfileId,
promotedStory.classroomId,
promotedStory.topicId,
promotedStory.storyId
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ class RecentlyPlayedActivity :

override fun routeToExploration(
profileId: ProfileId,
classroomId: String,
topicId: String,
storyId: String,
explorationId: String,
Expand All @@ -72,6 +73,7 @@ class RecentlyPlayedActivity :
ExplorationActivity.createExplorationActivityIntent(
this,
profileId,
classroomId,
topicId,
storyId,
explorationId,
Expand All @@ -83,6 +85,7 @@ class RecentlyPlayedActivity :

override fun routeToResumeLesson(
profileId: ProfileId,
classroomId: String,
topicId: String,
storyId: String,
explorationId: String,
Expand All @@ -93,6 +96,7 @@ class RecentlyPlayedActivity :
ResumeLessonActivity.createResumeLessonActivityIntent(
this,
profileId,
classroomId,
topicId,
storyId,
explorationId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
explorationCheckpointLiveData.removeObserver(this)
routeToResumeLessonListener.routeToResumeLesson(
profileId,
promotedStory.classroomId,
promotedStory.topicId,
promotedStory.storyId,
promotedStory.explorationId,
Expand All @@ -117,6 +118,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
} else if (it is AsyncResult.Failure) {
explorationCheckpointLiveData.removeObserver(this)
playExploration(
promotedStory.classroomId,
promotedStory.topicId,
promotedStory.storyId,
promotedStory.explorationId,
Expand All @@ -128,6 +130,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
)
} else {
playExploration(
promotedStory.classroomId,
promotedStory.topicId,
promotedStory.storyId,
promotedStory.explorationId,
Expand Down Expand Up @@ -162,6 +165,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
}

private fun playExploration(
classroomId: String,
topicId: String,
storyId: String,
explorationId: String,
Expand All @@ -174,13 +178,13 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
// cases, lessons played from this fragment are known to be in progress, and that progress
// can't be resumed here (hence the restart).
explorationDataController.restartExploration(
profileId.internalId, topicId, storyId, explorationId
profileId.internalId, classroomId, topicId, storyId, explorationId
)
} else {
// The only lessons that can't have their progress saved are those that were already
// completed.
explorationDataController.replayExploration(
profileId.internalId, topicId, storyId, explorationId
profileId.internalId, classroomId, topicId, storyId, explorationId
)
}
startPlayingProvider.toLiveData().observe(fragment) { result ->
Expand All @@ -192,6 +196,7 @@ class RecentlyPlayedFragmentPresenter @Inject constructor(
oppiaLogger.d("RecentlyPlayedFragment", "Successfully loaded exploration")
routeToExplorationListener.routeToExploration(
profileId,
classroomId,
topicId,
storyId,
explorationId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class OngoingTopicItemViewModel(
}

fun onTopicItemClicked() {
routeToTopic(internalProfileId, topic.topicId)
routeToTopic(internalProfileId, topic.classroomId, topic.topicId)
}

fun computeStoryCountText(): String {
Expand All @@ -36,10 +36,11 @@ class OngoingTopicItemViewModel(
)
}

override fun routeToTopic(internalProfileId: Int, topicId: String) {
override fun routeToTopic(internalProfileId: Int, classroomId: String, topicId: String) {
val intent = intentFactoryShim.createTopicActivityIntent(
activity.applicationContext,
internalProfileId,
classroomId,
topicId
)
activity.startActivity(intent)
Expand Down
Loading

0 comments on commit 6e12374

Please sign in to comment.