Skip to content

Commit fcb87dc

Browse files
theayushyadav11theayushyadav11manas-yuadhiamboperesRd4dev
authored
Fixes part of #5345: Fixing restarting of app on config change. (#5953)
Fixes part of #5345 : This pull request improves the handling of dialog persistence in the Feature Flags and Platform Parameters developer options screens, specifically ensuring that pending changes and restart dialogs remain visible after configuration changes (such as device rotation). It introduces a new state variable to track dialog visibility, updates fragment and presenter logic to restore dialogs, and adds tests to verify this behavior. Additionally, it refines the app restart logic to avoid unnecessary restarts during configuration changes. **Dialog Persistence and State Management** * Added an `PendingDialogListener` and implemented in `PlatformParametersFragment` and `FeatureFlagsFragment` when the save and discard are called from `PendingChangesDialogFragment`. **App Restart Logic** * Modified the app restart logic in both presenters to only restart the app if the configuration is not changing, preventing unnecessary restarts during device rotation. **Testing** * Added new instrumented tests for both fragments to verify that dialogs (pending changes and restart dialogs) persist after configuration changes, improving reliability and user experience. **Code Cleanup** * Removed extra `onBackNavigation` code in the `PlatformParametersFragmentPresenter.kt` that might have been duplicated. These changes ensure a smoother and more predictable experience for developers using the feature flags and platform parameters screens, especially when making changes that require confirmation dialogs. #### Screenrecording: https://github.com/user-attachments/assets/4642d72f-bfce-4179-9524-0e7aaa3971a9 ## 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)). --------- Co-authored-by: theayushyadav11 <[email protected]> Co-authored-by: Manas <[email protected]> Co-authored-by: Adhiambo Peres <[email protected]> Co-authored-by: RD Rama Devi <[email protected]>
1 parent 629c0db commit fcb87dc

14 files changed

+544
-157
lines changed

app/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ LISTENERS = [
9292
"src/main/java/org/oppia/android/app/devoptions/RouteToMathExpressionParserTestListener.kt",
9393
"src/main/java/org/oppia/android/app/devoptions/RouteToPlatformParametersListener.kt",
9494
"src/main/java/org/oppia/android/app/devoptions/RouteToViewEventLogsListener.kt",
95+
"src/main/java/org/oppia/android/app/devoptions/SavePendingChangesDialogListener.kt",
9596
"src/main/java/org/oppia/android/app/drawer/RouteToProfileProgressListener.kt",
9697
"src/main/java/org/oppia/android/app/help/LoadFaqListFragmentListener.kt",
9798
"src/main/java/org/oppia/android/app/help/LoadLicenseListFragmentListener.kt",

app/src/main/java/org/oppia/android/app/devoptions/AppRestartDialogFragmentPresenter.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@ class AppRestartDialogFragmentPresenter @Inject constructor(
2222
)
2323
binding.lifecycleOwner = fragment
2424

25+
val appRestartInterface = fragment.parentFragment as AppRestartListener
2526
val dialog = AlertDialog.Builder(activity, R.style.OppiaAlertDialogTheme)
2627
.setView(binding.root)
2728
.create()
2829
dialog.setCanceledOnTouchOutside(false)
2930

3031
binding.restartButton.setOnClickListener {
32+
appRestartInterface.restartApp()
3133
dialog.dismiss()
32-
activity.finishAffinity()
3334
}
3435
return dialog
3536
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
package org.oppia.android.app.devoptions
2+
3+
import android.app.Dialog
4+
import android.content.Context
5+
import android.os.Bundle
6+
import org.oppia.android.app.fragment.FragmentComponentImpl
7+
import org.oppia.android.app.fragment.InjectableDialogFragment
8+
import javax.inject.Inject
9+
10+
/** Dialog fragment shown to prompt the user to save/discard the changes. */
11+
class PendingChangesDialogFragment : InjectableDialogFragment() {
12+
@Inject
13+
lateinit var pendingChangesDialogFragmentPresenter:
14+
PendingChangesDialogFragmentPresenter
15+
16+
companion object {
17+
/** Returns a new instance of [PendingChangesDialogFragment]. */
18+
fun newInstance(): PendingChangesDialogFragment =
19+
PendingChangesDialogFragment()
20+
}
21+
22+
override fun onAttach(context: Context) {
23+
super.onAttach(context)
24+
(fragmentComponent as FragmentComponentImpl).inject(this)
25+
}
26+
27+
override fun onCreateDialog(savedInstanceState: Bundle?): Dialog {
28+
return pendingChangesDialogFragmentPresenter.handleOnCreateDialog()
29+
}
30+
}
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package org.oppia.android.app.devoptions
2+
3+
import android.app.AlertDialog
4+
import android.app.Dialog
5+
import androidx.appcompat.app.AppCompatActivity
6+
import androidx.fragment.app.Fragment
7+
import org.oppia.android.app.databinding.databinding.PendingChangesDialogFragmentBinding
8+
import org.oppia.android.app.ui.R
9+
import javax.inject.Inject
10+
11+
/** Presenter for the [PendingChangesDialogFragment]. */
12+
class PendingChangesDialogFragmentPresenter @Inject constructor(
13+
private val fragment: Fragment,
14+
private val activity: AppCompatActivity,
15+
) {
16+
/** Creates a dialog that prompts the user to save or discard the changes in the dashboard. */
17+
fun handleOnCreateDialog(): Dialog {
18+
val binding = PendingChangesDialogFragmentBinding.inflate(
19+
activity.layoutInflater,
20+
/* parent= */ null,
21+
/* attachToRoot= */ false
22+
)
23+
binding.lifecycleOwner = fragment
24+
25+
val pendingChangesInterface = fragment.parentFragment as SavePendingChangesDialogListener
26+
val dialog = AlertDialog.Builder(activity, R.style.OppiaAlertDialogTheme)
27+
.setView(binding.root)
28+
.create()
29+
dialog.setCanceledOnTouchOutside(true)
30+
31+
binding.saveButton.setOnClickListener {
32+
pendingChangesInterface.savePendingChanges()
33+
dialog.dismiss()
34+
}
35+
36+
binding.discardButton.setOnClickListener {
37+
pendingChangesInterface.discardPendingChanges()
38+
dialog.dismiss()
39+
}
40+
return dialog
41+
}
42+
}
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
package org.oppia.android.app.devoptions
2+
3+
/** Interface to be implemented by classes that need to handle pending changes. */
4+
interface SavePendingChangesDialogListener {
5+
6+
/** Saves the pending changes. */
7+
fun savePendingChanges()
8+
9+
/** Discards the pending changes and exits the screen. */
10+
fun discardPendingChanges()
11+
}

app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragment.kt

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import android.os.Bundle
55
import android.view.LayoutInflater
66
import android.view.View
77
import android.view.ViewGroup
8+
import org.oppia.android.app.devoptions.AppRestartListener
9+
import org.oppia.android.app.devoptions.SavePendingChangesDialogListener
810
import org.oppia.android.app.fragment.FragmentComponentImpl
911
import org.oppia.android.app.fragment.InjectableFragment
1012
import org.oppia.android.app.model.FeatureFlagId
@@ -15,7 +17,10 @@ import org.oppia.android.util.extensions.putProto
1517
import javax.inject.Inject
1618

1719
/** Fragment to provide functionality to view and modify feature flags of the app. */
18-
class FeatureFlagsFragment : InjectableFragment() {
20+
class FeatureFlagsFragment :
21+
InjectableFragment(),
22+
AppRestartListener,
23+
SavePendingChangesDialogListener {
1924
@Inject
2025
lateinit var featureFlagsFragmentPresenter: FeatureFlagsFragmentPresenter
2126

@@ -86,8 +91,15 @@ class FeatureFlagsFragment : InjectableFragment() {
8691
outState.putProto(FEATURE_FLAGS_FRAGMENT_SAVED_STATE_KEY, proto)
8792
}
8893

89-
override fun onDestroy() {
90-
super.onDestroy()
91-
featureFlagsFragmentPresenter.handleOnDestroy()
94+
override fun restartApp() {
95+
featureFlagsFragmentPresenter.restartApp()
96+
}
97+
98+
override fun savePendingChanges() {
99+
featureFlagsFragmentPresenter.savePendingChanges()
100+
}
101+
102+
override fun discardPendingChanges() {
103+
featureFlagsFragmentPresenter.discardPendingChanges()
92104
}
93105
}

app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentPresenter.kt

Lines changed: 48 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
package org.oppia.android.app.devoptions.featureflags
22

3-
import android.app.AlertDialog
43
import android.content.Intent
54
import android.view.LayoutInflater
65
import android.view.View
@@ -12,8 +11,8 @@ import androidx.fragment.app.Fragment
1211
import androidx.recyclerview.widget.LinearLayoutManager
1312
import org.oppia.android.app.databinding.databinding.FeatureFlagsFragmentBinding
1413
import org.oppia.android.app.databinding.databinding.FeatureFlagsItemBinding
15-
import org.oppia.android.app.databinding.databinding.PendingChangesDialogFragmentBinding
1614
import org.oppia.android.app.devoptions.AppRestartDialogFragment
15+
import org.oppia.android.app.devoptions.PendingChangesDialogFragment
1716
import org.oppia.android.app.fragment.FragmentScope
1817
import org.oppia.android.app.model.FeatureFlagId
1918
import org.oppia.android.app.model.OverriddenFeatureFlag
@@ -31,6 +30,9 @@ import kotlin.system.exitProcess
3130
/** Tag for displaying [AppRestartDialogFragment]. */
3231
const val TAG_FEATURE_FLAG_RESTART_DIALOG = "FEATURE_FLAG_RESTART_DIALOG_TAG"
3332

33+
/** Tag for displaying [PendingChangesDialogFragment]. */
34+
const val TAG_FEATURE_FLAG_PENDING_CHANGES_DIALOG = "FEATURE_FLAG_PENDING_CHANGES_DIALOG_TAG"
35+
3436
/** The presenter for [FeatureFlagsFragment]. */
3537
@FragmentScope
3638
class FeatureFlagsFragmentPresenter @Inject constructor(
@@ -44,7 +46,6 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
4446
private lateinit var binding: FeatureFlagsFragmentBinding
4547
private lateinit var linearLayoutManager: LinearLayoutManager
4648
private lateinit var bindingAdapter: BindableAdapter<FeatureFlagItemViewModel>
47-
private var restartRequired: Boolean = false
4849

4950
/** Called when [FeatureFlagsFragment] is created. Handles UI for the fragment. */
5051
fun handleCreateView(
@@ -58,13 +59,6 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
5859
container,
5960
/* attachToRoot= */ false
6061
)
61-
binding.featureFlagsToolbar.setNavigationOnClickListener {
62-
onBackNavigation()
63-
}
64-
binding.saveButton.setOnClickListener {
65-
val overriddenFlags = computeOverriddenFlags()
66-
savePendingFeatureFlags(overriddenFlags)
67-
}
6862

6963
activity.onBackPressedDispatcher.addCallback(
7064
fragment,
@@ -82,16 +76,23 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
8276
featureFlagsViewModel.resetFlags.value = resetFlags.toMutableMap()
8377
}
8478

85-
binding.apply {
86-
this.lifecycleOwner = fragment
87-
this.viewModel = featureFlagsViewModel
88-
}
89-
9079
linearLayoutManager = LinearLayoutManager(activity.applicationContext)
9180
bindingAdapter = createRecyclerViewAdapter()
92-
binding.featureFlagsRecyclerView.apply {
93-
layoutManager = linearLayoutManager
94-
adapter = bindingAdapter
81+
82+
binding.apply {
83+
lifecycleOwner = fragment
84+
viewModel = featureFlagsViewModel
85+
saveButton.setOnClickListener {
86+
val overriddenFlags = computeOverriddenFlags()
87+
savePendingFeatureFlags(overriddenFlags)
88+
}
89+
featureFlagsToolbar.setNavigationOnClickListener {
90+
onBackNavigation()
91+
}
92+
featureFlagsRecyclerView.apply {
93+
layoutManager = linearLayoutManager
94+
adapter = bindingAdapter
95+
}
9596
}
9697

9798
return binding.root
@@ -111,33 +112,15 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
111112
val resetFlags = getResetFeatureFlags()
112113

113114
if (overriddenFlags.isNotEmpty() || resetFlags.isNotEmpty()) {
114-
showPendingChangesDialog(overriddenFlags)
115+
showPendingChangesDialog()
115116
} else {
116117
activity.finish()
117118
}
118119
}
119120

120-
private fun showPendingChangesDialog(overriddenFlags: List<OverriddenFeatureFlag>) {
121-
val dialogBinding = PendingChangesDialogFragmentBinding.inflate(
122-
LayoutInflater.from(activity),
123-
/* root= */ null,
124-
/* attachToRoot= */ false
125-
)
126-
val dialog = AlertDialog.Builder(activity)
127-
.setView(dialogBinding.root)
128-
.create()
129-
130-
dialogBinding.saveButton.setOnClickListener {
131-
dialog.dismiss()
132-
savePendingFeatureFlags(overriddenFlags)
133-
}
134-
135-
dialogBinding.discardButton.setOnClickListener {
136-
dialog.dismiss()
137-
activity.finish()
138-
}
139-
140-
dialog.show()
121+
private fun showPendingChangesDialog() {
122+
val dialog = PendingChangesDialogFragment.newInstance()
123+
dialog.showNow(fragment.childFragmentManager, TAG_FEATURE_FLAG_PENDING_CHANGES_DIALOG)
141124
}
142125

143126
private fun savePendingFeatureFlags(overriddenFlags: List<OverriddenFeatureFlag>) {
@@ -190,7 +173,6 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
190173
.observe(fragment) { result ->
191174
when (result) {
192175
is AsyncResult.Success -> {
193-
restartRequired = true
194176
val dialog = AppRestartDialogFragment.newInstance()
195177
dialog.showNow(fragment.childFragmentManager, TAG_FEATURE_FLAG_RESTART_DIALOG)
196178
}
@@ -273,21 +255,15 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
273255
}
274256
}
275257

276-
/**
277-
* Called when [FeatureFlagsFragment] is destroyed.
278-
* Performs a fresh restart of the app to load any updated feature flag states, if required.
279-
*/
280-
fun handleOnDestroy() {
281-
if (restartRequired) {
282-
val intent = Intent(activity, SplashActivity::class.java).also {
283-
it.action = Intent.ACTION_MAIN
284-
it.addCategory(Intent.CATEGORY_LAUNCHER)
285-
}
286-
activity.startActivity(intent)
287-
// App is terminated to ensure a fresh restart and kill all the current process
288-
// so that ProcessState can be reinitialised on the fresh restart.
289-
exitProcess(0)
290-
}
258+
/** Called when user opts to save changes in [PendingChangesDialogFragment]. */
259+
fun savePendingChanges() {
260+
val overriddenFlags = computeOverriddenFlags()
261+
savePendingFeatureFlags(overriddenFlags)
262+
}
263+
264+
/** Called when user opts to discard changes in [PendingChangesDialogFragment]. */
265+
fun discardPendingChanges() {
266+
activity.finish()
291267
}
292268

293269
/**
@@ -309,4 +285,20 @@ class FeatureFlagsFragmentPresenter @Inject constructor(
309285
fun getFeatureFlagStates(): Map<FeatureFlagId, Boolean> {
310286
return featureFlagsViewModel.featureFlagStates.value ?: mapOf()
311287
}
288+
289+
/**
290+
* Performs a fresh restart of the app to reload feature flag states and reinitialize
291+
* the app processState.
292+
*/
293+
fun restartApp() {
294+
val intent = Intent(activity, SplashActivity::class.java).also {
295+
it.action = Intent.ACTION_MAIN
296+
it.addCategory(Intent.CATEGORY_LAUNCHER)
297+
}
298+
activity.finishAffinity()
299+
activity.startActivity(intent)
300+
// App is terminated to ensure a fresh restart and kill all the current process
301+
// so that ProcessState can be reinitialised on the fresh restart.
302+
exitProcess(0)
303+
}
312304
}

app/src/main/java/org/oppia/android/app/devoptions/platformparameters/PlatformParametersFragment.kt

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import android.os.Bundle
55
import android.view.LayoutInflater
66
import android.view.View
77
import android.view.ViewGroup
8+
import org.oppia.android.app.devoptions.AppRestartListener
9+
import org.oppia.android.app.devoptions.SavePendingChangesDialogListener
810
import org.oppia.android.app.fragment.FragmentComponentImpl
911
import org.oppia.android.app.fragment.InjectableFragment
1012
import org.oppia.android.app.model.OverriddenPlatformParameter
@@ -16,7 +18,10 @@ import org.oppia.android.util.extensions.putProto
1618
import javax.inject.Inject
1719

1820
/** Fragment to provide functionality to view and modify platform parameters of the app. */
19-
class PlatformParametersFragment : InjectableFragment() {
21+
class PlatformParametersFragment :
22+
InjectableFragment(),
23+
AppRestartListener,
24+
SavePendingChangesDialogListener {
2025
@Inject
2126
lateinit var platformParametersFragmentPresenter: PlatformParametersFragmentPresenter
2227

@@ -42,7 +47,6 @@ class PlatformParametersFragment : InjectableFragment() {
4247
val platformParameterStates:
4348
MutableMap<PlatformParameterId, PlatformParameterValue?> = mutableMapOf()
4449
val resetParamList: MutableMap<PlatformParameterId, PlatformParameterValue> = mutableMapOf()
45-
4650
if (savedInstanceState != null) {
4751
val args = savedInstanceState.getProto(
4852
PLATFORM_PARAMETERS_FRAGMENT_SAVED_STATE_KEY,
@@ -85,6 +89,7 @@ class PlatformParametersFragment : InjectableFragment() {
8589
.setOverriddenValue(value)
8690
.build()
8791
}
92+
8893
val proto = PlatformParametersFragmentStateBundle.newBuilder()
8994
.addAllPlatformParameterStates(validParameterOverrides)
9095
.addAllResetPlatformParameters(resetParamList)
@@ -94,8 +99,15 @@ class PlatformParametersFragment : InjectableFragment() {
9499
)
95100
}
96101

97-
override fun onDestroy() {
98-
super.onDestroy()
99-
platformParametersFragmentPresenter.handleOnDestroy()
102+
override fun restartApp() {
103+
platformParametersFragmentPresenter.appRestart()
104+
}
105+
106+
override fun savePendingChanges() {
107+
platformParametersFragmentPresenter.savePendingChanges()
108+
}
109+
110+
override fun discardPendingChanges() {
111+
platformParametersFragmentPresenter.discardPendingChanges()
100112
}
101113
}

0 commit comments

Comments
 (0)