I have two relevant attributes for my products, a startdate and a duration.
If I switch the startdate in the "Add to Cart" form the updated values don't fit to the selected duration.

Attached patch should fix this.
Btw:

  • The code unset($form_states['values']['attributes'][$key]); didn't work anyway since the variable is named $form_state ;)
  • I introduced the variable $matching_product_keys to get rid of a "Only variables should be passed by reference" notice.

My tests look promising - but I'm not sure that the patch has no sideeffects

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rszrama’s picture

Status: Needs review » Needs work

Ok, so what's going on here... I can't just remove the code outright, as that introduces a regression where the form won't accept a $default_product until the attribute options are actually updated to values that exist on a referenced product. I have a setup with the following products on a single form (where the number and letter map to separate attributes):

1A, 1B, 2B, 2C, 3A, 3B, 3C

Currently, if I select 3C and then change the number to 2, it keeps the C selected but actually uses 2B as the default product - so the $default_product and the product represented by the current form element selections is mismatched.

If I apply your patch, though, if I'm at 1A and change the number to 2, the letter options don't update to just show B and C - it appears they don't change at all until a valid letter is then chosen. At that point they update.

If I just fix the $form_states to $form_state, then it thinks there are multiple matching products... so if I go from 1A and change the number to 2, it then prompts me to pick from product 2B or 2C beneath the attribute elements even though it shows that the $default_product has been set to 2B.

Through a crazy amount of debug messages with the $form_states variables fixed (that were actually quite helpful, talking through the default product selection in plain English), I discovered that once the changed attribute had been isolated, because I was unsetting the subsequent attribute values from the $form_state, the comparison wasn't comparing enough fields against the first matching product to find subsequent matches... Also, once the form state was altered, the data was lost that was being used to determine which attribute had been changed each time through the loop.

This commit fixes the underlying data problem so there is now a valid $default_product and no additional selection steps are necessary. Unfortunately, it doesn't fix the default value problem for the widgets... according to my debug message, for example, when starting from product 2B and switching the number attribute to 1, it is properly setting 1A as the default product but the form is still showing 2B in the select lists. I think Peter mentioned this elsewhere and wondered if the autocomplete element would have to be used. I'm wondering if there's some other way to trick the browser, as I'm pretty sure this is now just a browser issue.

https://github.com/rszrama/drupalcommerce/commit/7835d8192850a18aba5e170...

jpstrikesback’s picture

Subscribe

rszrama’s picture

Status: Needs work » Fixed

Woohoo! Success! I finally tracked it down to the fact that unsetting form state variables actually wasn't doing anything (as das-peter's initial patch indicated), but because I thought it was solving something it was a bit of a distraction. I discovered that it's the value in the $form_state['input'] array that will actually be used to set default values for the select lists, so I switched to unsetting that instead.

And now it seems to be fixed. : )

https://github.com/rszrama/drupalcommerce/commit/13785a08fa15ec8049bed92...

Status: Fixed » Closed (fixed)

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