0

I have a trigger with a trigger handler class on Opportunity. I am writing a batch class to update the stage in Opportunity which should call the trigger to create a record in "Order" object. In the Opportunity trigger, I am looking for recursion and only allowing the trigger to fire once. The issue I am having here is that since the trigger is getting fired multiple times, the handler class is not getting called. If I remove the recursion check, two records get created in "Order" object. I tried the same insert through update of opportunity from Anonymous window and I am having the same issue.

Below is my trigger

trigger OpportunityTrigger on Opportunity (before insert, before update, before delete, after insert, after update, after delete, after undelete) {


    system.debug('recursive');
    system.debug('checkRecursive');
  if(checkRecursive.runOnce()){
        system.debug('Not recursive');
       OpportunityHandler handler1 = new OpportunityHandler();
        if(Trigger.isUpdate && Trigger.isBefore){
            system.debug('Entered');
            handler1.OnBeforeUpdate(Trigger.old, Trigger.new, Trigger.oldMap, Trigger.newMap);
        }
}
}

Here is the handler class:

    public class OpportunityHandler {

        public void OnBeforeUpdate(Opportunity[] oldOpps, Opportunity[] updatedOpps, Map<ID, Opportunity> oldOppMap, Map<ID, Opportunity> updatedOppMap){
           createOrders1(updatedOpps,oldOppMap,updatedOppMap);
        } 


        private void createOrders1(Opportunity[] updatedOpps, Map<ID, Opportunity> oldOppMap, Map<ID, Opportunity> updatedOppMap){
//perform logic

}

Here is the code that I am trying in console and in batch

Opportunity o = new Opportunity();
o.Name = 'Test';
o.Entity__c = 'Invoice';
o.Type__c = 'Invoice';
o.AccountId = '001Z000001gS4Xg';
o.Billing_Contact__c = '003Z000000ZC2A8';
o.CloseDate = Date.newInstance(2030, 06, 01);
o.Category__c = 'Sales';
o.StageName = 'Proposal';

insert o;

List<OpportunityLineItem> oli = new List<OpportunityLineItem>();
 OpportunityLineItem olinew=New OpportunityLineItem();
                        olinew.OpportunityId=o.Id;                       
                        olinew.Quantity = 1;
                        olinew.TotalPrice = 0;
                        olinew.PricebookEntryId = '01uZ00000090HF9';
                        olinew.Date__c = Date.newInstance(2030, 06, 01);
                        olinew.Month__c = 'June';

                        oli.add(olinew);
                       OpportunityLineItem olinew1=New OpportunityLineItem();
                        olinew1.OpportunityId=o.Id;
                        olinew1.Quantity = 1;
                        olinew1.TotalPrice = 5850;
                        olinew1.PricebookEntryId = '01uZ0000008vEjyIAE';
                        olinew1.Date__c = Date.newInstance(2030, 06, 01);
                          olinew1.Month__c = 'June';

                        oli.add(olinew1);
                       OpportunityLineItem olinew2=New OpportunityLineItem();
                        olinew2.OpportunityId=o.Id;
                        olinew2.Quantity = 1;
                        olinew2.TotalPrice = 2000;
                        olinew2.PricebookEntryId = '01uZ0000008vEkIIAU';
                        olinew2.Date__c = Date.newInstance(2030, 06, 01);
                          olinew2.Month__c = 'June';

                        oli.add(olinew2);
insert oli;

Opportunitycontactrole ocr = new OpportunityContactrole(ContactId = o.Billing_Contact__c, OpportunityId=o.Id, IsPrimary=true, Role='Business User');
insert ocr;

o.StageName = 'Closed/Signed';
update o;

1 Answer 1

5
if(checkRecursive.runOnce()){

This is perhaps the worst possible pattern to control recursion, specifically because it causes behavior like what you're observing right now. Here, you seem to have an especially pathological situation where you're allowing the entire trigger to run exactly once per transaction, regardless of the trigger event. That is, the trigger fires once on before insert, and then your recursion blocker turns it off for the entirety of the remainder of the transaction.

There are two patterns you can use that will work for your objective. In either case remove the runOnce() call.

Check Before Taking Action

Make your OnBeforeUpdate() method perform a bulkified SOQL query to see whether the Opportunities have Orders attached to them before it creates them.

This is the best route: have your code check before it takes action to see whether or not it needs to do anything. Then you're safe from recursion.

Store a Set<Id> of Opps for which Orders Have Been Created

This is a similar approach that might be applicable if, for example, checking whether action needs to be taken is extraordinarily complex or expensive.

Maintain a static Set<Id> in your trigger handler. Add to that Set the Ids of those Opportunities for which you create Orders, after you create them. Before creating, check whether, via the Set, you've already taken action for that record.

1
  • I second this. As unbelievable as it sounds if you put "trigger checkRecursive.runOnce" into Google you get a surprising number of hits - most of which are links to best answers from the Salesforce developer forum.. I don't know where this "pattern" got started but it surely has spread like wildfire.
    – Mossi
    Dec 19, 2019 at 19:17

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.