Problem/Motivation

Configuration translations are not imported if Interface translation module is enables after adding a language.

Steps to reproduce:

  1. Install in English.
  2. Enable Language module.
  3. Add a language. Enable Locale module.
  4. Go to report > Translation updates and run a translation update.
  5. Observe: Configuration translations are not updated.

Configuration translation is imported when:

  1. Install in English.
  2. Enable Language and Locale modules at once.
  3. Add a language.
  4. Observe: The batch that runs downloads translations and updates configuration translations.

Proposed resolution

Update configuration translations as well from the localization update process.

Remaining tasks

  • Review. Commit.
  • Add tests

User interface changes

The batch will run longer.

API changes

None.

Files: 
CommentFileSizeAuthor
#30 2344967-config-translation-update-30.patch3.9 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,773 pass(es). View
#30 interdiff.txt1.15 KBGábor Hojtsy
#28 2344967-config-translation-update-27.patch3.91 KBGábor Hojtsy
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,527 pass(es). View
#25 interdiff.txt740 bytesGábor Hojtsy
#25 2344967-config-translation-update-25.patch3.9 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php. View
#23 interdiff.txt1.73 KBGábor Hojtsy
#23 2344967-config-translation-update-23.patch3.9 KBGábor Hojtsy
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,452 pass(es), 1 fail(s), and 0 exception(s). View
#18 2344967-config-translation-update-18.patch3.92 KBSutharsan
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,320 pass(es). View
#18 interdiff-2344967-16-18.txt975 bytesSutharsan
#16 2344967-config-translation-update-15.patch3.92 KBSutharsan
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,999 pass(es), 1 fail(s), and 0 exception(s). View
#10 2344967-config-translation-update-10-must-fail.patch5.13 KBSutharsan
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,732 pass(es), 1 fail(s), and 0 exception(s). View

Comments

Gábor Hojtsy’s picture

FileSize
1.2 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,625 pass(es). View

This should fix it.

Gábor Hojtsy’s picture

Issue tags: +D8MIAMS

This is needed for our Lab. Tagging D8MIAMS.

Sutharsan’s picture

Status: Needs review » Reviewed & tested by the community

Code looks good and fixes the problem.

Désiré’s picture

I tested it twice (patch #1): once with a minimal, once with a standard install profile.
First I enabled language and locale module and added a language, dumped the language related config from the database.
Then I enabled only language module, added a language and then enabled locale module and imported the translations, dumped the language related config from the database.

As I experienced the language related config were the same on both cases, so I assume that the patch works well.

webchick’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

This needs tests, but I can see writing them would be difficult. OTOH it would be good to get this fixed prior to beta. Therefore, I think I'm going to let this in, but leave it needs work for tests. If folks could please follow back up with that during/after Amsterdam, that would be really awesome. This is definitely something we don't want to break a second time. :)

Committed and pushed to 8.x. Thanks!

Now back to needs work.

  • webchick committed 1848068 on 8.0.x
    Issue #2344967 by Gábor Hojtsy: Fixed Localization update does not...
Gábor Hojtsy’s picture

Title: Localization update does not update configuration translations » Tests for localization update does not update configuration translations
Gábor Hojtsy’s picture

Issue tags: +Amsterdam2014
Sutharsan’s picture

Assigned: Unassigned » Sutharsan
Sutharsan’s picture

Assigned: Sutharsan » Unassigned
Status: Needs work » Needs review
FileSize
3.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,740 pass(es). View
5.13 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 77,732 pass(es), 1 fail(s), and 0 exception(s). View

This patch adds a test that follows the steps from the issue summary to reproduce the problem.
For the 'must-fail' patch I've added the reverse of #1 patch.

The last submitted patch, 10: 2344967-config-translation-update-10-must-fail.patch, failed testing.

lazysoundsystem’s picture

Status: Needs review » Needs work

Tests look good. Just one tiny change in the comments:

+++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
@@ -0,0 +1,86 @@
+    // Check and update the translation status. This will import the African

'Afrikaans' for 'African' here.

Sutharsan’s picture

Status: Needs work » Needs review
Issue tags: -Needs tests
FileSize
908 bytes
286.42 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Unable to apply patch 2344967-config-translation-update-12.patch. Unable to apply patch. See the log in the details link for more information. View

Thanks for the review.
Only a new patch this time, no 'test only' patch for this trivial change.

lazysoundsystem’s picture

Status: Needs review » Reviewed & tested by the community

Thanks. RTBC.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 13: 2344967-config-translation-update-12.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
3.92 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 78,999 pass(es), 1 fail(s), and 0 exception(s). View

Ignore the patch in #13. New patch containing the comment in #12 is included.

Status: Needs review » Needs work

The last submitted patch, 16: 2344967-config-translation-update-15.patch, failed testing.

Sutharsan’s picture

Status: Needs work » Needs review
FileSize
975 bytes
3.92 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 81,320 pass(es). View

Test failing due to removal of url(). For now replaced by _url().

Sutharsan’s picture

Issue summary: View changes
Gábor Hojtsy’s picture

Status: Needs review » Needs work

Looks good, some minor comments:

  1. +++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
    @@ -0,0 +1,85 @@
    + * Tests translation of configuration strings.
    

    This is a bit too generic :) I would try to get the same 1 line text that is on the test method.

  2. +++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
    @@ -0,0 +1,85 @@
    +   * Tests configuration translation import when locale module is enabled after
    +   * a language was added.
    

    Would be great to get this on one line, eg. "Test localization update will change configuration translations if enabled after language." (did not check if this fits on one line in fact :D)

  3. +++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
    @@ -0,0 +1,85 @@
    +    // Add translation permissions now the locale module has been enabled.
    

    "now that the"

  4. +++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
    --- /dev/null
    +++ b/core/modules/locale/tests/test.af.po
    

    There is no code apparent in this patch that makes this file be the result of update checking, where is that located? I suspect preexisting in core.

Status: Needs work » Needs review
Gábor Hojtsy’s picture

@Sutharsan: this seemed like was so close, are you still around to fix this one?

Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community
FileSize
3.9 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,452 pass(es), 1 fail(s), and 0 exception(s). View
1.73 KB

The answer to #20/4 is this snippet from locale_translate_test which module is used in the test. The other minor text fixes I made myself. That should not disqualify me from RTBC-ing it since the changes are so minor.

'interface translation project': locale_test_translate
'interface translation server pattern': core/modules/locale/tests/test.%language.po

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 23: 2344967-config-translation-update-23.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Needs review
FileSize
3.9 KB
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php. View
740 bytes
Gábor Hojtsy’s picture

Status: Needs review » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 25: 2344967-config-translation-update-25.patch, failed testing.

Gábor Hojtsy’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
3.91 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,527 pass(es). View

Minor PHP issue.

alexpott’s picture

Status: Reviewed & tested by the community » Needs work
+++ b/core/modules/locale/src/Tests/LocaleConfigTranslationImportTest.php
@@ -0,0 +1,84 @@
+    \Drupal::config('update.settings')->set('fetch.url', _url('update-test', array('absolute' => TRUE)))->save();

Is it necessary to use _url() here?

Gábor Hojtsy’s picture

Status: Needs work » Reviewed & tested by the community
FileSize
1.15 KB
3.9 KB
PASSED: [[SimpleTest]]: [PHP 5.4 MySQL] 82,773 pass(es). View

@alexpott: the problem with the update-test path/route is it would expect some arguments, namely project and version. Then it actually ignores version later on (haha). For this test, all we need is a URL that would not return much valid info, because we only work off of local stuff here (see locale_test_translate.info.yml), so this override is for the rest of the projects so they don't interfere with the test. To fix the update-test route that requires unrelated changes, so we can just use the front page here just as well.

Better?

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I wished we had opened a follow-up for this - anyway nice to have additional test coverage for this. This issue addresses a major bug and is allowed per https://www.drupal.org/core/beta-changes. Committed 3f28dd7 and pushed to 8.0.x. Thanks!

  • alexpott committed 3f28dd7 on 8.0.x
    Issue #2344967 followup by Gábor Hojtsy, Sutharsan: Tests for...
Gábor Hojtsy’s picture

Issue tags: -sprint

Thanks a lot!

Gábor Hojtsy’s picture

Title: Tests for localization update does not update configuration translations » Localization update does not update configuration translations

Also restoring original title.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.