It's a long story, but due to fatal errors in certain environments triggered by Migrate's automated registration of migration and handler classes, as of Migrate 2.5 it will be possible to disable auto-registration, and any module implementing migration or handler classes should explicitly register them via hook_migrate_api() to continue working when auto-registration is disabled. Adding this registration will have no effect with Migrate 2.4 or earlier.

Comments

Haza’s picture

Status: Needs review » Reviewed & tested by the community

Thanks Mike !

Looking on that.

Haza’s picture

Status: Reviewed & tested by the community » Active
jantoine’s picture

Status: Active » Needs review
FileSize
1.07 KB

Attached is a patch that declares all migration classes in the hook_migrate_api() implementation.

mrP’s picture

Status: Needs review » Needs work

#3: While the patch applied correctly, none of the classes registered as it did previously with Migrate 2.4 (tested using migrate-7.x-2.6-rc1).

related issue: #1943420: Not seeing any ubercart migration options

With the rewrites necessary to make this Migrate 2.5+ compatible, would it make sense to leverage migrate_d2d in say, a 2.x branch of commerce_migrate_ubercart?

Ryan Weal’s picture

I've done an overhaul of this module to work with migrate 2.6... going to be doing some tests before posting the code. Aim is to have something working posted here this week.

rootwork’s picture

Looking forward to this -- right now it's incompatible with the latest version of Commerce Kickstart.

Ryan Weal’s picture

OK! I haven't looked at it for a few days so I hope this isn't broken, I had it working last Thursday. It is a *major* overhaul.

  • Files and images are no longer supported with this patch. You can build a files migration with migrate_d2d, and the UI will allow you to "edit" the commerce_migrate_ubercart migration where you want the files to go.
  • It will ask you to create any missing product_types rather than force you to delete the default one.
  • Clear the caches after patching!
  • Your "group" of migrations will appear at install time. Use the "configure ubercart migration" button, then after doing that, go to "configuration" and click "re-register statically-defined classes" just in case.

I believe the updates contained herein may also solve the following:

#2004570: Not all Ubercart stores have product images...
#1325670: Commerce Migrate Ubercart uninstall requires Migrate to be enabled
#1438048: Overriding dynamic migrations?
#1992316: Pass $group parameter to Migration base class constructor
#1421048: admin/content/migrate should not throw a fatal if commerce migrate ubercart not yet configured
#1645820: References to commerce_migrate_uc_file_s3 module *must* be moved into a subclass

If it makes more sense to separate this out as a separate module I'm open to it. I have already published a webform migration package that uses some of my learnings in refactoring this module: https://drupal.org/project/migrate_webform

mrP’s picture

@Ryan Weal -- thanks for your work on this. the patch doesn't apply for me because of the missing commerce_migrate_ubercart.migrate.inc file. could you reroll so it will apply or submit your patched module?

1832736-support-migrate-2-5d.patch:101: trailing whitespace.
    if (isset($target_types[$type])) {
1832736-support-migrate-2-5d.patch:198: trailing whitespace.

Checking patch commerce_migrate_ubercart.info...
Checking patch commerce_migrate_ubercart.install...
Checking patch commerce_migrate_ubercart.migrate.inc...
error: commerce_migrate_ubercart.migrate.inc: No such file or directory
Checking patch commerce_migrate_ubercart.module...
Checking patch customer_billing_profile.inc...
Checking patch line_item.inc...
Checking patch order.inc...
Checking patch product.inc...
Checking patch product_node.inc...
Checking patch product_type.inc...
Ryan Weal’s picture

Status: Needs work » Needs review
FileSize
28.32 KB

Rerolled to include the new file. Updated README.txt and cleaned up some extra spacing that came up in your error message above. Thanks for looking at it!

mrP’s picture

awesome! #9 applies cleanly as of http://drupalcode.org/project/commerce_migrate_ubercart.git/commit/d23f2...

i'll get to testing and let you know what i find.

mrP’s picture

Status: Needs work » Needs review

@Ryan Weal re:#9 --

I tested a D6 Ubercart to D7 Commerce migration and had the following experiences:

- commerce migrate ubercart options for ubercart images are gone at admin/content/migrate/ubercart_migration_options
- CommerceMigrateUbercartProductType

- CommerceMigrateUbercartCustomerBillingProfile

  • billing_street2 and delivery_street2 missing as source field (fixed in #15)
  • country missing as destination field in migrate_ui but hardcoded with prepare() function
  • Missing bundle property on entity of type commerce_customer_profile: must define default_value for destination field [type] = commerce_customer_billing (for commerce_kickstart)

- CommerceMigrateUbercartProductFoo

  • on migration view -
    "product_id" was used as source field in the "" mapping but is not in list of source fields
    "path" was used as source field in the "" mapping but is not in list of source fields
  • status missing as source field

- CommerceMigrateUbercartNodeFoo

  • on migration view - "path" was used as source field in the "path" mapping but is not in list of source fields
  • status missing as source field
  • initial Source Migration incorrect when there are multiple product types, references

- CommerceMigrateUbercartProductBar

  • migration successful unless you edit, save, and import a node - SQLSTATE[22007]: Invalid datetime format: 1366 Incorrect integer value: '' for column 'uid' at row 1: UPDATE {node_revision} SET timestamp=:db_update_placeholder_0, uid=:db_update_placeholder_1 WHERE (vid = :db_condition_placeholder_0) ; Array ( [:db_update_placeholder_0] => 1333819456 [:db_update_placeholder_1] => [:db_condition_placeholder_0] => 609 )
  • can't rollback node migrations for which there are errors

- CommerceMigrateUbercartOrder
- CommerceMigrateUbercartLineItem

  • Source Migration incorrect when there are multiple product types, references CommerceMigrateUbercartProductBar
  • Missing bundle property on entity of type commerce_line_item: must define default_value for destination field [type] = commerce_line_items (for commerce_kickstart)
Ryan Weal’s picture

Wow, great notes! Thanks.

Regarding the first two points, the removal of ProductType and Images are intentional. Images can be handled by using D2D to build a files migration and you can map it to your fields that way. All of the images-related stuff needed to be completely refactored to use the new API and I simply don't have any sites that even use that field. Since there is a way to get it to work using the UI I think that is fine.

For ProductTypes, it should alert you if one is missing. This is better than the old way - which was to fail completely unless you deleted a default setting before installing. This is easily doable in the UI by a novice in just a few clicks but probably won't be needed if people were running with the defaults.

The fields that are not mapped were probably not there in the original code, so that would be why those are missing... though the product_id one is curious and might be important.

Path module is supported but must be enabled on the site I think... that functionality is probably the same as the original. Maybe someone could add some detection code for this... personally I always use path.

I don't know what you mean by this: "edit, save, and import a node" ... are you saying you are editing and re-importing? Were you running this with mirage 2.5 or 2.6? I haven't seen any rollbacks fail yet on any of the sites I've tried. It could be a problem with the date handler... and I know the date module was not up-to-date with their migration includes until recently so it might be that. Could also try migrate_extras, I can't recall if it has a specific date handler or not.

The last two items are worth a look and probably an easy fix. The multiple source migrations one is likely hardcoded when it should be an array from the arguments. It probably just missed the last refactoring around that issue.

Anyway, greatly appreciated. I'm not sure if and when I'll get to any of this, but I will be looking at it again before I roll anything live.

mrP’s picture

Status: Needs review » Needs work

Regarding the first two points, the removal of ProductType and Images are intentional. Images can be handled by using D2D to build a files migration and you can map it to your fields that way. All of the images-related stuff needed to be completely refactored to use the new API and I simply don't have any sites that even use that field. Since there is a way to get it to work using the UI I think that is fine.

I figured this was intentional, but thought I'd note it just in case. Thanks for the clarification.

For ProductTypes, it should alert you if one is missing. This is better than the old way - which was to fail completely unless you deleted a default setting before installing. This is easily doable in the UI by a novice in just a few clicks but probably won't be needed if people were running with the defaults.

My experience was that the alerts weren't entirely friendly. In my test cast, I had three product types and it nicely alerted me about one of them. It wasn't until I went into the migration group that it threw a nasty SQL error about missing migration classes for the other two product types.

Path module is supported but must be enabled on the site I think... that functionality is probably the same as the original. Maybe someone could add some detection code for this... personally I always use path.

path was enabled on both destination and source. path was available as a destination field, but not a source field.

I don't know what you mean by this: "edit, save, and import a node" ... are you saying you are editing and re-importing? Were you running this with mirage 2.5 or 2.6? I haven't seen any rollbacks fail yet on any of the sites I've tried. It could be a problem with the date handler... and I know the date module was not up-to-date with their migration includes until recently so it might be that. Could also try migrate_extras, I can't recall if it has a specific date handler or not.

Yeah, this was a bit of a weird one as it only happened for one node type. And, it just so happened that it was the one node type I made manual changes to the migration mapping. Even after reverting all the fields to the same as an OOTB node migration, I still got this error. It seemed to me that it wasn't storing the default uid properly on edit, but to be frank, I didn't spend much time on this error.

Anyways, thanks again for making strides on this. I'll try to make some time and dig through the code to make some improvements. Marking back as needs work for now.

Ryan Weal’s picture

Interesting note on the detection of missing types. I thought I had covered multiples, but I must not have tested that enough.

Path... that we should definitely fix!

Thanks again for the review. My next rollout will likely be in the next 1-2 weeks so if you don't get to it within that time I probably will.

mrP’s picture

I got the low[est] hanging fruit. This patch adds in billing_street2 and delivery_street2 to customer_billing_profile.inc while bundling all your changes in #9.

After further digging wrt customer_billing_profile.inc, country does get migrated properly during the migration as its hardcoded in the prepare($profile, stdClass $row) function. There just aren't options to change this in migrate_ui, though I doubt its needed for nearly all use cases.

Ryan Weal’s picture

Status: Needs review » Needs work

I have some country code processing in another one of my migrations, maybe I can bring it into this. That country code thing has always been a problem with this module I think.

Good stuff with the latest patch! Every bit helps.

DamienMcKenna’s picture

Also, the commerce_migrate_ubercart.migrate.inc file was missing from the patch in #15 again ;-)

One other thing - you don't need to add commerce_migrate_ubercart.module to the info files[] list.

Ryan Weal’s picture

This cleans up many of the things for #9 to address issues in #11 and merges in the changes in #15.

  • First name / Last name now works
  • Product status mapped (no longer just default to active)
  • Country is confirmed working (question in #11 - code is only there to default to US if none given)
  • Company is confirmed working
  • Products now assigned LANGUAGE_NONE (nodes were previously default to this - should get changed)
  • Missing bundle property for billing profiles is now a configuration option that must be set
  • Path warning message is indeed just a warning - confirmed working... "problem" is that the variable isn't defined yet (but is at runtime)
  • Currency is queried from the Ubercart site and set for each item (may have caused items not to show in some cases).

No changes for:

  • Duplicates / integrity constraint errors - for now this is by design to preserve IDs - just delete your bad content occupying that ID and re-import.
  • Missing bundle on commerce_line_item - not sure how to approach this one yet
  • Multiple sourceMigrations problem with product types - have not looked into this
Ryan Weal’s picture

You guys, we forgot to celebrate 1 year of this issue! Happy birthday #1832736 !

DamienMcKenna’s picture

Ryan Weal’s picture

Hello everyone, rfay granted me commit access... so if I can get some reviews on this latest patch then some non-Ryan Weal person should RTBC this if it works. ;)

DamienMcKenna’s picture

I'm looking through it while manually writing scripts to finish a site converted using some custom scripts (yeah, I know).

I noticed the order statuses needed some review. The Ubercart v6.x-2.something version this particular site runs has seven statuses:

  • canceled
  • completed
  • in_checkout
  • payment_received
  • paypal_pending
  • pending
  • processing

Commerce Order, on the other hand, has the following:

  • cart
  • pending
  • canceled
  • checkout_checkout
  • completed
  • processing
  • checkout_shipping
  • checkout_review
  • checkout_payment
  • checkout_complete

I'm trying out the following mapping:

  $statuses = array(
    'canceled' => 'canceled',
    'completed' => 'completed',
    'in_checkout' => 'checkout_checkout',
    'payment_received' => 'pending',
    'paypal_pending' => 'pending',
    'pending' => 'pending',
    'processing' => 'processing',
  );

Is that correct?

mrP’s picture

Good call Damien. As it currently stands in the module, the only manually adjusted order status is payment_received => checkout_payment with the note that @todo Revisit this, these two statuses are not really the same.

My opinion is that payment_received => processing.

  function prepare($order, stdClass $row) {
    // Most order statuses (pending, cancelled, completed) are the same
    // between the two systems. However, some are not.
    if ($order->status == 'payment_received') {
      $order->status = 'processing';
    }
    if ($order->status == 'in_checkout') {
      $order->status = 'checkout_checkout';
    }
    if ($order->status == 'paypal_pending') {
      $order->status = 'pending';
    }
  }
mrP’s picture

Another note, if you want to use the "User ownership should be mapped" option, you can use migrate_ui + #2105037: is_new: can't map destination uid/nid for users/nodes in migrate_ui to maintain uid. This keeps all the Customer Billing Profiles intact.

mrP’s picture

I'll go ahead and reroll the patch with changes from #23 for further testing/refinement.

Also, @Ryan, do you want to create a 2.x branch so we can tag this and other issues to it specifically?

Ryan Weal’s picture

@mrP yes I think a 2.x branch is definitely in order here, I was thinking the same thing. Today I'm swamped with tasks already so will try to put that live over the next few days.

Ryan Weal’s picture

The 2.x branch has been created, it will start doing nightly builds tonight.

I reviewed the change in #24 and committed that. We should start using interdiffs!

Ryan Weal’s picture

The branch did not appear on the drupal.org side of things... even though it has been pushed. I'm thinking this is a drupal.org upgrade bug or something. Going to wait it out a bit and see if it appears.

mrP’s picture

Nice nice nice!!

git clone --branch 7.x-2.x http://git.drupal.org/project/commerce_migrate_ubercart.git works perfectly.

And its browseable at http://drupalcode.org/project/commerce_migrate_ubercart.git/shortlog/ref...

Ryan Weal’s picture

Thanks for the tip mrP! Glad to see it is somewhat available. It sounds like a broader issue as to why it is not available on the project page yet. Confirmed in IRC that it is a problem.

creativepragmatic’s picture

I am trying to migrate from an Ubercart 6 store using version 2. With some code modifications (that I hope to share here as a patch when finished), CustomerBillingProfile was successful but I don't understand how to migrate Product.

I first deleted the default Product type.

After doing so, the following warnings appear at 'admin/content/migrate/groups/commerce_ubercart/Product':

"commerce_price" was used as destination field in "sell_price" mapping but is not in list of destination fields
"commerce_price:currency_code" was used as destination field in "" mapping but is not in list of destination fields

Using the module to migrate with no product types defined, results in a successful migration of the product 'SKU', 'Title' and 'Status' fields. 'Type' is empty. The price and currency code are not created and migrated.

I know Drupal Commerce but have not used the Migrate module. Is there something I should be doing that I am not?

Ryan Weal’s picture

@creativepragmatic - the "delete product type" step should be removed from any documentation - we're not doing that step anymore in the 2.x branch (I assume you are using this?). So you are seeing the warning to *create* that type yourself because the migration sees it as missing. The subsequent notes about the fields are the mandatory fields needed for commerce to work so you will have to add those back also. If you haven't configured Commerce yet, you might just want to disable then uninstall the module, then re-enable (that *should* recreate the type). Or just add those two fields.

We did this to better support the default configurations since people might already have data in the product type.

joelstein’s picture

I'd like to point out that the commit mentioned in #27 (from #25) doesn't include the new commerce_migrate_ubercart.migrate.inc file. Thus, no migrations appear in the UI.

joelstein’s picture

Here's the missing commerce_migrate_ubercart.migrate.inc file (note that I had to use .txt to upload it, but you'll change it to .inc in your module). This is from one of the patches in #20. If you put this in your module, and then click "Register statically-defined classes" at admin/content/migrate/configure, you'll see the Commerce Migrate Ubercart migration group on the Migrate dashboard.

This should be committed to the repo.

mrP’s picture

dang, sorry joel -- that was totally my fault. i was pushing to get a commit and rushed the patch.

@Ryan would you mind committing in commerce_migrate_ubercart.migrate.inc when you get a sec? I can roll a new patch if thats easiest.

mrP’s picture

Version: 7.x-1.x-dev » 7.x-2.0-alpha1
FileSize
4.34 KB

adds missing commerce_migrate_ubercart.migrate.inc

joelstein’s picture

Thanks!

Ryan Weal’s picture

Ugh, I should have caught that. I'm sorry! I swore I had fixed it... I'll try to get another update in over the next few days.

Ryan Weal’s picture

The missing file has been corrected in the 7.x-2.0-alpha2 release published this morning.

If someone wants to give this a test maybe we can RTBC this meta issue and move on to individual issues for new/missing functionality.

Ryan Weal’s picture

Status: Needs review » Fixed

I'm declaring this to be fixed since 7.x-2.0-alpha2.

For those that are interested, there was a major fix in 7.x-2.0-alpha3 (tag, not release) which is included in the now-published 7.x-2.0-alpha4 release. It will fix any broken Views you have that aren't showing your products correctly. The alpha4 update will dedupe your SKUs rather than discarding them for better data integrity. Enjoy!

Thank you all for the reviews and your help to make the 2.x branch possible!

Status: Fixed » Closed (fixed)

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

mrP’s picture

@Ryan Weal -- Thanks for making this happen