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

Add autotranslate base #971

Merged
merged 18 commits into from
Jul 23, 2018
Merged

Add autotranslate base #971

merged 18 commits into from
Jul 23, 2018

Conversation

mbeacom
Copy link
Contributor

@mbeacom mbeacom commented Apr 25, 2018

Description

The goal of this PR is to add autotranslate and introduce German locale.

Adds:

  • autotranslate django app
  • make autotranslate command for automatically translating/updating existing translations

Langauge support for:

[
    ('en', _('English')),
    ('es', _('Spanish')),
    ('de', _('German')),
    ('hi', _('Hindi')),
    ('it', _('Italian')),
    ('ko', _('Korean')),
    ('pl', _('Polish')),
    ('zh-hans', _('Simplified Chinese')),
    ('zh-hant', _('Traditional Chinese')),
]

TODO:

Add mechanism to automatically replace poorly parsed special characters for python dynamic vars.
Example:

#: dashboard/templates/shared/nav_auth.html:54
#, fuzzy, python-format
#| msgid "Claim $%(tip.value_in_usdt_now)s Tip "
msgid "Claim $%(tip.value_in_usdt_now)s Tip "
msgstr "Claim $% (tip.value_in_usdt_now) s Tipp"

should be:

#: dashboard/templates/shared/nav_auth.html:54
#, fuzzy, python-format
#| msgid "Claim $%(tip.value_in_usdt_now)s Tip "
msgid "Claim $%(tip.value_in_usdt_now)s Tip "
msgstr "Claim $%(tip.value_in_usdt_now)s Tipp"
Checklist
  • linter status: 100% pass
  • changes don't break existing behavior
  • commit message follows commit guidelines
Affected core subsystem(s)

i18n

Testing

Tested locally

Refers/Fixes

Ref #814

@mbeacom mbeacom added in progress backend This needs backend expertise. labels Apr 25, 2018
@mbeacom mbeacom requested a review from owocki April 25, 2018 16:42
@owocki
Copy link
Contributor

owocki commented Apr 25, 2018

can we source someone who is german to tell us how good/bad the autotranslate is? if not german, maybe another language? vivek might be able to help us with sourcing this person

@codecov
Copy link

codecov bot commented Apr 26, 2018

Codecov Report

Merging #971 into master will decrease coverage by <.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #971      +/-   ##
==========================================
- Coverage   28.57%   28.57%   -0.01%     
==========================================
  Files         128      128              
  Lines       10070    10067       -3     
  Branches     1328     1327       -1     
==========================================
- Hits         2878     2877       -1     
+ Misses       7091     7089       -2     
  Partials      101      101
Impacted Files Coverage Δ
app/marketing/views.py 15.51% <0%> (+0.1%) ⬆️
app/app/settings.py 81.14% <100%> (ø) ⬆️
app/retail/views.py 31.3% <0%> (-0.32%) ⬇️

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 a6ea114...3e188fa. Read the comment docs.

@ghost ghost assigned mbeacom Jun 7, 2018
@mbeacom mbeacom changed the title WIP: Add autotranslate base and DE test HOLD: Add autotranslate base and DE test Jun 7, 2018
@mbeacom
Copy link
Contributor Author

mbeacom commented Jul 11, 2018

screenshot 2018-07-11 12 47 02

@mbeacom mbeacom changed the title HOLD: Add autotranslate base and DE test HOLD: Add autotranslate base - German, Hindi, Spanish, Italian, and Korean Jul 11, 2018
@mbeacom mbeacom changed the title HOLD: Add autotranslate base - German, Hindi, Spanish, Italian, and Korean Add autotranslate base - German, Hindi, Spanish, Italian, and Korean Jul 11, 2018
@mbeacom
Copy link
Contributor Author

mbeacom commented Jul 11, 2018

@owocki This PR should be good to merge - Test the latest at: https://stage.gitcoin.co -> Settings -> Preferred Language - Temporarily disabled zh (Chinese) and po (Polish).

owocki
owocki previously approved these changes Jul 11, 2018
Copy link
Contributor

@owocki owocki left a comment

Choose a reason for hiding this comment

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

LGTM once the concerns noted in slack are addresssed

@owocki
Copy link
Contributor

owocki commented Jul 11, 2018

@mbeacom go for it

Temporarily disabled zh (Chinese) and po (Polish).

How come ?

@owocki
Copy link
Contributor

owocki commented Jul 11, 2018

@PixelantDesign @mbeacom Do we want to auto-translate visitors based upon IP addresses?

#1695

@owocki
Copy link
Contributor

owocki commented Jul 11, 2018

oh also, it might be worth testing the emails @mbeacom

@mbeacom
Copy link
Contributor Author

mbeacom commented Jul 11, 2018

@owocki We can serve the appropriate language based on their Accept-Language header, enable language endpoints gitcoin.co/en, gitcoin.co/es, etc, use the ip aware tracking based on the locale of the IP, or simply use the preferred language established in settings. As it stands right now, the majority of the handling is done based on the user's preferences and based on the language header provided by the browser. I am not 100%, but the copy not displaying in the requested language might be a product of serving over CDN. 🤔

Also, testing emails now!

@mbeacom
Copy link
Contributor Author

mbeacom commented Jul 11, 2018

lang

@owocki
Copy link
Contributor

owocki commented Jul 11, 2018

@owocki We can serve the appropriate language based on their Accept-Language header, enable language endpoints gitcoin.co/en, gitcoin.co/es, etc, use the ip aware tracking based on the locale of the IP, or simply use the preferred language established in settings.

i think the Accept-Language header stuff is important for conversion (before the user has an account, i.e. upon first landing page visit)

As it stands right now, the majority of the handling is done based on the user's preferences and based on the language header provided by the browser. I am not 100%, but the copy not displaying in the requested language might be a product of serving over CDN. 🤔

got it. we should figure out the CDN issue before going live, since the prod site is on the CDN

@owocki
Copy link
Contributor

owocki commented Jul 20, 2018

@mbeacom this is a prioroity for decentralland -- any way we can prioritize shipping it?

@mbeacom mbeacom changed the title Add autotranslate base - German, Hindi, Spanish, Italian, and Korean Add autotranslate base Jul 20, 2018
@owocki
Copy link
Contributor

owocki commented Jul 20, 2018

@mbeacom what was the issue with chinese translation and is it fixed on this PR?

@mbeacom
Copy link
Contributor Author

mbeacom commented Jul 23, 2018

@owocki It was my mistake. I specified zh vs with the necessary secondary identifier zh-hant or zh-hans. They are present in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend This needs backend expertise.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants