Cannot add variation to cart, due to failed store detection

I think it is broken. See my comment in #2894733-13: Convert Commerce Product and ProfileBilling tests to use CommerceMigrateTestTrait

We have a failing test in #2899321: Refactor Commerce Product and ProfileBilling tests.

Postponing on #2899426: Product and variation id mapping issues

#2899426: Product and variation id mapping issues is in back to active

Tested this today. On a fresh install, I used the UI to migrate the test fixture, ck2. After migration, navigated to product/29, clicked add to cart and there was no error. Navigated to /cart and the product is shown.

I reckon this is fixed, so closing.

@mglaman, I'm sure you'll reopen this if it is still a problem.

I'm having this issue. None of my migrated products will add to the cart.

Here's part of my yml file.

stores/target_id: stores
variations/target_id:
    plugin: migration_lookup
    migration: uc6_product_variation
    source: tnid

When I look in devel it appears to be set:

"stores" => array:1 [▼
      "x-default" => array:1 [▼
        0 => array:1 [▼
          "target_id" => "1"
        ]
      ]
    ]

But I still get this error when I try to add any migrated product to the cart. If I create a new product it's fine.

The given entity is not assigned to any store. in <em class="placeholder">Drupal\commerce_cart\Form\AddToCartForm-&gt;selectStore()</em> (line <em class="placeholder">255</em> of <em class="placeholder">modules/contrib/commerce/modules/cart/src/Form/AddToCartForm.php</em>). <pre class="backtrace">Drupal\commerce_cart\Form\AddToCartForm-&gt;buildEntity(Array, Object) (Line: 187)
Drupal\Core\Entity\ContentEntityForm-&gt;validateForm(Array, Object) ...

I should also mention I'm migrating from D6 Ubercart.

I've figured it out. I noticed the product_id backreference in commerce_product_variation_field_data was for a product_id that no longer existed. I’ve done the product migration a number of times and rolled back to make changes and try again but never rolled back the variations migration because they kept being referenced in the product so I thought I didn’t need to. It appears the product_id is only updated correctly after the initial products migration. I'm not sure if there is a solution to this particular issue, it appears they are set to NULL initially is it possible to reset variation's product_id to null when rolling back the product migration? Is this noted somewhere and I missed it? Perhaps it should be noted in the uc6_product_variation.yml file where this is process is explained.

@jphelan, Thanks for the tracking down the cause!

So, mglaman's suspicion was correct. And this has to do with how the rollbacks are done.

it appears they are set to NULL initially is it possible to reset variation's product_id to null when rolling back the product migration?

Yes, it is possible, but is wasn't done because of the performance impact. Clearly, that needs to be looked at agai.

Is this noted somewhere and I missed it?

No, you were the first to find this particular problem. The rollback is explained a bit in the Known Issues page, but it doesn't touch on the problem you've found.

The next step is to reproduce the problem in Ubercart since that is the source where jphelan found this problem. Setting the component to Code since this is probably a problem for Commerce 1 as well.

I have been able to reproduce a case where the product variation back reference becomes NULL, though it isn't identical to what jphelan reported. To begin I used the UI to migrate the test fixture, then tried to add a product to the cart, which failed but with a different error that there was a missing property, 'territories'. That has to do with taxes, so I set up a territory and then was able to add a product to the cart. I then removed the product from the cart and started using drush to investigate the problem reported by jphelan.

drush mr uc6_product_variation
drush mim uc6_product_variation

The back references are now NULL, and the product variations have a new id and so will no match the reference in the product table. The products were not rolled back. It wasn't possible to add a product to the cart here because the 'add to cart' button was not displayed.

To fix this problem rollback the product and re-import them and that will correct the references.

drush mr d6_node:product
drush mim d6_node:product

The back references are now correct.

@jphelan, is it possible you encountered this scenario, that is that the product variations were rolled back while the products were still migrated.

Since that has reappeared again maybe there should be a test for adding to cart.

Change the path for a product in Commerce 1 test.

Add the property.

Now, to follow up on the failure reported in #11. When I tried to add a product after the Ubercart migration I received this error,

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">Drupal\Core\Entity\EntityStorageException</em>: Missing required property &quot;territories&quot;. in <em class="placeholder">Drupal\Core\Entity\Sql\SqlContentEntityStorage-&gt;save()</em> (line <em class="placeholder">829</em> of <em class="placeholder">core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php</em>). <pre class="backtrace">Drupal\commerce_tax\Plugin\Commerce\TaxType\Custom-&gt;buildZones() (Line: 352)
Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase-&gt;getZones() (Line: 185)
Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase-&gt;matchesAddress(Object) (Line: 96)
Drupal\commerce_tax\Plugin\Commerce\TaxType\LocalTaxTypeBase-&gt;applies(Object) (Line: 39)
Drupal\commerce_tax\TaxOrderProcessor-&gt;process(Object) (Line: 155)
Drupal\commerce_order\OrderRefresh-&gt;refresh(Object) (Line: 92)
Drupal\commerce_order\OrderStorage-&gt;doPreSave(Object) (Line: 389)
Drupal\Core\Entity\EntityStorageBase-&gt;save(Object) (Line: 820)

The tax type migration is not setting the territories array. To fix that, the source plugin now gets the country code variable, converts it to a 2 character code and puts in on the row. The migration then puts into the territories configuration. This patch include those changes and updates the source plugin, source plugin test and migration test.

@quietone I believe I was migrating the reverse of you. I migrated the product variations first and then the products. I believe the nid backreference should be NULL and commerce_migrate sets it to the new nid when you migrate the products. I was rolling back the products but not the product variations, so the nid backreference was referencing the old products and was not updated to the new ones. It's also odd to me that resaving the product variation does not update the backreference to the correct nid. Seems like either way they both have to be rolled back at the same time right now.

I moved the fixes to the tax type to a new issue, #2977682: Add territory to tax type migration, which in turn means this is postponed on that.

This patch removes the tax type changes. With this patch the adding of a product to the cart post migration works for Commerce 1 but not Ubercart 6.

Ubercart needs a default store and that migration is in this issue #2977978: Add default store migration.

This patch works with the territory patch and the default store patch installed. However, after the migration for Commerce 1 and Ubercart 6 an additional drupalGet is needed, otherwise attempting to load the product page results in a 502.

Both #2977682: Add territory to tax type migration and #2977978: Add default store migration are committed so this is no longer postponed.

The failure is because core is not validating the file paths. The ubercart migration test was using a source_base_path that did not exist. The source_base_path is now an empty string.#2562845: Validate file path on Credential form.

Improve the summary line for the test classes.

This is not testing the entity counts of the full migration, so removing the entity counts.

Restesting

One question, as mentioned in #22. After the migration and before loading a product page another page must be loaded or a 502 occurs. Why is that needed?

1. +++ b/modules/commerce/tests/src/Functional/commerce1/MigrateUpgradeAddToCartTest.php
   @@ -0,0 +1,163 @@
   +    $this->drupalGet('/about');
   +    $this->drupalGet('/product/15');
   

   In this test access '/about' before a product page.

2. +++ b/modules/ubercart/tests/src/Functional/uc6/MigrateUpgradeAddToCartTest.php
   @@ -0,0 +1,163 @@
   +    $this->drupalGet('/admin/commerce/config/stores');
   +    \Drupal::service('commerce_cart.cart_provider')->createCart('default');
   +    $this->drupalGet('/product/2');
   

   In this test access an admin page before the product page.

Ran the Ubercart 6 test locally without the extra step of loading a page before loading the product and it passed. Trying again with the testbot.

OK, the extra page load appeared in #21, although it is mentioned in the next comment. It should not have been needed, I never investigated why it was needed, but it isn't anymore. So, let's go with it.

Woohoo!