Fix #52: Finalize release 0.11 #5010
Conversation
This commit: - Enables the language picker (being completed in #4762). - Bumps version codes (x2 since an extra re-release of 0.10 was necessary, and the updated version codes for that release weren't checked in).
These strings come from learner studies as a backup to logging events to Firebase. Local decoding is necessary to inspect and investigate the logs locally.
Relevant link: https://translatewiki.net/w/i.php?title=Special:ExportTranslations&language=pcm&group=oppia-android-app. This fixes some of the spacing issues that were introduced by during translation of the app strings.
This also adds support for Arabic in production builds (along with Nigerian Pidgin), and fixes some issues with the Swahili configuration. Nigeria was added as a new region for languages, including for production.
These changes aren't release blockers but they've been irking me for a while, so it's nice to get this output nicer. :)
Specifically: - Follow-up fixes to add Naija support to the new language selector. - Two post-merge nit fixes for the new language selector. - Fixed locale selection by adjusting the matching algorithm and also moving the pcm translations to be NG-specific. Some UI tests were removed that are particularly difficult to make pass, and don't add a lot of value.
This enables spotlights specifically for alpha builds (via a new alpha-only platform parameter module). This commit also adjusts spotlights' overlay background color since it's a bit too dark as-is. A new counter was added to count events during the app being open to help track for lost events between two events that are received.
Specifically: - Addressed failing static checks (including adding one test exemption for the new alpha-specific platform parameters module). - Fixed lint issues. - Added tests for the new debug event sequence number (and fixed existing tests for it). - Added some notes in languages.proto on creole languages. - Updated the profile selection logic in AndroidLocaleProfileFactory to more correctly prioritize Android language codes. This fixes language selection more in-line with the existing infrastructure than the previous solution, however it will require a large set of changes to AndroidLocaleProfileFactoryTest (which will happen in a follow-up commit). - Removed all temporary TODOs that are either addressed, or are being tracked elsewhere. There are still some items that need to be addressed yet. Note that Nigerian Pidgin has not yet been tested with content. Any fixes necessary for that will be brought in via a follow-up commit.
As part of this, a few small issues were fixed in AndroidLocaleFactory and its implementation was essentially recreated from the ground up to better leverage code reuse (and thus gain more confidence in test breadth since the actual contract of this factory is quite complex to test comprehensively).
This removes some no-longer-needed comments from AndroidLocaleFactory. It also updates the factory to be a bit more robust against missing region definitions (rather than trying to proceed which could result in an exception being thrown when it doesn't need to be). This also fixes InitializeDefaultLocaleRule incorrectly creating an empty region in cases when no region data is provided. This change fixes the StateFragmentLocalTest failures reported by CI.
For some reason, fixing these 3 tests as part of the 5 that were failing before is causing them to hang on Gradle (probably because they're actually working correctly now, though it's not clear why it's hanging vs. failing). Rather than investigating a potentially hard-to-track-down issue, this just disables the tests in Gradle since Gradle is going away soon, anyway, and the tests already run & pass with Bazel.
Specifically: - Disable Swahili again since it's not actually read for production launch. - Fix audio language localization during audio selection. - Fix a one-off crash that I noticed while changing languages and navigating. - Added a built-in recovery mode to the decode event string utility to ensure that even truncated strings can at least be partially recovered.
Specifically: - This adds integrity-checking data to the shared event string from the learner analytics admin screen to help with troubleshooting if data is lost. - This splits the learner study feature into 3 total (2 new) parameters: one for controlling whether to log the sensitive profile/installation IDs, one for the in-lesson fast language switcher, and one for the convenient top-level facilitator features for user studies (including the learner analytics screen). - Lint fixes for past changes. Note that alpha builds of the app will now have the learner analytics screen enabled by default (though the in-lesson language switcher and user extra ID logging will be off by default).
…lize-release-0.11
|
@seanlip @adhiamboperes PTAL for codeowners. |
There was a problem hiding this comment.
Thanks @BenHenning -- this looks amazing from a product standpoint.
FYI I am unable to see the spotlight videos due to high download latency (for some reason they're only coming in at 3.5 kb/s), but I imagine your changes should be fine and am not worried about that.
- EDIT: Finally downloaded the videos! I think both are actually too dark, in terms of background -- I can barely see the background on either. I think it's worth learners still being able to see the background because it would help them see where the spotlit element is in relation to the other elements, so I would suggest having the background be a bit lighter still.
One other thought: I imagine that translatewiki will almost certainly push to values-pcm as it has done previously (both #5009 and #5019). What would you like to happen in that case -- should we accept it? /cc @adhiamboperes
app/src/main/java/org/oppia/android/app/player/state/StateViewModel.kt
Outdated
Show resolved
Hide resolved
utility/src/main/java/org/oppia/android/util/platformparameter/PlatformParameterConstants.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Thanks @BenHenning! I do have a couple of questions but nothing blocking.
app/src/main/java/org/oppia/android/app/drawer/NavigationDrawerFragmentPresenter.kt
Show resolved
Hide resolved
app/src/main/java/org/oppia/android/app/translation/AppLanguageResourceHandler.kt
Show resolved
Hide resolved
domain/src/test/java/org/oppia/android/domain/locale/LanguageConfigRetrieverProductionTest.kt
Show resolved
Hide resolved
domain/src/test/java/org/oppia/android/domain/locale/LanguageConfigRetrieverTest.kt
Outdated
Show resolved
Hide resolved
|
Unassigning @adhiamboperes since they have already approved the PR. |
This goes from an 87% blend to black to a 78%.
|
Thanks @seanlip & @adhiamboperes. I addressed all follow-up comments. @adhiamboperes could you PTAL at changes & address resolved comments from both you and Sean (since Sean is probably unavailable at the moment)?
The background comes across lighter on a device than in the video, but I lowered the darkness by another 9%. See comparison:
Regarding Translatewiki, I think we might be stuck there temporarily (as in, merging those PRs will result in translation duplications that are ignored by the build). We should start with asking the Translatewiki team if it's possible for us to configure the export to ensure Nigerian Pidgin translations go to values-pcm-rNG instead of just values-pcm. If that's not an option, we're going to have to look into automation to fix the location of the files during build time (which isn't too bad, but it's a bit hacky so it's not preferred). I won't have time to look into this before I'm gone. |
|
Unassigning @BenHenning since a re-review was requested. @BenHenning, please make sure you have addressed all review comments. Thanks! |
Re: Spotlight background, I think the latest version has acceptable visibility. |
There was a problem hiding this comment.
All the latest changes look good, both mine and @seanlip's comments have been addressed.
|
Everything looks great. Thanks! @BenHenning Noted on your comment re translatewiki. Will send them an email. |
|
Thans @adhiamboperes & @seanlip! |
Explanation
Fixes #52
This PR finishes up a bunch of separate tasks relating to the 0.11 release of the Oppia Android app.
Release plans
Note that a lot of the tasks addressed were completed out-of-band (hence the lack of tracking issues). These are needed to prepare for the following release changes:
Overview
Language support changes
AndroidLocaleFactorywas largely rebuilt to make better use of code sharing, but its actual functionality needed to change due to the Naija support problems mentioned below.New feature: spotlights (alpha-only)
New feature: in-app language selector (all users)
Event system changes
Infrastructure changes
Improved support for future user studies
Nigerian Pidgin support issues
The Android system does not formally support Nigerian Pidgin (Naija) despite the fact that it has a recognized ISO 639-3 language code.
Naija is a creole language which means it's mostly a derivative of English with vernacular adaptations (such as recreated words and grammar alterations). I think that because it's mostly a derivative language and is mainly used for casual speaking (vs. formal communications), there isn't as much of a desire on the Android side to formally support it as a UI language.
Fortunately, the Oppia Android app's localization system already supports forcing the system to rendering app strings that are not natively supported. However, this results in several considerations and has turned up one new issue that needed addressing:
values-pcm-rNG(for Nigeria locking). This doesn't stop the language from being used outside Nigeria, it's just a limitation in Android's resource qualifier system when working with custom languages.AndroidLocaleFactory.DecodeUserStudyEventStringutilityA new utility was introduced to decode the compressed Base64 string of events logs that can be exported from devices being used for user studies (i.e. whose admins have access to the learner analytics screen). These logs represent the entirety of both pending and uploaded logs since the introduction of the feature (but only for devices that have the learner study enabled).
The utility reads an input file containing a single instance of the string (and automatically strips newline and horizontal space formatting since the output from the app includes line-wrapping) and outputs it to one of three indicated formats: textproto, JSON, or Yaml.
The script can be run in a local terminal from this branch (or
developonce it's checked in) at the repository root by running:(where 'input.log' exists in the local repository root, and 'output.json' will be written to the local repository root).
For use outside of Bazel, one can use a pre-built deployment Jar file by running:
(Note that the error output for this command will be assuming Bazel is being used, so some adjustment may be needed to interpret CLI errors when using the deploy Jar).
Third-party dependency updates
Some new dependencies were needed for the new
DecodeUserStudyEventStringutility to make the implementation much simpler:This required an update to maven_install.json, but no changes are needed in tracked Maven dependency licenses since these are not dependencies that are deployed with the app to end users.
The above do result in an incidental update to Gson 2.8.6 (from the current 2.8.5 used). I don't expect that this actually will affect the release much since Gson isn't used broadly, and this is a minor update to Gson.
Exemptions & test notes
Two new files were exempted for tests:
PlatformParameterAlphaModule: while we can test modules, I'm not actually keen on testing the platform parameter modules at this time because staging is generally messy at the moment with a lot of duplication across multiple modules, so the test complexity isn't small. A lot of the platform parameter system needs to be streamlined, but that's largely dependent on Add support for Dagger Hilt [Blocked: #59] #1720 being finished.DecodeUserStudyEventStringwhile not particularly difficult to test, this is a special local analytics tool that will rarely be used and is very unlikely to break. Since I'm strapped for time on this PR, I'm forgoing adding tests for this utility.Regarding broad testing coverage, everything has had corresponding testing additions or changes where applicable. This PR might not have quite as much testing thoroughness as other PRs, but it does largely cover the core scenarios.
Essential Checklist
For UI-specific PRs only
Spotlights changes before/after:
Video demonstrating the new functionality in the app, including alpha access to the learner analytics screen and support for both Nigerian Pidgin and Arabic (and via the in-app language selector):
oppia_overview_new_features.mp4