Skip to content

Tracking applied to wrong code if using auto-discount where there's also another valid discount with greater ID

Recipe to reproduce:

  1. Create an auto-discount for an event, to be automatically applied for current members (other auto-discount configurations may also work to reproduce this), e.g., "MyAutoDiscount"
  2. Create another discount code valid for the same event, but not an auto-discount, e.g., "MyWriteInDiscount"; the important thing here is that this step comes after creating the "MyAutoDiscount" code.
  3. Visit the CiviDiscount tracking page; observe that the usage tracking for both discounts is at "0".
  4. Log in as a contact fitting the auto-discount criteria, and visit the event registration page; leave the "Discount Code" field blank; observe that the discount for "MyAutoDiscount" is properly applied and the registration fee is discounted correctly, and the discount code is correctly described in the fee description.
  5. Complete the event registration; again, observe that the discount for "MyAutoDiscount" is properly applied and the registration fee is discounted correctly, and the discount code is correctly described in the fee description.
  6. As an administrator, visit the CiviDiscount tracking page; observe that the usage tracking for "MyAutoDiscount" is at 0, and the usage for "MyWriteInDiscount" is at 1.
  7. Disable the "MyAutoDiscount" code; create a new code with identical configuration, e.g., "MyAutoDiscount2"
  8. Repeat steps 4 and 5, observing the same correct behavior for "MyAutoDiscount2"
  9. As an administrator, visit the CiviDiscount tracking page; observe that the usage tracking for "MyAutoDiscount2" is at 1, which is the correct value.

My thoughts so far: This appears be happening because of some faulty foreach logic in cividiscount_civicrm_buildAmount(). The code at https://github.com/dlobo/org.civicrm.module.cividiscount/blob/master/cividiscount.php#L453 (note the comment, "// this seems to incorrectly set to only the last discount but it seems not to matter in the way it is used") is assigning the last value of the foreach loop in $form->set('_discountInfo'), and that value is used later (around https://github.com/dlobo/org.civicrm.module.cividiscount/blob/master/cividiscount.php#L625) to record the tracking.

That foreach loop is doing a lot, and I haven't had a time to dissect it and be sure any changes don't produce unexpected results.