Postponed on #2897114: Products migrated to both nodes and products
Problem/Motivation
The test fixture needs to be multilingual. Once translations are in the fixtures, we need to solve the following:
I have successfully migrated orders + products + order products. I don't have variations/attributes in my D6 use case. After import I modify a product variation and it gives me this error: "The SKU "birthday sea experience" is already in use and must be unique.". This is because migrate has migrated the English and Spanish nodes from D6 as two separate product variations in D8, both marked as English
See image attached. This is a query in the database for other variations with the same SKU. As you can see, its the same product, translated, but imported as 2 separate products.
I have come across this issue with migrate with normal nodes. Not sure if this is therefore migrate or commerce_migrate.
Proposed resolution
Add translations
Fix Product migrations ending up in 2 entities: Products migrated to both nodes and products
Import product variation language properly and do not import the products to two different NIDs
Also apply the same to product_kits
Remaining tasks
Do it.
Make patch
..
User interface changes
N/A
API changes
N/A
Data model changes
N/A
Comment | File | Size | Author |
---|---|---|---|
#39 | interdiff.txt | 600 bytes | quietone |
#39 | 2902425-39.patch | 842.04 KB | quietone |
#37 | interdiff-31-37.txt | 6.82 KB | quietone |
#37 | 2902425-37.patch | 841.91 KB | quietone |
#35 | interdiff.txt | 5.92 KB | quietone |
Comments
Comment #2
willeaton CreditAttribution: willeaton as a volunteer commentedFixed the various errors the installation produced. Enabled translation modules, created Spanish language, translated taxonomy terms (simply translation not language vocabs), created product translations, created order with translated product.
Documentation page also created: https://www.drupal.org/docs/8/modules/commerce-migrate/generating-databa...
Comment #3
willeaton CreditAttribution: willeaton as a volunteer commentedComment #4
willeaton CreditAttribution: willeaton as a volunteer commentedComment #5
willeaton CreditAttribution: willeaton as a volunteer commentedThe patch I created for the D6 fixtures failed and I need help to fix it. Also, modified the spec of this issue to be more related to the product translations import.
Comment #6
willeaton CreditAttribution: willeaton as a volunteer commentedAdded missing reference to product_kits
Comment #7
quietone CreditAttribution: quietone at Acro Commerce commentedReroll.
Comment #9
quietone CreditAttribution: quietone at Acro Commerce commentedUpdate the timestamps in the test to reflect what is really in the fixture.
Comment #10
quietone CreditAttribution: quietone at Acro Commerce commentedPostponing on #2897114: Products migrated to both nodes and products
Comment #11
quietone CreditAttribution: quietone at Acro Commerce commentedOver in #2897114: Products migrated to both nodes and products willeaton tested the patch in #150
with a db with translations, nodes and products and reported the results #153. Referencing it here so we are sure to fix those problems. I had similar problems testing with the fixture in this patch but haven't looked at the results in detail.
edit: fix the issue number.
Comment #12
quietone CreditAttribution: quietone at Acro Commerce commentedDid some testing with this and #2897114: Products migrated to both nodes and products. There are still lots of errors, no need to list them all, to sort out. But for this issue lets select the first one, the migration of the language content settings. These are not migrated for the products, which makes sense because d6_language_content_settings is another migration with a constant value 'node' for the entity type.
Keeping this to adding the fixture and one migration, d6_language_content_settings with followups for any other error. That way this is a manageable size and sensible scope.
Comment #13
willeaton CreditAttribution: willeaton as a volunteer commentedSounds good. There are various translation issues which we can break down into manageable issues. I'll be monitoring the issue to help whenever you have doubts or ready to test anything
Comment #14
willeaton CreditAttribution: willeaton as a volunteer commentedHi @quietone. Can I help you out in any way with this? If you give me some pointers I could try and look into this but Im not even sure where to start really. Do you have any time booked in to look at this soon?
Thanks
Will
Comment #15
quietone CreditAttribution: quietone at Acro Commerce commentedMaking a brief note, I plan on making a patch after I take a walk. I made a d6_ubercart_language_content_settings migration based on the existing language_content_settings migration. Once that is run the various node_translation migrations seem to work. Promising
I have noticed 3 problems. One, the language_content_settings migration does not rollback properly. Once is it rolled back when the migration is run again this error is reported,
[error] 'language_content_settings' entity with ID 'commerce_product.product' already exists.
Two, the content_language_settings enables translation on the content but does not tick the individual fields (title, body etc). And three, there is nothing to enable translation of product variations.Comment #16
quietone CreditAttribution: quietone at Acro Commerce commentedThis adds a migration for language content settings for commerce product and a test. No interdiff since it needed a reroll.
Comment #17
quietone CreditAttribution: quietone at Acro Commerce commentedNoting that I was able to migrate translations, the text fixture, when running migrations by hand. Starting from a clean install, enable commerces. Then migrate the node and product types and enable translations for product and product kit types. The migrate nodes and node translations. Still working on a reliable way to replicate that.
Comment #18
heddnAdding a review only patch.
Comment #19
quietone CreditAttribution: quietone at Acro Commerce commentedBeen a bit of a comedy of errors to get the steps to reproduce migrating translations.
nid: tnid
toproduct_id: tnid
destination: plugin: 'entity:node'
todestination: plugin: 'entity:commerce_product'
- upgrade_d6_ubercart_product_type
Comment #20
quietone CreditAttribution: quietone at Acro Commerce commentedThe first problem was that d6_ubercart_language_content_settings migration was not setting the body field translatable on the translated products. Finally tracked that down to MigrateProductType::postRowSave(), which add the stores, variation_ids and body fields to the product type. But the body field was defaulting to non-translatable. Therefor, a test was added that will set the body field translatable if content_translation exists.
Next was to get the translations migrated. Here, it was simplest to let the node deriver do its work and just modify the product migrations as needed. But still no translations were migrated. That was because the product migration was modified to use the source plugin 'd6_ubercart_product' and that did not have any code to handle translations. So, changed the plugin to extend the core node source plugin and updated the query, fields, and prepareRow methods as needed. After that the translations were migrated.
Now, since the d6_ubercart_product source plugin and the d6_ubercart_product migrate are no longer used, they have been deleted and tests updated as needed.
And thanks to heddn I have remembered to add a review only patch.
Comment #23
quietone CreditAttribution: quietone at Acro Commerce commentedThat error is no surprise, I get it locally, and good news there were no others. The failure is in the ProductTest on this test taken from the core MigrateNodeTest. I don't know about the content_translation manager, yet.
Comment #24
quietone CreditAttribution: quietone at Acro Commerce commentedThe test was missing migrate_plus, which is needed for the prepare row event. Once that was running found that the d6_node_type wasn't migrating the node types because the node configuration wasn't installed for the test.
Comment #25
quietone CreditAttribution: quietone at Acro Commerce commentedComment #26
quietone CreditAttribution: quietone at Acro Commerce commentedComment #27
quietone CreditAttribution: quietone at Acro Commerce commentedI found rerolling the test fixture extremely difficult, which just continues to convince me we should have a 'complete as possible' fixture to start with and thus avoid difficult rerolls. This is not passing ProductTest and I don't see why. Let's find out if anything else is failing and that may point to the problem.
Comment #29
quietone CreditAttribution: quietone at Acro Commerce commentedIts always simple when you find it. Think this will work. Added a few comments as well.
Comment #31
quietone CreditAttribution: quietone at Acro Commerce commentedYes, the source plugin was modified so updated the tests. Removed debug stuff left in.
Comment #33
quietone CreditAttribution: quietone at Acro Commerce commentedTests are passing
Comment #34
heddnIs there a core issue for this? Is is good to fix this upstream so it is configurable? If so, let's add an @TODO with link to the issue.
Let's not leave code commented out in the code base unless there's a good reason and a comment.
Comment #35
quietone CreditAttribution: quietone at Acro Commerce commented1. Made an issue, https://www.drupal.org/node/2930050
2. Looks like I wasn't finished with this at all. I had 'parked' the variations and didn't get back to it. Removed the commented lines, updated tests, and was able to remove the postRow save. Let's see if I broke something else.
Comment #37
quietone CreditAttribution: quietone at Acro Commerce commentedHead moved while I was working on this. Ignore #35.
Comment #39
quietone CreditAttribution: quietone at Acro Commerce commentedAdd the dependencies to the test.
Comment #40
heddnI cannot see anything wrong here. Tests seem to cover everything. LGTM.
Comment #42
heddnComment #43
willeaton CreditAttribution: willeaton commentedGreat news. I'll test it out tomorrow!