Problem/Motivation

Product attributes and their options need a migration. The db tables are:

  • uc_attributes - defines the attribute name, label, ordering, require, display type(text, checkbox etc) and description.
  • uc_attribute_options - the option, cost, price, weight, and ordering.
  • uc_product_attributes links the attribute to a product.

Proposed resolution

Create field storage from uc_attributes - added in patch #3
Migrate the product attribute - added in patch #4
Migrate the product attribute options - added in patch #6

Remaining tasks

Review

User interface changes

N/A

API changes

N/A

Data model changes

N/A

CommentFileSizeAuthor
#23 2893389-23.patch19.17 KBquietone
#23 interdiff.txt2.65 KBquietone
#21 interdiff.txt7.36 KBquietone
#21 2893389-21.patch19.31 KBquietone
#20 2893389-20.patch19.17 KBquietone
#19 2893389-19.patch19.24 KBquietone
#18 2893389-18.patch16.15 KBquietone
#16 2893389-15.patch19.28 KBquietone
#15 interdiff.txt1.35 KBquietone
#11 2893389-11.patch19.31 KBquietone
#10 2893389-10.patch19.23 KBquietone
#8 interdiff.txt2.12 KBquietone
#8 2893389-8.patch18.55 KBquietone
#6 interdiff.txt14.34 KBquietone
#6 2893389-6.patch18.57 KBquietone
#5 2893389-4.patch8.78 KBquietone
#4 interdiff.txt4.26 KBquietone
#3 2893389-3.patch5.65 KBquietone
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

quietone created an issue. See original summary.

quietone’s picture

quietone CreditAttribution: quietone commented
Issue summary: View changes
quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
2893389-3.patch5.65 KB

For the field storage

quietone’s picture

quietone CreditAttribution: quietone commented
Issue summary: View changes
FileSize
interdiff.txt4.26 KB

Add in the migration of the product attribute. This doesn't include the attribute options. This is rough, the docs are incomplete and I'm not sure about the elementType in the migration. That needs more testing.

 elementType:
   plugin: static_map
   bypass: true
   source: display
   map:
   # Text field
     0: text
   # Select box
     1: select
   # Radio buttons
     2: radios
   # Checkboxes
     3: checkbox
quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
2893389-4.patch8.78 KB

That was odd, I am sure I uploaded the patch. Anyway, here it is.

quietone’s picture

quietone CreditAttribution: quietone commented
Issue summary: View changes
FileSize
2893389-6.patch18.57 KB
interdiff.txt14.34 KB
quietone’s picture

quietone CreditAttribution: quietone commented
Status: Active » Needs review
quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
2893389-8.patch18.55 KB
interdiff.txt2.12 KB

Change migration label to start with 'Ubercart' so they will be easier to find in the config UI. Correct doc comments

quietone’s picture

quietone CreditAttribution: quietone commented
Priority: Normal » Major

Moving product migrations to major.

quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
2893389-10.patch19.23 KB

Move the changes to the ubercart submodule.

quietone’s picture

quietone CreditAttribution: quietone commented
FileSize
2893389-11.patch19.31 KB

Added source_provider to the source plugin annotation.

Status: Needs review » Needs work

The last submitted patch, 11: 2893389-11.patch, failed testing. View results

quietone’s picture

quietone CreditAttribution: quietone commented
Status: Needs work » Needs review

The test aborted. Passes on retest.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Needs work 
  1. +++ b/modules/ubercart/migration_templates/d6_ubercart_attribute.yml
@@ -0,0 +1,43 @@
+id: d6_ubercart_attribute

    Can we call this d6_ubercart_attribute_field_config, or some such? It took me a while to figure out this was different than d6_ubercart_product_attribute 

  2. +++ b/modules/ubercart/migration_templates/d6_ubercart_attribute.yml
@@ -0,0 +1,43 @@
+  - Drupal 6
+  - Drupal 6 Ubercart

+++ b/modules/ubercart/migration_templates/d6_ubercart_attribute_value.yml
@@ -0,0 +1,20 @@
+  - Drupal 6
+  - Drupal 6 Ubercart

+++ b/modules/ubercart/migration_templates/d6_ubercart_product_attribute.yml
@@ -0,0 +1,28 @@
+  - Drupal 6
+  - Drupal 6 Ubercart

    Can we just call this Ubercart and drop the D6 part? 

  3. +++ b/modules/ubercart/migration_templates/d6_ubercart_attribute.yml
@@ -0,0 +1,43 @@
+    langcode: en
...
+  langcode: 'constants/langcode'

+++ b/modules/ubercart/migration_templates/d6_ubercart_attribute_value.yml
@@ -0,0 +1,20 @@
+    langcode: en
...
+  langcode: constants/langcode

    Are we sure that it 'en' is the only language possible? 

  4. +++ b/modules/ubercart/migration_templates/d6_ubercart_attribute.yml
@@ -0,0 +1,43 @@
+    plugin: static_map
+    bypass: true
+    source: display
+    map:
+    # Text field
+      0: 1
+    # Select box
+      1: 1
+    # Radio buttons
+      2: 1
+    # Checkboxes
+      3: -1

    Should a default value be provided? Just asking, I don't know the answer.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
interdiff.txt1.35 KB

Only fixed #2.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2893389-15.patch19.28 KB

Silly me, I didn't upload the patch.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs review » Needs work

Back to needs work for #14.1, #14.3 and #14.4.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
2893389-18.patch16.15 KB

First, reroll.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2893389-19.patch19.24 KB

Let's try that reroll again.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2893389-20.patch19.17 KB

Oh, you've got to be kidding. I fixed that.

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
FileSize
2893389-21.patch19.31 KB
interdiff.txt7.36 KB

14.1. Rename d6_ubercart_attribute to d6_ubercart_field_attribute. I chose that because core d6_field write the field config and it puts emphasis on the field rather than the specific field, attribute.

14.3. Don't know about other languages and fields. Core has settled on using configuration 'translations: true' for migrations translations and having a separate migration for the translations. I grabbed langcode pattern from core d6_field. There is issue about translatable fields which has just the one comment from mikeryan. #2789927: How to migrate translatable fields?

#14.4. Don't think it needs a default value. But can be convinced otherwise. Anyone have an example where a default is needed/helpful? Added a comment in the yml about where the data comes from.

Removed migration_groups from the migrations, moved the new assertions to be alphabetical in the CommerceMigrateTestTrait.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Needs work

Just some small nits.

  1. +++ b/modules/ubercart/tests/fixtures/uc6.php
@@ -20666,11 +20702,38 @@ $connection->insert('uc_attributes')
+  'description' => 'Avaiable towel colors',

    Nit: Available. 

  2. +++ b/modules/ubercart/tests/src/Kernel/Migrate/d6/MigrateAttributeTest.php
@@ -0,0 +1,82 @@
+  protected function assertEntity($id, $expected_type, $expected_translatable, $expected_cardinality, array $expected_dependencies) {

+++ b/tests/src/Kernel/CommerceMigrateTestTrait.php
@@ -193,6 +195,47 @@ trait CommerceMigrateTestTrait {
+  protected function assertProductAttributeEntity($id, $expected_label, $expected_element_type) {
...
+  protected function assertProductAttributeValueEntity($id, $expected_attribute_id, $expected_name, $expected_label, $expected_weight) {

    Can we drop the 'expected_' part from the variable name?

quietone’s picture

quietone CreditAttribution: quietone at Acro Commerce commented
Status: Needs work » Needs review
FileSize
interdiff.txt2.65 KB
2893389-23.patch19.17 KB

1. Fixed
2. Old habit. Yes, expected_ has been removed.

heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Needs review » Reviewed & tested by the community

  • heddn committed 9994925 on 8.x-2.x authored by quietone 
    Issue #2893389 by quietone, heddn: [Ubercart D6] Migrate product...
heddn’s picture

heddn
Primary language English
Location Nicaragua
CreditAttribution: heddn at MTech, LLC, Acro Commerce commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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