Fix #482: Remove default admin for onboarding V2 (3/12) #5968

adhiamboperes · 2025-09-16T00:06:06Z

Explanation

In the legacy onboarding flow, the admin profile is hardcoded into the profile chooser screen. In V2 of the onboarding flow, an admin user creates their profile during onboarding. This makes the hardcoded profile redundant, and this PR removes it.

Main Changes

Gating addProfile()
- profileManagementController.addProfile() is now gated behind the enableOnboardingFlowV2 flag.
- This ensures it is only called in the legacy flow.
Cleanup of profileNickname
- The profileNickname argument (introduced in a previous PR to ensure the created profile had the correct display name) is no longer needed.
- This PR removes its usages and updates the relevant tests.
Fix for App Data Reset Edge Case
- Previously, deleting all profiles during an app data reset left the app in a bad state: the app was considered onboarded, but no profiles existed.
- To address this, a new function resetOnboardingState has been added in AppStartupStateController.kt to reset the onboarding state.

Considered Approaches

There were multiple possible solutions for handling the reset logic:

(a) UI-Layer Reset
- Update alreadyOnboardedApp to false and call the function in the UI layer.
- Minimal impact, no refactor required.
- Requires keeping the delete profiles DataProvider and app startup state DataProvider in sync.
- Harder to test the reset flow in ProfileLoginFragmentTest.kt.
(b) Domain-Layer Reset
- Update alreadyOnboardedApp to false inside profileManagementController.deleteAllProfiles().
- Ensures the two providers are always in sync.
- Violates single responsibility principle of deleteAllProfiles().
(c) Refactor to Remove alreadyOnboardedApp
- Remove the field and instead rely on whether the profiles list is empty.
- Requires significant refactors across multiple areas, including handling both states of the enableAppAndOsDeprecation feature flag.
- Startup logic would need to move away from onboardingFlowStore, adding complexity.

Option (a) was chosen because it:

Requires the least amount of change.
Minimizes side effects.
Keeps the implementation cleaner, even though testing and DataProvider synchronization require extra care.

Essential Checklist

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: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

Before	After
https://github.com/user-attachments/assets/a2f635e9-4078-49c3-b157-374f9f6b8ba9	https://github.com/user-attachments/assets/cce07588-912e-4b44-85b5-6007381106bc

oppiabot · 2025-09-23T00:36:41Z

Hi @adhiamboperes, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

github-actions · 2025-09-24T00:09:29Z

Coverage Report

Results

Number of files assessed: 8
Overall Coverage: 91.57%
Coverage Analysis: PASS ✅

Passing coverage

Files with passing code coverage

File	Coverage	Lines Hit	Status	Min Required
AppStartupStateController.kt domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt	91.57%	76 / 83	✅	70%

Exempted coverage

Files exempted from coverage

File	Exemption Reason
ProfileChooserFragment.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragment.kt	This file is incompatible with code coverage tooling; skipping coverage check.
ProfileLoginFragmentPresenter.kt app/src/main/java/org/oppia/android/app/profile/ProfileLoginFragmentPresenter.kt	This file is exempted from having a test file; skipping coverage check.
ProfileChooserActivity.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivity.kt	This file is exempted from having a test file; skipping coverage check.
ProfileChooserActivityPresenter.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt	This file is exempted from having a test file; skipping coverage check.
SplashActivity.kt app/src/main/java/org/oppia/android/app/splash/SplashActivity.kt	This file is incompatible with code coverage tooling; skipping coverage check.
AdminIntroFragmentPresenter.kt app/src/main/java/org/oppia/android/app/onboarding/AdminIntroFragmentPresenter.kt	This file is exempted from having a test file; skipping coverage check.
AdminIntroFragment.kt app/src/main/java/org/oppia/android/app/onboarding/AdminIntroFragment.kt	This file is incompatible with code coverage tooling; skipping coverage check.

Refer test_file_exemptions.textproto for the comprehensive list of file exemptions and their required coverage percentages.

To learn more, visit the Oppia Android Code Coverage wiki page

adhiamboperes · 2025-09-24T00:28:05Z

@BenHenning, PTAL. This is probably the smallest PR in the entire project 😁.

BenHenning

Thanks @adhiamboperes! Just a few comments--PTAL.

BenHenning · 2025-09-30T17:59:35Z

model/src/main/proto/arguments.proto

 message ProfileChooserActivityParams {
-  // TODO(#482): Remove once the default admin profile has been removed.
-  // The nickname associated with a newly created profile.
-  string profile_nickname = 1;


This should be marked as reserved probably, just for posterity (since the interoperability risk is low).

This does not need to be reserved because it has never actually been used in logs. I introduced it in one of the earlier PRs for this project, so there is no risk to removing it completely. I noticed that I should have re-numbered the fields however, which I have now done.

BenHenning · 2025-09-30T21:42:58Z

app/src/main/java/org/oppia/android/app/profile/ProfileLoginFragmentPresenter.kt

  }

+  /**
+   * Deletes all app data by removing all profile,s resetting onboarding state, and closing the app.


Suggested change

* Deletes all app data by removing all profile,s resetting onboarding state, and closing the app.

* Deletes all app data by removing all profile's resetting onboarding state, and closing the app.

BenHenning · 2025-09-30T21:44:00Z

app/src/main/java/org/oppia/android/app/profile/ProfileLoginFragmentPresenter.kt

  private fun deleteAppData() {
    profileManagementController.deleteAllProfiles().toLiveData().observe(fragment) {
+      // Reset onboarding so the app starts fresh.
+      appStartupStateController.resetOnboardingState()


Perhaps worth verifying via test that this step happens?

Added, and here is proof that the test fails if the wrong StartupMode is asserted.

BenHenning · 2025-09-30T21:44:54Z

Reminder check the checkboxes on the PR description as well.

Other than that, super cool to see an issue from 2019 get resolved! :D

adhiamboperes · 2025-10-02T19:19:30Z

@BenHenning, PTAL.

github-actions · 2025-10-02T20:38:57Z

Coverage Report

Results

Number of files assessed: 9
Overall Coverage: 91.57%
Coverage Analysis: PASS ✅

Passing coverage

Files with passing code coverage

File	Coverage	Lines Hit	Status	Min Required
AppStartupStateController.kt domain/src/main/java/org/oppia/android/domain/onboarding/AppStartupStateController.kt	91.57%	76 / 83	✅	70%

Exempted coverage

Files exempted from coverage

File	Exemption Reason
SplashActivity.kt app/src/main/java/org/oppia/android/app/splash/SplashActivity.kt	This file is incompatible with code coverage tooling; skipping coverage check.
ProfileLoginFragment.kt app/src/main/java/org/oppia/android/app/profile/ProfileLoginFragment.kt	This file is incompatible with code coverage tooling; skipping coverage check.
ProfileChooserFragment.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserFragment.kt	This file is incompatible with code coverage tooling; skipping coverage check.
ProfileLoginFragmentPresenter.kt app/src/main/java/org/oppia/android/app/profile/ProfileLoginFragmentPresenter.kt	This file is exempted from having a test file; skipping coverage check.
ProfileChooserActivity.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivity.kt	This file is exempted from having a test file; skipping coverage check.
ProfileChooserActivityPresenter.kt app/src/main/java/org/oppia/android/app/profile/ProfileChooserActivityPresenter.kt	This file is exempted from having a test file; skipping coverage check.
AdminIntroFragmentPresenter.kt app/src/main/java/org/oppia/android/app/onboarding/AdminIntroFragmentPresenter.kt	This file is exempted from having a test file; skipping coverage check.
AdminIntroFragment.kt app/src/main/java/org/oppia/android/app/onboarding/AdminIntroFragment.kt	This file is incompatible with code coverage tooling; skipping coverage check.

Refer test_file_exemptions.textproto for the comprehensive list of file exemptions and their required coverage percentages.

To learn more, visit the Oppia Android Code Coverage wiki page

oppiabot · 2025-10-09T21:00:17Z

Hi @adhiamboperes, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

oppiabot · 2025-10-21T09:28:51Z

Hi @adhiamboperes, I'm going to mark this PR as stale because it hasn't had any updates for 7 days. If no further activity occurs within 7 days, it will be automatically closed so that others can take up the issue.
If you are still working on this PR, please make a follow-up commit within 3 days (and submit it for review, if applicable). Please also let us know if you are stuck so we can help you!

Remove default profile

147fbb1

adhiamboperes self-assigned this Sep 16, 2025

adhiamboperes changed the base branch from develop to create-admin-intro September 16, 2025 00:06

adhiamboperes changed the title ~~Fix #482: Remove default admin for onboarding V2~~ Fix #482: Remove default admin for onboarding V2 (3/12) Sep 16, 2025

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 23, 2025

adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Sep 23, 2025

adhiamboperes added 2 commits September 23, 2025 22:07

Merge branch 'create-admin-intro' into remove-default-admin

0f5605e

Reset onboarding state

8fbd042

adhiamboperes marked this pull request as ready for review September 24, 2025 00:26

adhiamboperes requested review from a team as code owners September 24, 2025 00:26

adhiamboperes requested review from BenHenning and removed request for a team September 24, 2025 00:26

adhiamboperes assigned BenHenning and unassigned adhiamboperes Sep 24, 2025

BenHenning requested changes Sep 30, 2025

View reviewed changes

BenHenning assigned adhiamboperes and unassigned BenHenning Sep 30, 2025

Address reviewer comments

6dfdc60

adhiamboperes assigned BenHenning and unassigned adhiamboperes Oct 2, 2025

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 9, 2025

adhiamboperes removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 14, 2025

oppiabot bot added the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 21, 2025

oppiabot bot closed this Oct 28, 2025

adhiamboperes reopened this Oct 29, 2025

oppiabot bot removed the stale Corresponds to items that haven't seen a recent update and may be automatically closed. label Oct 29, 2025

	* Deletes all app data by removing all profile,s resetting onboarding state, and closing the app.
	* Deletes all app data by removing all profile's resetting onboarding state, and closing the app.

Fix #482: Remove default admin for onboarding V2 (3/12) #5968

Are you sure you want to change the base?

Fix #482: Remove default admin for onboarding V2 (3/12) #5968

Uh oh!

Conversation

adhiamboperes commented Sep 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation

Main Changes

Considered Approaches

Essential Checklist

For UI-specific PRs only

Uh oh!

oppiabot bot commented Sep 23, 2025

Uh oh!

github-actions bot commented Sep 24, 2025

Coverage Report

Results

Passing coverage

Exempted coverage

Uh oh!

adhiamboperes commented Sep 24, 2025

Uh oh!

BenHenning left a comment

Choose a reason for hiding this comment

Uh oh!

BenHenning Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

adhiamboperes Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

BenHenning Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

adhiamboperes Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

BenHenning Sep 30, 2025

Choose a reason for hiding this comment

Uh oh!

adhiamboperes Oct 2, 2025

Choose a reason for hiding this comment

Uh oh!

BenHenning commented Sep 30, 2025

Uh oh!

adhiamboperes commented Oct 2, 2025

Uh oh!

github-actions bot commented Oct 2, 2025

Coverage Report

Results

Passing coverage

Exempted coverage

Uh oh!

oppiabot bot commented Oct 9, 2025

Uh oh!

oppiabot bot commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

adhiamboperes commented Sep 16, 2025 •

edited

Loading