I am the maintainer of Commerce Variation Cart Form, which provides a form mode for the order item form that can be used on individual product variation entities, providing a dedicated add-to-cart form for each product variation.
I have had a request to integrate with Commerce Stock Enforcement. I see that only the default add-to-cart form is supported in commerce_stock_enforcement_form_alter().
Simplest solution: Implement a helper function that other form providers can call, passing in &$form and $selected_variation parameters. The helper method would simply be the contents of
commerce_stock_enforcement_form_alter() lines 40-55 (and be called instead of those).
Does that sound reasonable?
| Comment | File | Size | Author |
|---|---|---|---|
| #16 | commerce_stock-other-cart-forms-3147163-16.patch | 1.19 KB | mrweiner |
| #14 | commerce_stock-other-cart-forms-3147163-14.patch | 1.23 KB | johnpitcairn |
Comments
Comment #2
johnpitcairn commentedArchitecturally that's a bit suboptimal, relying on calling a helper from the module's own
hook_form_alter()implementation, and would be Commerce Stock specific. I'd take it for now.Long-term, it's probably a commerce core problem, right? Provide a way to declare your module provides an alternate cart form implementation, via a standard interface, service or config key, so any modules needing to work with cart forms can discover and use that. The current standard implementation of needing to use
hook_form_alter()which runs for every form and checking the form_id for predefined implementations feels very Drupal 7.Comment #3
guy_schneerson commentedHi John,
Thanks for bringing this up. we already added support for Commerce Ajax Add to Cart (I think that's the one we did). And it will not be reasonable to keep adding other cart solutions and we do need to find a better solution for this. Hopefully, the availability manager in commerce will sort this issue out. Alternatively, we can have a system where carts register themselves with commerce stock but that is not likely to be a quick fix and is not worth the investment if commerce will eventually sort this out.
So it may be easiest to have a temporary solution like you say. I am not sure I understood your suggestions. can you put a patch in?
Also would not mind if you do add specific code for your module like we did for the ajax add to cart if that works.
Comment #4
johnpitcairn commentedThanks - adding explicit support by
form_idwon't work anyway, the structure ofcommerce_variation_cart_formis a little different from the default add-to-cart forms.I will file a patch, should be fairly simple. Might not be very soon, unless the user who originally requested it chimes in.
Comment #5
guy_schneerson commentedJohn Pitcairn, cool, looking forward to your patch
Comment #6
sjvellenga commentedHas there been any progress on this? Keen for a solution here, as I require all variations to have separate add to cart forms with stock enforcement enabled.
Comment #7
johnpitcairn commentedHere's a patch adding the suggested helper function, and using it for the already supported cart forms. It won't do anything for unsupported cart forms by itself.
If no tests break, I'll add a patch for commerce_variation_cart_form that will make use of the helper function. @sjvellenga, I will need you to test following that (not yet).
Comment #8
johnpitcairn commented@sjvellenga: Please apply this patch and the patch at #3146653: Support for Commerce Stock Enforcement, test, and report back in that other issue.
@guy_schneerson: I'm not sure this is the best approach, maybe we would be better to specifically support commerce_variation_cart_form, so the "temporary" fix is only in one module. I'm not very clear on how you intend to integrate with the availability manager to support other cart forms.
Comment #9
kristofferromHi guys,
Had the same issues and needs as @sjvellenga and the patches seem to work for me.
Best,
Comment #10
johnpitcairn commentedI've opened an issue in Commerce to try to address 3rd-party integration difficulties: #3208853: Provide and document a better way for modules to identify and modify add-to-cart forms
Comment #11
johnpitcairn commentedHere's an attempt at an alternate way of identifying add-to-cart forms, as above. It might break for Commerce Ajax Add To Cart, not sure.
I will commit a line to dev for Commerce Variation Cart Form that adds
$form_state['selected_variation'], and then that should work at least as well as it does for the patch at #7, without requiring the additional patch at #3146653: Support for Commerce Stock Enforcement.Please re-test using latest Commerce Variation Cart Form 8.x-1.x-dev.
Comment #13
johnpitcairn commentedLooks like Commerce Ajax Add To Cart could potentially work without special-casing, since it also implements
AddToCartFormInterface.Think the patch above is just missing a
usestatement.Comment #14
johnpitcairn commentedAdded the
usestatement.Comment #15
johnpitcairn commentedComment #16
mrweiner commentedHad issues applying #14 for some reason -- just rerolling for latest dev.
Comment #17
johnpitcairn commented@mrweiner did you reroll the patch?
If this is working for you and allows commerce_variation_cart_form to integrate without needing special-casing here, would you consider marking this RTBC to gain some maintainer attention?
Comment #18
mrweiner commentedYup, #16 should apply as expected and seems to solve the issue. I agree that checking the instanceof the form is probably the proper way to be handling this conditional.
Comment #19
lexsoft commented+1 to get this committed
Comment #21
guy_schneerson commentedThank you for the patch and all the testing and sorry it has taken so long :)