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

Language translations not working when not supported by make-plural #6848

Closed
craig-landry opened this issue Jan 15, 2021 · 7 comments
Closed
Assignees
Labels
Regression Affects a feature that worked in a previous release Type: Bug Fix something that isn't working as intended
Projects

Comments

@craig-landry
Copy link
Member

This was previously addressed with #5705 and regressed in CHT 3.8. The regression comes due to an updated messageformat dependency in CHT 3.8.

The prior fix to messageformat was done via monkey patching, which was applicable for messageformat 2.2.1 and does not work in the current version of messageformat (2.3.0).

@dianabarsan
Copy link
Member

Just wanted to point out that the issue manifests a little bit differently now, compared to before.
The reports list loads but when navigating to a contact, where we actually use pluralization, those translations will fail and the console will list errors:

image

image

@ngaruko
Copy link
Contributor

ngaruko commented Jan 19, 2021

Tested using the branch 6848-dual-monkey-patch-message-format, on a local machine, following these steps

  1. checkout the master branch

  2. do a local deployment as per documentation (basically npm ci, then grunt, then grunt dev-api, then grunt-sentinel, in a nutshell)

  3. Go to languages tab in the admin portal > display

  4. Add new language (code :ceb, name :Cebuano)

  5. Upload a translation file messages-ceb.properties (I copied the tagalog translation from the config-icm repo and added a few translations (make Google Translate your friend) - Upload either manually from the UI/admin portal or with medic-conf --local upload-custom-translations - There is a warning in the terminal: WARN 'cel' is not a recognized ISO 639 language code, please ask admin to set the name

  6. Create a user (first a health facility if it is a new db, with no seed data)

  7. Log in as the new created user

  8. Add a new person and a report that generates tasks

  9. Go to tasks and observe all translations look ok (default being English)
    image

  10. Go to 'Edit User Profile', change the language to Cebuano

  11. Go to 'Tasks, observe missing translations (task.days.left`) and error in console....
    image

  12. Go to contact and observe same...

  13. Now, checkout the `` branch

  14. Log out and log in again as the user created above & change language to cebuano

  15. Go to Task and observe translations work:
    image

  16. Same with contact page :
    image

@ngaruko
Copy link
Contributor

ngaruko commented Jan 19, 2021

One observation that need further investigation but not related to this PR:
Switching between language seems to be glitchy , one has to move between 2 or more clicks before the choice works. We have seen this before, maybe a regression ?

@MaxDiz MaxDiz added the Regression Affects a feature that worked in a previous release label Jan 19, 2021
@MaxDiz MaxDiz added this to To do in 3.11.0 via automation Jan 19, 2021
@ngaruko ngaruko moved this from To do to AT in progress in 3.11.0 Jan 20, 2021
@dianabarsan dianabarsan added this to Ready for AT in 3.10.2 Jan 20, 2021
@dianabarsan
Copy link
Member

@ngaruko please tell me if you consider this as ready to be merged.

@ngaruko ngaruko moved this from Ready for AT to AT in progress in 3.10.2 Jan 20, 2021
@ngaruko ngaruko moved this from AT in progress to Ready to merge in 3.10.2 Jan 20, 2021
@ngaruko
Copy link
Contributor

ngaruko commented Jan 20, 2021

@dianabarsan Yes - This is ready. Please feel free to merge and release a beta (unless @binokaryg would like to include the datepicker in the release?) Then we will do a quick RT around languages/translations before the end of the week.

@binokaryg
Copy link
Member

I am still waiting for confirmation from the partner. Will it help if add the draft translations now and change them later before the final release if required?

@MaxDiz MaxDiz removed this from AT in progress in 3.11.0 Jan 20, 2021
dianabarsan added a commit that referenced this issue Jan 20, 2021
Adds a browserify alias for the messageformat package, to point at its source code instead of the default, minified file.
This makes the existent patch work again.
Adds e2e test to protect against future regressions.

#6848
dianabarsan added a commit that referenced this issue Jan 20, 2021
Adds a browserify alias for the messageformat package, to point at its source code instead of the default, minified file.
This makes the existent patch work again.
Adds e2e test to protect against future regressions.

#6848

(cherry picked from commit b747f47)
@dianabarsan
Copy link
Member

Merged into master and backported to 3.10.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Regression Affects a feature that worked in a previous release Type: Bug Fix something that isn't working as intended
Projects
No open projects
Development

No branches or pull requests

5 participants