0

I am new to Apex and I am trying to make SOQL to be outside of FOR Loops but then my trigger stopped working. Here is my code:

trigger AccountFieldUpdate on Contract (after insert, after update) {

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

    for (Contract contr : Trigger.new) {
            accountIds.add(contr.AccountId);
    }
        for (Contract con: Trigger.new) {
        Account act = [SELECT ID FROM ACCOUNT WHERE ID =:con.AccountId];
            if (con.Contract_Year__c == '2018' ){ 
                act.Contract_2018__c = con.Id;
                act.Contract_2019__c = null;
                act.Contract_2018__c = null;
            } else if (con.Contract_Year__c == '2019') {
                act.Contract_2019__c = con.Id;
                act.Contract_2020__c = null;
                act.Contract_2018__c = null;
            } else if (con.Contract_Year__c == '2020') {
                act.Contract_2020__c = con.Id;
                act.Contract_2019__c = null;
                act.Contract_2018__c = null;
            }
            update act;
            }
}

Can anyone help me to solve this issue? Thank you!

0
2

Questions of this nature have been discussed multiple times in this forum already. For instance: here, here, here, here and here. This falls under Salesforce Apex Programming 101 so I suggest you go over articles like this and trailhead modules like this that shed light on the fundamentals. Thankfully the internet is flush with content that would help you in this stage.

That said, here is how I would solve your problem. You don't really need a SOQL query to accomplish what you are doing here because, in order to instantiate an Account with a specific Id (so you can update it later), you just need the Id which you are already getting from the Contract object.

trigger AccountFieldUpdate on Contract (after insert, after update) {

    List<Account> accounts = new List<Account>();
    
    for (Contract con: Trigger.new) {
        Id accountId = con.AccountId;
        
        if (accountId != null){
            String contractYear = con.Contract_Year__c;
            
            if(String.isNotBlank(contractYear)){
                Account act = new Account(Id = accountId);
                
                if (contractYear.equals('2018')){ 
                    act.Contract_2018__c = con.Id;
                    act.Contract_2019__c = null;
                    act.Contract_2020__c = null;
                } else if (contractYear.equals('2019')) {
                    act.Contract_2018__c = null;
                    act.Contract_2019__c = con.Id;
                    act.Contract_2020__c = null;
                } else if (contractYear.equals('2020')) {
                    act.Contract_2018__c = null;
                    act.Contract_2019__c = null;
                    act.Contract_2020__c = con.Id;
                }
                
                accounts.add(act);
            }
        }
    }
    
    if (!accounts.isEmpty()) {
        update accounts;
    }
}

I took the liberty of adding some extra data integrity checks and reorganized the code a bit to my liking.

Some other observations:

  1. For the Contract Year 2018, you seem to have missed setting the value for Contract Year 2020 to null. Instead, you are overriding the value for 2018 back to null.
  2. Also, I get a sense that the database design could be somewhat better. At the moment you have Contract fields for specific years stored at the account level. What would happen in 2021? Would you need to create another Contract_2021__c field at the account level? And for the year after Contract_2022__c and so on demanding a change to this trigger code each year? Why not store that data in a related list?
1
  • 2
    N.B. Remove the final if. DML on empty lists does not burn any limits. – cropredy Dec 25 '20 at 20:46

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