0

I wrote a trigger which will count the no of primary contacts associated to an account which is working for insert, delete events but not when I update a contact's Primary checkbox from True to False or vice versa. Kindly help with what is going wrong here for update operation.

This the Trigger code:

trigger ContactTriggerforCountingTotalnumberforanAccount on Contact (before insert, after insert, after update, after delete, after undelete) {
    if( Trigger.isAfter ) {

        if( Trigger.isInsert ) {
            ContactTriggerHandler.updateCount( trigger.new, null );
        }
        else if( Trigger.isUpdate) {
            ContactTriggerHandler.updateCount( trigger.new, trigger.oldMap );
        }
        else if( Trigger.isUndelete) {
            ContactTriggerHandler.updateCount( trigger.new, null );
        }
        else if( Trigger.isDelete ) {
            ContactTriggerHandler.updateCount( trigger.old, null );
        }
    }
    
}

Trigger Handler code:

public class ContactTriggerHandler {
    
    public static void updateCount( List<Contact> listContacts, Map<Id,Contact> mapOldContacts ) {

        Set<Id> setAccountId = new Set<Id>();
        if( mapOldContacts == null || mapOldContacts.isEmpty() ) {
            for( Contact objCont : listContacts ) {
                if( objCont.AccountId != null ) {
                    setAccountId.add( objCont.AccountId );
                }
            }
        }
        else if( mapOldContacts != null && !mapOldContacts.isEmpty()) {
            for( Contact objCont : listContacts ) { 
                if( objCont.AccountId != null 
                && mapOldContacts.get( objCont.Id ).AccountId != objCont.AccountId ) {
                    setAccountId.add( objCont.AccountId );
                    if( mapOldContacts.get( objCont.Id ).AccountId != null ) {
                        setAccountId.add( mapOldContacts.get( objCont.Id ).AccountId );
                    }
                }
                else if( objCont.AccountId == null && mapOldContacts.get( objCont.Id ).AccountId != objCont.AccountId ) {
                    setAccountId.add( mapOldContacts.get( objCont.Id ).AccountId );
                }
            }
        }
        if( !setAccountId.isEmpty() ) {
            List<Account> listAccount = new List<Account>();
            for( Account objAcc : [ SELECT 
                                        Id,
                                         ( SELECT Id FROM Contacts WHERE Primary__c=true)
                                    FROM
                                        Account
                                    WHERE
                                   Id In : setAccountId ] ) {
                objAcc.Total_no_Of_Contacts__c = objAcc.Contacts != null && ( !objAcc.Contacts.isempty() ) ? objAcc.Contacts.size() : 0;
                listAccount.add( objAcc );
            }

            if( !listAccount.isEmpty() ) {
                update listAccount;
            }
        }
    }
}

1 Answer 1

1

"Getting it right" is usually pretty hard. If you can, I'd suggest avoid trying to do this yourself and just using DLRS. It takes care of all the trigger work for you.

Your update logic only concerns itself if the AccountId changes, not if the Primary checkbox changes. This means that your logic doesn't work correctly for the scenario where the Account ID doesn't change, but the Primary checkbox does.

Instead, you can make your trigger act correctly by doing less work. Here's an example version that I might write from your code.

public class ContactTriggerHandler {
    public static void updateCount(Contact[] contacts1, Contact[] contacts2) {
        Set<Id> accountIds = new Set<Id>();
        for(Contact record: contacts1) {
            accountIds.add(record.AccountId);
        }
        for(Contact record: contacts2) {
            accountIds.add(record.AccountId);
        }
        // Ignore null Id
        accountIds.remove(null);
        Map<Id, Account> accounts = new Map<Id, Account>([
            SELECT Total_no_Of_Contacts__c FROM Account WHERE Id = :accountIds
        ]);
        Map<Id, Account> accountUpdates = accounts.deepClone();
        // Reset the counters
        for(Account record: accountUpdates.values()) {
            record.Total_no_of_Contacts__c = 0;
        }
        // Get counts for each account Id
        for(AggregateResult result: [
            SELECT AccountId Id, COUNT(Id) sum
            FROM Contact
            WHERE AccountId = :accountIds AND Primary__c = true
            GROUP BY AccountId
        ]) {
            accountUpdates.put(
                (Id)result.get('Id'),
                new Account(Id=(Id)result.get('Id'), Total_no_Of_Contacts__c = (Decimal)result.get('sum'))
            );
        }
        // If the number hasn't changed, ignore the account
        for(Id accountId: accountIds) {
            if(accounts.get(accountId).Total_no_Of_Contacts__c == accountUpdates.get(accountId).Total_no_Of_Contacts__c) {
                accountUpdates.remove(accountId);
            }
        }
        update accountUpdates.values();
    }
}

Note that I don't try to see if an Account needs to be updated until the final step. To avoid nulls, we'll update the trigger to always pass in both values. Since we're not doing as much work, even a list of 200 records won't be inefficient.

trigger ContactTriggerforCountingTotalnumberforanAccount on Contact (before insert, after insert, after update, after delete, after undelete) {
    if( Trigger.isAfter ) {
        Contact[] contacts1 = Trigger.new == null? Trigger.old: Trigger.new;
        Contact[] contacts2 = Trigger.old == null? Trigger.new: Trigger.old;
        ContactTriggerhandler.updateCount(contacts1, contacts2);
    }
}
2
  • When I change the account the update logic counts the total no of related contacts correctly. However when I edit the contact to make it not primary it is not working. As my code was working correct for insert, delete and undelete operations with out using Aggregate function can you please help to find where exactly the code is breaking when I update a contact to Primary or change- the Primary field from true to false. Any help in this regard will be appreciated Aug 20, 2023 at 8:33
  • @AritraDasgupta I already explained it. Your code must check if AccountId or Primary__c changes. Your code right now checks only for AccountId. However, my answer was written to give you superior code that runs fast.
    – sfdcfox
    Aug 20, 2023 at 16:45

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .