-
Notifications
You must be signed in to change notification settings - Fork 565
Fixes part of #5345: Implemented Reset functionality of Platform Parameters with new UI. #5894
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Coverage ReportResultsNumber of files assessed: 3 Exempted coverageFiles exempted from coverage
|
.../java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImplTest.kt
Outdated
Show resolved
Hide resolved
.../java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImplTest.kt
Outdated
Show resolved
Hide resolved
…o reset-platform-parameters
Huh. I don't think my review is actually needed here. I don't see any files specific to owners that @adhiamboperes can't approve, so perhaps it is a holdover from an earlier version of the PR. Updating the reviewer list accordingly. Please assign back if my approval is needed after @adhiamboperes's (I will quickly approve whatever remaining files need approval at that point since it's usually something small). |
Hi @adhiamboperes , PTAL. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Rd4dev for the extensive review!
@theayushyadav11, the PR looks solid. I may not have been as thorough as I would like due to time and size, but the PR is in a good shape.
...src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsFragmentPresenter.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagItemViewModel.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/devoptions/featureflags/FeatureFlagsViewModel.kt
Outdated
Show resolved
Hide resolved
...main/java/org/oppia/android/app/devoptions/platformparameters/PlatformParametersViewModel.kt
Outdated
Show resolved
Hide resolved
...t/java/org/oppia/android/app/devoptions/platformparameters/PlatformParametersActivityTest.kt
Outdated
Show resolved
Hide resolved
.../java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImplTest.kt
Show resolved
Hide resolved
.../java/org/oppia/android/domain/platformparameter/PlatformParameterControllerDebugImplTest.kt
Show resolved
Hide resolved
Unassigning @adhiamboperes since the review is done. |
Hi @theayushyadav11, it looks like some changes were requested on this pull request by @adhiamboperes. PTAL. Thanks! |
…o reset-platform-parameters
PTAL @adhiamboperes |
Unassigning @theayushyadav11 since a re-review was requested. @theayushyadav11, please make sure you have addressed all review comments. Thanks! |
Hi @adhiamboperes , I have addressed all the comments PTAL. |
Hi @BenHenning PTAL at this comment. |
Coverage ReportResultsNumber of files assessed: 9 Exempted coverageFiles exempted from coverage
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @theayushyadav11!.
Please see my comment about filing and issue to clarify test naming and outcomes.
Fixes part of #5345:
This pull request introduces functionality to reset locally overridden platform parameters and feature flags to their server or default values, along with UI updates to support these features.
New Functionality
Platform Parameters
resetPlatformParameter
method to handle resetting parameters to server/default valuesFeature Flags
resetFeatureFlag
method for boolean flagsUI Updates
isResetAvailable
,isResetButtonActive
)Save
button which in the future PR will have an Alert Dialog attached.Testing
Screen Recording
dashboard.mp4
Screenshots
Essential Checklist