Let's see if I can explain.

We have a product with two attributes. One is a select list. The other is a text field.

The product SKU changes based on the value of the select list. However, if a number is put in the text field, the model adjustment lookup fails.

The problem can be traced to this function:

function uc_attribute_add_to_cart_data($form_values) {
  $combination = array();
  if (!isset($form_values['attributes'])) {
    return array('attributes' => array(), 'model' => null);
  }
  foreach ($form_values['attributes'] as $aid => $value) {
    if (is_numeric($value)) {
      $combination[$aid] = $value; // <-- Our text field is given numeric values, and added to this array.
    }
  }
  $result = db_query("SELECT model FROM {uc_product_adjustments} WHERE nid = %d AND combination LIKE '%s'", $form_values['nid'], serialize($combination));
  $model = db_result($result);
  // drupal_set_message('Attributes to add to order Array(aid => oid):<pre>'. print_r($form_values['attributes'], true) . $model .'</pre>');
  // Preserve the 'attributes' key to allow other modules to add to the data field.
  return array('attributes' => $form_values['attributes'], 'model' => $model);
}

So the query ends up as:

SELECT model FROM uc_product_adjustments WHERE nid = 425 AND combination LIKE 'a:2:{i:5;s:6:\"123123\";i:68;s:3:\"125\";}'

When it should be:

SELECT model FROM uc_product_adjustments WHERE nid = 425 AND combination LIKE 'a:1:{i:68;s:3:\"125\";}'

I don't see an obvious way to patch this, since hook_add_to_cart_data() only gives me the $form_values, when I really need to check the $form element to see if this is a text string (or, really, a free-form entry field).

Make sense?

Comments

rszrama’s picture

Ken's analysis looks right, and a quick look confirms this is an issue w/ 2.x as well. At the same time this is fixed, it might be nice to remove that commented debug code and tidy up this function a tad.

rszrama’s picture

Issue tags: +code cleanup, +ubercamp sprint

Ken's analysis looks right, and a quick look confirms this is an issue w/ 2.x as well. At the same time this is fixed, it might be nice to remove that commented debug code and tidy up this function a tad.

Island Usurper’s picture

Version: 5.x-1.7 » 6.x-2.x-dev
Assigned: Unassigned » Island Usurper
Status: Active » Needs review
StatusFileSize
new1.28 KB

I'm not really happy about adding a call to uc_attribute_load() there, but it's better than trying to change the API to accommodate both 1.x and 2.x. Or even just one of the versions. But then, it doesn't look like those queries will take very long to run, especially if there's a query cache.

johnalbin’s picture

subscribe

agentrickard’s picture

StatusFileSize
new1.37 KB

Here's a D5 version as well, which seems to fix the issue.

Island Usurper’s picture

Status: Needs review » Fixed

In that case, consider it committed, to both branches. Thanks, Ken.

Status: Fixed » Closed (fixed)
Issue tags: -code cleanup, -ubercamp sprint

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