3

After many attempts on my part, I'm hoping someone is able to help me out with this :)

Background and Desired Result:

I have a seemingly straightforward Apex Trigger that fires as Contacts are inserted, updated, etc...

I would like this trigger to simply update a single field on the Account that the Contact is tied to.

The Issue:

The trigger works as expected when Contacts are created or edited manually through the SFDC interface. However, the trigger does not appear to fire when thousands of contacts are updated in mass (i.e. via the Import Wizard).

I have included my code below. Any help would be greatly appreciated!

My Apex Code:

Trigger setNTCOnAccountViaContact on Contact (after INSERT, after UPDATE) {

Set<ID> setAccountIDs = new Set<ID>();
for (Contact c : Trigger.new) {
    setAccountIDs.add(c.AccountID);
}

String accountID = null;
for (String setElement : setAccountIDs) {
    accountID = setElement;
    break;
}

List<Contact> nextTargetContactObject = [SELECT Id FROM Contact
                                    WHERE Target_Contact__c = TRUE AND
                                    Account.Id = :accountID
                                    ORDER BY Target_Contact_Order__c ASC
                                    LIMIT 1];

List<Account> accounts = [SELECT Id FROM Account WHERE Id = :accountID];                    

if (nextTargetContactObject.size() == 1) {
    for (Account account : accounts) {
        account.Next_Target_Contact_ID__c = nextTargetContactObject[0].Id;
    }
} else {
    for (Account account : accounts) {
        account.Next_Target_Contact_ID__c = NULL;
    }
}
update accounts;                                        

}
  • You will only be working on ONE account since you limit everything via the * accountID* property. If the mass update of contacts spans multiple accounts only ONE of the accounts will be updated – Eric Aug 19 '17 at 4:34
  • What Eric said is true, and it appears your making this extremely complicated, why are you break;? This seems like a rubber ducky problem – EricSSH Aug 19 '17 at 4:35
5

You'll want to read Best Practice: Bulkify Your Code. Your code essentially only works on the first contact in each batch of 200 records processed, because despite all the gyrations your code goes through, it really only works on one record.

A more appropriate trigger would look like the following:

trigger setNTConAccountViaContact on Contact (after INSERT, after UPDATE) {
    // Data we'll use in this trigger
    Set<Id> accountIds = new Set<Id>();
    Account[] updateAccounts = new Account[0];
    Map<Id, String> error = new Map<Id, String>();

    // Find all account Id values
    for(Contact record: Trigger.new) {
        accountIds.add(record.AccountId);
    }
    // Ignore AccountId = null
    accountIds.remove(null);

    // Abort early if no accounts to process
    if(accountIds.isEmpty()) {
        return;
    }
    // Query accounts that need to be updated
    for(Account record: [
            SELECT (SELECT Id FROM Contacts
                    WHERE Target_Contact__c = TRUE
                    ORDER BY Target_Contact_Order__c
                    LIMIT 1)
            FROM Account WHERE Id = :accountIds]) {
        updateAccounts.add(
            new Account(
                Id=record.Id, 
                Next_Target_Contact_ID__c=
                    (record.Contacts.isEmpty()?null:record.Contacts[0].Id)));
    }

    // Attempt to update
    Database.SaveResult[] results = Database.update(updateAccounts, false);

    // NOTE: this is optional; skip if you don't want to report errors.

    // Process errors
    for(Integer index = 0, size = results.size(); index < size; index++) {
        if(!results[index].isSuccess()) {
            error.put(updateAccounts[index].Id, results[index].getErrors()[0].getMessage());
        }
    }
    // Report errors to parent context
    for(Contact record: Trigger.new) {
        if(error.containsKey(record.AccountId)) {
            record.addError('Failed to update account record: '+error.get(record.AccountId));
        }
    }
}
  • What is the benefit to return; isn't it somewhat redundant at that point? – EricSSH Aug 19 '17 at 4:36
  • @EricSSH If a transaction has only "private" contacts, we don't want to continue on. This saves a query and a sub-query (governor limits, you know). Then again, this may be considered a trivial amount of processing, and you just don't care (not everyone will). This is just how I'd write this. – sfdcfox Aug 19 '17 at 4:41
  • Thanks for the explanation, what are the implications if you were to wrap that accountIds.isEmpty() in !accountIds.isEmpty() I think I'm nit picking you just trying to soak it in.. I know it's all personal preference, but is it just habit that you like to do or is there a reason behind it? – EricSSH Aug 19 '17 at 4:43
  • Terrific! I'll give it a shot. Thank you, @sfdcfox ! – Brandon Aug 19 '17 at 4:44
  • 1
    @EricSSH Using if(!empty) { code } means you don't need to write a "negative unit test", but will indent all of your code by an additional level, which makes makes it harder to read if you do this repeatedly-- if(...) { if(...) { if(...) is three levels of indentation, while if(...) ... if(...) ... if(...) ... is only 1. It's really a matter of preference and/or coding styles. There's pros and cons to both. – sfdcfox Aug 19 '17 at 4:52

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.