0

I have written the following code to update a custom field (Total_Amount__c = Sum of the amount of all opportunities related to the account) on the update of Account. The following code works but updates all the accounts in the org when an account is updated.

//Scenario: Apex class to fetch the Sum of Amount of all the opportunities present under an account 
public class AccountTotalAmountHandler {
    public static Boolean isFirstTime = true;
    public static void accountAmount(List<Account> listOfUpdatedAccounts){
        Map<Id, Account> mapAcc = new Map<Id, Account>();
        Set<Id> opportunityWithAccountSet = new Set<Id>();
        // List of opportunity records without Account
        List<Opportunity> opportunityRecords = [SELECT Id, AccountId, Name FROM Opportunity WHERE AccountId != Null];
        // Storing the Accounts with opportunities
        for(Opportunity oppObj : opportunityRecords){
            opportunityWithAccountSet.add(oppObj.AccountId);
        }
        if(opportunityWithAccountSet!=null && isFirstTime == TRUE){
            isFirstTime = FALSE;
            for(AggregateResult agrObj: [SELECT   AccountId, Sum(Amount) sumAmount FROM Opportunity 
                                         WHERE    AccountId IN :opportunityWithAccountSet 
                                         GROUP BY AccountId]){                        
                                             Account accObj = new Account();
                                             accObj.Id = (Id)agrObj.get('AccountId');                           
                                             accObj.Total_Opportunity_Amount__c = (Decimal)agrObj.get('sumAmount');  
                                             mapAcc.put(accObj.Id, accObj);
                                         }
        }
        if(mapAcc.values()!= null  && mapAcc.size()>0){
            update mapAcc.values();
        }
    }
}

I just want to update the Total Amount of the account which is getting updated.

5
  • Why dont you use a trigger? trigger triggername on Account (before update) Jun 26 at 13:36
  • It is a trigger. I have just added the handler class
    – goku
    Jun 26 at 13:37
  • Okay so than List<Account> listOfUpdatedAccounts contains all accounts? how do you obtain this list? Jun 26 at 13:42
  • Is there a particular reason why you're not using a rollup summary field here? If you don't want to use that, then why not use Andrew Fawcett's DLRS? I usually advocate for writing code, but both of those options are performant and mature.
    – Derek F
    Jun 26 at 13:47
  • I am just practicing and found this question in one of the forums
    – goku
    Jun 26 at 13:48
0

Looking at your code, it shouldn't come as a surprise that it's updating all accounts (with opps).

  • You query for Opportunities where AccountId != null (which is likely to be all of the Opportunities in your org)
  • You iterate over them to add the account ids
  • You then use a query to sum all opps on all accounts you just found

Based on the argument that your method takes, you're getting a List of accounts. If you're only interested in recalculating this sum for the Accounts that are being updated in some transaction, then there's literally no reason for you to perform the first query.

Set<Id> opportunityWithAccountSet = new Set<Id>();
// List of opportunity records without Account
List<Opportunity> opportunityRecords = [SELECT Id, AccountId, Name FROM Opportunity WHERE AccountId != Null];
// Storing the Accounts with opportunities
for(Opportunity oppObj : opportunityRecords){
    opportunityWithAccountSet.add(oppObj.AccountId);
}

can be replaced with

for(Account acct :listOfUpdatedAccounts){
    opportunityWithAccountSet.add(acct.Id);
}

Some other notes:

  • You should never use a static Boolean to try to prevent trigger recursion. If you update more than 200 Accounts, this code would only run on the first 200.
  • If you see something like <boolean value> == true, you can (should?) replace it with simply <boolean value>. The == operator checks for equality and gives you a boolean value as a result. Since you already have a boolean, there is no need to do an equality comparison x == true returns true when x is true, and false when x is false. In other words, x == true and x are exactly equivalent (and likewise, x == false is the same as !x).
  • Checking things like if(mapAcc.values()!= null && mapAcc.size() > 0) before performing DML generally provides no benefit and is a waste of effort. An initialized map will never be null (nor will the list you get from values(), and Salesforce is smart enough to not perform DML if the collection is empty.
  • Since you're working on accounts, and wanting to update the same accounts that are taking part in the trigger, this should be run as part of a before update trigger. That makes the DML at the end (as well as your recursion checking) unnecessary.
    • The "automatic update" for records in before triggers only happens when you modify the instance of the record contained in trigger.new or trigger.newMap. Lucky for us, putting records into Lists and Maps makes a reference to the original record rather than a copy of it. If you pass trigger.new into listOfUpdatedAccounts (or loop over trigger.new to make a list that you then pass in as an argument to your handler), then you have a reference to the record in trigger.new. Changes to a reference also change the original instance.

The one case that most people forget about when trying to "roll their own" rollup code is what happens when a record no longer has any children?. I think that's what your initial query was attempting to handle (finding out which accounts either have no opps or no longer have any opps).

There are times when figuring out what to do with that situation is hard, but in this case you can use the easy approach of "assume all Accounts have no Opps, set a default value for your rollup field, and then roll up data for accounts that do have opps".

Map<Id, Account> accountsToUpdateMap = new Map<Id, Account>();

// default the rollup field to 0
// assuming you're working in a before trigger (which you should be)
for(Account acct :incomingAccountsListFromTriggerDotNew){
    accountsToUpdateMap.put(acct.Id, acct);
    acct.Rollup_Field__c = 0;
}

// perform the "rollup"
// If the account has no opps, then we won't update the value in the map.
// Such accounts will then assume the default value when Salesforce automatically
//   updates the values
for(AggregateResult ar :[SELECT AccountId, SUM(Amount) total FROM Opportunity WHERE AccountId IN :accountsToUpdateMap.keySet() GROUP BY AccountId]){
    Account acct = accountsToUpdateMap.get((Id)ar.get('AccountId'));
    acct.Rollup_Field__c = (Decimal)ar.get('total');
}
1
  • Thanks, Derek for the detailed explanation
    – goku
    Jun 26 at 20:23
0

You almost got it. You can receive the Trigger.newMap from the trigger in the before update event and perform the sum without additional DML operation.

The Account trigger

trigger accountTrigger on Account(before update) {
    if (Trigger.isBefore && Trigger.isUpdate) {
        AccountTotalAmountHandler.accountAmount(Trigger.newMap);
    }
}

The Handler class

//Scenario: Apex class to fetch the Sum of Amount of all the opportunities present under an account
public with sharing class AccountTotalAmountHandler {
    public static Boolean isFirstTime = true;

    public static void accountAmount(Map<Id, Account> mapOfUpdatedAccounts) {
        // verify if map is not empty and if is first time
        if (!mapOfUpdatedAccounts.isEmpty() && isFirstTime == true) {
            isFirstTime = false;
            // Aggregate the opportunities belongs to the updated accounts.
            for (AggregateResult agrObj : [SELECT AccountId, Sum(Amount) sumAmount FROM Opportunity WHERE AccountId IN :mapOfUpdatedAccounts.keySet() GROUP BY AccountId]) {
                // Get the current account id to use as key in the received map
                Id accountId = (Id) agrObj.get('AccountId');
                // perform the field update in the received map
                mapOfUpdatedAccounts.get(accountId).Total_Opportunity_Amount__c = (Decimal) agrObj.get('sumAmount');
            }
        }
    }
}
3
  • Hi Luis, Thanks for your inputs but I think the trigger should be working only after the account is updated instead of before update. If I use it in the before update context the value will not be updated
    – goku
    Jun 26 at 14:19
  • Hi goku, I tested and it's working as expected, when you update an Account the Total Opportunity Amount is updated. However, since you're attempting to update the same accounts that are in the trigger execution you must do it in the 'before' event to avoid the recursion you were getting. In the before event the DML operation is performed at the end. Jun 26 at 15:23
  • Thanks Luis! I missed something very silly
    – goku
    Jun 26 at 20:20

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.