Problem/Motivation

Working with the dump files is unpleasant and data is needed for testing the various i18n migrations.

Historically, the dump files have been updated as part of the patch for each single migration. That certainly allows the developer to really understand the data source but it adds extra work to each patch. That extra work can be avoided if the data is already in the dump file. And hopefully, it will make it easier to work on the migrations.

There is a precedence in #2594263: Add translation data to Migrate Drupal's test fixtures.

Proposed resolution

Add the i18n data needed in one patch.

i18n allows translations of the following items and we need data for testing migrations.

  • Built-in interface
  • Blocks
  • CCK
  • Content type
  • Menu
  • Profile
  • Taxonomy

Remaining tasks

Do it.

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

Status: Active » Needs review
FileSize
109.78 KB

Enabled Internationalization, String translation, Profile translation. Added data for Profile translation.

quietone’s picture

Enabled block translation, added 'full html' to string translation list, set language negotiation to 'path prefix' and added translations for the second custom block title and body. The change in language negotiation setting might cause an error.

quietone’s picture

Enabled taxonomy and content type translation and added translations. CCK and Poll still to do.

quietone’s picture

Poll has been removed from Core, so no work to do there. Enabled CCK string translation and added translations.

Assuming this passes the tests, this is good to use for working on the i18n string migrations. However, there was one problem. In the content type translation the english string contained the tags, which resulted in an 'invalid html' error on save. The html tag has been removed from the translation. Anyone know how to fix that?

quietone’s picture

Was interrupted and uploaded the patch twice, so canceled one test.

quietone’s picture

@TODO: Add data for i18n_blocks, which is different than the block data in i18n_strings.

quietone’s picture

Issue tags: +migrate-d6-d8
quietone’s picture

Title: Add i18n data to d6_dump » Add i18n string data to d6_dump
FileSize
783.63 KB

Added string translations for all text groups list in D6 admin/build/translate/search (Built-in interface, Blocks, CCK, Content type, Menu, Profile, Taxonomy). Translations exist for every item in every text group, except for the Built-in interface. About 5 were done for the Built-in interface. This may or may not be enough for testing, but that can be determined later when that work actually happens.

The point is this gets the bulk of the string data in the test fixture which should mean string translation patches can just concentrate on the migration and avoid the steps to create the test data.

Note that the translations are the same as the english text but with a preceeding 'fr - '. That is, except for a few which were translated by a French speaking friend who was visiting.

Status: Needs review » Needs work

The last submitted patch, 9: 2670170-9.patch, failed testing.

quietone’s picture

Don't know why file_directory_temp changed, but it is back to what it was.

quietone’s picture

Status: Needs work » Needs review
quietone’s picture

Title: Add i18n string data to d6_dump » Add i18n string & variable data to d6_dump
Status: Needs review » Needs work

This has the i18n_variable data in it as well, changing the title to reflect that.

A few weeks ago vasi suggested on IRC that another language is needed. Might as well do that here and add some i18n data in that language as well.

quietone’s picture

Status: Needs work » Needs review
FileSize
783.43 KB

Removed trailing whitespace in some strings.

quietone’s picture

Added another language, 'zu', and translated more strings.

quietone’s picture

Issue tags: +blocker

This is ready for review. Tagging as a blocker since it is needed for all the i18n strings and i18n variable migrations.

I've been using this locally to develop the i18n migrations without any problems.

vasi’s picture

Just a note: You get a much easier to review diff if you use the --patience or --minimal arguments to git diff.

vasi’s picture

FileSize
602.37 KB

Here's the minimal version. Not testing, since it's literally the exact same patch, just a different representation.

vasi’s picture

vasi’s picture

Ok, I've looked over most of it and it seems reasonable. The menu_links table is kinda hard to review in any format though, because of things being reordered I guess? I'll see if I can diff it in a different format so that it's easier to read.

vasi’s picture

FileSize
1.59 KB
5.99 KB

I wrote a little script to take a fixtures file, and dump the menu links from fixtures in a nice manner, see attached.

When I use it to compare the old and new fixtures, I get the attached diff. Most things look ok, but the addition of 'admin/content/node-type/story/fields/field_test_text_single_checkbox2/remove' is really weird. Why was that missing in the old fixtures?

quietone’s picture

@vasi, thanks for the tips on git diff. What are you using for the 'old' fixture? The link appears to be in HEAD, http://cgit.drupalcode.org/drupal/tree/core/modules/migrate_drupal/tests.... Or am I missing something?

quietone’s picture

The change to menu_links appeared in the first patch, #2. To find what cause the change I started with a clean D6 using d6_dump from HEAD. Then, verified that that 'admin/content/node-type/story/fields/field_test_text_single_checkbox2/remove' was not in menu_links:

MariaDB [dev0_dump]> select menu_name,mlid,link_path from menu_links where `link_path` like '%checkbox%';
+------------+------+-----------------------------------------------------------------------------------+
| menu_name | mlid | link_path |
+------------+------+-----------------------------------------------------------------------------------+
| navigation | 380 | admin/content/node-type/story/fields/field_test_float_single_checkbox/remove |
| navigation | 388 | admin/content/node-type/story/fields/field_test_text_single_checkbox/remove |
| navigation | 406 | admin/content/node-type/test-planet/fields/field_test_text_single_checkbox/remove |
+------------+------+-----------------------------------------------------------------------------------+
3 rows in set (0.00 sec)

Then I enabled the Internationalization module and checked again. There was now an entry for 'admin/content/node-type/story/fields/field_test_text_single_checkbox2/remove' in menu_links.

MariaDB [dev0_dump]> select menu_name,mlid,link_path from menu_links where `link_path` like '%checkbox%';
+------------+------+-----------------------------------------------------------------------------------+
| menu_name | mlid | link_path |
+------------+------+-----------------------------------------------------------------------------------+
| navigation | 380 | admin/content/node-type/story/fields/field_test_float_single_checkbox/remove |
| navigation | 388 | admin/content/node-type/story/fields/field_test_text_single_checkbox/remove |
| navigation | 406 | admin/content/node-type/test-planet/fields/field_test_text_single_checkbox/remove |
| navigation | 408 | admin/content/node-type/story/fields/field_test_text_single_checkbox2/remove |
+------------+------+-----------------------------------------------------------------------------------+
4 rows in set (0.00 sec)

So, enabling internationalization added the entry. Or was it something else? I started again, installed d6_dump from head, verified that the link is question wasn't in menu_links. Then went to 'admin/build/modules' and clicked 'save configuration' without enabling or disabling any modules. After that the link in question was again in menu_links;

Based on that it seems that the d6_dump was missing some data. Good find.

vasi’s picture

Status: Needs review » Reviewed & tested by the community

Ok, so it looks like it's the old dump that was in error. I'm happy with everything else, though I would like if someone else with more D6 experience could chime in.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 75b07c4 and pushed to 8.1.x and 8.2.x. Thanks!

I think it is fine if we have to make small changes when we actually start to write migrations against the new data.

  • alexpott committed 74d31f2 on 8.2.x
    Issue #2670170 by quietone, vasi: Add i18n string...

  • alexpott committed 75b07c4 on 8.1.x
    Issue #2670170 by quietone, vasi: Add i18n string...
quietone’s picture

Status: Needs review » Fixed

Asked on IRC why this didn't get committed to 8.0.x. Gábor Hojtsy explained a bit about the release schedules to me and said that it is safe to close this one.

Status: Fixed » Closed (fixed)

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