Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
When adding a coupon via the admin order UI form, the coupon is not validated.
Before I add a custom call to commerce_coupon_code_is_valid
in a form_alter, I'm just wondering if perhaps it makes sense to merge the two functions and to call
commerce_coupon_code_is_valid
within
commerce_coupon_redeem_coupon
That way the coupon will always be validated prior to redemption, rather than manually validating elsewhere.
Let me know if suits and can submit a patch.
Cheers,
DT
Comments
Comment #1
pcambraYeah, I was thinking on this one when I developed the UI for orders, but in the end I think that when you're in the admin UI you know what you're doing and have the right permissions so the validation doesn't make much sense.
About joining them, I think it's better for the API (we should have API docs btw), to have them separated.
If we can somehow manage this by settings, I.E one specific permission to redeem coupons via UI admin form without validation and another one that requires validation, I'm fine with that.
Comment #2
davidwhthomas CreditAttribution: davidwhthomas commentedThanks for the reply.
I'd have to disagree on that one, for example the ability to add expired coupons, inactive coupons and coupons that have exceeded their maximum use should proabably be restricted.
A user who can administer orders may not necessarily be able to administer coupons.
Our current project has a subscription administrator type role managing subscription orders via the admin interface and I need to prevent them shooting themselves in the foot via adding invalid coupons to subscription, that are then processed via the payment gateway.
It looks like that form_alter may be the ticket as I did get the coupons validating by that approach.
If I get time, I'll look into adding a permission to bypass validation on the admin page, along with the form_alter to submit as a patch.
Cheers,
DT
Comment #3
pcambraIt makes totally sense, let's reuse the same issue.
Probably we need to check that permission in the validation of the redeem of the coupon so the form alter is not required?
Thanks
Comment #4
davidwhthomas CreditAttribution: davidwhthomas commentedWell, if the validation check
commerce_coupon_code_is_valid
was run when the coupon was redeemed ( in the same function ) then the permission check could be there only.I'm not sure when a coupon would need to be validated without redeeming it - is there a use case for the separate API functions there?
If there's a permission to control validation bypass, then there's presumably no use case for redeeming a coupon without validating it ( assuming the access permission check passes )
Given they are currently two separate functions, and there's no validation in
commerce_coupon_redeem_coupon
I think it will still need the form_alter to prevent the coupon being directly redeemed when invalid.Alternatively, merging the validation with coupon redemption and adding a permission to bypass validation could be the way forward here - is that what you mean?
Thanks again,
DT
Comment #5
pcambraLet's call this a feature request, but it would be really nice to have and clear up a little the validation process.
Comment #6
davidwhthomas CreditAttribution: davidwhthomas commentedI ended up doing this in
hook_form_commerce_order_ui_order_form_alter
by adding an#element_validate
handler to the entityreference fieldcommerce_coupon_order_reference
.The validation is borrowed from entityreference, but adds some additional checks for a valid coupon.
First the hook_form_commerce_order_ui_order_form_alter code
then the element validate handler
If interested, perhaps I can roll it as a patch.
Cheers,
David
Comment #7
liupascal CreditAttribution: liupascal commentedBy not "Validated" you mean that the coupon's conditions are not checked before attaching it to the order ?
I just create an order with coupon fixed amount on a fresh kickstart install, and it seems that the coupon's "effect " (say -20€ on the order) is not applied on the order either.
-> Should I open a new issue for that ?
-> Is it the intended behavior ? (not to automatically apply the coupon through the ui, just let an admin attach it ?)
-> Or should we consider this related to this issue ?
Many thanks
Comment #8
pcambraI think that's nothing to do with this issue, does it?
Comment #9
davidwhthomas CreditAttribution: davidwhthomas commented@liupascal #7 That's a different issue, this issue is about validating the coupon when adding as admin.
I'm still using the snippet from #6, tis working ok.
DT
Comment #10
pcambra@davidwhthomas could you please provide a patch for what you think should fix this issue?
Thanks
Comment #11
liupascal CreditAttribution: liupascal commentedI was indeed talking about coupons being added to an order while "manually" creating the order.
But it's clear that this topic is about the conditions of the coupon. Sorry I misunderstood, i'll create another topic !