Curious about a recommended course of action... Should it just fail, and then browse where? Should it browse to the product page? What if the cart link had a redirect on it?

Right, I think the best course of action would be to fail the cart link (is it possible to keep the redirect intact?) and just navigate to the product's node.

Perhaps the redirect gets translated, I'm not sure how it works in that regard. But it would be awesome if for example:

Cart Link in Upsell -> gets clicked
- Attributes are required, none specified in cart link
- Proceeds to product node
- Product's A/O combo is selected
- Product is added, redirects back to cart (default action for Upsell Cart Links)

What do you think?

I like some of the ideas, but I don't necessarily have the code on hand to implement them. The quick fix in the short term is of course to use restricted cart links so that customers can't just modify their link and make an incorrect combination. I'll definitely keep this feature request around, and I'd be more than happy to review a patch for it in the future. Gotta keep pushing for the 1.0 right now.

Okay, I managed to hack something together. It's not pretty, but it works. Put this little bit of code around line 240 of uc_cart_links.module, right before the line

Here's the added code:

So basically it just looks up the product real quick, and if there was no "option" part of the URL (in other words if it was just a product nid and quantity), AND there is an attribute required for that product, it will kick you to the product's page, with an error message to select an option.

Bumping this, since I noticed no solution has been added since this was first posted. I had forgotten about this until now; and the upgrade to Ubercart 1.0 blew away my fix, resulting in some products being sold via Cart Links without their required attributes being honored. If I can make this cleaner, I will - and then submit a patch.

Did you get the fix in / a patch by any chance? If you don't want to roll it into a patch, you can always just post up your copy and I can copy/paste.

Sure, here's a patch. Let me know if it works. Thanks for considering it If you think it can be improved, by all means, feel free to modify.

Ahh, that looks easy enough. Quick question - what to do about sites that don't have the attribute module? I'm happy to just wrap it in module_exists(), but I didn't know if someone might have another idea.

Ah, I hadn't thought of that. I would say just throw in an extra condition to see if the module exists (is enabled), as you are thinking.

Although - will the code not pass in that case? I'm using this snippet / patch on our live site and it only fails the Cart Link if the attribute requirement is set; on products where there are no attributes, or attributes are not required, they pass the condition and are added to the cart successfully. I'm not sure how this will change if the Attributes module is disabled or doesn't exist.

Guess it'd be interesting to look into.

I'm not sure if I totally understood you, but I did get the idea while I was reading your post that maybe the check should be done under case 'a'. It should be safe to assume that the attribute module is enabled if someone makes cart links that specify them. This would also let us check each attribute in the event that the cart link specified more than one.

Ah - an even better solution. If you want me to work on another patch, let me know.

That'd be great... I'm about to head out for the night but would be happy to review in the morning.

Here's a patch that sets a watchdog error if 'a' is even reached if the uc_attribute module isn't enabled.

The patch also stores the options chosen, mapped to the attribute id they were chosen for. Then later after parsing the cart link, the attributes are iterated over, and confirmed. Multiple errors are allowed to be echoed to the redirected product page, plaus another watchdog error for each required attribute that wasn't specified (It could be nice for admins trying to figure out why their cart links are failing?)

Lemme know what ya think.

EDIT: I made it better because invalid options could still be erroneously used with an attribute. This latest patch fixes that by adding a function uc_attribute_option_is_valid, which is evaluated if the attribute is required.

Alrighty, finally applied and tested this. I mentioned the minor code style things earlier, but also I noticed in testing that this patch only checks availability for required attributes. I wonder if there's any way to base whether or not to check availability based on something besides the required setting. The reason I say this is I had select lists with specific options but I didn't make them required... effectively the user still has to pick one (b/c there's a default), but I didn't have to make it required. So, I was able to make cart links that added the product to the cart w/ incorrect options... it displayed as a blank option in the shopping cart.

I'm not sure what the solution is... but perhaps it's to check the display type of the attribute on the nid? If it's a textfield, than anything works... if it's a select or radio list, then you must have an option match.

Thanks for these patches - I was running into the same issue. Is there any chance these patches will be incorporated into the next 5.x maintenance release?