Problem/Motivation

Ubercart D7 product variations need a migration.

Proposed resolution

Refer to the existing D6 migrations.
But for tests, please use the new fixture in #2871812: New complete fixture for Ubercart D7 schema.

Remaining tasks

Do it. There are novice tasks here, details in comment #3.
Write a D7 Product Variation migration test
change all d6_ubercart_product_variation to ubercart_product_variation, after the D7 test is working
Test it.

User interface changes

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone’s picture

Issue summary: View changes
Issue tags: +Novice

And a patch.
Note that this uses the d6_ubercart_product_variation source plugin. And that the d6 and d7 migrations are the same. Therefor there is some renaming work needed. And is suitable for novice.

Next steps (suitable for novice):

  • Rename d6_ubercart_product_variation.yml to ubercart_product_variation.yml
  • Edit ubercart_product_variation.yml
    • Remove 'd6_' from the id:
    • Add two tags, Drupal 7 and Drupal 7 Ubercart
    • Change the line plugin: d6_ubercart_product_variation to plugin: ubercart_product_variation
  • Edit the source plugin:
    • Change id = "d6_ubercart_product_variation" to id = "ubercart_product_variation"
    • Change namespace Drupal\commerce_migrate\Plugin\migrate\source\ubercart;
    • Now git mv the plugin from the current directory '..source/d6/ProductVariation' to '..source/ProductVariation'
quietone’s picture

Status: Active » Needs work
karthikkumarbodu’s picture

Status: Needs work » Needs review
StatusFileSize
new8.53 KB

Status: Needs review » Needs work
mohit1604’s picture

StatusFileSize
new1.96 KB

@quietone, did changes as per #3, please review it :)

quietone’s picture

Issue summary: View changes
Issue tags: -Novice

Thanks, the patch looks correct. Since this issue was created there are a lot more migrations using the product variation source plugin and the product variation migration and they are looking for d6_ubercart_product_variation and not ubercart_product_variation. Thus many of the test fails with this type of error.

1) Drupal\Tests\commerce_migrate_ubercart\Kernel\Migrate\d6\ViewModesTest::testMigration
Drupal\Component\Plugin\Exception\PluginException: Plugin (ubercart_product_variation) instance class "Drupal\commerce_migrate_ubercart\Plugin\migrate\source\ProductVariation" does not exist.

Before fixing all those errors, I think the next step is to write a d7 Migration test to make sure it will work with both d6 and d7 sharing the migration and source plugin. Once that is confirmed then the next step would be to change all the other occurrences of the migration 'd6_ubercart_product_variation' to 'ubercart_product_variation' and similarly for the source plugin, d6_ubercart_product_variation to ubercart_product_variation.

Removing the novice tag for the writing of the new test.

quietone’s picture

Status: Needs work » Needs review
StatusFileSize
new6.11 KB
new5.39 KB

Added a copy/paste of the D7 product migration test, it still has d6 code in it. The list of things to do in #3 was a bit out of date and so fixed some of the renaming. Created both a d6_ubercart_product_variation.yml and a d7_ubercart_product_variation.yml because of differences in the dependencies.

There will be test failures, but hopefully the d6 ProductVariationTest will pass now.

The next step is to add a MigrateStore helper function to Ubercart7TestBase.php.

Status: Needs review » Needs work

The last submitted patch, 9: 2873500-9.patch, failed testing. View results

quietone’s picture

Issue summary: View changes
Status: Needs work » Postponed
StatusFileSize
new13.23 KB

A lot has changed since the last patch here. Starting over with a new patch so there is no interdiff.

The code for the source plugins for uc6 and uc7 product variations is the same but the extend from difference base classes, the respective node source plugins. To combine the code here a trait is used. That may not be the best solution.

TODO:
Add non product nodes to the fixture and test that they are not migrated as products.
This needs is postponed on #2995418: [Ubercart D7] Migrate field formatter

quietone’s picture

Issue summary: View changes

No longer postponed. Adding a test

quietone’s picture

Issue summary: View changes
Status: Postponed » Needs review
StatusFileSize
new32.81 KB
new23.73 KB

This adds a page node to the fixture for testing the product migration, which isn't done here. I got a little ahead of myself. Maybe I should just add the product migration here.

But first, lets test this.

quietone’s picture

StatusFileSize
new35.59 KB
new3.08 KB

Yes, add the product test here. It is just one test and no migration or extra plugins.

Status: Needs review » Needs work

The last submitted patch, 14: 2873500-14.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

quietone’s picture

Status: Needs work » Needs review
StatusFileSize
new3.03 KB

Didn't update the product id in the test properly. And remove unused use statement.

quietone’s picture

StatusFileSize
new35.54 KB

And the patch.

quietone’s picture

StatusFileSize
new39 KB
new4.96 KB

Combine the d6 and d7 code for node migration in commerce_migrate_ubercart.module

Status: Needs review » Needs work

The last submitted patch, 18: 2873500-18.patch, failed testing. View results

quietone’s picture

Status: Needs work » Needs review
StatusFileSize
new38.05 KB
new3.54 KB

Restore an if statement accidentally removed.

quietone’s picture

Status: Needs review » Reviewed & tested by the community

This is large a copy of the ubercart 6 version with the addition of combining code where possible. Because of that and tests are passing, I'm going to venture an RTBC.

  • quietone authored 4ccc084 on 8.x-2.x
    Issue #2873500 by quietone, karthikkumarbodu, mohit1604: [Ubercart D7]...
quietone’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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