Skip to content
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

Remove Kenya user study specific constructs once study is over #4419

Closed
BenHenning opened this issue Jul 3, 2022 · 0 comments · Fixed by #5417
Closed

Remove Kenya user study specific constructs once study is over #4419

BenHenning opened this issue Jul 3, 2022 · 0 comments · Fixed by #5417
Assignees
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.

Comments

@BenHenning
Copy link
Member

BenHenning commented Jul 3, 2022

See TODO locations in code for this issue for places that need cleanup.

@Broppia Broppia added issue_type_infrastructure Impact: Low Low perceived user impact (e.g. edge cases). labels Jul 6, 2022
BenHenning added a commit that referenced this issue Aug 19, 2022
## Explanation
Fixes #4410

This PR builds on #4414 by introducing a new alpha component that's specific to the team's ongoing research project in Kenya. This build serves three benefits:
1. It allows us to ensure end users can easily distinguish between alpha & study-specific alpha flavors of the app (since they will now have different build flavors).
2. It ensures we can explicitly version the binaries such that the study binary always takes precedence on Play Store.
3. It allows us to define study-specific parameters without needing manual changes during release (such as automatically enabling the learner study feature, which this PR is doing for the new build flavor).

A few important things to note:
- There are no new tests for the new components since they are mostly uninteresting to test (similar to other build flavor components).
- The new components are temporary are will be reoved (#4419 is tracking this).
- This PR is a recreation of #4420 since the new build flavor was being originally being introduced after the new beta & GA flavors (but this is no longer the case due to the beta release being delayed).
- The new build flavor is not being added to CI build tests mainly because it's unlikely for this build to break versus the alpha build (as the two are so similar), and because of the temporary nature of this version of the app.
- The new classes needed to be exempted for Gradle builds since Gradle was having difficulty with the fact that the new study-specific alpha component was a separate Dagger graph (our Gradle configuration, unlike Bazel, can't manage multiple Dagger graphs in the same build tree). This seems like a reasonable thing to do since the study-specific version of the app can't be built on Gradle, anyway.
This 
## Essential Checklist
- [x] 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: ...".)
- [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation.
- [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
This PR has no user-facing side effects except the fact that the version name is different now for the study-specific version of the app (which doesn't seem particularly important to show a video/screenshot for).

Commits:

* Create dedicated alpha application component.

This simplifies application component management significantly and
allows individual build flavors to have their own unique module lists.

* Fix broken test per earlier changes.

* Add Kenya study-specific alpha build flavor.

This new build flavor automatically enables the study-specific learner
study feature flag.

Cherry-pick of eef2f4c from the
original #4420.

* Update build_flavors.bzl

Remove beta version declaration since that doesn't belong in this branch.

* Post-CP fixes.

* Fix Gradle builds.

* Update TransformAndroidManifestTest.kt

Correct typos.
BenHenning added a commit that referenced this issue Sep 8, 2022
…#4421)

## Explanation
Fixes #4389
Fixes #4407

This PR "future-proofs" the learner analytics logging infrastructure by:
- Introducing separate event names that should be more understandable to data analysts than used in the Kenya version of the app (though compatibility is still kept with existing Kenya logged metrics by means of a separate implementation used only in that build of the app).
- Logging a new ``event_type`` parameter that contains the actual proto integer of the event rather than just logging a string representation (so that we can rename both the events in code, and the corresponding strings logged to Firebase). The event names, at this point, act as a nice point-of-reference while analyzing events but are no longer meant to be the primary identifying keys for events (``event_type`` should be used for this purpose, instead).
- The device's Android SDK version, the app's version name, and the app's version code are now logged with all events for better event dimension slicing and analysis.
- Effort is now made to ensure event names don't exceed 40 characters (Firebase's limit) to help avoid events being potentially rejected during upload.

Specifics about tests and exemptions:
- ``EventTypeToHumanReadableNameConverter`` is exempted from tests since it's an interface.
- ``KenyaAlphaEventTypeToHumanReadableNameConverterImpl`` is exempted from tests since it's temporary, and its values are at least indirectly tested through ``KenyaAlphaEventBundleCreatorTest``.
- ``KenyaAlphaEventBundleCreatorTest`` was added as a fork of ``EventBundleCreatorTest`` to ensure the existing Kenya-specific event names are properly logged, but it will be removed in the future (as tracked by #4419). Note that this suite wasn't updated to verify the new fields like ``EventBundleCreatorTest`` was.
- A BUILD.bazel file was added for the existing logging/firebase and logging/performancemetrics tests since one was added for the tests under logging/ (and subpackages must also have BUILD files if parent packages do).

## Essential Checklist
- [x] 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: ...".)
- [x] Any changes to [scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets) files have their rationale included in the PR explanation.
- [x] The PR follows the [style guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android Studio ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers ([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
N/A -- This is a metrics infrastructure change that will have zero effect on the UI (except maybe how events are displayed in the developer options menu, but that's not important to include here since it isn't end user-facing).

Commits:

* Create dedicated alpha application component.

This simplifies application component management significantly and
allows individual build flavors to have their own unique module lists.

* Add beta & GA update notices.

This also introduces dedicated beta & GA build flavors which is a
necessary prerequisite.

It also introduces an extra beta, alpha, and dev mode labels for the splash
screen (the latter 2 were extra) with 2 second minimum wait timers for
beta and alpha to ensure they are seen. A 5-second safety timer was
added to ensure the splash screen can always be passed even if something
goes wrong at the domain level (since there are now quite a few moving
pieces to determine the user's current onboarding state).

* Add build tests for the new beta & GA flavors.

* Add Kenya study-specific alpha build flavor.

This new build flavor automatically enables the study-specific learner
study feature flag.

* Future-proof metrics infrastructure.

This is mainly done by:
1. Logging an immutable event type for each event and de-coupling the
event names so that the latter can be changed over time to improve event
management quality-of-life.

2. Logging the Android SDK version, app version name, and app version
code with each event.

3. Ensuring the existing Kenya-specific alpha versions of the app
continue to use their existing event names so that historical data can
be matched (since those queries rely on event names rather than this new
event type).

* Fix broken test per earlier changes.

* Fix general broken tests & builds.

Tests broken due to changes to the app startup experience haven't yet
been fixed.

* Lint fixes.

* First part of adding tests for GA notices.

There's a bunch left to do here, this is mainly needed so that I can
transfer changes to a different machine.

* Update TransformAndroidManifestTest.kt

Correct typos.

* Fix tests & static checks.

This also removes temporary debug code and TODOs, and finishes the tests
for SplashActivity.

* Fix & add testing.

* Static check fixes + missing tests.

* Post-merge fixes.

* Test fixes.

* Fix failing tests.

* Fix Gradle test.

* Post-merge fixes.

* Follow-up adjustments after self-review.
@BenHenning BenHenning added Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Z-ibt Temporary label for Ben to keep track of issues he's triaged. issue_user_developer labels Sep 15, 2022
@seanlip seanlip added enhancement End user-perceivable enhancements. bug End user-perceivable behaviors which are not desirable. and removed issue_type_infrastructure enhancement End user-perceivable enhancements. labels Mar 28, 2023
@MohitGupta121 MohitGupta121 added the Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. label Jun 16, 2023
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue May 27, 2024
Remove application\alphakenya package
Remove codes in bazel files,test_file_exemptions.textproto that is related with alphakenya
Remove AlphaKenya related modules and corresponding tests
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jun 18, 2024
…ppia#4419

� Conflicts:
�	app/src/main/java/org/oppia/android/app/application/alphakenya/AlphaKenyaApplicationComponent.kt
�	app/src/main/java/org/oppia/android/app/application/alphakenya/BUILD.bazel
�	domain/src/main/java/org/oppia/android/domain/platformparameter/PlatformParameterAlphaKenyaModule.kt
�	utility/src/main/java/org/oppia/android/util/logging/BUILD.bazel
�	utility/src/main/java/org/oppia/android/util/logging/KenyaAlphaEventTypeToHumanReadableNameConverterImpl.kt
�	utility/src/test/java/org/oppia/android/util/logging/BUILD.bazel
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jun 30, 2024
…ppia#4419

� Conflicts:
�	scripts/assets/test_file_exemptions.textproto
�	version.bzl
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Jul 1, 2024
…ppia#4419

� Conflicts:
�	utility/src/main/java/org/oppia/android/util/logging/KenyaAlphaEventTypeToHumanReadableNameConverterImpl.kt
�	utility/src/test/java/org/oppia/android/util/logging/KenyaAlphaEventBundleCreatorTest.kt
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Aug 19, 2024
…ppia#4419

� Conflicts:
�	utility/src/main/java/org/oppia/android/util/logging/KenyaAlphaEventTypeToHumanReadableNameConverterImpl.kt
�	utility/src/test/java/org/oppia/android/util/logging/KenyaAlphaEventBundleCreatorTest.kt
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Aug 19, 2024
…ppia#4419

� Conflicts:
�	scripts/assets/test_file_exemptions.textproto
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Aug 27, 2024
@BenHenning BenHenning added this to the 1.0 Global availability milestone Aug 29, 2024
adhiamboperes added a commit to XichengSpencer/oppia-android that referenced this issue Sep 2, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Sep 20, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Sep 20, 2024
adhiamboperes added a commit to XichengSpencer/oppia-android that referenced this issue Oct 4, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Oct 25, 2024
# Conflicts:
#	utility/src/main/java/org/oppia/android/util/logging/EventTypeToHumanReadableNameConverter.kt
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Oct 25, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Oct 29, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Oct 30, 2024
XichengSpencer added a commit to XichengSpencer/oppia-android that referenced this issue Oct 30, 2024
BenHenning added a commit to XichengSpencer/oppia-android that referenced this issue Nov 6, 2024
BenHenning added a commit that referenced this issue Nov 6, 2024
…es (#5417)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix #4419 
Removed application\alphakenya package as Adhiambo suggests.
Removed codes in bazel files,test_file_exemptions.textproto that is
related with alphakenya
Remove AlphaKenya related modules under android/util/logging including
KenyaAlphaPlatformParameterModule, EventLoggingModule etc. and
corresponding tests and Bazel configs
Inlined StandardEventTypeToHumanReadableNameConverterImpl.kt into
EventTypeToHumanReadableNameConverter.kt
Removed EventLoggingConfigurationModule
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
subhajitxyz pushed a commit to subhajitxyz/oppia-android that referenced this issue Nov 19, 2024
…d Files (oppia#5417)

<!-- READ ME FIRST: Please fill in the explanation section below and
check off every point from the Essential Checklist! -->
## Explanation
Fix oppia#4419 
Removed application\alphakenya package as Adhiambo suggests.
Removed codes in bazel files,test_file_exemptions.textproto that is
related with alphakenya
Remove AlphaKenya related modules under android/util/logging including
KenyaAlphaPlatformParameterModule, EventLoggingModule etc. and
corresponding tests and Bazel configs
Inlined StandardEventTypeToHumanReadableNameConverterImpl.kt into
EventTypeToHumanReadableNameConverter.kt
Removed EventLoggingConfigurationModule
<!--
- Explain what your PR does. If this PR fixes an existing bug, please
include
- "Fixes #bugnum:" in the explanation so that GitHub can auto-close the
issue
  - when this PR is merged.
  -->

## Essential Checklist
<!-- Please tick the relevant boxes by putting an "x" in them. -->
- [x] 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: ...".)
- [x] Any changes to
[scripts/assets](https://github.com/oppia/oppia-android/tree/develop/scripts/assets)
files have their rationale included in the PR explanation.
- [x] The PR follows the [style
guide](https://github.com/oppia/oppia-android/wiki/Coding-style-guide).
- [x] The PR does not contain any unnecessary code changes from Android
Studio
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#undo-unnecessary-changes)).
- [x] The PR is made from a branch that's **not** called "develop" and
is up-to-date with "develop".
- [x] The PR is **assigned** to the appropriate reviewers
([reference](https://github.com/oppia/oppia-android/wiki/Guidance-on-submitting-a-PR#clarification-regarding-assignees-and-reviewers-section)).

## For UI-specific PRs only
<!-- Delete these section if this PR does not include UI-related
changes. -->
If your PR includes UI-related changes, then:
- Add screenshots for portrait/landscape for both a tablet & phone of
the before & after UI changes
- For the screenshots above, include both English and pseudo-localized
(RTL) screenshots (see [RTL
guide](https://github.com/oppia/oppia-android/wiki/RTL-Guidelines))
- Add a video showing the full UX flow with a screen reader enabled (see
[accessibility
guide](https://github.com/oppia/oppia-android/wiki/Accessibility-A11y-Guide))
- For PRs introducing new UI elements or color changes, both light and
dark mode screenshots must be included
- Add a screenshot demonstrating that you ran affected Espresso tests
locally & that they're passing

---------

Co-authored-by: Adhiambo Peres <[email protected]>
Co-authored-by: Ben Henning <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug End user-perceivable behaviors which are not desirable. Impact: Low Low perceived user impact (e.g. edge cases). Issue: Needs Clarification Indicates that an issue needs more detail in order to be able to be acted upon. Work: Medium The means to find the solution is clear, but it isn't at good-first-issue level yet. Z-ibt Temporary label for Ben to keep track of issues he's triaged.
Development

Successfully merging a pull request may close this issue.

5 participants