Patch alters the configuration for "Apply percentage coupons to product line item", doing the work this module currently asks the user to perform via the Rules UI.

Additionally, this simplifies features that might want to depend on this module, as they no longer have to account for overriden Rules configuration, and can instead simply depend on this module.

Busting the ArrayAccess interface like this is ugly, but (unless I'm mistaken) is required for PHP < 5.4 (that or adding LoC for temporary arrays, which is even less clean, IMO).

CommentFileSizeAuthor
#5 commerce_couponprodref-automatic_install-1882828-4.patch1.76 KBandyf
#1 1882828-rules-default-configuration-alter-2.patch1.39 KBlotyrin
0002-Alter-commerce_coupon_pct_apply_pct_coupons_to_line_.patch1.77 KBlotyrin

Comments

lotyrin’s picture

lotyrin commented
StatusFileSize
new 1882828-rules-default-configuration-alter-2.patch1.39 KB

Here's a slightly simpler version (replaces the action, keeps the loop).

andyf’s picture

andyf commented

I just wanted to say thanks so much for the code - things have been hectic recently, but I'll be able to take a look very soon.

star-szr’s picture

star-szr
he/him
Primary language English
commented

Patch looks good and works as expected. Thanks @lotyrin!

andyf’s picture

andyf commented
Status: Needs review » Needs work

Looks good, and I've just written some tests and it works with them. Thanks! My one worry is that it assumes the coupon_pct rule hasn't been modified (eg. it wouldn't work if the loop wasn't the first action).

andyf’s picture

andyf commented
Status: Needs work » Needs review
StatusFileSize
new commerce_couponprodref-automatic_install-1882828-4.patch1.76 KB

It's not beautiful, but I'd like to play as nicely with other modules as possible (ie if commerce_coupon_pct changes its rule, or if another module alters the rule before this one - dunno if I'm being (too) anal here!). I wasn't sure if it's safe to add an action to the loop while iterating it, so I used $deleted. I never enjoy using Rules classes; I'd appreciate feedback if anyone has time.

Thanks again @lotyrin, I've been meaning to get tests written for a while and this was always standing in the way.

bjlewis2’s picture

bjlewis2 commented

It appears that there are whitespace errors in #5. I'm not sure what the appropriate way is to get rid of those, or I'd have supplied another patch.

git apply -v commerce_couponprodref-automatic_install-1882828-4.patch
commerce_couponprodref-automatic_install-1882828-4.patch:17: trailing whitespace.

commerce_couponprodref-automatic_install-1882828-4.patch:20: trailing whitespace.

commerce_couponprodref-automatic_install-1882828-4.patch:24: trailing whitespace.

commerce_couponprodref-automatic_install-1882828-4.patch:33: trailing whitespace.

commerce_couponprodref-automatic_install-1882828-4.patch:45: trailing whitespace.

Checking patch commerce_couponprodref.rules_defaults.inc...
Applied patch commerce_couponprodref.rules_defaults.inc cleanly.
warning: 5 lines add whitespace errors.
andyf’s picture

andyf commented

@bjlewis2 Thanks, I'll kill those before committing!

andyf’s picture

andyf commented
Status: Needs review » Fixed

Thanks for everyone's help, finally committed: f6c55db.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.