Skip to content
Snippets Groups Projects

Draft: [WIP] Resolution for #214, !215, #167

Open elilisseck requested to merge elilisseck/cividiscount:issues/214/167 into master

This branch is for development towards resolving issues #214 and #167 (closed) that have been inactive. There is a long conversation on each issue and on MR !215 (closed) but what we are currently experiencing boils down to:

CiviDiscount gives admins the ability to configure a discount for just a subset of price options on a price set with no warning on current limitations of this feature.

There are several issues that arise from this feature around users now expecting multiple automated discounts to apply to an event / membership form with a price set that can be described as:

  • Multiple automated discounts do not apply when they're configured as one-applicable-per-price-option. This used to work before #167 (closed) and I would categorize it as a regression and aim to fix it in this branch (!215 (closed) fixes this in my experience) Ordering issues.
  • Multiple discounts do not apply to the same price option. Cividiscount appears to choose the automated discount with the highest ID number but it isn't clear to admins what is happening. @agh1 noted a nice idea in #214 of adding weights to discount code configurations with some help text, and then choosing on weight, or this could be as simple as help-text as a first step. I would be interested in thoughts on that.
  • Multiple automated discounts do not stack on a single price option if they're all applicable. I'm not sure if folks are interested in this but it got confused with the issues above at some point. This never worked and wasn't supposed to AFAIK. I would categorize it as a feature request that I don't aim to fix on this branch.

My proposal to move forward is:

  • Get the commit from !215 (closed) in (cherry-picked here, basically reverts the fix for #167 (closed))
  • Confirm whether #167 (closed) is now an issue again (Issue is 4-5 years old)
  • Find an alternative solution for #167 (closed) if the issue persists, without resorting to this problematic break

I'm not sure if these usernames still exist in gitlab but tagging some folks from the old issues who may have input: @mtnpavlas @h-c-c @kewljuice @freeform-sg

Edited by elilisseck

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • elilisseck changed the description

    changed the description

  • so sorry, guys, just noticed the call-out for my comments. This stuff is pretty old, and a few things have changed. Here is what I'm seeing on our end at the moment:

    Regarding 214: If multiple auto-discounts are set up for the same event type (each applying to DIFFERENT price options in different price fields and price sets), then the discount with the lowest row ID does NOT work. E.g. we have events of type "Class", one with $90 price field option and another one $160 price field option. Two autodiscounts - one for $20 applied to the $90 price option, and a $35 one applied to the $160 price option. The $35 one works, the 20 does not. Adjusting line 446 in the cividiscount.php file from if (autodiscount) { to if ($autodiscount && $discountApplied) { does the trick for us - easy peasy fix, but a PITA to apply this patch manually every time we upgrade CiviDiscount. Because we're talking about autodiscounts applied to completely different price sets, I believe this is a bug.

    Regarding "Multiple automated discounts do not stack on a single price option if they're all applicable." ... Clarifying question: I'm assuming you meant to say "on a single price field" (vs. "option") ... our experience is we're applying only ONE discount to each price option, but these price options are part of the same price field. E.g. Price field "Event Fee" has two options: $50 for a one-day and $80 for a two-day pass. We want our members to get a $20 discount for the one-day pass and $35 discount for the two-day pass - thus, we need to set up two discounts, each applying to a different price field OPTION, which are both part of the same price field. Sooo, is this a bug or an enhancement?

    Thank you!

  • Chance someone can clarify if my assumptions above re "bug" vs. "enhancement" are accurate, and what it would take to address both? Funding? Other? Thx.

  • I've started an issue (#303 (closed)) to hopefully get to some sort of resolution on this problem, as there are many issues and PRs trying to address it, but tradeoffs are in play, so I think we need either someone to step up to fix the problem in a complete way (or come up with some funding towards that, yes, @mtnpavlas) or we need to get a consensus on a suboptimal fix for now.

Please register or sign in to reply
Loading