Applying multiple discounts versus correct discount tracking
There are a lot of issues and PRs stemming from the change in !171 (merged). I've looked through the code, PRs and issues and I think the current proposed solutions aren't really going to fix the fundamental issue or they are going to revert the fix in !171 (merged). I'm not sure what the best solution is here, but I'm hoping if I tag everyone and lay out the options, we can get some sort of consensus and put this one to bed after many years.
Disclaimer: I haven't tested any of this, so it's quite possible that I'm wrong in the details here.
Basically, I think the fundamental issue is that the code that adds the tracking only works with one discount code per participant or contribution/membership. Before !171 (merged), it sounds like it was possible to use multiple autodiscount codes on different price options, perhaps even an autodiscount code and a regular code at the same time, maybe multiple regular codes (but not multiple codes on the same option). We also have the problem that autodiscount codes aren't applied properly when there are two codes that apply to the same event type (I think only the code with the lowest id is available).
Obviously, the tracking is not going to work properly if we have multiple codes, because only one code can be tracked in the postProcess hook. Additionally, if there is an autodiscount code and a regular code, when the regular code has a higher id, the regular code will be tracked even when it isn't actually used if we try to allow multiple codes.
The real fix here would be to re-write the code that handles the tracking to handle multiple codes. It looks like we would also have to make some changes in the buildAmount hook and to handle the situation where we have a regular code and an autodiscount code on the same form. If someone wants to take that on, that would be great. It's too big of a project for me to justify with how little we use CiviDiscount.
If no one wants to step forward to do that, the easy options at this point are going to focus on the code changed in !171 (merged):
- Leave it as is. The tracking works, but we can't use multiple codes on the same form and autodiscount codes aren't applied properly when there are two codes for the same event type.
- Remove the
break
entirely, which I think will allow multiple discounts, fix the event type issue, but break the tracking again. - Change to
if ($autodiscount && $discountApplied) { break; }
as proposed. I think that will fix the event type issue and allow multiple discount codes for different price options (though not multiple discounts on the same price option, and not an autodiscount plus a regular discount if the regular discount has a higher id or multiple autodiscounts on the same form). Tracking will work when only one code is in use; when multiple codes are used, the code with the highest id will be tracked.
Tagging everyone who has been involved in this, though seems like many aren't on Gitlab: @bgm @colemanw @gh1 @freeform-sg @kewljuice @AllenShaw @jitendra @mtnpavlas @midnightspenguin @petednz @seamuslee @mattwire @herbdool @Stoob