Skip to content

feat: add i18n support #115

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

Merged
merged 29 commits into from
Jan 30, 2021
Merged

Conversation

Gonzom
Copy link
Contributor

@Gonzom Gonzom commented Jan 22, 2021

This feature includes the core functions added to /internal, English and Hebrew json files, removal of text strings from the html files, updates to the routes and model updates. Also included are tests. Please note, that this feature is waiting on user registration for final hooks.

Issue: #23

This feature includes the core functions added to /internal, English and Hebrew json files, removal of text strings from the html files, updates to the routes and model updates. Also included are tests. Please note, that this feature is waiting on user registration for final hooks.
… move foreignkey to last column as requested in CR.

Additional minor doc fixes.
@codecov-io
Copy link

codecov-io commented Jan 22, 2021

Codecov Report

Merging #115 (f96f221) into main (2055db7) will increase coverage by 0.15%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #115      +/-   ##
==========================================
+ Coverage   98.72%   98.88%   +0.15%     
==========================================
  Files          10       11       +1     
  Lines         236      269      +33     
==========================================
+ Hits          233      266      +33     
  Misses          3        3              
Impacted Files Coverage Δ
app/internal/languages.py 96.00% <96.00%> (ø)
app/database/models.py 100.00% <100.00%> (ø)
app/dependencies.py 100.00% <100.00%> (ø)
app/main.py 100.00% <100.00%> (+5.88%) ⬆️
app/routers/agenda.py 100.00% <100.00%> (ø)
app/routers/profile.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2055db7...f96f221. Read the comment docs.

Copy link
Member

@yammesicka yammesicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work. Added some of my insights.

… and "localized" and the creation of the Jinja2TemplateResponse is now handled in internal/utilities.
The translations are now handled with @lru_cache and the function is only re-handled when a new language code parameter is used. This should be the last edit (unless any crash) to this version of the translation feature.
@Gonzom
Copy link
Contributor Author

Gonzom commented Jan 26, 2021

@yammesicka any idea on how to handle the missing code coverage where I do a test to check if I'm running from tests or live?

A few notes:

  1. gettext.install() adds _() to the global namespace, so to make the flake F821 errors disappear, the following needs to be added to the flake config file, which I think is python-app.yml: --builtins=_ (see doc).

  2. To remove the "unresolved references" errors in PyCharm for the same error, do the following:
    2.1. In the Settings/Preferences dialog Ctrl+Alt+S, select Editor | Inspections.
    2.2. Go to "Unresolved references".
    2.3. Click the "Add" button.
    2.4. Add "_".

  3. And here are the steps to add new translations:
    3.1. Create base.pot file: pybabel extract --mapping-file=app/babel_mapping.ini app tests -o app/locales/base.pot
    3.2. cd app
    3.3.1. Create he base.po file: pybabel init -l he -i locales/base.pot -d locales -D base
    3.3.2. Create en base.po file: pybabel init -l en -i locales/base.pot -d locales -D base
    3.4. Translate strings
    3.5. Create .mo files: pybabel compile -d locales -D base
    3.6. Update translations.
    3.7. Update .mo files: pybabel update -i locales/base.pot -d locales

@yammesicka
Copy link
Member

3. And here are the steps to add new translations:
3.1. Create base.pot file: pybabel extract --mapping-file=app/babel_mapping.ini app tests -o app/locales/base.pot
3.2. cd app
3.3.1. Create he base.po file: pybabel init -l he -i locales/base.pot -d locales -D base
3.3.2. Create en base.po file: pybabel init -l en -i locales/base.pot -d locales -D base
3.4. Translate strings
3.5. Create .mo files: pybabel compile -d locales -D base
3.6. Update translations.
3.7. Update .mo files: pybabel update -i locales/base.pot -d locales

Awesome work! Can you please add this to the workflow? We can do it automagically using GitHub Actions.
It can be considered as another ticket :)

Fixed missing curly parenthesis in profile.html
Changed _get_supported_languages() into a generator expression
Merged testing parameters in test_language.py
@yammesicka yammesicka merged commit 779de2d into PythonFreeCourse:main Jan 30, 2021
yammesicka added a commit that referenced this pull request Jan 30, 2021
yammesicka added a commit that referenced this pull request Jan 30, 2021
@yammesicka
Copy link
Member

Hi,

I've merged it by mistake, but this actually should go into the develop branch.
I've reverted the changes to avoid poor repo structure.
Can you please re-create the PR to the develop branch and not to the main branch?

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 this pull request may close these issues.

5 participants