Fixes part of #5345 : Introduced Feature Flag Screen #5861
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comes from #5725. These configurations have been adapted for being interoperable with the existing platform parameter system, but there's downstream work needed yet for actually making the app build and work.
This removes a bunch of stuff from #5725 but introduces some of the initial interop pieces needed to work without substantial integration requirements. A lot more needs to change yet to get the components properly hooked up and the app building, not to mention tests.
The dev build actually opens and runs, though it's still relying on the old parameter values. This also introduces support for flavor-specific overrides, though they are as yet untested. These may have been lost at some point in the development of #5725 since they weren't present in the latest changes.
This actually introduces full interoperability between the new and old system, and in a way that should guarantee start-up safety (mostly). Tests still need to be fixed.
Plus some other clean-ups. This hopefully avoids a dependency explosion for test updates, though of course this functionality will need to be reintroduced in a later branch. This is effectively a no-op since remote syncing is broken in the app.
This also fixes a bunch of tests, but there are a bunch left to go in order to get tests set up correctly. This is a very strong start, however. New tests still need to be added.
This includes removing a lot of old platform parameter test code that needs to be updated. Otherwise, it's primarily about using the test platform parameter module in a bunch of places (since that automatically initializes platform parameters and feature flags for prod code), which in turn requires AssetModule (and both AssetModule and TestPlatformParameterModule require explicit dependencies in Bazel). There are still 31 known failing tests to fix, but the majority have been addressed in this commit.
This includes adding KDocs and temporary test exemptions, plus removing the platform parameters static check since it's not really needed anymore with the new Dagger-bound feature flag map plus other tests for verifying feature flag logging.
|
Unassigning @theayushyadav11 since a re-review was requested. @theayushyadav11, please make sure you have addressed all review comments. Thanks! |
Coverage ReportResultsNumber of files assessed: 16 Failure Cases
Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
adhiamboperes
requested changes
Jul 3, 2025
There was a problem hiding this comment.
Thanks @theayushyadav11! I have a few very minor changes. Should be able to merge by next pass. PTAL.
app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagItemViewModel.kt
Outdated
Show resolved
Hide resolved
...src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentPresenter.kt
Outdated
Show resolved
Hide resolved
.../main/java/org/oppia/android/app/devoptions/featureflags/testing/FeatureFlagsTestActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsViewModel.kt
Outdated
Show resolved
Hide resolved
...rc/sharedTest/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentTest.kt
Outdated
Show resolved
Hide resolved
...rc/sharedTest/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentTest.kt
Outdated
Show resolved
Hide resolved
...rc/sharedTest/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentTest.kt
Outdated
Show resolved
Hide resolved
...rc/sharedTest/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentTest.kt
Outdated
Show resolved
Hide resolved
...rc/sharedTest/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentTest.kt
Outdated
Show resolved
Hide resolved
|
Hi @adhiamboperes , I have updated as requested PTAL. |
|
Unassigning @theayushyadav11 since a re-review was requested. @theayushyadav11, please make sure you have addressed all review comments. Thanks! |
Coverage ReportResultsNumber of files assessed: 16 Passing coverageFiles with passing code coverage
Exempted coverageFiles exempted from coverage
|
adhiamboperes
approved these changes
Jul 4, 2025
|
@BenHenning, PTAL for owners. |
BenHenning
approved these changes
Jul 7, 2025
|
@theayushyadav11 I think you can bring this up-to-date with the latest |
|
Develop was added cleanly, merging. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes part of #5345
This pull request introduces a new "Feature Flags" functionality in the developer options of the app, allowing developers to view and modify feature flags. The changes include adding a new activity, fragment, view models, and necessary wiring to integrate this feature into the existing developer options structure.
Feature Flags Functionality Integration:
New Activity and Fragment for Feature Flags:
FeatureFlagActivityandFeatureFlagFragmentto provide a dedicated UI for managing feature flags. (FeatureFlagActivity.kt[1]],FeatureFlagFragment.kt[2]])FeatureFlagActivityPresenterandFeatureFlagFragmentPr 10000 esenterto handle the UI logic for the activity and fragment. (FeatureFlagActivityPresenter.kt[1]],FeatureFlagFragmentPresenter.kt[2]])View Models for Feature Flags:
FeatureFlagViewModelandFeatureFlagItemViewModelto manage the state and data of feature flags in the UI. (app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagItemViewModel.kt[FeatureFlagItemViewModel.ktR1-R12])Developer Options Updates:
Routing to Feature Flags:
RouteToFeatureFlagsListenerinterface and implemented it inDeveloperOptionsActivityto enable navigation to the new feature flags activity. (app/src/main/java/org/oppia/android/app/devoptions/RouteToFeatureFlagsListener.kt[1]],DeveloperOptionsActivity.kt[2]], [3]])Integration with Developer Options UI:
DeveloperOptionsViewModelandDeveloperOptionsOverrideAppBehaviorsViewModelto include a new option for navigating to the feature flags screen. (app/src/main/java/org/oppia/android/app/devoptions/DeveloperOptionsViewModel.kt[1]], [2]],app/src/main/java/org/oppia/android/app/devoptions/devoptionsitemviewmodel/DeveloperOptionsOverrideAppBehaviorsViewModel.kt[3]], [4]])Configuration and Dependency Updates:
Bazel Build File Updates:
LISTENERS,VIEW_MODELS, and dependencies inBUILD.bazel. (app/BUILD.bazel[1]], [2]], [3]])Manifest and Dependency Injection:
FeatureFlagActivityin the Android manifest. (app/src/main/AndroidManifest.xml[app/src/main/AndroidManifest.xmlR307-R310])ActivityComponentImplto support dependency injection forFeatureFlagActivity. (app/src/main/java/org/oppia/android/app/activity/ActivityComponentImpl.kt[1]], [2]])Screenshots
Event Log Screen
Essential Checklist
For UI-specific PRs only
If your PR includes UI-related changes, then: