Fix #565: Domain on-boarding flag

[nikitamarysolomanpvt]

Explanation

Onboarding flow should be visible only once irrespective of learner profile. Introduced domain layer code to control this feature.
Oppia app now showing On-boarding on initial app open, and directly to choose profile page upon returning to the app. Both of these states are also properly retained across configuration changes due to Dagger not leading to activities trying to recreate their dependent controllers.

From a testing side, testcase for OnboardingFlowControllerTest is introduced to test OnboardingFlowController.

Also this added new SplashActivity tests which rely on sleeping rather than idling resource similar like HomeActivity.

Checklist

• The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
• The PR explanation includes the words "Fixes #bugnum: ..." (or "Fixes part of #bugnum" if the PR only partially fixes an issue).
• The PR follows the style guide.
• The PR does not contain any unnecessary auto-generated code from Android Studio.
• The PR is made from a branch that's not called "develop".
• The PR is made from a branch that is up-to-date with "develop".
• The PR's branch is based on "develop" and not on any other branch.
• The PR is assigned to an appropriate reviewer in both the Assignees and the Reviewers sections.

[nikitamarysolomanpvt]

@rt4914 & @BenHenning The new SplashActivity tests similar like HomeActivity have some issue in generating DaggerSplashActivityTest_TestApplicationComponent and blocked further not able to work on testing because of the same. PTAL

[rt4914]

I believe that this PR has been assigned just to check the overview approach. (I am assuming this keeping in mind that the PR has not been self reviewed and there are a lot of nit changes).

Can you please update this code, because I think we do not need any change in files related to UserApphistory.

[nikitamarysolomanpvt]

@rt4914 & @BenHenning The new SplashActivity tests similar like HomeActivity have some issue in generating DaggerSplashActivityTest_TestApplicationComponent and blocked further not able to work on testing because of the same. PTAL

@rt4914 PTAL on this.

[nikitamarysolomanpvt]

Check my comments and resolve them and assign it back to me.

All my comments have been lost, so just assign me back when are done with this PR.

Okay i merged and resolved conflicts

[rt4914]

Please check the comments. Also, I suggest using on-board in comments instead of onBoard to make it consistent.

[rt4914]

LGTM, Just one comment.

[BenHenning]

Sorry that this might cause a delay, but can you actually split this into 2 PRs? I think it would be a bit easier to follow & review if there was a first PR implementing just the domain controller & its tests, and a follow-up PR that's based on the first which introduces the initial activity/fragment that connects into that domain layer.

I think this is a general pattern we should use moving forward since it helps keep reviews focused on specific layers rather than reviewers needing to understand vertical implementations. This helps improve reviewing correctness and test coverage since reviewing domain layer code is quite different than frontend UI code, and more complex examples of vertically implemented features can easily reach into the thousands of lines of code.

FYI @rt4914 & @veena14cs as well for future PRs.

[rt4914]

Sorry that this might cause a delay, but can you actually split this into 2 PRs? I think it would be a bit easier to follow & review if there was a first PR implementing just the domain controller & its tests, and a follow-up PR that's based on the first which introduces the initial activity/fragment that connects into that domain layer.

I think this is a general pattern we should use moving forward since it helps keep reviews focused on specific layers rather than reviewers needing to understand vertical implementations. This helps improve reviewing correctness and test coverage since reviewing domain layer code is quite different than frontend UI code, and more complex examples of vertically implemented features can easily reach into the thousands of lines of code.

FYI @rt4914 & @veena14cs as well for future PRs.

@BenHenning In that case, lets just keep this as single PR which implements the domain layer as I already have a PR #558 which works on low-fi, so once this domain layer code is approved and merged in develop, I can just update #558 such that it uses domain layer code and implements everything.

[nikitamarysolomanpvt]

Sorry that this might cause a delay, but can you actually split this into 2 PRs? I think it would be a bit easier to follow & review if there was a first PR implementing just the domain controller & its tests, and a follow-up PR that's based on the first which introduces the initial activity/fragment that connects into that domain layer.

I think this is a general pattern we should use moving forward since it helps keep reviews focused on specific layers rather than reviewers needing to understand vertical implementations. This helps improve reviewing correctness and test coverage since reviewing domain layer code is quite different than frontend UI code, and more complex examples of vertically implemented features can easily reach into the thousands of lines of code.

FYI @rt4914 & @veena14cs as well for future PRs.

I agree.
PTAL on
#619 and
#619