2

I have a trigger written on Opportunity and I think it's fairly efficient but when I tried to bulk test it, I received the CPU time limit exceeded error. I know I can get around this by putting it in a future method but was curious as to whether or not there is something simple that I'm missing?

Any help is greatly appreciated!

Trigger

trigger Opportunity_Trigger on Opportunity (after insert, after update, after delete) {

    if(Trigger.IsInsert){
        for(Opportunity o : Trigger.new){
            if(o.Decision_Maker__c != null){
                Opportunity_Methods.addContactRole(Trigger.new);
            }
            if(o.StageName == 'Closed Won'){
                Opportunity_Methods.updateAcctClosedWon(Trigger.new);
            }
        }
    }

    if(Trigger.isUpdate){
        for(Opportunity o : Trigger.new){
            if(o.StageName == 'Closed Won' && Trigger.oldMap.get(o.id).StageName != 'Closed Won'){
                Opportunity_Methods.updateAcctClosedWon(Trigger.new);
            }
            if(o.Decision_Maker__c != null && Trigger.oldMap.get(o.id).Decision_Maker__c == null){
                Opportunity_Methods.addContactRole(Trigger.new);
            }
        }
    }
}

Apex Class

public class Opportunity_Methods {

    public static void updateAcctClosedWon(List<Opportunity> newList){

        Set<Id> actIds = new Set<Id>();
        Map<Id,Id> idMap = new Map<Id,Id>();

        for(Opportunity o : newList){
            idMap.put(o.AccountId, o.id);
        }

        List<Account> acctList = [SELECT Id, (SELECT Id, AccountId, Order__c, PDF_Attached__c, commission__c, Delivery_Date__c, Inflation__c, Fax__c, Email__c, Payments__c, Main__c FROM Opportunities WHERE Id IN :idMap.values() ) FROM Account WHERE Id IN :idMap.keySet()];

        for(Account a :acctList){
            for(Opportunity o : a.Opportunities){

                    a.Order__c = o.Order__c;
                    a.Commission__c = o.Commission__c;
                    a.inflation__c = o.Inflation__c;
                    a.delivery_Date__c = o.Delivery_Date__c;
                    a.fax__c = o.Fax__c;
                    a.order_Email__c = o.Order_Email__c;
                    a.Payments__c = o.Payments__c;
                        }
            }
        update acctList;
    }
6

You are adding the entire trigger context to your maps for each execution of the loop on a single record then you are calling a method that updates records based on this list each execution of the for loop.

Basically if you had 200 records in context you would be processing each set of 200 records 200 times and performing up to 200 dml statements if all of them entered the updateAccounts method. I believe you want something like this:

trigger Opportunity_Trigger on Opportunity (after insert, after update, after delete) {

Opportunity[] addConRole = New Opportunity[]{};
Opportunity[] updateAcct = New Opportunity[]{};

    if(Trigger.IsInsert){
        for(Opportunity o : Trigger.new){
            if(o.Decision_Maker__c != null){
                addConRole(o);
            }
            if(o.StageName == 'Closed Won'){
                updateAcct(o);
            }
        }
    }

    if(Trigger.isUpdate){
        for(Opportunity o : Trigger.new){
            if(o.StageName == 'Closed Won' && Trigger.oldMap.get(o.id).StageName != 'Closed Won'){
                updateAcct(o);
            }
            if(o.Decision_Maker__c != null && Trigger.oldMap.get(o.id).Decision_Maker__c == null){
                addConRole(o);
            }
        }
    }

      Opportunity_Methods.addContactRole(addConRole);
      Opportunity_Methods.updateAcctClosedWon(updateAcct);
}

This can be refined further and may not work as expected as you did not provide the full code.

| improve this answer | |
  • Thanks @Eric, didn't catch the issue on the trigger, was mostly concerned about the Class! Appreciate it. – sfdcJameson Jun 20 '15 at 23:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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