upload, build, download and update translations to/from Crowdin using js client api

[krmanik]

Pull Request template

Purpose / Description

Move to new Crowdln JS Client API

Fixes

Fixes #8274

Approach

1. Created a yarn project in tools/localization dir

mkdir ./tools/localization
cd ./tools/localization
mkdir src
yarn init

2. Installed required dependencies

yarn add @crowdin/crowdin-api-client

3. Created following files in src dir index.ts : For running functions defined below files upload.ts : For uploading English res/values files to Crowdin download.ts : For building/downloading English res/values files to Crowdin update.ts : For updating the files from extracted ankidroid.zip file

4. Updated sync_translations.yml file

5. Available commands, run from ./tools/localization upload : yarn start upload build and download : yarn start download extract ankidroid.zip : yarn start extract update : yarn start update

6. The script in ts files are line by line conversion of python code to typescript

How Has This Been Tested?

It is tested on local repository. (Needs to add tests dir with test script and sample xml files) Actions result https://github.com/krmanik/Anki-Android-Copy/runs/6754662446?check_suite_focus=true

i18n_sync branch https://github.com/krmanik/Anki-Android-Copy/compare/main...i18n_sync

(Change i18n_sync are all in English because not translated)

Learning (optional, can help others)

https://yarnpkg.com/cli/install https://www.typescriptlang.org/docs/ https://github.com/crowdin/crowdin-api-client-js https://support.crowdin.com/api/v2/#section/Introduction/Crowdin-API-Clients

Links to blog posts, patterns, libraries or addons used to solve this problem

Checklist

Please, go through these checks before submitting the PR.

• [x] You have not changed whitespace unnecessarily (it makes diffs hard to read)
• [x] You have a descriptive commit message with a short title (first line, max 50 chars).
• [x] Your code follows the style of the project (e.g. never omit braces in if statements)
• [x] You have commented your code, particularly in hard-to-understand areas
• [x] You have performed a self-review of your own code
• [ ] UI changes: include screenshots of all affected screens (in particular showing any new or changed strings)
• [ ] UI Changes: You have tested your change using the Google Accessibility Scanner

[Arthur-Milchior]

I'm looking at the API. It seems, if I understand correctly, that there is now a way to interact with screenshot too. If so, do you think you could, in another PR, maybe have a way to:

• download all screenshot (right now, I can't find a way to make a back-up. Which means if crowd-in stop, we lose hours of work)
• upload screenshot from PR; which will hopefully win a lot of time and avoid forgetting to do the work.

I'm still reviewing.

[david-allison]

We do have a way to download screenshots, let's move this to: https://github.com/ankidroid/Anki-Android/issues/8621.

I feel uploading screenshots is overengineering at this point - far too much that can go wrong with it for it to be useful

[krmanik]

I went through each review and updated it. If I missed any review let me know.

[krmanik]

Getting url undefined errors, may be 30 minutes wait.

$ node ./dist/index.js download
downloading...
Building ZIP on server...
Built.
Downloading Crowdin file
node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^
TypeError [ERR_INVALID_ARG_TYPE]: The "url" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)
    at Url.parse (node:url:168:3)
    at Object.urlParse [as parse] (node:url:155:13)
    at dispatchHttpRequest (/home/runner/work/Anki-Android-Copy/Anki-Android-Copy/tools/localization/node_modules/axios/lib/adapters/http.js:133:22)
    at new Promise (<anonymous>)
    at httpAdapter (/home/runner/work/Anki-Android-Copy/Anki-Android-Copy/tools/localization/node_modules/axios/lib/adapters/http.js:4[9](https://github.com/krmanik/Anki-Android-Copy/runs/6791577721?check_suite_focus=true#step:9:10):[10](https://github.com/krmanik/Anki-Android-Copy/runs/6791577721?check_suite_focus=true#step:9:11))
    at dispatchRequest (/home/runner/work/Anki-Android-Copy/Anki-Android-Copy/tools/localization/node_modules/axios/lib/core/dispatchRequest.js:58:10)
    at Axios.request (/home/runner/work/Anki-Android-Copy/Anki-Android-Copy/tools/localization/node_modules/axios/lib/core/Axios.js:109:[15](https://github.com/krmanik/Anki-Android-Copy/runs/6791577721?check_suite_focus=true#step:9:16))
    at wrap (/home/runner/work/Anki-Android-Copy/Anki-Android-Copy/tools/localization/node_modules/axios/lib/helpers/bind.js:9:15) {
  code: 'ERR_INVALID_ARG_TYPE'
}
error Command failed with exit code 1.

[krmanik]

I get following messages in Find broken strings in this actions results and actions run on macos. So it is ok?

Found grep errors but if you are not on macOS they are likely false positive. Ignoring
No issues found.
Exiting with status 0

Also first run gave following errors, after re-run it run successfully.

node ./dist/index.js download
downloading...
Building ZIP on server...
Built.
Downloading Crowdin file
node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^
TypeError [ERR_INVALID_ARG_TYPE]: The "url" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)

To-Do

• [x] prettier
• [x] eslint
• [x] node16

[mikehardy]

Found grep errors but if you are not on macOS they are likely false positive. Ignoring

:weary: - ugh, that whole script may be bogus now, I can't remember the last time it flagged anything, perhaps we are ignoring it completely in current execution as well so adding it has no real value? I'm not sure. If you look in there and discover either a) we're ignoring it full time in current i18n sync or b) it's worthless for some other reason, maybe just kill the script (don't use it / delete it)

If node16 is still not working, don't worry about it, bail and just use the "regular" (non-ESM) modules now

Building ZIP on server...
Built.
Downloading Crowdin file
node:internal/errors:465
    ErrorCaptureStackTrace(err);
    ^
TypeError [ERR_INVALID_ARG_TYPE]: The "url" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:372:5)
    at validateString (node:internal/validators:120:11)

That's worrisome. Does it happen every time ?

[krmanik]

I don't see anything in here in the code at the moment. Just some questions on the find broken translations script (maybe delete?) and the error you mentioned

I have removed find broken string from yml. I think script can be translated to TypeScript for finding broken string. The error occurs often I think some wait time need to add using timeout function. I am testing it. Now it needs to add test for comparing result with correct xml file and replacement using regex.

[krmanik]

I have used setInterval to wait for building the translations which takes approx. 1 min. Then download the built translations. The error now fixed. Now next is fixing the broken strings using typescript.

[krmanik]

I have used https://regexr.com/6spn2 to build regex for matching broken strings. It should be added more to regex for matching more broken strings.

[david-allison]

I have used https://regexr.com/6spn2 to build regex for matching broken strings. It should be added more to regex for matching more broken strings.

This class/documentation may be of use. It's originally converted from the C++ which handled 'valid' format string testing:

https://github.com/ankidroid/Anki-Android/blob/f40239ddadd0c57d053c3ce5fbe313f5c42b40b5/lint-rules/src/main/java/com/ichi2/anki/lint/utils/Aapt2Util.kt

tests:

https://github.com/ankidroid/Anki-Android/blob/f40239ddadd0c57d053c3ce5fbe313f5c42b40b5/lint-rules/src/test/java/com/ichi2/anki/lint/rules/NonPositionalFormatSubstitutionsTest.kt

[krmanik]

I think Aapt2Util.kt and NonPositionalFormatSubstitutionsTest.kt should be translated to TypeScript because regex approach may be not find errors, for e.g. %s %s. I will try to translate it to typescript. Thanks

[david-allison]

I think Aapt2Util.kt and NonPositionalFormatSubstitutionsTest.kt should be translated to TypeScript because regex approach may be not find errors, for e.g. %s %s. I will try to translate it to typescript. Thanks

I'd consider using https://cs.android.com/android/platform/superproject/+/master:frameworks/base/tools/aapt2/util/Util.cpp;drc=fbb7fe0da32896a871dd395845f1f510b075f8d5 as a base.

The C++ -> Java -> Kotlin probably wasn't the cleanest translation, and it may be a good idea to go to the original source

[krmanik]

I'd consider using https://cs.android.com/android/platform/superproject/+/master:frameworks/base/tools/aapt2/util/Util.cpp;drc=fbb7fe0da32896a871dd395845f1f510b075f8d5 as a base.

The C++ -> Java -> Kotlin probably wasn't the cleanest translation, and it may be a good idea to go to the original source

I will use the original source.

[david-allison]

@krmanik Ready for review? Will you want this squashed or rebased when you're done?

[krmanik]

It is not done yet, the check.ts file needs improvement. The check.ts file in tests dir fail on pluralMultiple and validStr. I am trying to fix it but not passing the tests.

[krmanik]

I have tried to implement the check.ts for finding broken strings but it is not passing for pluralMultiple .

const pluralMultiple = `<plurals name="reschedule_card_dialog_interval">
    <item quantity="one">Current interval: %d day</item>
    <item quantity="few">Keisti Kortelių mokymosi dieną</item>
    <item quantity="many">Current interval: %d days</item>
    <item quantity="other">Current interval: %d days</item>
</plurals>`

I am getting same result for the kotlin class mentioned above (on kotlin playground https://pl.kotl.in/07ZnNzCxV), and how the test class mark it as clean?

https://github.com/ankidroid/Anki-Android/blob/f40239ddadd0c57d053c3ce5fbe313f5c42b40b5/lint-rules/src/test/java/com/ichi2/anki/lint/rules/NonPositionalFormatSubstitutionsTest.kt#L173-L180

[krmanik]

The plurals will be implemented in next commit. More tests will be added also.

[mikehardy]

Scoped storage is done!! Note that the list of languages in lint_release needs an update when a language is added - just needs an addition to the comment in that area of the new implementation

[eginhard]

@krmanik @mikehardy It looks like the Crowdin API v1 will finally be shut down at the end of November (https://github.com/microsoft/pxt/issues/9539, https://github.com/PrestaShop/PrestaShop/issues/33655), so this is getting more urgent. Not sure what is left to be done for this?

[mikehardy]

Oh! Welll happy coincidence at least, I have time to finish getting it merged in now so we should make the deadline. I really appreciate the heads-up thank you!

[krmanik]

This PR is almost functional, I have not tested it yet. Thanks to @mikehardy for looking into this.

[krmanik]

@mikehardy should I continue this?

[mikehardy]

@krmanik if you have time, sure! We have a deadline here of the end of the month and the work is (as you mentioned) almost done. I have not had time to look into it yet, so any help is appreciated of course

[krmanik]

I am also out of time but I will complete this PR.

[mikehardy]

fixed fairly trivial merge conflict and pushed it back starting process of final review+testing+fixups, not 100% committing that I personally have time to do it but I think I will, and the deadline is now rapidly approaching 😬 😄

[mikehardy]

TODO list for me as I look at this

• [x] eliminate string checking from this PR / the new implementation
  • it appears that the previous string checking that happened on crowdin import is now handled well by default lint which is run in CI on any commit and is where these failures show up in current process, so it is not a regression
  • Current (python) implementation only works on certain operating systems so was not great to begin with
  • new typescript implementation is slow and is apparently just duplicating lint
• [x] verify this: the list of languages in lint_release needs an update when a language is added - just needs an addition to the comment in that area of the new implementation
• [x] use new personal access token generated for ankidroid org
• [x] use correct project id
• [x] verify download/extract/update generates minimal diff
• [x] verify we want to change ... to unicode char 8230
• [x] fix market title infinite append
• [x] verify upload generates zero diff for no changes, correct diff for new strings / string deletes

[mikehardy]

I'm busy rest of the day today but will merge this tomorrow morning my time (about 16hrs from now?) assuming no one sees anything dire

Then I can do the first few runs with it and make sure it is well behaved. Strings are up to date so we'll have small diffs to start...

[krmanik]

I have tested it locally and it is working as expected. I have added Readme file for the commands.

[mikehardy]

Fantastic, thanks again @krmanik - I'll do what I can on David's suggestion then merge this and do some careful runs of the i18n sync afterwards. There is always some tiny thing so I won't be surprised if there is some little thing on the yaml changes in the workflow but this is fantastic and I'm excited to get this in

[mikehardy]

Just to confirm: it was good to go before and is still good to go

prompted great additions though, much appreciated

[mikehardy]

Okay - now bear with me while I do the first couple runs.

I know that it will do some html-entitification on the first run as I've seen it locally (but not committed those changes to the language files, saving it for the run...) and I've verified it is actually what we want, so it has a nice first test case

I'm probably going to YOLO fixes direct on main to the YAML though until I get it working. I can't risk having the cloud state of crowdin (which is unbranchable) not tracking the local state of workflow runs here. Not my favorite thing to do to walk on a tightrope without a net but I believe it will work out...