Support for Persian (fa) language

CodeWithEmad commented 3 months ago

Bug description

Hi. I wanted to start translating for the Persian language on the project. As mentioned here, I created the language directory, added translations, built it for the frontend, and added it to superset_configs.py but frontend crashes when changing the language to Persian.

How to reproduce the bug

I've created fa directory using: pybabel init -i superset/translations/messages.pot -d superset/translations -l fa and started translating some strings. then I build-translations for the forntend and then, added

LANGUAGES = {
    "en": {"flag": "us", "name": "English"},
    "fr": {"flag": "fr", "name": "French"},
    "fa": {"flag": "ir", "name": "Persian"},
}

inside superset_config.py and as I expected, I saw the English, French, and Persian languages, and English and French work fine, but after clicking on the Persian language, the frontend raises an error and the top bar doesn't render. This the error in in console

Uncaught                                                                         jed.js:107
TypeError: Cannot read properties of null (reading 'domain')

Screenshots/recordings

https://github.com/apache/superset/assets/49523567/f2a0395e-4d69-4e03-a79f-da55884a5d8b

Superset version

master / latest-dev

Python version

3.10

Node version

18 or greater

Browser

Chrome

Additional context

I've checked the superset logs. nothing weird is happening in the logs.

superset_worker_beat  | [2024-07-07 16:55:00,038: INFO/MainProcess] Scheduler: Sending due task reports.scheduler (reports.scheduler)
superset_worker       | [2024-07-07 16:55:00,041: INFO/MainProcess] Task reports.scheduler[3b8cfbed-5d9f-4b01-997a-84e5cb1ae2b3] received
superset_worker       | [2024-07-07 16:55:00,042: DEBUG/ForkPoolWorker-1] [stats_logger] (incr) reports.scheduler
superset_worker       | [2024-07-07 16:55:00,068: INFO/ForkPoolWorker-1] Task reports.scheduler[3b8cfbed-5d9f-4b01-997a-84e5cb1ae2b3] succeeded in 0.026297561002138536s: None
superset_app          | 172.23.0.1 - - [07/Jul/2024:16:55:03 +0000] "GET /lang/fa HTTP/1.1" 302 189 "http://localhost:8088/login/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
superset_app          | 172.23.0.1 - - [07/Jul/2024:16:55:03 +0000] "GET / HTTP/1.1" 302 223 "http://localhost:8088/login/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
superset_app          | 2024-07-07 16:55:03,275:WARNING:root:Class 'werkzeug.local.LocalProxy' is not mapped
superset_app          | 2024-07-07 16:55:03,275:DEBUG:superset.stats_logger:[stats_logger] (incr) welcome
superset_app          | 172.23.0.1 - - [07/Jul/2024:16:55:03 +0000] "GET /superset/welcome/ HTTP/1.1" 302 201 "http://localhost:8088/login/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"
superset_app          | 2024-07-07 16:55:03,322:WARNING:superset.db_engine_specs:Unable to load SQLAlchemy dialect gsheets: No module named 'google'
superset_app          | 172.23.0.1 - - [07/Jul/2024:16:55:03 +0000] "GET /login/ HTTP/1.1" 200 4002 "http://localhost:8088/login/" "Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36"

Checklist

[X] I have searched Superset docs and Slack and didn't find a solution to my problem.
[X] I have searched the GitHub issue tracker and didn't find a similar bug report.
[X] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

dosubot[bot] commented 3 months ago

Hey, @CodeWithEmad! I'm here to assist you with your issue. Let's work together to figure this out! 🤖

The error TypeError: Cannot read properties of null (reading 'domain') typically occurs when the language pack JSON file is missing or not properly loaded, or if the structure of the language pack JSON file is incorrect or incomplete.

Here are the steps to ensure your Persian language pack is correctly set up:

Check the Structure of the Language Pack: Ensure that the fa language pack JSON file includes the domain property and follows the correct structure. Here is an example of the correct structure:

{
 "domain": "superset",
 "locale_data": {
   "superset": {
     "": {
       "domain": "superset",
       "lang": "fa",
       "plural_forms": "nplurals=2; plural=(n != 1);"
     },
     "Hello": ["سلام"],
     "You have %d message": ["شما %d پیام دارید", "شما %d پیام دارید"]
   }
 }
}

Update the Locale Type: Ensure that the Locale type in the frontend code includes the new language code. Update the Locale type as follows:

export type Locale =
 | 'de'
 | 'en'
 | 'es'
 | 'fr'
 | 'it'
 | 'ja'
 | 'ko'
 | 'pt'
 | 'pt_BR'
 | 'ru'
 | 'zh'
 | 'fa'; // added Persian to supported locales in Superset

Add the Language Pack to the Frontend: Ensure that the Persian language pack is properly added to the frontend. You can do this by adding the language pack JSON file to the appropriate directory and ensuring it is loaded correctly.

Enable Language Selection: Ensure that the LANGUAGES variable in your superset_config.py is correctly set up:

LANGUAGES = {
   "en": {"flag": "us", "name": "English"},
   "fr": {"flag": "fr", "name": "French"},
   "fa": {"flag": "ir", "name": "Persian"},
}

Build and Compile Translations: After making the necessary changes, rebuild and compile the translations:
```
npm run build-translation
pybabel compile -d superset/translations
```

By following these steps, you should be able to resolve the TypeError: Cannot read properties of null (reading 'domain') error and successfully add Persian language support to the Superset frontend ^[1]^[2]^[3].

To continue talking to Dosu, mention @dosu.

CodeWithEmad commented 3 months ago

This is so cool, @dosu. the messages.json has the appropriate structure:

{
  "domain": "superset",
  "locale_data": {
    "superset": {
      "22": [""],
      "": {
        "domain": "superset",
        "plural_forms": "nplurals=2; plural=(n==0 || n==1);",
        "lang": "fa"
      },

No, I didn't have fa in my Locale because nowhere in the docs mentioned this step, so we need to update the docs for translations. Even having 'fa` in my Locale and running npm ci && npm run build didn't do the trick and I still get the same error.

dosubot[bot] commented 3 months ago

To resolve the issue, ensure the following steps are correctly implemented:

Verify Language Pack Structure: Your messages.json structure looks mostly correct, but ensure it includes translations for the keys. For example:

{
  "domain": "superset",
  "locale_data": {
    "superset": {
      "22": ["translation for key 22"],
      "": {
        "domain": "superset",
        "plural_forms": "nplurals=2; plural=(n != 1);",
        "lang": "fa"
      }
    }
  }
}

Update Locale Type: Ensure fa is added to the Locale type in the frontend code:

export type Locale =
  | 'de'
  | 'en'
  | 'es'
  | 'fr'
  | 'it'
  | 'ja'
  | 'ko'
  | 'pt'
  | 'pt_BR'
  | 'ru'
  | 'zh'
  | 'fa'; // added Persian to supported locales in Superset

Rebuild Translations: After making these changes, rebuild the translations:
```
npm run build-translation
pybabel compile -d superset/translations
```

Check Configuration: Ensure the LANGUAGES variable in superset_config.py is correctly set:

LANGUAGES = {
    "en": {"flag": "us", "name": "English"},
    "fr": {"flag": "fr", "name": "French"},
    "fa": {"flag": "ir", "name": "Persian"},
}

Frontend Configuration: Ensure the frontend is correctly configured to load the language pack. The warning in your console suggests that configure(...) should be called before other methods. Ensure the Translator is properly initialized before using it.

By following these steps, you should be able to resolve the TypeError: Cannot read properties of null (reading 'domain') error and successfully add Persian language support to the Superset frontend ^[1]^[2].

To continue talking to Dosu, mention @dosu.

CodeWithEmad commented 3 months ago

didn't solve my issue.

CodeWithEmad commented 3 months ago

Sorry to bother you here @rusackas. I followed the documentation but encountered this error. Did I miss a step? Maybe there's a limitation for some languages? I should mention that Persian is a right-to-left (RTL) language, but I just wanted to see the changes in the UI.

rusackas commented 3 months ago

All good. As you've surmised, these are indeed the best/current instructions. As for possibly missing a step here, did you do these two as well?

I wonder if you can compare the resulting .json files with other languages, and see if there are any differences. Or even run it through GPT/Claude/AI and have it look for anything wrong. You might also open the Persian .po file using Poedit and see if there are any warnings/errors visible there.

If you're getting any visible errors, those might be worth sharing here as well.

CodeWithEmad commented 3 months ago

Thanks for the response, @rusackas .

did you do these two as well?

Yes, both .json and .mo files are created.

You might also open the Persian .po file using Poedit and see if there are any warnings/errors visible there.

I'm using PoEdit. No warnings/errors on save. (I haven't translated everything yet. can it be that?)

I compared jsons for fa and fr. The only difference is in the translated strings.

Something is not right here. in the login screen, there's this message: "Enter your login and password below:", but I can't find it anywhere in the codebase. neither it's French translations: "Entrez votre login et mot de passe:"

Update I deleted everything and did all the steps as mentioned. the result was the same.

Update 2 I found another issue. since jinja2 is not mentioned in superset/translations/requirements.txt, it will not be installed and someone like me only wants to update translations and follow the steps mentioned here, will encounter this error running ./scripts/translations/babel_update.sh:

Traceback (most recent call last):
  File "/tmp/superset/.venv/bin/pybabel", line 8, in <module>
    sys.exit(main())
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/frontend.py", line 929, in main
    return CommandLineInterface().run(sys.argv)
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/frontend.py", line 853, in run
    return cmdinst.run()
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/frontend.py", line 481, in run
    for filename, lineno, message, comments, context in extracted:
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/extract.py", line 149, in extract_from_dir
    for message_tuple in check_and_call_extract_file(
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/extract.py", line 207, in check_and_call_extract_file
    for message_tuple in extract_from_file(
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/extract.py", line 243, in extract_from_file
    return list(extract(method, fileobj, keywords, comment_tags,
  File "/tmp/superset/.venv/lib/python3.10/site-packages/babel/messages/extract.py", line 320, in extract
    raise ValueError('Unknown extraction method %r' % method)
ValueError: Unknown extraction method 'jinja2'

I'll create #29548 for this issue

rusackas commented 3 months ago

Thanks for the dependency PR!

I'm not sure about the strings for the login scree, but I would guess those are implemented in Flask-AppBuilder (FAB) and just don't have the _() translation wrapper around them.

Otherwise, it's not yet clear to me what the PR is. If you want to open a draft PR, maybe we can see what (if anything) fails when I pull/run it.

CodeWithEmad commented 3 months ago

Sorry, I shared a lot of details in one comment!

About the Persian translations and Jed issue, I found no workarounds yet. I did everything step by step, but no luck. If it's not too much to ask, can you confirm that following the steps works for any language (like fa)?

Following the "Creating new language directory" I found another issue. The script fails in a fresh virtual environment where Jinja2 is not installed. I've added it in #29548

rusackas commented 3 months ago

Thanks for the (now merged) PR, that helps a bunch! Indeed, those steps should work for any language.

As far as the Right to Left (RTL) situation, there was someone who was proposing to do some CSS/template tweaks to support Arabic, but I'm not sure if their effort went anywhere. We should track that down and see if anything took shape :)

I haven't translated everything yet. can it be that?

That shouldn't be an issue. Most of our translations are partial, so this wouldn't be unusual.

Something is not right here. in the login screen, there's this message: "Enter your login and password below:", but I can't find it anywhere in the codebase. neither it's French translations: "Entrez votre login et mot de passe:"

I was able to confirm that these strings are part of a view in FAB. @dpgaspar might have a good means to rectify this, but the solutions that spring to mind are (1) we should make this view a part of our React Single Page App at some point, and (2) in the meantime, we can probably override the FAB jinja template with one in Superset, and then provide our own translations in any/all supported languages.

CodeWithEmad commented 3 months ago

Thanks for the extensive comment. I really appreciate it.

those steps should work for any language.

I'm out of ideas. My last guess is that probably something is wrong with my setup. Although, I did everything from scratch again. I don't mean to keep this open and can close it if you or someone can confirm the steps works for Persian.

We should track that down and see if anything took shape :)

Awesome. I found iasue #25258

I was able to confirm that these strings are part of a view in FAB.

Awesome man. Thanks. I'll try to find my way in that repo for login page translations but I love the idea of overriding the template.

rusackas commented 3 months ago

If you want to open a draft PR with the code you have, maybe I can pull the branch and see if my luck is any different.

Side note, thank you for all the PRs you've opened thus far. Community contributions like this mean everything :D

CodeWithEmad commented 3 months ago

That's so nice of you. well, it's not much of a change, just a .po file, and fa language added to a Local variable in jed package and superset_config which I should remove but I just tried to create what I've got here. https://github.com/apache/superset/pull/29580

Community contributions like this mean everything

Indeed, I love open source. I'm currently contributing to the openedx project and its related repositories, but I'm starting to like Superset. Maybe one day I can review pull requests. :)

apache / superset