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
Comment | File | Size | Author |
---|---|---|---|
#15 | interdiff-2670170-14-15.txt | 161.95 KB | quietone |
#15 | 2670170-15.patch | 849.98 KB | quietone |
#14 | 2670170-14.patch | 783.43 KB | quietone |
#11 | 2670170-11.patch | 783.44 KB | quietone |
#9 | 2670170-9.patch | 783.63 KB | quietone |
Comments
Comment #2
quietone CreditAttribution: quietone commentedEnabled Internationalization, String translation, Profile translation. Added data for Profile translation.
Comment #3
quietone CreditAttribution: quietone as a volunteer commentedEnabled 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.
Comment #4
quietone CreditAttribution: quietone as a volunteer commentedEnabled taxonomy and content type translation and added translations. CCK and Poll still to do.
Comment #5
quietone CreditAttribution: quietone as a volunteer commentedPoll 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?
Comment #6
quietone CreditAttribution: quietone as a volunteer commentedWas interrupted and uploaded the patch twice, so canceled one test.
Comment #7
quietone CreditAttribution: quietone as a volunteer commented@TODO: Add data for i18n_blocks, which is different than the block data in i18n_strings.
Comment #8
quietone CreditAttribution: quietone as a volunteer commentedComment #9
quietone CreditAttribution: quietone as a volunteer commentedAdded 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.
Comment #11
quietone CreditAttribution: quietone as a volunteer commentedDon't know why file_directory_temp changed, but it is back to what it was.
Comment #12
quietone CreditAttribution: quietone as a volunteer commentedComment #13
quietone CreditAttribution: quietone as a volunteer commentedThis 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.
Comment #14
quietone CreditAttribution: quietone as a volunteer commentedRemoved trailing whitespace in some strings.
Comment #15
quietone CreditAttribution: quietone as a volunteer commentedAdded another language, 'zu', and translated more strings.
Comment #16
quietone CreditAttribution: quietone as a volunteer commentedThis 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.
Comment #17
vasi CreditAttribution: vasi at Evolving Web commentedJust a note: You get a much easier to review diff if you use the --patience or --minimal arguments to git diff.
Comment #18
vasi CreditAttribution: vasi at Evolving Web commentedHere's the minimal version. Not testing, since it's literally the exact same patch, just a different representation.
Comment #19
vasi CreditAttribution: vasi at Evolving Web commentedComment #20
vasi CreditAttribution: vasi at Evolving Web commentedOk, 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.
Comment #21
vasi CreditAttribution: vasi at Evolving Web commentedI 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?
Comment #22
quietone CreditAttribution: quietone as a volunteer commented@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?
Comment #23
quietone CreditAttribution: quietone as a volunteer commentedThe 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.
Comment #24
vasi CreditAttribution: vasi at Evolving Web commentedOk, 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.
Comment #25
alexpottCommitted 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.
Comment #28
quietone CreditAttribution: quietone as a volunteer commentedAsked 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.