The AddToCartForm needs to work with any purchasable entity.
But when we fixed the "multiple add to cart forms interfere with each other" bug, we introduced this into getFormId():

    $product_id = $this->entity->getPurchasedEntity()->getProductId();
    $form_id = $this->entity->getEntityTypeId();
    if ($this->entity->getEntityType()->hasKey('bundle')) {
      $form_id .= '_' . $this->entity->bundle();
    }
    if ($this->operation != 'default') {
      $form_id = $form_id . '_' . $this->operation;
    }
    $form_id .= '_' . $product_id;

That code needs to be replaced with something generic.
I thought about a variation ID, but that one changes on #ajax, which would probably break the form.
Perhaps we simply use a static counter?

Comments

bojanz created an issue. See original summary.

bojanz’s picture

steveoliver’s picture

Title: AddToCartForm uses a product variation specific method » AddToCartForm should support all purchasable entities
Component: Developer experience » Cart
Status: Active » Needs review
steveoliver’s picture

(Build passed.)

steveoliver’s picture

While working on something else I noticed Drupal\Component\Utility\Html::getUniqueId. Would that be helpful?

mglaman’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me, we have coverage in MultipleCartFormsTest and tests passing. I guess the problem with getUniqueId is that it's for HTML (outputs - when form IDs need _ [which maybe they do not anymore?]). So we'd have to copy a lot of that code for seenIds and such.

I think the static counter is simplistic and gets the job done.

steveoliver’s picture

This is ready after a small change suggested by @bojanz on the GitHub PR.

See https://github.com/drupalcommerce/commerce/pull/559/files:

diff --git a/modules/cart/src/Form/AddToCartForm.php b/modules/cart/src/Form/AddToCartForm.php
index 257a8dd..f1cbaac 100644
--- a/modules/cart/src/Form/AddToCartForm.php
+++ b/modules/cart/src/Form/AddToCartForm.php
@@ -55,6 +55,13 @@ class AddToCartForm extends ContentEntityForm {
   protected $chainPriceResolver;
 
   /**
+   * A unique id to use for each instance of this form.
+   *
+   * @var int
+   */
+  protected static $formUniqueId = 0;
+
+  /**
    * Constructs a new AddToCartForm object.
    *
    * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager
@@ -78,6 +85,8 @@ public function __construct(EntityManagerInterface $entity_manager, CartManagerI
     $this->orderTypeResolver = $order_type_resolver;
     $this->storeContext = $store_context;
     $this->chainPriceResolver = $chain_price_resolver;
+
+    self::$formUniqueId++;
   }
 
   /**
@@ -105,7 +114,6 @@ public function getBaseFormId() {
    * {@inheritdoc}
    */
   public function getFormId() {
-    $product_id = $this->entity->getPurchasedEntity()->getProductId();
     $form_id = $this->entity->getEntityTypeId();
     if ($this->entity->getEntityType()->hasKey('bundle')) {
       $form_id .= '_' . $this->entity->bundle();
@@ -113,7 +121,7 @@ public function getFormId() {
     if ($this->operation != 'default') {
       $form_id = $form_id . '_' . $this->operation;
     }
-    $form_id .= '_' . $product_id;
+    $form_id .= '_' . self::$formUniqueId;
 
     return $form_id . '_form';
   }

  • bojanz committed 332ecf0 on 8.x-2.x authored by steveoliver
    Issue #2827721 - AddToCartForm should support all purchasable entities...
bojanz’s picture

Status: Reviewed & tested by the community » Fixed

Thanks!

steveoliver’s picture

Related issues: +#2831895: AddToCartForm should support multiple instances
StatusFileSize
new764 bytes

This caused this issue: #2831895: AddToCartForm should support multiple instances - the static counter doesn't work when having multiple products on one page.

By committing this diff:

diff --git a/modules/cart/src/Form/AddToCartForm.php b/modules/cart/src/Form/AddToCartForm.php
index b7efc34..9d18170 100644
--- a/modules/cart/src/Form/AddToCartForm.php
+++ b/modules/cart/src/Form/AddToCartForm.php
@@ -88,7 +88,6 @@ public function __construct(EntityManagerInterface $entity_manager, CartManagerI
     $this->storeContext = $store_context;
     $this->chainPriceResolver = $chain_price_resolver;
 
-    self::$formInstanceId++;
   }
 
   /**
@@ -123,7 +122,7 @@ public function getFormId() {
     if ($this->operation != 'default') {
       $form_id = $form_id . '_' . $this->operation;
     }
-    $form_id .= '_' . self::$formInstanceId;
+    $form_id .= '_' . $this->entity->getPurchasedEntity()->id();
 
     return $form_id . '_form';
   }

we can still support all purchasable entities. Should we commit that to this issue?

bojanz’s picture

Status: Fixed » Needs work

I don't see how that can work since #ajax changes the selected variation, thus changing the purchasable entity ID.

Why would the static counter fail? Core reuses the form instance?

steveoliver’s picture

Status: Needs work » Needs review

I'm not sure exactly why the static counter fails - the form ids do seem to be incremented as expected. But currently multiple add to cart forms are broken, and are fixed by using an entity id.

There may be issue(s) with #ajax--in fact I know of at least one, and will create a separate issue--but they are not caused by current committed code or the change I'm proposing (to use the purchased entity id), as the forms ids are already mangled when rebuilt and sent back from ajax calls. Ajax field replacement on variation change is handled by ProductVariationFieldRenderer::buildAjaxReplacementClass, and that is actually broken with the current static counter and fixed by using the purchasable entity id in the form id.

Ajax field replacement does not work as expected if you have the same product add to cart form more than once on a page -- it seems because of the replaceRenderedField targeting only a field class, when it should probably be targeting that class within the scope of the form in question.

I propose we commit the follow-up child issue, which will get multiple add to cart forms working again. Then we can work on the ajax issue(s).

bojanz’s picture

Okay, so we've committed a fix that passed tests, it introduced a regression, and we don't know why, while our tests are still green.
We now have a new fix, which works, but we also don't know why.
That means it's time to stop committing things, and start investigating / improving tests.

1) In #2831895: AddToCartForm should support multiple instances drugan mentioned that the static counter failed because of self VS static, can you check if that's true?
2) Can you check how come our tests are green?

If we can't find a quick answer to #1 and #2 I'll go ahead and revert the original fix that we did in this issue. However, I still like the simplicity of a static counter.

drugan’s picture

As for the counter we can even to make it work like this:

    $this->chainPriceResolver = $chain_price_resolver;
    $this->currentUser = $current_user;
   
-   self::$formInstanceId++;
  }

// ...
// ...

    if ($this->operation != 'default') {
      $form_id = $form_id . '_' . $this->operation;
    }
-    $form_id .= '_' . self::$formInstanceId;
+    $form_id .= '_' . static::$formInstanceId++;

But the counter is not the actual reason for not working multiple carts views. For now I've discovered that on the view we have a lot of variations but any $selected_variation ->getProductId() always returns the ID of the first product in a view list. That is interesting because when you select attributes' options or even click at the relative Add to cart button you have an expected result (if #575 is applyed).

Yesterday I had a hope that steveoliver's suggestion to replace counter with $this->entity->getPurchasedEntity()->id() will help to find the involved product but this is actually the ID of every first (chosen by default) variation on every product. If anyone know how to fetch the actually selected product in multiple carts view - please post it here, so we all know.

drugan’s picture

StatusFileSize
new2.81 KB

Just in order to not multiply issues and PRs this patch could be applied above the latest commit on #578.

The static counter does not work because:

1. The sorting order of ajax automatically added counters on a view is a total mess up. At first when you load the page they are reversed starting from 10 to 1 Add to cart forms. But when trigger the ajax by choosing an attribute's option it changes to normal 1 to 10 order. Obviously that our counter cannot not work in such environment because it always starts from 1++.

2. Automatically generated #id for each attribute select list appears not unique and that is why you thrown to the top of the page when ajax replaces elements.

3. The suggested by steveoliver $this->entity->getPurchasedEntity()->id() is also not unique enough as there are might be a lot of variations with the same id just because they are added to a product as "existing variation".

My suggestion is to concatenate variation ID, product ID, created and changed time and sha1() the resulted string just to hide this data from prying eyes. Not sure with this kind of hashing but think it should be enough.

Yes, there still could be the case when one and the same product/variation (add to cart) appears two or more times on the page but it is not the big issue because whatever instance of the same add to cart form you use - you will add to a shopping cart the same variation.

agoradesign’s picture

The intention behind the static counter was to get a product/product variation independent solution, as the AddToCartForm should be able to handle any purchasable entity... Maybe the PurchasableEntity Interface needs to be extended to return a correct identifier, where e.g. product variations will include their parent product, other implementations may behave differently. However, that would not be an ideal solution, if this additional function would have no other usage justification than this.

Does your solution work properly with the Ajax callback on selecting different variations (does the field replacement work properly)?

drugan’s picture

@agoradesign

Yes, it works. To reproduce:

1. Create a view of product type(s) having variable number of attributes.
2. Go to a view page and try choose different options on attributes.
3. Check if title, sku, price changes for a chosen variation.
4. Add to cart chosen variation.
5. Go to cart page and check if the variation is added correctly.

As for the counter it was said that incrementing nature of a that identifier will not work with the current views/ajax workflow. Because it reverts the view order after the first ajax field replacing. Look, not the order of products (add to cart forms) but the automatically generated by ajax counter identifiers. For example, when you first load the page the identifiers are in reverse order: --10 through NULL if you have 10 add to cart forms on the page. But when you trigger ajax the order becomes NULL through --10. At the same time our add to cart form counter always starts from 1 and then increments. And that does not work. To abstract from any external code handling an ajax request we need unique identifier for each add to cart form based on some persist data, not the counter. In addition to concatenated product ID, variation (purchasable entity) ID, variation created time and changed time might be added a SKU which is unique for each variation but not sure with the latest because current drupalcommerce has some possibility to add SKU-less variation to a product.

bojanz’s picture

I'll review the entire issue later, just wanted to clarify this:

3. The suggested by steveoliver $this->entity->getPurchasedEntity()->id() is also not unique enough as there are might be a lot of variations with the same id just because they are added to a product as "existing variation".

Variations cannot be shared between products, they always belong to a single one.
We'll need to remove the IEF "add existing" button, otherwise the system will explode in fun ways.

drugan’s picture

StatusFileSize
new3.92 KB

The #17 patch is outdated as it does not work in some circumstances. Instead use this one or grab the latest version of it here.

Also, as no moving were made on #578 in the last time I've made a new #582 PR.

The solution works with multiple add to cart forms having different number of attributes. Also works with add to cart duplications on a page but the only deficiency is that all SKUs, titles and prices are replaced properly on all the twin carts but the attribute value is replaced just on the element which is actually triggered an ajax event. Obviously it requires a solution for this complex add to carts use case but for now it might be easily resolved by excluding the same add to cart forms on a page or just ignoring ,,the feature,, as it does not break checkout workflow in any way except UX issue.

Important note:
if you still require multiple duplicated add to cart forms on a page you need to disable views caches on the each next view (page or block) which has the same add to cart forms as the previous one.

no cache

drugan’s picture

StatusFileSize
new176.65 KB
steveoliver’s picture

Status: Needs review » Needs work

@drugan - This patch supports multiple add to cart forms, but it still assumes "Product/variation" is the purchasable entity. We need to support any purchasable entity.

drugan’s picture

@steveoliver

Now it should work with any purchasable entity.

Before the latest commit:

//...
    $product_id = $this->entity->getPurchasedEntity()->getProductId();
    $str = $product_id . serialize($this->entity->getPurchasedEntity()->toArray());
    $id = sha1($str);
    // For the case when on a page 2+ exactly the same add_to_cart forms.
    while (in_array($id, static::$FormInstanceIds)) {
//...

After:

//...
   $id = sha1(serialize($this->entity->getPurchasedEntity()->toArray());
    // For the case when on a page 2+ exactly the same purchasable entities.
    while (in_array($id, static::$FormInstanceIds)) {
//...

As I understand purchased entity cannot exist without some parent entity would be that Product or instance of some other class. So let's sha1() only serialized purchased entity and if that was already assigned earlier to some twin entity then make it unique in the while loop. Initially I've added product ID prefix just to avoid as much as possible entering into the while loop because of performance considerations but the overload in this case is so tiny that using product ID prefix could be skipped. The suggested solution will work without it.

At any time you can take the latest version of this patch here.

drugan’s picture

Status: Needs work » Needs review
joake’s picture

#17 fixed this issue for me, thanks @drugan

steveoliver’s picture

Issue tags: +Needs tests

(We need to get this handled sooner than later) Someone please write tests :)

hansfn’s picture

I guess the value of "me too" comments is limited. Anyway, I came here from #2786245: Wrong product added to cart and drugan's patch in comment 21 fixed the issue for me. (Using latest dev version.)

bojanz’s picture

The main blocker to this issue is the fact that we still don't have a failing test.
Unfortunately, I won't have time for this issue in the next few days.

drugan’s picture

@bojanz

ok, I'll try it.

drugan’s picture

The test for multiple Add to cart forms on a page has been moved to a FunctionalJavascript namespace and in a lot is rewritten. It requires javascript as without the AJAX it is pretty difficult to reproduce failure case. That's why the previous test was always been green despite the fact that this and some other related issues are still remaining alive.

Also, two helper methods are introduced to facilitate fetching Add to cart and Shopping cart values displayed on a test page. If you ever need to write a test for those elements I'd recommend to look into the modules/cart/tests/src/Functional/CartBrowserTestBase.php file.

eric heydrich’s picture

@drugan I used your patch and it works. But sometimes it's required to add the a product to the cart twice. Maybe this is an AJAX error. I disabled the cache for the view.

1. I click on add to cart
2. Page reloads, but no product is added to cart, no AJAX error is shown in console and no error is shown in watchdog
3. I once more click on add to cart
4. Product is normally added to cart

drugan’s picture

@Eric Heydrich

I suppose you've applied the latest diff below (not the patches posted on this page):

https://patch-diff.githubusercontent.com/raw/drupalcommerce/commerce/pul...

To reproduce the error I need the following info:

  1. Was something reported on admin/reports/dblog page or browser console after failing to add a product variation to cart?
  2. How much products (Add to cart forms) were displayed on a page?
  3. Was that the only view (page view, page view + block view, page view + block + regular product/{id} page) with multiple products displayed?
  4. Was the failed Add to cart form above, in the middle, at the bottom of the view(s)?
  5. Were the products of the same type or different?
  6. What the attributes were on a product type(s) (how much, optional? required?)?
  7. Were any visible issues when selecting attributes on an Add to cart form?
  8. Was the drupalcommerce install fresh or any additional patches/code hacks were applied?
eric heydrich’s picture

@drugan

1. There were no errors at all. No error in chrome dev tools or in dblog.
2.+3. I have two block views on one page. Each has approximately 30 products and each product has its own add to cart form.
4. Every form can produce this error. The position doesn't matter.
5. They are all the same product type with the the same product variation type.
6. There is only one attribute at each product. The attribute is prepopulated. It's a predefined number of copies of a product. For example 100,250,500. The 100 is the prepopulated default value.
7. When I choose an attribute the AJAX-Throbber is shown. But I think this is like it's expected.
8. There is only the patch mentioned above installed.

13jupiters’s picture

#24 is working without problems for me, fixing the multi add to cart issue. (Core 8.2.7/commerce-dev 3/27.) Just a note: despite the suggestion that views cache would have to be turned off for pages with multiple add to cart forms, so far I've had no problems despite leaving caching on. Just lucky?

drugan’s picture

@Eric Heydrich

Can't reproduce your issue.
What I've done:

  1. Created an attribute with 100, 250, 500 options.
  2. Created a variation type with the attribute.
  3. Created a product type with the variation type.
  4. Created 30 products of the product type with 3 variation on a product each having different SKU and price.
  5. Created a view for the product type with a page and two blocks (fields - Product: Rendered entity).
  6. Disabled cache for the blocks, leaving Tag based cache for the page.
  7. Enabled the view blocks on the Content region.
  8. Visited the view page and tested add to cart variations from different blocks and the page (30 products each duplicated 3 times on the page, total 90 products).
  9. Visited a page where only two blocks from the view are displayed and tested add to cart variations from different blocks (30 products each duplicated 2 times, total 60 products).

@13jupiters

despite the suggestion that views cache would have to be turned off for pages with multiple add to cart forms, so far I've had no problems despite leaving caching on. Just lucky?

The cache should be disabled for each next view having duplicated Add to carts: page or block (panel? not tested). So if you don't have duplicated Add to carts - you don't need to disable cache.

The reason is the views uses the result from the previous DB query made with same parameters (product ID) and therefore skipping to render a form which was already rendered and instead using the views cache for this purpose. That is great for performance but not for our Add to cart forms which get their unique AJAX wrapper ID exactly in the form rendering process.

mglaman’s picture

Issue tags: +MidCamp2017

Can someone create a View export and paste the YAML here so we can start to work on a test? Anyone, just dump a YAML here which allows us to reproduce so we can start the test process. Or how to configure it.

drugan’s picture

Well, there is a view which I use for testing the #582 PR patch.

While uploading the .yml I've got this:

For security reasons, your upload has been renamed to views.view_.commerce_products_page_blocks.yml.

So, please rename the file to the original name: views.view.commerce_products_page_blocks.yml

drugan’s picture

StatusFileSize
new967.99 KB

@mglaman

I've thought would it be more convenient for you to import DB dump with ready to use products and view and blocks enabled.

For security reasons, your upload has been renamed to acommerce.sql_.gz.

The same as on the previous upload. Just rename the file to acommerce.sql.gz
After importing run this and then log in with user admin password admin:

drush cr

I assume you have devel and admin_toolbar folders in the web/modules/contrib directory.

First, try choose a not default attribute and then add to cart. You should get an uncaught exception like this:

InvalidArgumentException: Unknown attribute field name "attribute_vehicles_attribute". in Drupal\commerce_product\Entity\ProductVariation->getAttributeValueId() (line 261 of modules/contrib/commerce/modules/product/src/Entity/ProductVariation.php).

That's because all errors reporting is enabled. Then apply the patch and try again. The exception should be gone and variations added to cart as expected.

eric heydrich’s picture

I still have the problem from #34. I have tried everything from removing modules to changing themes. I get an ajax-Error now.

1. I choose a product variation and put it in the cart. Everythings fine. The form token is y8Ut7gxb9IF2ro97FsAjbMjuqPFKkXvt6qiTJLMVn_o. Page reloads as usual.

2. I try to choose a variation and ajax-Process is terminating. Then I try to put it in the cart but nothing happens. Form Token in dev Tools is now AHa_vrhpi-__E_uyQAYR8W3O3VvGNT1WyYgFqgiJ4K0.

3. See step 1.

Is it possible that because of the changing Form Tokens AJAX can't finish every second clicks on add to cart button?

drugan’s picture

@Eric Heydrich

Please, do this:

Fresh install drupalcommerce. I suppose you have a copy no older than this commit:

commit d60f6b42627023d972478178027c57a71cb67795
Author: Bojan Zivanovic
Date: Tue May 9 13:32:31 2017 +0200

Issue #2876660 by bojanz: The Custom tax type should allow decimal amounts

Install devel and admin_toolbar modules.

Dump or copy the database of the fresh installed site. You can use it later to restore the initial state of the site.

Drop the database.

Create empty database with the same name as the dropped one.

Download the DB dump on the #39 comment.

Rename the downloaded file to acommerce.sql.gz

Import the file into the empty database created above.

cd to the site's root and run this:

drush updatedb --entity-updates

Go to /my-products page and log in to the site:

user: admin
password: admin

Try to add any product variation to cart without changing attributes. It should be added as expected.

Try to add any product variation to cart after changing some of the attributes. It should result into the fatal error. Actually, changing attributes does not change product variation title, sku and price on the Add to cart form.

Apply this patch and refresh the page with Ctrl+F5.

The products variations should be added to cart as expected.

If you still observe errors while adding product variations to cart then you need to post more info on the environment you are running your drupalcommerce site.

Idriss.BS’s picture

StatusFileSize
new581 bytes

I create a patch for this issue "Wrong product added to the cart" https://www.drupal.org/node/2880939 .The add to cart and the Ajax attributes change working fine .I hope the patch fix your problem.

pavelculacov’s picture

I dont know if i can write this bug here.

1) Content type Product with field Product reference (View mode Render Entity > View mode (Default))
2) Create single product with two variant type
3) Create two content type Product with same reference to Product
4) Create view where display all Content Type Product, where field product type is (Render Entity > View mode default)
5) Change Variant type, ajax working only for first element, not for second.
6) Cache on view is disabled.

If i understand correctly only one Product Type add to card must be on page?

make some debug, form generate only once, and for all content type is the same form. And Ajax work only for first match id in html.

Issue is found AddToCartForm should support multiple of the same purchasable entity add to cart form.

drugan’s picture

@Regnoy

Please, fresh install drupalcommerce site. I suppose you have a copy no older than this commit:

commit 178702a59749ff4698d2e8b5ca122606bb4b0059
Author: smccabe
Date: Sun May 21 23:46:13 2017 +0200

Issue #2874051 by bojanz, smccabe: Create a tax type plugin for Canadian sales tax

Make modifications on the site to reproduce your bug: add product/variation/attribute/content types, views, whatever...

Dump sites' database to my-dump.sql.gz file.

Upload the file on this page.

pavelculacov’s picture

After install fresh commerce bug persis.

if try change on second content type product attribute not working, after change on first content type product attribute , start working second form.

pavelculacov’s picture

StatusFileSize
new1.04 MB

Upload my db.

drugan’s picture

@Regnoy

Thanks for sharing the DB dump.

The set up that you use on your site (product referenced by a content type) does not work with the current patch.

I wonder why you don't use actual products to display Add to cart forms? Basically they are the same content types which could be extended with any fields, references, view modes, etc.. Also, you can display your products on the front page like content type entities do by default. How:

  • Create a page view of all product types.
  • Go to admin/config/system/site-information and change the /node path for the front page to the path of the view created.

After that you can see your products displayed dynamically just visiting example.com address without view path appended.

Note: those who uses the current patch and have duplicated Add to cart forms (the same products) on a page I'd recommend to read the #21 comment.

pavelculacov’s picture

I know that path not work for me

"I wonder why you don't use actual products to display Add to cart forms?"
For me, ease use reference, that content type is single with all fields (body, features,image,tags,related link, overview, specification, compatible devise etc more that 10 fields) and have single field reference to product type (title and custom variants). In project have more than 5 product type with custom variation types. And for me not need too duplicate fields for each Product Type, all field have Content Type, ease use reference to product type.

"Also, you can display your products on ..." i always do like you describe, for exemple in dump file i made like this.

Issue that i have i this issue links.
AddToCartForm should support multiple of the same purchasable entity add to cart form.

Thanks for response. @drugan

drugan’s picture

@Regnoy

Now I see what you require. If I were you I'd try this:

Create a desirable product type.

Go admin/commerce/config/product-types/MY-PRODUCT-TYPE/edit/fields and remove body field.

Create desirable content type with all fields (body, features,image,tags,related link, overview, specification, compatible devise etc more that 10 fields).

Create a content of the just created type.

Go admin/commerce/config/product-types/MY-PRODUCT-TYPE/edit/fields and add a reference field to Content -> Reference type -> just created content type.

Go to /admin/commerce/config/product-types/MY-PRODUCT-TYPE/edit/form-display and admin/commerce/config/product-types/MY-PRODUCT-TYPE/edit/display and set up order/widget type for the reference field to your content.

Create a product of the MY-PRODUCT-TYPE type and choose an instance of the content as a reference. Then use product as usual.

The only question remains how to remove a Title field of the referenced content. As a quick and dirty solution it might be done via CSS. If someone knows how to do it with a different method, please share so we all know.

pavelculacov’s picture

Why you suggest me to revers my ideea?:) it the same like your but visa versa. O lot of module not working with Product Type like shareThis (I must change in module to be share the product type).

Back to issue, did you see that second form not working in my backup ?

The only question remains how to remove a Title field of the referenced content. >>> override node.html.twig and use Display Suite or Pseudo Field

drugan’s picture

@Rednoy

Back to issue, did you see that second form not working in my backup ?

Yes, I did. It does not work neither before nor after applying the current patch.

Unfortunately at the moment I have no so much time to adjust the patch for working with such cases like yours one. Sorry about that.

pavelculacov’s picture

I know, thanks for attention, but please describe error in this issue. I think issue is the same with error that i found or already existed :).. Thanks again and good luck

AddToCartForm should support multiple of the same purchasable entity add to cart form.

mglaman’s picture

Assigned: Unassigned » mglaman

Sitting down and reviewing today, along with test writing.

mglaman’s picture

I just realized that in #2868637: AddToCartFormatter not working in Views a view was added which renders multiple add to cart forms for each product, and tests it. So maybe we can improve that test to prove what was written in #21?

mglaman’s picture

Status: Needs review » Postponed

Okay, I'm postponing this. The original issue was fixed. However I think this thread is now discussing other problems and we need to discern.

It seems we have two bugs being talked about in this thread

mglaman’s picture

Status: Postponed » Fixed
Related issues: +#2886614: Rework the AddToCartForm form ID handling

I am putting this back to fixed from #9, as the original report was about how we generated the form ID. Which has been fixed.

We have tests which pass for Views + multiple add to cart forms, which was tested and fixed in #2868637: AddToCartFormatter not working in Views. Please try the latest -dev of Commerce and make sure to test with the fix from that issue.

If you are still experiencing bugs, please review the following two issues and see if they related. Please provide detailed reproduction steps (such as big_pipe enabled, multiple products via Views, changing attribute AJAX.) We need to expand our test coverage and the best way to do so is having explicit details we can reproduce in tests with succinct issue reports.

Thanks, everyone.

drugan’s picture

I've tested against this commit on a fresh install:

commit 797522dc4886ef7c70cb02a70bc89913963c9333
Author: Bojan Zivanovic
Date: Sun Jul 2 22:13:56 2017 +0200

Use a 4-space indent for ludwig.json

... and had this exception when adding to cart from a view of products:

InvalidArgumentException: Unknown attribute field name "attribute_mysizesattr". in Drupal\commerce_product\Entity\ProductVariation->getAttributeValueId() (line 272 of modules/contrib/commerce/modules/product/src/Entity/ProductVariation.php).

Seems the #9 still does not work.

mglaman’s picture

@drugan

I've tested against this commit on a fresh install:

So as I requested for detailed steps: you added an attribute, and added how many variations? Please open an issue detailing multiple add to carts in views with attributes and how to replicate in a test.

Thanks!

drugan’s picture

StatusFileSize
new853.87 KB

To reproduce the #57 issue:

Navigate to your webroot and run:

composer create-project drupalcommerce/project-base mysite --stability dev

In the end it asks whether you want to remove .git folders. Answer n (no).

Run this:

cd mysite && composer require drupal/devel drupal/admin_toolbar

(Should we require admin_toolbar module by default for the drupalcommerce?)

Install mysite site using any method.

Dump or copy the database of the fresh installed site. You can use it later to restore the initial state of the site.

Drop the database.

Create empty database with the same name as the dropped one.

Download the DB dump file on this comment.

Rename the downloaded file to mysite.sql.gz

Import the file into the empty database created above.

Run this:

cd web/modules/contrib/commerce && drush cr

Go to /all-my-products page and log in to the site:

user: admin
password: admin

Try to add any product variation to cart without changing attributes or otherwise. It should result into the fatal error:

InvalidArgumentException: Unknown attribute field name "attribute_xxxxxx". in Drupal\commerce_product\Entity\ProductVariation->getAttributeValueId() (line 272 of modules/contrib/commerce/modules/product/src/Entity/ProductVariation.php).

Run this and refresh the page with Ctrl+F5:

wget https://patch-diff.githubusercontent.com/raw/drupalcommerce/commerce/pull/582.diff && git apply 582.diff

The products variations should be added to cart as expected.

Go to admin/structure/block/list/bartik and place All My Products block into the Sidebar first region.

Go to /all-my-products page. You'll see that all products in the block are basically duplications of the view page products.

Try to change attributes on the My Colors & Sizes - FIRST product in the block. You'll see that AJAX does not work there. If you try to add this product to cart you'll see that disregarding on the select list values it is always Red, Small variation added to cart.

Try to change attributes on the My Colors & Sizes - FIRST product in the page view. Now, the AJAX works not only for the page product but for the block's one too. Though select lists' values are still not changed. It is expected behaviour because AJAX wrappers for both displays of the product are different. Only formatters are the same because their wrappers based on the class not the id. That's why the title, sku and price are synced for both product displays.

So, when you changed the page product's attribute values now the same product also works on the block as expected. Obviously that this introduces a nasty UX despite no any errors are emitted. To mitigate this issue do this:

Go admin/structure/views/view/all_my_products/edit/block_1?destination=all-my-products and set Caching: None only for the block (override).

cache

Try to change attributes on the block. You'll see that all works as expected from the first attempt though title, sku and price are still synced on both the displays of a product. Which is also a UX issue but only a minor one. Obviously that solution for this should be found but for now it would be enough to solve not working products' views which is critical issue and must be fixed before the first RC.

drugan’s picture

Priority: Normal » Critical
Status: Fixed » Active
bojanz’s picture

Priority: Critical » Normal
Status: Active » Fixed

Continuing in #2886614: Rework the AddToCartForm form ID handling because it has better information about the actual problem.

"Unknown attribute field name "attribute_xxxxxx" might actually be unrelated to the main issue here.

Status: Fixed » Closed (fixed)

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

amrausch00’s picture

Where am I supposed to be saving the patch file? I am new to Drupal. I saved it within all\modules\commerce and tried to run the patch, but it can't find the file to patch. When I go through the file directory, I do not see any folders or files labeled src. Can someone tell me what I am doing wrong?

drugan’s picture

@amrausch00

You may save a patch in any place on your system. Then open the Terminal, cd to the drupalcommerce module's root folder and run this if you have dev version of the module:

git apply path/to/file.patch

Or...

git apply path/to/file.diff

Or, if you don't have a git:

patch -p1 path/to/file.patch

Or...

patch -p1 path/to/file.diff

Read more here.

As for the current issue most of the patches posted on this page are outdated. If you have problems with multiple add to carts on a page I'd recommend to read this comment:

https://www.drupal.org/node/2707721#comment-12225526

bojanz’s picture

@amrausch00
This issue is fixed, which means that the fix is present in the 2.0-rc1 release. You don't need to patch anything.