Improve trigger check in rollup summaries trigger / Better Rollup Dev API

[wes1278]

Line 162 of this trigger needs to be smarter in my opinion. https://github.com/afawcett/declarative-lookup-rollup-summaries/blob/master/rolluptool/src/classes/RollupSummaries.cls

Right now, it's just looking for a trigger of a certain naming convention. I propose we use the snazzy new metadata api wrapper to discover a particular line of code that makes sure the rollup service is actually being called instead of looking for a trigger name.

The reason for this is if we're using this tool "in the wild" we need to still abide by one trigger to rule them all methodology. This tool breaks that rule. Mind you, it is still possible to comment the line from the trigger and then implement the method where you want it in your class. But if you want to push from sandbox to production, you have to push all those unused classes to make your rollups active.

@afawcett , thoughts?

[afawcett]

Yeah it does break that rule, though we don't really have an option if the other trigger is from a managed package. As you say there is the option for sandbox developers to merge the triggers if they are concerned about this. Though not sure if all other classes related to the trigger would need to be pushed, would it not just be the new merged trigger?

Regarding improving the checking at line 162, i'm not clear on your motivation for this? Whats the use case / error / background for you making this observation, if i knew that it might help me understand.

As regards using the Apex MD API from a Trigger context thats a no no, since its a HTTP callout and they are not permitted from a Trigger context.

[afawcett]

Sorry for the late reply btw, i've been on holiday.

[wes1278]

No need to apologize! I'm sure your holiday was much deserved and I'm actually glad you took some away.

Let me explain what I ran into so you can understand what I mean about how it's a pain having the trigger as-is.

Let's say I have a pre-existing org with triggers on say, every object (for the sake of the argument) If I want to use DLRS, I have to go and create a new child trigger BEFORE I can make my dlrs records "active". That's no big deal since I'm developing in a sandbox. Let's say I want to use DLRS on 4-5 other objects and I deploy child triggers for all of them. Great, now I can activate all my DLRS records and I see the results I was looking for. But, let's say I start to have some unexpected behavior because triggers are firing "out of order" Uh-oh! Well, I know what I need to do because I subscribe to the one trigger per object methodology and went and combined all the triggers. Now, the DLRS rollup method was commented out of the trigger that was auto-generated, and the method call line now exists in the pre-exising triggers.

Fast forward and it's time for deployment. In order to get any of this to work, you have to deploy all the triggers that were auto-generated and have no active code in them. If you don't, you cannot insert your dlrs records as active because the trigger will fire and say no, the class that I'm expecting to be there to run the DLRS method isn't there and you cannot activate the DLRS records. BTW, those dummy triggers will have to stay there forever too. They can't be removed after the fact if you ever want to to edit a dlrs record for that same child.

I'd much rather build that trigger logic into some sort of diagnostic log than prevent the saving of the dlrs records as active. If they are active, and don't have the trigger, the worst thing that will happen is that nothing happens. I'm ok with that.

What I was imagining we might want to do is to put a callout to the metadata api in a future method of the trigger on dlrs records. This future method would "snif out" the code that calls DLRS for whatever object is in context. Then, it would update some field on the DLRS record saying, "This dlrs record is active but it doesn't seem like it will be called from any child triggers. Please deploy a child trigger" That way, if DLRS was being called from some non-auto-generated trigger, we won't have to deploy these dummy triggers just to get the records set to active.

Let me know your thoughts.

-Wes

[afawcett]

Ah ok i get it now. I am wondering if the Developer mode would help here though? Have you tried that, in that mode you can active the rollup without it checking for a trigger, because it assumes you have called the rollup elsewhere from another trigger. However i think the entry point into the tool for the generated trigger code (that you would be merging into your other triggers) only looks for Realtime mode ones. So i think maybe we need a new mode Realtime (Developer Managed) or something? Make sense?

[wes1278]

Agreed. I like the idea of a different type that would treated exactly like a realtime except for the DLRS trigger doesn't enforce the existence of the generated code.

[jondavis9898]

Hello @wes1278 - If I'm understanding your situation, I think #236 & #221 should provide what you need.

[wes1278]

hey @jondavis9898 / @afawcett Can someone summarize/explain the end-state of developer API options that are changing? I'm trying to retrace all the amazing commits/issues that you and @jondavis9898 have been working through.

[jondavis9898]

Hi @wes1278 - Happy to explain. In short, the goal of the new dev API was to mimic the behavior of triggerHandler() functionally speaking. There are two primary benefits to this:

1. Ability to invoke rollups in a trigger context from somewhere other than the trigger that is required when using "Realtime" mode. In your case, this would allow you to configure your rollups as "Developer" mode and then invoke them using the new API as follows:

rollup(Trigger.oldMap, Trigger.newMap, Account.SObjectType)

2. Ability to gain the benefit of change detection when manually evaluating/processing rollups. The existing developer API would recalculate every master id. The new developer API takes old & new maps and performs change detection so that only the parents that have a child that changed will actually be updated. Additionally, the new API takes care of the updates as compared to returning the new values. There are still times when the previous API can/should be used but the new API more closely aligns to the typical method of evaluating/processing rollups. Along these lines, the new dev API does not require that you use Trigger context maps as you can pass any combination of old/new record maps to the API. This provides a means to manually determine what needs to be rolled and incorporates change detection to minimize the surface area of what actually gets updated.

One final note is that the new Dev API does not handle Scheduled rollups, only Developer rollups. The triggerHandler handles Realtime & Scheduled so this is the one difference between triggerHandler and new Dev API. If this should ever become a need, after discussions with @afawcett, my thinking is that we would add a new overload to rollup that would accept a boolean to process the rollups asynchronously.

rollup(Map<Id, SObject> existingRecords, Map<Id, SObject> newRecords, Schema.SObjectType sObjectType, Boolean async)

Hope this helps. Let me know if you have any questions or would like any additional info.

[daveerickson]

This seems to be an extension to this request, but I can put it in it's own issue if preferred.

I think we need a way to say Scheduled - Developer. I've had to deploy some triggers and inactivate them to achieve the same functionality. I am using Apex Enterprise Patterns and the Developer mode on my DLRS records. That works great for making realtime calculations work, however I cannot keep the record Active if I change it to Scheduled. The desired effect would be able to use the dlrs.RollupService.rollup calls in the Domain class and be able to set the DLRS record to Scheduled and Active.

[afawcett]

Yes actually in hindsight having a checkbox for Developer Mode (preventing the default DLRS engine entry points from processing), instead of it being a picklist item in Calculation Mode. This would have been better i feel. I agree in the meantime this is a valid extension to this discussion so lets keep it here for now.

[shawnholt]

Note that I started to receive error after winter 17 update on production. tried to disable/re-enable triggers but error persists:

"First error: Update failed. First exception on row 0 with id 0016100000eVLPSAA4; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, dlrs_AccountTrigger: execution of AfterUpdate
caused by: System.NoAccessException: Entity is not api accessible
Class...."
Context: npsp__RLLP_OppAccRollup_BATCH
Stack Trace:
null

[afawcett]

@shawnholt i'm not sure your update is relevant to this issue, but thanks for sharing. A number of other users are seeing this since Winter'17 rolled out. There is an issue raised #429 if you want to chip in. I've marked it as a priority fix.