This is how to reproduce:

  1. Create a product with a "Product Minimum & Maximum" feature of, say, max=1
  2. Add product to your cart and increase quantity to 2
  3. Update the cart and see the warning message of exceeding the quantity you are allowed
  4. Press "Remove" to remove the product
  5. The product is not removed.

What a user would expect is to have the product removed from the cart regardless of the min/max rule.

CommentFileSizeAuthor
#4 1356878-uc_product_minmax_remove-4.patch2.96 KBrickmanelius

Comments

marcus178’s picture

marcus178 commented

I've got the same problem on 6.x-1.x-dev

Cablestein’s picture

Cablestein commented

I have the same problem too, 6.x-1.x-dev.

Basically it's an annoyance to the user to have to go through their cart items and make sure all min-max rules are okay, THEN they are able to remote an item from the cart.

The logic for min-max rules and the messages should occur after the logic for removing items from cart.

natuk’s picture

natuk
Primary language English
Location Cymru
commented

I think the problem is with the form_alter function. I made a few changes as follows:

/**
 * Prevent illegal modifications of quantities in cart
 */
function uc_product_minmax_form_uc_cart_view_form_alter(&$form, &$form_state, $form_id) {
  $item_removal = FALSE; //assume that no Remove button has been pressed
  foreach(element_children($form['items']) as $i) {    
    if(!array_key_exists('nid', $form['items'][$i])) continue;
    
    $nid = $form['items'][$i]['nid']['#value'];
    $minmax = uc_product_minmax_feature_load($nid);
    if($minmax) {
      // if removing all items or items with errors, skip validation
      if (array_key_exists('input', $form_state)) {
        if (array_key_exists('remove-'.$i, $form_state['input'])) {
          $item_removal = TRUE;
        }
      }
    }
  }
  
  if (!$item_removal) { //if no Remove button has been pressed
    foreach(element_children($form['items']) as $i) {    
      if(!array_key_exists('nid', $form['items'][$i])) continue;

      $nid = $form['items'][$i]['nid']['#value'];
      $minmax = uc_product_minmax_feature_load($nid);
      if($minmax) {

        // If item in cart is part of a product kit, do not apply minmax rules
        if (module_exists('uc_product_kit')) {
          if (in_array('uc_product_kit', $form['items'][$i]['module'])) continue;
        }
    
        // if removing all items or items with errors, skip validation
        if(array_key_exists('input', $form_state)) {
          if (array_key_exists('items', $form_state['input'])) {
            $form['items'][$i]['#element_validate'][] = 'uc_product_minmax_cart_validate';
          }
        }
      }
    }
  } //otherwise, if Remove button has been pressed, then ignore validation
}

Basically, we first check to see if any of the fields is to be removed and if there is one, then we skip validation altogether. This, obviously, means that if there are additional invalid fields then they are skipped as well. But they are picked up with checkout.
Otherwise, I am not sure how it is possible to limit form validation to certain fields only when it comes to removal.

rickmanelius’s picture

rickmanelius commented
StatusFileSize
new 1356878-uc_product_minmax_remove-4.patch2.96 KB

I've done nothing but convert #3 into patch form so we can test it. I'll review myself in a little bit, but I'll set the status to "needs review" for now.

rickmanelius’s picture

rickmanelius commented
Status: Active » Needs review

*ahem. Setting status

rickmanelius’s picture

rickmanelius commented
Status: Needs review » Reviewed & tested by the community

FYI I can confirm that the function behaves as expected (the product min/max restrictions still work but removing from the cart is now possible). I'd like to set this to RBTC, but I'd feel more comfortable to get a code spot check first before it's fully committed.