Rebuild only the discount rule that got inserted / updated

Would this affect this issue in any way? #2555419: Limit length of the rules names

Here's a kick start patch but I don't know where to prune.

Needs Pruning

Here's updated patch that only builds the single rule.. Instead of _commerce_discount_rebuild_rules_config() allowing null and optionally rebuilding entire rules_config, I've made it require a discount entity. The discount entity controller invokes regular rules_config rebuild.

This patch worked for me, saving a Discount now only took a few seconds instead of 30 :) Very happy to see this change. Thanks!

P.S. - in the early days we always had a bunch of discount offers that suddenly were unlinked from their discount entities, for reasons we could never figure out. Essentially it seemed like the Rule was created anew, with a new identifier and everything. I'm not sure if the brute-force rebuilding of Discount Rules was the cause of this (we actually haven't seen this happen in a while, anyway) but in any event this is a very welcome change, because I'm sure it will help cut down more issues like the one I described.

Hello mglaman,

A quick observation about the last patch you have submitted.

function _commerce_discount_rebuild_rules_config(CommerceDiscount $commerce_discount_entity) 

The variable name "commerce_discount_entity" seems too long and not really conventional compared to other variable name used for a CommerceDiscount entity. Usually, we use the variable name: $discount. May you update your patch in order to keep standardized commerce_discount's code ?

Sure, done.

Bah I forgot to merge 7.x-1.x into my issue branch. Disregard patch from #7.

Thank you mglaman !

Since I worked on this patch a bit and not comfortable blind committing it(though I would) I'm assigning it to @rszrama for the honours and to check over that it's doing what his original request has intended.

Thanks @mglaman and @jkuma for finishing this up.

Adding to the borg.

+++ b/includes/commerce_discount.controller.inc
@@ -77,7 +77,8 @@ class CommerceDiscountControllerExportable extends EntityAPIControllerExportable
     parent::delete($ids, $transaction);
-    // Rebuild entities.
+
+    // Rebuild rules config.
     entity_defaults_rebuild(array('rules_config'));

Can/Should we use our custom function here too? Also, should we avoid the rebuild if the transaction fails?

I left it to rebuild all as multiple could be deleted. The helper function doesn't support multiple to reduce complexity, as insert and update only affect a single instance. Delete is multiple. Figure we could open an enhancement ticket to work around delete. I felt it was out of scope of the original ticket.

@mglaman: I don't know if the multiple part is the problem, as you could just do a little foreach on the $ids before you passed them into the helper function. The problem is the helper won't support deletes at all as far as I know. It always assumes a create or update and then saves. The helper function would have to be significantly modified or a second delete one created if I understand things correctly. I'd agree with mglaman that it is probably best for a follow-up issue.

Here's the /admin/modules page before and after this patch!

It shaved off 3 seconds on my admin/modules page! My only concern is this logic looks like it should belong to rules. Maybe we can open up a rules issue to see if we can't get this in there?

Right now, this module uses brute force to rebuild discount rules when a discount is inserted / updated:

hm, I'm missing why it is doing that in the first place. Rebuilding is the process of applying defaults exported to code - how does that relate to the creation of further discount rules?

fago, discounts generate rules. So the default configuration needs to be re-imported for that rule to reflect changes.

Fago's done some work in rules that has some benefits to this issue it seems. There is a patch over in #2189645: Avoid full cache clear whenever a rules component or reaction rule is edited that would be good to review.

Just a small refactor, shouldn't change code. I'll commit this after it passes.

Thanks, I've committed this to -dev and if something comes of change in rules or entity to help do this without duplicating the code then I'm all for ripping this out later.

Even though I didn't write any code for this issue I'm thankful for the commit credit ;) Glad to see this in! It's made a positive impact on our discount creation workflow.

It's an appreciation of the company;)

Created a follow-up issue #2919251: Improve the rebuilding of discount rules instead of reopening this one which is old.