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

Use Symfony Translations to synchronize translations with Lokalise #1648

Merged
merged 23 commits into from
Oct 2, 2024

Conversation

osma
Copy link
Member

@osma osma commented Aug 14, 2024

Reasons for creating this PR

This PR brings in more Symfony components so that we can use Symfony Translations to manage the translation files and to synchronize them with Lokalise (see #1527). Also, the message format for translations is changed from po files to JSON (see #1525).

Despite adding more Symfony dependencies, we won't go all-in for Symfony, so we are not using it as the main MVC framework but rather rely on our own, simple ad-hoc MVC application structure.

Support for PHP 8.0 is dropped because Symfony 6.4 doesn't support it and it's EOL in any case.

Link to relevant issue(s), if any

Description of the changes in this PR

  • upgrade Symfony components to 6.4 (current stable version)
  • install Symfony Console and Lokalise support for Symfony Translations
  • drop PHP 8.0 from the list of PHP versions tested in GitHub Actions CI
  • pull newest translations from Lokalise (the messages themselves did not change, but some metadata got dropped)
  • switch the message format from PO to JSON and rename the translation files to message.XX.json, where XX is the language code
  • rename Twig templates that used the extension .inc to .inc.twig in order to enable extracting translation strings from them

Known problems or uncertainties in this PR

  • It's not possible to extract translatable strings from Vue templates (Symfony Translations doesn't know how to do it), so debug:translations shows a lot of "unused" messages. We need a separate solution for translating Vue messages. (See also Translate messages shown by Vue components #1523)

Checklist

  • phpUnit tests pass locally with my changes
  • I have added tests that show that the new code works, or tests are not relevant for this PR (e.g. only HTML/CSS changes)
  • The PR doesn't reduce accessibility of the front-end code (e.g. tab focus, scaling to different resolutions, use of .sr-only class, color contrast)
  • The PR doesn't introduce unintended code changes (e.g. empty lines or useless reindentation)

@osma osma added this to the 3.0 milestone Aug 15, 2024
Copy link

codecov bot commented Aug 20, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 70.69%. Comparing base (2a391f4) to head (0e49f02).
Report is 63 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1648      +/-   ##
============================================
- Coverage     70.71%   70.69%   -0.03%     
- Complexity     1647     1649       +2     
============================================
  Files            32       33       +1     
  Lines          4323     4330       +7     
============================================
+ Hits           3057     3061       +4     
- Misses         1266     1269       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

sonarcloud bot commented Aug 20, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

composer.json Show resolved Hide resolved
config/packages/twig.yaml Outdated Show resolved Hide resolved
@osma
Copy link
Member Author

osma commented Aug 21, 2024

TODO:

  • change domain to "messages" in Lokalise so that pull works
  • document the push/pull commands, maybe write a wrapper script?

… to get the console working, so they could perhaps be moved into dev dependencies
@osma
Copy link
Member Author

osma commented Oct 2, 2024

It was pretty difficult to properly initialize the project on the Lokalise side so that it works with Symfony Translate push and pull commands. One crucial thing is that all the translation keys need to have the filename messages.xliff on the Lokalise side (which is completely separate from the filenames used on the application side, which are messages.XX.json). The push and pull commands assume this filename and will not work if it is something else. Also, the push command is very unreliable and breaks easily. Here is a procedure that seems to work:

  1. Initialize the Lokalise project, or delete all keys, so that it has no keys
  2. Push the English language translations using this command: bin/console translation:push lokalise --domains messages --locales en --force. This will define the necessary keys and they will all have the filename messages.xliff as required later.
  3. Use the Upload function in Lokalise to upload all the other translation files. (This will take a while to process, check the Activity view)
  4. Make sure that all keys have been assigned the filename messages.xliff (some keys that were not in the English language file could have another filename). In the Multilingual view, look for keys that have been assigned to a filename other than messages.xliff using a Filter. Then click on each key to open the details and on the Advanced tab, change the filename to messages.xliff under Assigned to file. You may have to refresh a few times to change them all...

Now the project should have all keys and all available translations on the Lokalise side. After this point, only English language translations need to be pushed (e.g. after extracting new keys from templates). Pulling is more reliable and should be done for all languages regularly.

Push command for English:

bin/console translation:push lokalise --domains messages --locales en --force

Pull command for a single language (the format parameter is important!):

bin/console translation:pull lokalise --domains messages --format json --locales fi

Pull command for all languages:

bin/console translation:pull lokalise --domains messages --format json

NOTE: Pulling the English language translations with the --force option causes all entries that have an "Empty" translation on the Lokalise side (i.e. the message is identical to the key, or used to be) to be dropped from the file entirely. That's probably not a good idea, so --force shouldn't be used for pulling. But for pushing it is required, otherwise Symfony won't find the translations to push and will send an empty list to Lokalise...

@osma
Copy link
Member Author

osma commented Oct 2, 2024

Steps for extracting new message ids, adding them to Lokalise, translating them to other languages, then pulling back:

  1. Make a change in the code that adds a new translated message into the twig templates
  2. Check that it can be extracted: bin/console translation:extract --dump-messages en (should be shown in the list) or bin/console debug:translation en (should show it as missing)
  3. Extract the message(s) and add them to the English language translation file: bin/console translation:extract --force en --format json --prefix ""
  4. Push the translation file to Lokalise: bin/console translation:push lokalise --domains messages --locales en --force
  5. In the Lokalise UI, look up the message and add translations in other languages
  6. Pull the translations back to the codebase: bin/console translation:pull lokalise --domains messages --format json

Copy link

sonarcloud bot commented Oct 2, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
C Reliability Rating on New Code (required ≥ A)

See analysis details on SonarCloud

Catch issues before they fail your Quality Gate with our IDE extension SonarLint

@osma osma marked this pull request as ready for review October 2, 2024 12:46
@osma osma changed the title WIP: Use Symfony Translations to synchronize translations with Lokalise Use Symfony Translations to synchronize translations with Lokalise Oct 2, 2024
@osma
Copy link
Member Author

osma commented Oct 2, 2024

@osma osma requested a review from miguelvaara October 2, 2024 13:08
Copy link
Contributor

@miguelvaara miguelvaara left a comment

Choose a reason for hiding this comment

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

At first, there was some uncertainty about whether the system was actually updating the data correctly (deletion in Lokalise -> deletion locally as well), but it seems that it worked correctly after all. Although the feature branch can be merged, further testing should still be done just to be sure.

@miguelvaara miguelvaara merged commit 5e4ea21 into main Oct 2, 2024
9 of 10 checks passed
joelit added a commit that referenced this pull request Oct 8, 2024
Translations can be maintained with the Symfony-Lokalise combination (pull, push, extract)  but not yet by extracting from Vue code (#1648)

At first, there was some uncertainty about whether the system was actually updating the data correctly (deletion in Lokalise -> deletion locally as well), but it seems that it worked correctly after all. Although the feature branch can be merged, further testing should still be done just to be sure.

Add cypress tests for alphabetical index + add new test vocab

Add cypress test for alt labels

Add cypress files for alphabetical index and hierarchy

Move alphabetical index cypress tests from vocab-home.cy.js + add checks for spinners

Change test vocab name

Fix uriSpace

Fix uri space in test vocab
joelit added a commit that referenced this pull request Oct 8, 2024
Translations can be maintained with the Symfony-Lokalise combination (pull, push, extract)  but not yet by extracting from Vue code (#1648)

At first, there was some uncertainty about whether the system was actually updating the data correctly (deletion in Lokalise -> deletion locally as well), but it seems that it worked correctly after all. Although the feature branch can be merged, further testing should still be done just to be sure.

Add cypress tests for alphabetical index + add new test vocab

Add cypress test for alt labels

Add cypress files for alphabetical index and hierarchy

Move alphabetical index cypress tests from vocab-home.cy.js + add checks for spinners

Change test vocab name

Fix uriSpace

Fix uri space in test vocab
joelit added a commit that referenced this pull request Oct 8, 2024
Translations can be maintained with the Symfony-Lokalise combination (pull, push, extract)  but not yet by extracting from Vue code (#1648)

At first, there was some uncertainty about whether the system was actually updating the data correctly (deletion in Lokalise -> deletion locally as well), but it seems that it worked correctly after all. Although the feature branch can be merged, further testing should still be done just to be sure.

Add cypress tests for alphabetical index + add new test vocab

Add cypress test for alt labels

Add cypress files for alphabetical index and hierarchy

Move alphabetical index cypress tests from vocab-home.cy.js + add checks for spinners

Change test vocab name

Fix uriSpace

Fix uri space in test vocab
@osma osma deleted the issue1527-translation-symfony branch October 10, 2024 13:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done (verified in test.dev.finto.fi, set Milestone 3.0 for both issue & PR)
2 participants