Fix #3360: Platform Parameter Dagger Module and Constants File [BLOCKED on #3323]

[Arjupta]

Explanation

Fixes #3360
This PR introduce a new Dagger Module which will provide the PlatformParameters and a Parameter Constants file to store the annotations, names, default values at one place.

Note - This PR containes the changes from PR1.2 and PR1.2 which were made from the fork of Oppia Repository, This PR is made directly from Oppia-Android repository in order to help us with Stacking PRs. Both of the previous PRs were almost reviewed so instead of reopening them we will copy the changes made in them here in order to develop over those changes. Any further change in these two PRs will be included here also (manually copy pasted) till the time those PRs are merged. Any further PR that will be made afterwards, will be stacked based on this PR (if needed).
/cc @Sarthak2601

Change associated with this PR -

• ApplicationComponent
• PlatformParameterModule
• TestPlatformParameterModule
• PlatformParameterConstants
• PlatformParameterModuleTest

Rest of the files are from previous two PRs.

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.

[jcqli]

This is interesting since I don't think we want to hard-code any example parameters into this module, but we don't yet have features that are using this module quite yet. For similar reasons we also don't have a means of testing this module.

Overall the PR looks pretty good but I'm not sure if we want to check this in with fake parameters. I'll defer to @vinitamurthi on her thoughts about how we might update this but I don't see anything else major in this PR -- nice job!

Only additional comment is to mentions which files or lines of code are newly-added in this PR so reviewers don't re-review code from the past PRs.

[Arjupta]

This is interesting since I don't think we want to hard-code any example parameters into this module, but we don't yet have features that are using this module quite yet. For similar reasons we also don't have a means of testing this module.

Overall the PR looks pretty good but I'm not sure if we want to check this in with fake parameters. I'll defer to @vinitamurthi on her thoughts about how we might update this but I don't see anything else major in this PR -- nice job!

Only additional comment is to mentions which files or lines of code are newly-added in this PR so reviewers don't re-review code from the past PRs.

I have added these Example Parameters just to show how are we trying to provide the parameters. We can remove them once we can actually add a PlatformParameter (which will happen once we reach the end of Milestone 1)

I will write the tests the tests now, just wanted to have first review over the kind of changes

Edited the description to highlight the newly added files.

@jcqli

[jcqli]

I have added these Example Parameters just to show how are we trying to provide the parameters. We can remove them once we can actually add a PlatformParameter (which will happen once we reach the end of Milestone 1)

I will write the tests the tests now, just wanted to have first review over the kind of changes

I understand why we have the examples parameters but it seems odd to check in code that's not used. That said, I do agree that it would still be best to include these examples largely as a means of testing the module especially since it's using the map in the earlier PR and we probably want to check that it's handled correctly. Please add the test file, and I also recommend being explicit that the parameters in the modules are only examples and not used for actual parameters yet. Maybe changing the names to provideExampleParamX and also including a comment in the file that they are only examples and not tied to parameters yet.

[Arjupta]

@jcqli @vinitamurthi PTAL at the PlatformParameterModuleTest file. If this approach looks good to you then we can have more test cases.

[jcqli]

@jcqli @vinitamurthi PTAL at the PlatformParameterModuleTest file. If this approach looks good to you then we can have more test cases.

LGTM, recommend naming "FakePlatformParameterModule" to "TestPlatformParameterModule".

[rt4914]

LGTM. Approving only for code-owner file.

[jcqli]

LGTM!

[BenHenning]

@Arjupta a couple of things:

1. If this includes changes from another PR, please mention that it is blocked on that PR in the PR title
2. Please reassign this to me after the dependent PR is merged & this is brought up-to-date with develop (if needed--I suspect you don't actually need my codeowners approval after that happens since the only BUILD file changes in this PR seem to be included in the previous one that I approved)

[vinitamurthi]

@anandwana001 @jcqli , if the changes look good to you, can you please approve the PR? The merge is currently blocked on your reviews

[anandwana001]

Code owners file LGTM

[jcqli]

LGTM!

[oppiabot]

Hi @Arjupta, this PR is ready to be merged. Please address any remaining comments prior to merging, and feel free to merge this PR once the CI checks pass and you're happy with it. Thanks!