+ Start a Discussion
kshannonkshannon 

Governor limit issue with Trigger on child to rollup summary to parent

Hey all,

I'm trying to bulkify this trigger I wrote, seems my original setup is causing too many SOQL queries, clearly due to the soql being in the loop. I am working with a few different ways to fix this, but looking for some community guidance if possible. I am triggering on the Revenue object, which is related back to the Account. What I need to do is depending on the Canvas_Id__c, I am updating the overall total per canvas field on the account. 

See code below:
trigger ProductsSoldRollUp on Revenue__c (after delete, after insert, after undelete, 
after update) {
    public static List<Account> accountsList;
    public static List<Revenue__c> updateRevenueList;
    accountsList = new List<Account>();
    
    String ProductRevenueRecordTypeId = [select Id from RecordType where (Name='Product Revenue') and (SobjectType='Revenue__c')].Id;
    String ShippingRevenueRecordTypeId = [select Id from RecordType where (Name='Shipping Revenue') and (SobjectType='Revenue__c')].Id;

    
    for (Integer i = 0; i < Trigger.new.size(); i++)    {
        Integer total_quantity = 0;
    
        Account acct = [SELECT Royalties_Paid__c FROM Account WHERE Id = :Trigger.new[i].Account__c];
        List<Revenue__c> revList = [SELECT Canvas_Id__c, Royalty__c,Quantity__c,RecordTypeId FROM Revenue__c WHERE Account__c = :Trigger.new[i].Account__c];              
            
        for(Revenue__c r : revList){         
            if(r.RecordTypeId == ProductRevenueRecordTypeId && r.Quantity__c != null){
                if(r.Canvas_Id__c == Canvas__c.getInstance('1').Canvas_Id__c.intValue()){ // T-Shirt
                    p_quantity += r.Quantity__c.intValue();
                } else if(r.Canvas_Id__c == Canvas__c.getInstance('2').Canvas_Id__c.intValue()){ // Tank
                    t_quantity += r.Quantity__c.intValue();
                } else if(r.Canvas_Id__c == Canvas__c.getInstance('3').Canvas_Id__c.intValue()){ // Kids
                    e_quantity += r.Quantity__c.intValue();
                }

                total_quantity++;
             }
        }
        acct.p_total__c = p_quantity;
        acct.t_total__c = t_quantity;
        acct.e_total__c = e_quantity;
        
        update acct;
    }
}

I simplified this trigger to just the concept, also need to make the account update into a map so that i'm not constantly updating inside the loop as well.

Thanks all!
Naval Sharma4Naval Sharma4
Hi Kshannon,

I have updated code to reduce your SOQL quaries.
 
trigger ProductsSoldRollUp on Revenue__c (after delete, after insert, after undelete, 
after update) {
    public static List<Account> accountsList;
    public static List<Revenue__c> updateRevenueList;
    accountsList = new List<Account>();
    
    String ProductRevenueRecordTypeId = [select Id from RecordType where (Name='Product Revenue') and (SobjectType='Revenue__c')].Id;
    String ShippingRevenueRecordTypeId = [select Id from RecordType where (Name='Shipping Revenue') and (SobjectType='Revenue__c')].Id;
   Set<Id> accountIds = new Set<Id>();
    for(Revenue__c  record : Trigger.new){
         accountIds.add(record.Account__c);
    }

    Map<Id, Account> mapAccounts = new Map<Id, Account>([SELECT Royalties_Paid__c, (SELECT Canvas_Id__c, Royalty__c,Quantity__c,RecordTypeId FROM Revenues__r) FROM Account WHERE Id in : accountIds]);  
    Map<Id, Revenue__c> mapAccounts = new Map<Id, Account>();
    List<Account> lstAccounts = new List<Account>();
    for (Integer i = 0; i < Trigger.new.size(); i++)    {
        Integer total_quantity = 0;
    
        Account acct = mapAccounts.get(Trigger.new[i].Account__c);
        for(Revenue__c r : acct.Revenues__r){         
            if(r.RecordTypeId == ProductRevenueRecordTypeId && r.Quantity__c != null){
                if(r.Canvas_Id__c == Canvas__c.getInstance('1').Canvas_Id__c.intValue()){ // T-Shirt
                    p_quantity += r.Quantity__c.intValue();
                } else if(r.Canvas_Id__c == Canvas__c.getInstance('2').Canvas_Id__c.intValue()){ // Tank
                    t_quantity += r.Quantity__c.intValue();
                } else if(r.Canvas_Id__c == Canvas__c.getInstance('3').Canvas_Id__c.intValue()){ // Kids
                    e_quantity += r.Quantity__c.intValue();
                }

                total_quantity++;
             }
        }
        acct.p_total__c = p_quantity;
        acct.t_total__c = t_quantity;
        acct.e_total__c = e_quantity;
        
        lstAccounts.add(acct);
    }
    update lstAccounts;
}

 
Mahesh DMahesh D
Hi KShannon,

Please find the below trigger:
 
trigger ProductsSoldRollUp on Revenue__c (after insert, after delete, after undelete,after update) {
    set<Id> accIdSet = new set<Id>();
    
    if(trigger.isinsert || trigger.isUpdate || trigger.Isundelete){
        for(Revenue__c rev: Trigger.new){
			if(Trigger.isInsert || Trigger.isUndelete || (rev.Account__c != Trigger.oldMap.get(rev.Id).Account__c))
				accIdSet.add(rev.Account__c);            
        }
    }
    
    if(trigger.isUpdate || trigger.isDelete) {
        for(Revenue__c rev: Trigger.old){
            if(Trigger.isDelete || (rev.Account__c != Trigger.oldMap.get(rev.Id).Account__c))
				accIdSet.add(rev.Account__c);
        }
    }    
    
	if(!accIdSet.isEmpty()) }
		List<Account> accList = [select Id, p_total__c, t_total__c, e_total__c (Select Id, Account__c, Quantity__c from Revenue__r) from Account Where ID IN: accIdSet];
		
		for(Account acc : accList){
			
			acc.p_total__c = 0;
			acc.t_total__c = 0;
			acc.e_total__c = 0;
			
			for(Revenue__c rev : acc.Revenue__r) {
				acc.p_total__c += rev.Quantity__c;
			}
		}
		update accList;
	}
}

Please do let me know if it helps you.

Regards,
Mahesh
kshannonkshannon
Thanks for the quick responses, Naval this fixes my issue with the SOQL, but the issue that occurs now is that accountsList has duplicatation errors. For example when importing 285 records for the same account of course it won't update, because accountsList has 285 of the same account to update.
kshannonkshannon
Found the solution to also bulkify, hopefully this thread can help someone along the path as well. To bulkify, instead of just "update acct", you must convert to map and update all values.
 
Map<Id,Account> mapAccounts2 = new Map<Id,Account>();
    for(Account acct : lstAccounts){
        mapAccounts2.put(acct.Id,acct);
    }
    if(!mapAccounts2.values().isEmpty()){
        update mapAccounts2.values();
    }

Thanks for the help on the SOQL out of the loop Naval.
Mahesh DMahesh D
Hi Shannon,

We don't need to put into map to bulkify the code. If the values are already exists in the list, we can directly insert into DB.

if there is list of Accounts stored in lstAccounts then you can use the below:
 
if(!lstAccounts.isEmpty()) {
      insert lstAccounts;
}

Regards,
Mahesh
kshannonkshannon
Thanks Mahesh, but this is not only on insert, so this will not work unless you specify the map. If you try to update the list with multiple of the same account id's you will receive an error.
Mahesh DMahesh D
Hi Shannon,

Yes if we are expecting duplicate records then thats true.

But in your scenario, we are storing the IDs in a set "accIdSet". Hence we will not get duplicate Ids any time.

Please do let me know if my thinking is wrong.

Regards,
Mahesh