Skip to content

Commit

Permalink
Fix part of #4865 and Fix #4986 : Move all fragment arguments, activ…
Browse files Browse the repository at this point in the history
…ity intent extras, and saved instance state over to protos (#5248)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fixes #4986 
Move all fragment arguments, activity extras, and saved instance state
bundle usage over to using protos entirely.

Fix part of #4865
Introduce a ProfileID decorator to pack ProfileId into all Intents so
that it can be easily accessed by underlying fragments and child
classes. Some classes already use this utility where it was not easy to
add profileId to the intent proto bundle.

## 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

---------

Co-authored-by: Vishwajith-Shettigar <[email protected]>
  • Loading branch information
Vishwajith-Shettigar and Vishwajith-Shettigar authored Jun 24, 2024
1 parent 92a561f commit dd80399
Show file tree
Hide file tree
Showing 142 changed files with 2,812 additions and 1,110 deletions.
1 change: 1 addition & 0 deletions app/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -804,6 +804,7 @@ kt_android_library(
"//utility/src/main/java/org/oppia/android/util/extensions:bundle_extensions",
"//utility/src/main/java/org/oppia/android/util/parser/image:image_loader",
"//utility/src/main/java/org/oppia/android/util/parser/image:image_parsing_annonations",
"//utility/src/main/java/org/oppia/android/util/profile:current_user_profile_id_intent_decorator",
"//utility/src/main/java/org/oppia/android/util/statusbar:status_bar_color",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,43 +8,33 @@ import org.oppia.android.app.activity.ActivityComponentImpl
import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity
import org.oppia.android.app.administratorcontrols.appversion.AppVersionActivity
import org.oppia.android.app.administratorcontrols.learneranalytics.ProfileAndDeviceIdActivity
import org.oppia.android.app.drawer.NAVIGATION_PROFILE_ID_ARGUMENT_KEY
import org.oppia.android.app.model.AdministratorControlActivityStateBundle
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ScreenName.ADMINISTRATOR_CONTROLS_ACTIVITY
import org.oppia.android.app.settings.profile.ProfileEditFragment
import org.oppia.android.app.settings.profile.ProfileListActivity
import org.oppia.android.app.settings.profile.ProfileListFragment
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.extensions.getStringFromBundle
import org.oppia.android.util.extensions.getProto
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
import javax.inject.Inject

/** Argument key for of title for selected controls in [AdministratorControlsActivity]. */
const val SELECTED_CONTROLS_TITLE_SAVED_KEY =
"AdministratorControlsActivity.selected_controls_title"

/** Argument key for of selected profile for selected controls in [AdministratorControlsActivity]. */
const val SELECTED_PROFILE_ID_SAVED_KEY =
"AdministratorControlsActivity.selected_profile_id"

/** Argument key for last loaded fragment in [AdministratorControlsActivity]. */
const val LAST_LOADED_FRAGMENT_EXTRA_KEY = "AdministratorControlsActivity.last_loaded_fragment"

/** Argument key used to identify [ProfileListFragment] in the backstack. */
const val PROFILE_LIST_FRAGMENT = "PROFILE_LIST_FRAGMENT"

/** Argument key used to identify [ProfileEditFragment] in the backstack. */
const val PROFILE_EDIT_FRAGMENT = "PROFILE_EDIT_FRAGMENT"

/** Argument key for the Profile deletion confirmation in [ProfileEditActivity]. */
const val IS_PROFILE_DELETION_DIALOG_VISIBLE_KEY =
"ProfileEditActivity.is_profile_deletion_dialog_visible"

/** Argument key used to identify [AppVersionFragment] in the backstack. */
const val APP_VERSION_FRAGMENT = "APP_VERSION_FRAGMENT"

/** Argument key used to identify [ProfileAndDeviceIdFragment] in the backstack. */
const val PROFILE_AND_DEVICE_ID_FRAGMENT = "PROFILE_AND_DEVICE_ID_FRAGMENT"

/** Argument key for Administrator Controls Activity saved state. */
const val ADMINISTRATOR_CONTROLS_ACTIVITY_STATE_KEY = "ADMINISTRATOR_CONTROLS_ACTIVITY_STATE_KEY"

/** Activity [AdministratorControlsActivity] that allows user to change admin controls. */
class AdministratorControlsActivity :
InjectableAutoLocalizedAppCompatActivity(),
Expand All @@ -68,17 +58,24 @@ class AdministratorControlsActivity :
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(activityComponent as ActivityComponentImpl).inject(this)
val extraControlsTitle =
savedInstanceState?.getStringFromBundle(SELECTED_CONTROLS_TITLE_SAVED_KEY)

val args = savedInstanceState?.getProto(
ADMINISTRATOR_CONTROLS_ACTIVITY_STATE_KEY,
AdministratorControlActivityStateBundle.getDefaultInstance()
)

val extraControlsTitle = args?.selectedControlsTitle

isProfileDeletionDialogVisible =
savedInstanceState?.getBoolean(IS_PROFILE_DELETION_DIALOG_VISIBLE_KEY) ?: false
args?.isProfileDeletionDialogVisible ?: false
lastLoadedFragment = if (savedInstanceState != null) {
savedInstanceState.getStringFromBundle(LAST_LOADED_FRAGMENT_EXTRA_KEY) as String
args?.lastLoadedFragment as String
} else {
// TODO(#661): Change the default fragment in the right hand side to be EditAccount fragment in the case of multipane controls.
PROFILE_LIST_FRAGMENT
}
val selectedProfileId = savedInstanceState?.getInt(SELECTED_PROFILE_ID_SAVED_KEY) ?: -1
val selectedProfileId = args?.selectedProfileId ?: -1

administratorControlsActivityPresenter.handleOnCreate(
extraControlsTitle,
lastLoadedFragment,
Expand Down Expand Up @@ -111,18 +108,17 @@ class AdministratorControlsActivity :
}

companion object {

/** Returns an [Intent] to start this activity. */
fun createAdministratorControlsActivityIntent(context: Context, profileId: Int?): Intent {
fun createAdministratorControlsActivityIntent(context: Context, profileId: ProfileId?): Intent {

val intent = Intent(context, AdministratorControlsActivity::class.java)
intent.putExtra(NAVIGATION_PROFILE_ID_ARGUMENT_KEY, profileId)
intent.decorateWithScreenName(ADMINISTRATOR_CONTROLS_ACTIVITY)
if (profileId != null) {
intent.decorateWithUserProfileId(profileId)
}
return intent
}

/** Returns the argument key used to specify the user's internal profile ID. */
fun getIntentKey(): String {
return NAVIGATION_PROFILE_ID_ARGUMENT_KEY
}
}

override fun onBackPressed() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ import org.oppia.android.app.activity.ActivityScope
import org.oppia.android.app.administratorcontrols.appversion.AppVersionFragment
import org.oppia.android.app.administratorcontrols.learneranalytics.ProfileAndDeviceIdFragment
import org.oppia.android.app.drawer.NavigationDrawerFragment
import org.oppia.android.app.model.AdministratorControlActivityStateBundle
import org.oppia.android.app.settings.profile.LoadProfileEditDeletionDialogListener
import org.oppia.android.app.settings.profile.ProfileEditFragment
import org.oppia.android.app.settings.profile.ProfileListFragment
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.databinding.AdministratorControlsActivityBinding
import org.oppia.android.util.extensions.putProto
import javax.inject.Inject

/** The presenter for [AdministratorControlsActivity]. */
Expand Down Expand Up @@ -198,13 +200,18 @@ class AdministratorControlsActivityPresenter @Inject constructor(
/** Saves the state of the views on configuration changes. */
fun handleOnSaveInstanceState(outState: Bundle) {
val titleTextView = binding.extraControlsTitle
if (titleTextView != null) {
outState.putString(SELECTED_CONTROLS_TITLE_SAVED_KEY, titleTextView.text.toString())
}
outState.putString(LAST_LOADED_FRAGMENT_EXTRA_KEY, lastLoadedFragment)
isProfileDeletionDialogVisible.let {
outState.putBoolean(IS_PROFILE_DELETION_DIALOG_VISIBLE_KEY, it)
}
selectedProfileId.let { outState.putInt(SELECTED_PROFILE_ID_SAVED_KEY, it) }
val args = AdministratorControlActivityStateBundle.newBuilder()
.apply {
if (titleTextView != null) {
selectedControlsTitle = titleTextView.text.toString()
}
lastLoadedFragment = this@AdministratorControlsActivityPresenter.lastLoadedFragment
this@AdministratorControlsActivityPresenter.isProfileDeletionDialogVisible.let {
isProfileDeletionDialogVisible = it
}
selectedProfileId = this@AdministratorControlsActivityPresenter.selectedProfileId
}
.build()
outState.putProto(ADMINISTRATOR_CONTROLS_ACTIVITY_STATE_KEY, args)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,23 +7,30 @@ import android.view.View
import android.view.ViewGroup
import org.oppia.android.app.fragment.FragmentComponentImpl
import org.oppia.android.app.fragment.InjectableFragment
import org.oppia.android.app.model.AdministratorControlsFragmentArguments
import org.oppia.android.util.extensions.getProto
import org.oppia.android.util.extensions.putProto
import javax.inject.Inject

/** Argument key for Administrator Controls Fragment. */
const val ADMINISTRATOR_CONTROLS_FRAGMENT_ARGUMENTS_KEY = "AdministratorControlsFragment.arguments"

/** Fragment that contains Administrator Controls of the application. */
class AdministratorControlsFragment : InjectableFragment() {
@Inject
lateinit var administratorControlsFragmentPresenter: AdministratorControlsFragmentPresenter

companion object {
private const val IS_MULTIPANE_ARGUMENT_KEY = "AdministratorControlsFragment.is_multipane"

/** Creates a new instance of [AdministratorControlsFragment]. */
fun newInstance(isMultipane: Boolean): AdministratorControlsFragment {
val args = Bundle()
args.putBoolean(IS_MULTIPANE_ARGUMENT_KEY, isMultipane)
val fragment = AdministratorControlsFragment()
fragment.arguments = args
return fragment
val args =
AdministratorControlsFragmentArguments.newBuilder().setIsMultipane(isMultipane).build()
return AdministratorControlsFragment().apply {
arguments = Bundle().apply {
putProto(ADMINISTRATOR_CONTROLS_FRAGMENT_ARGUMENTS_KEY, args)
}
}
}
}

Expand All @@ -37,11 +44,15 @@ class AdministratorControlsFragment : InjectableFragment() {
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
val args =
val arguments =
checkNotNull(arguments) {
"Expected arguments to be passed to AdministratorControlsFragment"
}
val isMultipane = args.getBoolean(IS_MULTIPANE_ARGUMENT_KEY)
val args = arguments.getProto(
ADMINISTRATOR_CONTROLS_FRAGMENT_ARGUMENTS_KEY,
AdministratorControlsFragmentArguments.getDefaultInstance()
)
val isMultipane = args.isMultipane
return administratorControlsFragmentPresenter
.handleCreateView(inflater, container, isMultipane)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import org.oppia.android.app.administratorcontrols.administratorcontrolsitemview
import org.oppia.android.app.administratorcontrols.administratorcontrolsitemviewmodel.AdministratorControlsItemViewModel
import org.oppia.android.app.administratorcontrols.administratorcontrolsitemviewmodel.AdministratorControlsProfileAndDeviceIdViewModel
import org.oppia.android.app.administratorcontrols.administratorcontrolsitemviewmodel.AdministratorControlsProfileViewModel
import org.oppia.android.app.drawer.NAVIGATION_PROFILE_ID_ARGUMENT_KEY
import org.oppia.android.app.fragment.FragmentScope
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.recyclerview.BindableAdapter
Expand All @@ -24,6 +23,7 @@ import org.oppia.android.databinding.AdministratorControlsFragmentBinding
import org.oppia.android.databinding.AdministratorControlsGeneralViewBinding
import org.oppia.android.databinding.AdministratorControlsLearnerAnalyticsViewBinding
import org.oppia.android.databinding.AdministratorControlsProfileViewBinding
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId
import java.security.InvalidParameterException
import javax.inject.Inject

Expand Down Expand Up @@ -56,7 +56,7 @@ class AdministratorControlsFragmentPresenter @Inject constructor(
/* attachToRoot= */ false
)

internalProfileId = activity.intent.getIntExtra(NAVIGATION_PROFILE_ID_ARGUMENT_KEY, -1)
internalProfileId = activity.intent.extractCurrentUserProfileId().internalId
profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
administratorControlsViewModel.setProfileId(profileId)

Expand Down Expand Up @@ -193,14 +193,19 @@ class AdministratorControlsFragmentPresenter @Inject constructor(
private enum class ViewType {
/** Represents [View] for the general section. */
VIEW_TYPE_GENERAL,

/** Represents [View] for the profile section. */
VIEW_TYPE_PROFILE,

/** Represents [View] for the download permissions section. */
VIEW_TYPE_DOWNLOAD_PERMISSIONS,

/** Represents [View] for the app information section. */
VIEW_TYPE_APP_INFORMATION,

/** Represents [View] for the account actions section. */
VIEW_TYPE_ACCOUNT_ACTIONS,

/** Represents [View] for the learner analytics section. */
VIEW_TYPE_LEARNER_ANALYTICS
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ import android.content.Intent
import android.os.Bundle
import org.oppia.android.app.activity.ActivityComponentImpl
import org.oppia.android.app.activity.InjectableAutoLocalizedAppCompatActivity
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ScreenName.COMPLETED_STORY_LIST_ACTIVITY
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId
import javax.inject.Inject

/** Activity for completed stories. */
Expand All @@ -17,21 +20,22 @@ class CompletedStoryListActivity : InjectableAutoLocalizedAppCompatActivity() {
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(activityComponent as ActivityComponentImpl).inject(this)
val internalProfileId: Int =
intent.getIntExtra(PROFILE_ID_EXTRA_KEY, -1)

val internalProfileId: Int = intent?.extractCurrentUserProfileId()?.internalId ?: -1
completedStoryListActivityPresenter.handleOnCreate(internalProfileId)
}

companion object {
// TODO(#1655): Re-restrict access to fields in tests post-Gradle.
const val PROFILE_ID_EXTRA_KEY =
"CompletedStoryListActivity.profile_id"

/** Returns a new [Intent] to route to [CompletedStoryListActivity] for a specified profile ID. */
fun createCompletedStoryListActivityIntent(context: Context, internalProfileId: Int): Intent {
val intent = Intent(context, CompletedStoryListActivity::class.java)
intent.putExtra(PROFILE_ID_EXTRA_KEY, internalProfileId)
intent.decorateWithScreenName(COMPLETED_STORY_LIST_ACTIVITY)
val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
val intent = Intent(context, CompletedStoryListActivity::class.java).apply {
decorateWithUserProfileId(profileId)
decorateWithScreenName(COMPLETED_STORY_LIST_ACTIVITY)
}

return intent
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import android.view.View
import android.view.ViewGroup
import org.oppia.android.app.fragment.FragmentComponentImpl
import org.oppia.android.app.fragment.InjectableFragment
import org.oppia.android.app.model.ProfileId
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId
import javax.inject.Inject

/** Fragment for displaying completed stories. */
Expand All @@ -15,17 +18,15 @@ class CompletedStoryListFragment : InjectableFragment() {
// TODO(#1655): Re-restrict access to fields in tests post-Gradle.
/** Key for accessing [CompletedStoryListFragment]. */
const val COMPLETED_STORY_LIST_FRAGMENT_TAG = "COMPLETED_STORY_LIST_FRAGMENT_TAG"
/** [String] key for mapping internalProfileId in [Bundle]. */
internal const val COMPLETED_STORY_LIST_FRAGMENT_PROFILE_ID_KEY =
"CompletedStoryListFragment.profile_id"

/** Returns a new [CompletedStoryListFragment] to display corresponding to the specified profile ID. */
fun newInstance(internalProfileId: Int): CompletedStoryListFragment {
val completedStoryListFragment = CompletedStoryListFragment()
val args = Bundle()
args.putInt(COMPLETED_STORY_LIST_FRAGMENT_PROFILE_ID_KEY, internalProfileId)
completedStoryListFragment.arguments = args
return completedStoryListFragment
val profileId = ProfileId.newBuilder().setInternalId(internalProfileId).build()
return CompletedStoryListFragment().apply {
arguments = Bundle().apply {
decorateWithUserProfileId(profileId)
}
}
}
}

Expand All @@ -42,12 +43,11 @@ class CompletedStoryListFragment : InjectableFragment() {
container: ViewGroup?,
savedInstanceState: Bundle?
): View? {
val args = checkNotNull(arguments) {
val arguments = checkNotNull(arguments) {
"Expected arguments to be passed to CompletedStoryListFragment"
}
val internalProfileId = args.getInt(
COMPLETED_STORY_LIST_FRAGMENT_PROFILE_ID_KEY, -1
)
val profileId = arguments.extractCurrentUserProfileId()
val internalProfileId = profileId.internalId
return completedStoryListFragmentPresenter.handleCreateView(
inflater,
container,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,12 @@ import org.oppia.android.app.devoptions.markstoriescompleted.MarkStoriesComplete
import org.oppia.android.app.devoptions.marktopicscompleted.MarkTopicsCompletedActivity
import org.oppia.android.app.devoptions.mathexpressionparser.MathExpressionParserActivity
import org.oppia.android.app.devoptions.vieweventlogs.ViewEventLogsActivity
import org.oppia.android.app.drawer.NAVIGATION_PROFILE_ID_ARGUMENT_KEY
import org.oppia.android.app.model.ProfileId
import org.oppia.android.app.model.ScreenName.DEVELOPER_OPTIONS_ACTIVITY
import org.oppia.android.app.translation.AppLanguageResourceHandler
import org.oppia.android.util.logging.CurrentAppScreenNameIntentDecorator.decorateWithScreenName
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.decorateWithUserProfileId
import org.oppia.android.util.profile.CurrentUserProfileIdIntentDecorator.extractCurrentUserProfileId
import javax.inject.Inject

/** Activity for Developer Options. */
Expand All @@ -40,7 +42,7 @@ class DeveloperOptionsActivity :
override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
(activityComponent as ActivityComponentImpl).inject(this)
internalProfileId = intent.getIntExtra(NAVIGATION_PROFILE_ID_ARGUMENT_KEY, -1)
internalProfileId = intent.extractCurrentUserProfileId().internalId
developerOptionsActivityPresenter.handleOnCreate()
title = resourceHandler.getStringInLocale(R.string.developer_options_activity_title)
}
Expand Down Expand Up @@ -80,11 +82,13 @@ class DeveloperOptionsActivity :
}

companion object {

/** Function to create intent for DeveloperOptionsActivity. */
fun createDeveloperOptionsActivityIntent(context: Context, internalProfileId: Int): Intent {
fun createDeveloperOptionsActivityIntent(context: Context, profileId: ProfileId): Intent {

return Intent(context, DeveloperOptionsActivity::class.java).apply {
putExtra(NAVIGATION_PROFILE_ID_ARGUMENT_KEY, internalProfileId)
decorateWithScreenName(DEVELOPER_OPTIONS_ACTIVITY)
decorateWithUserProfileId(profileId)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,9 @@ package org.oppia.android.app.devoptions

import android.content.Context
import android.content.Intent
import org.oppia.android.app.model.ProfileId

/** Interface to create intent for [DeveloperOptionsActivity]. */
interface DeveloperOptionsStarter {
fun createIntent(context: Context, internalProfileId: Int): Intent
fun createIntent(context: Context, profileId: ProfileId): Intent
}
Loading

0 comments on commit dd80399

Please sign in to comment.