Beta version of the app should only include fully available languages #4628
Assignees
Milestone
Comments
BenHenning
added this to the 0.10 Release Blockers (Beta "Early Access" MR2/Alpha MR6) milestone
6 tasks
BenHenning
added a commit
that referenced
this issue
Nov 11, 2022
) ## Explanation Fixes #4628 Fixes #4667 The core fix for #4628 is splitting the language configuration between "all languages the app supports" and "production-ready languages." This is now configurable per-binary where all pre-beta binaries include all languages, and the beta & GA binaries only include production-ready languages (those are, languages for which we always aim to have 100% content translations). Tests use the "all languages" configuration, by default, for parity with the developer version of the app. This required a small change where we define the language proto as part of the shared ``//model`` module rather than as a separate package. As was before, this proto is only loaded in Bazel builds of the app since our Gradle pipeline doesn't support automatically textproto->binary proto conversion during build time. To ensure that the app strings are filtered out, the supported languages configuration proto that's included in the app is passed through a new script that strips out all resources (including non-strings) from the binary. This has actually removed ~400KiB even from the pre-release binaries since our third-party assets include a bunch of strings outside our core languages. New output now shows when building the app with Bazel to make it clearer what sorts of resources are being dropped due to incompatible languages: ``` 82 resources are being removed that are tied to unsupported languages: [af, am, as, az, be, bg, bn, bs, ca, cs, da, de, el, en-AU, en-CA, en-GB, en-IN, en-XC, es, es-419, es-US, et, eu, fa, fi, fr, fr-CA, gl, gu, hr, hu, hy, in, is, it, iw, ja, ka, kk, km, kn, ko, ky, lo, lt, lv, mk, ml, mn, mr, ms, my, nb, ne, nl, or, pa, pl, pt, pt-PT, ro, ru, si, sk, sl, sq, sr, sr-Latn, sv, ta, te, th, tl, tr, uk, ur, uz, vi, zh-CN, zh-HK, zh-TW, zu] (size reduction: 391239 bytes). ``` (When building oppia_dev) ``` 85 resources are being removed that are tied to unsupported languages: [af, am, ar, as, az, be, bg, bn, bs, ca, cs, da, de, el, en-AU, en-CA, en-GB, en-IN, en-XC, es, es-419, es-US, et, eu, fa, fi, fr, fr-CA, gl, gu, hi, hr, hu, hy, in, is, it, iw, ja, ka, kk, km, kn, ko, ky, lo, lt, lv, mk, ml, mn, mr, ms, my, nb, ne, nl, or, pa, pl, pt, pt-PT, ro, ru, si, sk, sl, sq, sr, sr-Latn, sv, sw, ta, te, th, tl, tr, uk, ur, uz, vi, zh-CN, zh-HK, zh-TW, zu] (size reduction: 351087 bytes). ``` (When building oppia_beta) #4667 is included in this PR despite it being an asset downloader-only fix out of convenience & since it's a related issue. The problem in the downloader was that translations were being incorrectly copied over to the new proto structure for lessons (which isn't used in the app yet, but is used for image downloading) which led to non-English images not being scraped from content HTMLs and thus not downloaded and included in the embedded assets for releases. We'll be adding a manual QA test to make sure that non-English content images load correctly. ## 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 Screenshots showing how English, Brazilian Portuguese, and Swahili are handled with the changes in this PR (between developer and beta builds of the app): | | Dev flavor -- with PR changes | Beta flavor -- with PR changes | |----------------------|-----------------------------------------------------------------------------------------------------------------|-----------------------------------------------------------------------------------------------------------------| | English |  |  | | Brazilian Portuguese |  |  | | Swahili |  |  | As can be seen from the table, all three languages are fully supported except when using beta with the changes introduced by this PR (where Swahili is dropped since it's not yet a 100% supported, production-ready language for the app). Note that in the case of Swahili, it falls back to English simply because that's how my device was configured at the time (it will fall back to whatever the system-secondary language is set to). Note that other typical screenshots/tests are not included in this PR, including for Espresso. From a high-level perspective, the main difference in this PR is which app and content strings are ultimately included and supported within the app. Given this is inclusion/exclusion, the screenshots above + manual verification via e2e tests are sufficient. Beyond that, properly testing these changes would require e2e tests that can use the production assets (which current Espresso tests don't have access to).
|
I've verified that this is the case in the latest MR2 (only English and Brazilian Portuguese are available in the beta version of the app). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
App & content strings must only be available in the fully translated languages (English & Brazilian Portuguese), but only for beta.
The text was updated successfully, but these errors were encountered: