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

Beta version of the app should only include fully available languages #4628

Closed
BenHenning opened this issue Sep 28, 2022 · 1 comment · Fixed by #4687
Closed

Beta version of the app should only include fully available languages #4628

BenHenning opened this issue Sep 28, 2022 · 1 comment · Fixed by #4687
Assignees

Comments

@BenHenning
Copy link
Sponsor Member

App & content strings must only be available in the fully translated languages (English & Brazilian Portuguese), but only for beta.

@BenHenning BenHenning self-assigned this Sep 28, 2022
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 |
![image](https://user-images.githubusercontent.com/12983742/200829756-bbae1ab4-c40f-4b96-8404-eb12fdecb418.png)
|
![image](https://user-images.githubusercontent.com/12983742/200831324-0b03a8c5-654d-41b5-bbfd-206a5a4b143f.png)
|
| Brazilian Portuguese |
![image](https://user-images.githubusercontent.com/12983742/200829980-7025ce6b-5861-4a61-aff1-86b4c66fe774.png)
|
![image](https://user-images.githubusercontent.com/12983742/200831480-73ba6d73-2685-4dab-895d-ed915bcb6f65.png)
|
| Swahili |
![image](https://user-images.githubusercontent.com/12983742/200830150-7b3c5568-deaa-46af-83b2-e3da813e984a.png)
|
![image](https://user-images.githubusercontent.com/12983742/200831722-d5cc7ebf-1521-412a-947b-f2d9dc899957.png)
|

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).
@BenHenning
Copy link
Sponsor Member Author

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
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant