Sign Up ›

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
Jonathan Osgood 3Jonathan Osgood 3 

Trigger to replace a report formula calculation

Hi All,

I'm working on a trigger and could use some help. I've hit a report formula limitation and am trying to recreate the same logic in a trigger. The basic requirement is to collect a field value from child objects, run a simple calculation, then assign that value to a field on the parent account. From the debug statemtents, it seems that things seem to fall apart at line 55. Any feedback would be appreciated! 

Thank you!
  
trigger JobsIncomeCalc on Form__c (after insert, after update, after delete) {
    
    //Forms List
    List <Form__c> FormsList = [SELECT id,
                                Account__c,
                                Account__r.id,
                                X2_1_a_Total_of_jobs_Year_1__c, 
                                X2_1_a_Total_of_jobs_Year_2__c
                                FROM form__c];
    
    
    //Distinguish whether the list should act on insert, update or delete
    
    if(trigger.isInsert || trigger.isUpdate){
        FormsList = Trigger.New;
    }
    if(trigger.isDelete){
        FormsList = Trigger.Old;
    }
    
    //Account Ids Set 
    Set <id> accountIds = New Set <id>();
    
    //Add form account ids to set
    for(Form__c F: FormsList){
        
        accountIds.add(f.Account__c);
        System.debug('account id'+f.Account__c);
    }
    
    //Map to pair accounts and forms
    Map<id,Double> formAccountMap= New Map<id,Double>();
    
    for(form__c f: [SELECT id,
                    Account__r.id,
                    Account__r.X2_1_a_Total_of_Jobs_Hist_Cumulative__c
                    FROM Form__c 
                    WHERE Account__r.id IN: accountIds]){
                        
                        Double AccTotalJobsHist = f.Account__r.X2_1_a_Total_of_Jobs_Hist_Cumulative__c;                   
                        formAccountMap.put(f.Account__r.ID, AccTotalJobsHist);                
                        
                        system.debug('AccTotalJobsHist='+ AccTotalJobsHist);
                        system.debug('Account total jobs from form map='+ formAccountMap.get(f.Account__r.ID));
                        
                        
                    }
    
    //Accounts to update list
    List<Account> AcctsToUpdate = new List<Account>();
    
    system.debug('AcctsToUpdate='+AcctsToUpdate);
    //Apply calculation to all forms in FormList
    
    for(Account a: AcctsToUpdate){
        
        Double AccTotalJobsHist = formAccountMap.get(a.ID);
        system.debug('AccTotalJobsHist='+AccTotalJobsHist);
        system.debug('AccTotalJobsHist='+formAccountMap.get(a.ID));
        
        Double TotalJobsY1 = a.form__r.X2_1_a_Total_of_jobs_Year_1__c;
        Double TotalJobsY2 = a.form__r.X2_1_a_Total_of_jobs_Year_2__c;
        system.debug('TotalJobsY1='+TotalJobsY1);
        system.debug('TotalJobsY2='+TotalJobsY2);
        
        
        Double CalculatedTotalJobs = (TotalJobsY2 - TotalJobsY1);
        system.debug('CalculatedTotalJobs='+CalculatedTotalJobs);
        
        //Set calculated value in account field    
        
        AccTotalJobsHist = CalculatedTotalJobs; 
        
        AcctsToUpdate.add(a);
    }
    
    Update AcctsToUpdate;
    
}

 
Best Answer chosen by Jonathan Osgood 3
pconpcon
The problem with lines 4-9 is that the query is not restricted and if it ran in production then it could return too many rows and you are writing over the variable on line 14-19.

The code below should do what you are trying to do.
  
trigger JobsIncomeCalc on Form__c (after insert, after update, after delete) {
    List <Form__c> formsList = new List<Form__c>();

    if (Trigger.isInsert || Trigger.isUpdate){
        formsList = Trigger.new;
    } else if (trigger.isDelete){
        formsList = Trigger.old;
    }

    Set<Id> accountIds = new Set<Id>();

    for (Form__c form : formsList) {
        accountIds.add(form.Account__c);
    }

    accountIds.remove(null);

    if (!accountIds.isEmpty()) {
        Map<Id, Decimal> accountMap = new Map<Id, Descimal>();

        for (form__c f: [
            select Account__c,
                X2_1_a_Total_of_jobs_Year_1__c,
                X2_1_a_Total_of_jobs_Year_2__c
            from Form__c
            where Account__c in :accountIds
        ]){
            if (!accountMap.contiainsKey(f.Account__c)) {
                accountMap.put(f.Account__c, 0);
            }

            Double calculated = accountMap.get(f.Account__c);
            calculated += form.X2_1_a_Total_of_jobs_Year_1__c - X2_1_a_Total_of_jobs_Year_2__c;
            accountMap.put(f.Account__c, calculated);
        }

        if (!formAccountMap.isEmpty()) {
            List<Account> acctsToUpdate = new List<Account>();

            for (Id id : accountMap.keySet()) {
                acctsToUpdate.add(new Account(
                    Id = id,
                    X2_1_a_Total_of_Jobs_Hist_Cumulative__c = acctMap.get(id)
                ));
            }

            update acctsToUpdate;
        }
    }
}
NOTE: This code has not been tested and may contain typographical or logical errors
 

All Answers

pconpcon
So the first problem with your trigger is that line 4-9 are useless and also can get you into real problems with the governor limits on the platform.  Then your second problem is on line 55 you are iterating over an empty list that you created on line 50.  Can we take a step back and talk about what you are trying to accomplish with this trigger?  Are you trying to do the following:

When a Form is inserted, all of the forms for the account that it is inserted for should have it's total jobs for year 1 added together and then it's total jobs for year 2 added together and then those two subtracted and stored on the account?
Jonathan Osgood 3Jonathan Osgood 3
Thanks for the feedback. Yes, that is the general requirement for the trigger: populate a field on Account (X2_1_a_Total_of_Jobs_Hist_Cumulative__c) with the difference of X2_1_a_Total_of_jobs_Year_1__c and X2_1_a_Total_of_jobs_Year_2__c) on the child Form__c records. 

Can you tell me why 4-9 are problematic? The soql is not in the for loop? I see the problem with iterating over the empty list -- that is very helpful and my biggest hang up right now. How I can I fix it? I can't iterate accounts over trigger.new and got an error when I tried to poplulate the list with a soql statemnt where account id is in accountIds. 

Thanks!
pconpcon
The problem with lines 4-9 is that the query is not restricted and if it ran in production then it could return too many rows and you are writing over the variable on line 14-19.

The code below should do what you are trying to do.
  
trigger JobsIncomeCalc on Form__c (after insert, after update, after delete) {
    List <Form__c> formsList = new List<Form__c>();

    if (Trigger.isInsert || Trigger.isUpdate){
        formsList = Trigger.new;
    } else if (trigger.isDelete){
        formsList = Trigger.old;
    }

    Set<Id> accountIds = new Set<Id>();

    for (Form__c form : formsList) {
        accountIds.add(form.Account__c);
    }

    accountIds.remove(null);

    if (!accountIds.isEmpty()) {
        Map<Id, Decimal> accountMap = new Map<Id, Descimal>();

        for (form__c f: [
            select Account__c,
                X2_1_a_Total_of_jobs_Year_1__c,
                X2_1_a_Total_of_jobs_Year_2__c
            from Form__c
            where Account__c in :accountIds
        ]){
            if (!accountMap.contiainsKey(f.Account__c)) {
                accountMap.put(f.Account__c, 0);
            }

            Double calculated = accountMap.get(f.Account__c);
            calculated += form.X2_1_a_Total_of_jobs_Year_1__c - X2_1_a_Total_of_jobs_Year_2__c;
            accountMap.put(f.Account__c, calculated);
        }

        if (!formAccountMap.isEmpty()) {
            List<Account> acctsToUpdate = new List<Account>();

            for (Id id : accountMap.keySet()) {
                acctsToUpdate.add(new Account(
                    Id = id,
                    X2_1_a_Total_of_Jobs_Hist_Cumulative__c = acctMap.get(id)
                ));
            }

            update acctsToUpdate;
        }
    }
}
NOTE: This code has not been tested and may contain typographical or logical errors
 
This was selected as the best answer
Jonathan Osgood 3Jonathan Osgood 3
Thank you pcon, that did it. I really appreciate the explanations --incredibly helpful. Everything makes sense with one exception. Line 41, I see that you are adding accounts to the list, but it appears that it's a NEW account. I know this isnt the case, but I'm trying to understand what's happening there. 

Thanks again!
pconpcon
Sure, not a problem.  So line 41-44 are not creating a new account, but they are creating a new instance of the account object.  By setting the Id on line 42 we are setting up to be an update.  Then we populate the data we want to update and add it to the list.  We'll later run the update on that and it'll only update the single field we pass in.
Jonathan Osgood 3Jonathan Osgood 3
Got it, thanks! I've hit a snag on my difference of years calculation. Wondering if you've got any thoughts. I'm actually calculating years 1-5. (year2 - year1) + (year4 - year 3) etc etc. This works fine except when a year is not reported on yet. For example, if only years 1-3 have values, the calculation become 0 (year4) - year3 which produces a negative number and skews the data. In my Report formula, I acounted for this by using IF(year 2 >0, then Year2-Year1) etc etc. I know I can accomplish in apex using the same IF statement approach, but that seems sloppy. I've looked into the Double class documentation for a useful method but didn't find anything. Any ideas? 
  
calculated += (f.X2_1_a_Total_of_jobs_Year_2__c - f.X2_1_a_Total_of_jobs_Year_1__c)+
                          (f.X2_1_a_Total_of_jobs_Year_4__c - f.X2_1_a_Total_of_jobs_Year_3__c)+
                          (f.X2_1_a_Total_of_jobs_Year_5__c - f.X2_1_a_Total_of_jobs_Year_4__c);

 
pconpcon
You're best way is to just put the logic in an if statement
Jonathan Osgood 3Jonathan Osgood 3
Perfect, thanks. So this is what I came up with:
  
            Double diffYr21;
            Double diffYr32;
            
            if(f.X2_1_a_Total_of_jobs_Year_2__c > 0){
               diffYr21 = f.X2_1_a_Total_of_jobs_Year_2__c - f.X2_1_a_Total_of_jobs_Year_1__c;
            }else {
                diffYr21 = f.X2_1_a_Total_of_jobs_Year_1__c;
            }
            
            
            
            if(f.X2_1_a_Total_of_jobs_Year_3__c > 0){
                diffYr32 = f.X2_1_a_Total_of_jobs_Year_3__c - f.X2_1_a_Total_of_jobs_Year_2__c;
            }else {
                diffYr32 = f.X2_1_a_Total_of_jobs_Year_2__c;
            }
            
            
              
            Double calculated = accountMap.get(f.Account__c);
            calculated +=  diffYr21 + diffYr32;
            
            accountMap.put(f.account__c, calculated);    
        }
But now I seem to be writing over my diffYr21 variables with each iteration over form records. What I really need is something like 
  
​calculated +=  if(f.X2_1_a_Total_of_jobs_Year_3__c > 0){ 
                  diffYr32 = f.X2_1_a_Total_of_jobs_Year_3__c - f.X2_1_a_Total_of_jobs_Year_2__c;                                 
  } else { 
                  diffYr32 = f.X2_1_a_Total_of_jobs_Year_2__c; 
        }

I fell like I'm not utilizing the map properly to isolate each recrod individually.