Comments

czigor’s picture

Status: Active » Needs review
StatusFileSize
new865 bytes

Attached is a patch which looks for a #default_value in $form_state['values'] .

czigor’s picture

StatusFileSize
new873 bytes

This way it's better (because it's working).

czigor’s picture

Still not working. The attribute dropdowns are working right but the other parts of the product display node (like title, fields) still show the data belonging to the default Size.

rszrama’s picture

Status: Needs review » Active

The reason it's resetting is because there is a dependency between the attributes that respects the order of their widgets. Color comes first, so when its value changes, the Size select box is rebuilt to only include options available for the Color selected. I think I understand what you're trying to do - respect dependency and use the dependent option if it remains available after the select list is rebuilt. Is this correct?

Kohán Péter’s picture

We are working on the same project with czigor. Yes, you are correct, this is exactly we would like to do.

czigor’s picture

Status: Active » Needs review

I can just confirm Péter's confirmation. If the dependencies allow it, I would keep the selected value of Size, otherwise the default value should be selected. I think in most of the cases that's the expected behavior.

rszrama’s picture

Status: Needs review » Needs work

You said this isn't working, right? : D

czigor’s picture

Yes, thanks, I did not set the status on purpose.

czigor’s picture

Status: Needs work » Needs review
StatusFileSize
new4.69 KB

This is ugly but seems to be working.

czigor’s picture

StatusFileSize
new7.55 KB

The same, but prettified.

jimmins’s picture

This patch worked great! But when I click "Add to Cart", I see the same issue -- the Select Lists get reset to their default values. Is there another similar patch for that?

davidwhthomas’s picture

StatusFileSize
new7.55 KB

@czigor - thanks for the patch!

This issue where the select box options would reset to default after changing another attribute option, was bugging our QA & users.

The patch actually fixes that tricky issue.

The attached version fixes the patch whitespace errors reported,

Applied from commerce module dir with patch -p1 < attribute_selection_1286244_5.patch fine

Cheers!

DT

dpolant’s picture

StatusFileSize
new2.36 KB

Here is my take on fixing this problem without having to use a helper function that might get called twice. Basically it does two things:

1. Removes logic that was causing only the changed attribute to decide the value of the default product
2. Adds some improved logic to the part that decides what to use as the default product

Patch was rolled using git diff against latest dev.

davidwhthomas’s picture

I recently upgraded to Commerce 1.3 and found this patch was still required.

I tested out dpolant's patch in #13 and it's simpler, and works well.

Thanks!

DT

summit’s picture

Status: Needs review » Reviewed & tested by the community

Hi, Shouldn't this be committed then maybe?
greetings, Martijn

rszrama’s picture

Title: Attribute selection order » Allow dependent attributes to preserve their default options if a matching product exists on an Add to Cart form
Component: Product » Cart
Category: bug » feature
Status: Reviewed & tested by the community » Needs work

The patch as is actually introduces to attribute selection widgets the bug that was causing #1343192: "Select a product" element is ignoring its new #default_value on AJAX refresh. If I have 6 products covering two attributes (color and frame, which is optional):

  1. Blue Tile
  2. Green Tile
  3. Blue Tile with Gray Frame
  4. Blue Tile with Black Frame
  5. Green Tile with Gray Frame
  6. Green Tile with Black Frame

But only reference products 1 and 3-6 from a form, then I'll get an Add to Cart form with two attribute form elements - one for color and one for frame. However, from the default position, with the color Blue selected and the frame left to None, if I change the color to Green, my frame select list is going to be rebuilt with new options since the None option should no longer be available. Unfortunately, the AJAX form submission would have specified that that element should use the None option for its new default value, but since this option is no longer available, the element will be left without a default value. Here's the resulting HTML:

<select id="edit-attributes-field-frame--2" name="attributes[field_frame]" class="form-select ajax-processed">
  <option value="Gray">Gray</option>
  <option value="Black">Black</option>
</select>

Notice that neither option is checked as the default, even though the new default product for the form would definitely indicate that Gray should be the selected default value.

I can confirm that things appear to function as expected - all options being equal (i.e. if I use all 6 products on the form) my dependent attribute options will be preserved. I can't commit this as is due to this bug, though, and I'd be interested in seeing tests with 3 attributes to ensure the cascading attribute refresh works as expected.

For your reference, this is the line that was removed that's causing the problem:

  unset($form_state['input']['attributes'][$key]);
lakshmisundar’s picture

I applied the latest patch given in this discussion. But, it works only in two level. If the product has three attributes it shows wrong product image.

My Products
------------------
Shoe type : Formal
Brand : Lee cooper, Bata
Color : Black, Brown

Shoe type : Casual
Brand : Puma, Nike
Color : Black, Brown

Scenario
-------------
Choose
Formal --> Bata --> Brown --> Casual (shoe type)
Now, the third drop-down, i.e Color value is still Brown. But, the product image is showing the Puma Black color shoe image instead of Puma Brown.

When the previously selected value is not available in the current product combination, it should reset all the subsequent attributes to their default value to match with the default product image.

I have added a 3 line code to the patch at #13 to fix this issue. Please find the same below.

3rd_rail’s picture

I applied the latest patch from #17 and everything is o.k. But if I reference a product with more than 4 attributes i get an AJAX error message after selecting an attribute in the product display. This only happens if i select the last attribute first. Any help is appreciated...

jessepinho’s picture

StatusFileSize
new7.79 KB

Re-rolled #17 for the Commerce 1.8 update (see attached), and... it doesn't work. (Dependent attribute selectors are still reset.) Any ideas?

EDIT: Whoops, nevermind... looks like I accidentally ran the patch from #4 instead of #17.

summit’s picture

Hi, so is this needs work or RTBC? Greetings, Martijn

rszrama’s picture

Not if the bug I described in #16 hasn't been addressed.

Tmanagement’s picture

In my case, I have 500+ products that all have the same 5 categories on which they can be different. So for instance, category 1 has 4 options, category 2 and 3 has 4, cat 4 and 5 has 3 options. This results in 576 possible combinations.

However, although all combinations are present (products in drupal commerce). Selecting an option in a few categories might suddenly be discarded once another option in another category is selected and the form has been updated. In that case the form sets the other categories to the default option.

It does not happen when you start selecting at the first category down to the last. If you do it in a random manner you will experience that previous selections will be returned to their default state.

It seems the same bug as discussed here am I right? If so, I am more than happy to test some things as I need the solution. Not much of a programmer though...

For the information, I am not running def
Info
Drupal 7.22
Info
Install profile Commerce Kickstart (commerce_kickstart-7.x-2.8)

stockliasteroid’s picture

This worked for my site with 2 attribute selections.

philsward’s picture

Sweet! #19 works beautifully with two attributes. (Untested on more than 2)

This is the ideal workflow from a usability standpoint. Hope to see this fixed in future releases.

jurgenr’s picture

Issue summary: View changes

#19 works on 2 and 3 level attributes.

mglaman’s picture

#19 works, could use reroll.

diff --git a/sites/all/modules/contrib/commerce/modules/cart/commerce_cart.module b/sites/all/modules/contrib/commerce/modules/cart/commerce_cart.module
index cb77e83..f1b895a 100644
--- a/sites/all/modules/contrib/commerce/modules/cart/commerce_cart.module
+++ b/sites/all/modules/contrib/commerce/modules/cart/commerce_cart.module

Patch won't apply due to file paths.

joe huggans’s picture

I have 4 level attributes. Is there any hope for this?

joe huggans’s picture

I can confirm that it works with 4 attributes.. Thank you very much

wqmeng’s picture

19# work on my site too. With 4+ attributes.

Thanks

czigor’s picture

Issue summary: View changes
joe huggans’s picture

,

mesch’s picture

I had a go at resolving this as part of a larger rework of the cart form here.. It should preserve attribute options for any number of attributes so long as there is a product for each attribute combination.

joe huggans’s picture

Sweet Mesch!

wqmeng’s picture

I found a odd problem today, On my product display page. I have many product variations, and one attribute is used the field collection fields, as it can be unlimited numbers and also can combine each option with the unlimited numbers, so that it will cause huge variations, I have to get that attribute to be a field collection. It is not the problem but the problem I got is when the attribute add a option and click add more button to add more options to this field collection, then add the selected product to the cart, the selection of other attributes have be changed to the default one, that is the selection product not added to the cart but the default product added to the cart.

I have dpm some result within the function commerce_cart_add_to_cart_form_submit, and found that the product_id is not correct, if you click the add more button (ajax) of the field collection attribute , the product_id is always of the default product. Then I compared the $form_state['values'] and the form_state['input'], the product_id is not match, the input contain the correct one, but the values contains the default one.

So What's changed the $form_state['values']['product_id'] is not match the form_state['input']['product_id'] that submitted? I also try to find the hooks of other modules, do not get a result yet.

For myself usage, I just use the input form_state['input']['product_id'] instead of the $form_state['values']['product_id'], but which may has other bad effect.

So anybody who also in this case, may find a solution?

Thank you.

wqmeng’s picture

/**
 * Form submit handler: add the selected product to the cart.
 */
function commerce_cart_add_to_cart_form_submit($form, &$form_state) {
  $product_id = $form_state['values']['product_id'];
  if ($product_id != $form_state['input']['product_id']){
    $product_id = $form_state['input']['product_id'];
  }
rszrama’s picture

jantoine’s picture

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

I couldn't get the patch in #13 to work, even with only two attributes. I also didn't like the double pass that the latest patch uses. Attached is a patch that:

  1. Starts with a list of all products from the add to cart form
  2. Filters out products not associated with the changed attribute
  3. Filters out products not associated with other attributes in the order they appear in the form
  4. If we ever end up filtering out all products, it returns the set of products filtered on the previous attribute
  5. Otherwise it returns the final set of matched products
  6. Finally it removes any attributes from the forms input values that the default product isn't associated with

The last item should address the issue raised in #16.

jantoine’s picture

StatusFileSize
new9.44 KB

Some functionality was lost related to only returning the first matched product and subsequent products with identical attribute values. This resulted in the product selection widget appearing even when all selected attributes only matched a single attribute. That functionality is included in the attached patch.

goz’s picture

I make a few quick tests with patch #38 and seems to work.

es_es’s picture

Anybody can help solve this on Commerce 7.x-1.11? Patch #38 not working for me.

dave kopecek’s picture

YES !! #38 worked for me. Thanks @jantoine IMHO this is a BIG DEAL

This is an OLD issue that is potentially expensive to site owners and can make your phone ring.

This is still an issue. Here's an unpatched demo in current Commerce Kickstart:

http://master-azmn2qmrszabg.us.platform.sh/tops/guy-hoodie - Commerce Kickstart with 7.x-1.11 on platform.sh. I have added additional SKU's to the guy-hoodie so that it is available in small / med / large in pink and grey:

  1. Change the size to large
  2. Change the color to Grey & observe that the size MAGICALLY changes back to small (!!!)
  3. Bam! The user just bought the wrong size shirt.

With products with more than 2 attributes it becomes attribute wack-a-mole.

Patch #38 worked for me and fixes above. Also works with 4 attribute product. I'm happy to do additional testing if it will help get this patch committed.

dave kopecek’s picture

Status: Needs review » Reviewed & tested by the community

Changed status to RTBC #38

nelslynn’s picture

Is there a reason we cannot get this committed? What needs to happen? Every commerce client of mine complains about this issue. Some end users do not realize the "reset" of previously selected attributes and inadvertently order the wrong product. Every other commerce solution I've explored does not reset previously selected attributes.

EDIT: #38 works for me.

mglaman’s picture

nelslynn, this mostly just needs maintainer review. However writing a test would be a huge help. To prove the issue and that this fixes it.

bojanz’s picture

Resolved for 2.x in #2675836: Expand the add to cart form test coverage, doesn't need a port.

wqmeng’s picture

If you have a field collection attribute in your product and it can add unlimited values, which will add the "add another item" button on the product display page. When your customer click the button of the attribute, the patch in #38 will not work, try my fix in the #35.

What's cause the problem seems that the "add another item" will cause a ajax post to the page, and it will get the correct value of $form_state['values']['product_id'] missing. But the value of $form_state['input']['product_id'] still correct, not sure why. Do we need to hook the commerce cart form alter in the field collection module to add some ajax callback to correct the missing value of $form_state['values']['product_id']?

Thank you.

dubs’s picture

#38 works for me too. +1 to getting this committed.

kopeboy’s picture

Priority: Normal » Major

@wqmeng unlimited Field collection attributes is a special case imho..

Can we get this (#38) committed in the meantime please!?
Just tested and working!

jsacksick’s picture

I quickly tested the patch in #38, and it seems to work as expected.

rszrama’s picture

Issue tags: +release-1.14
rszrama’s picture

Wow, everyone. Thanks so much for sticking with this issue. I've reviewed it just now and tested it about as fully complicated an attribute scenario as you can imagine:

Everything worked fine, including the product select list appearing / disappearing when needed and the "Blue" attribute disappearing when going from Large / Blue -> Small (defaulting again to Red since Small, Blue doesn't exist).

I imagine there are bound to be side effects, but I agree, field collections are an edge case that don't need to be resolved to commit this patch for a 1.14 release. Feel free to open follow-ups.

rszrama’s picture

StatusFileSize
new38.42 KB

(Whoops. Thought the image file got lost in the last patch update.)

rszrama’s picture

Status: Reviewed & tested by the community » Needs work
StatusFileSize
new24.03 KB

Well, shoot. Re-reading my comment in #16 above, I realized that the patch may not be ready yet with respect to optional product attributes. This appears to still be the case, in that if I make Size optional, when I choose "- None -" I see Red, Green, and Blue available as options even though I only added a Red unsized shirt...

No time to track this down as I need to go throw a birthday party for my now 2 year-old, but I'll take a crack at debugging that later this evening.

rszrama’s picture

Status: Needs work » Needs review

Pausing where I am but leaving a note that I'm actually not sure the "None" behavior is any worse off with the patch than without it. It appears to be a general bug in the Add to Cart form builder unrelated to dependent attributes. Will confirm in the (later) AM.

rszrama’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed! The issue exists regardless of this patch, and I have a good lead on resolving it. Going to commit this and create a follow-up issue.

  • rszrama committed 09937d8 on 7.x-1.x authored by jantoine
    Issue #1286244 by czigor, jantoine, rszrama, davidwhthomas,...
rszrama’s picture

Status: Reviewed & tested by the community » Fixed

Committed! Credited everyone who reviewed through the years. Thanks so much!

Status: Fixed » Closed (fixed)

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

Marko B’s picture

We are using authcache with drupal commerce, also we added custom code so that this ajax requests to attribute variants are done with GET and not POST, so they can be cached. With all that I have to say that this patch is breaking our functionality as proper IDs are not fetched, not sure exactly which part does that, in the end I made reverse of this patch to have things work as before. So I am uploading it if someone else might need this also.