2

I have a custom checkbox infront of every related contact.That checkbox is to make a contact primary.I have written a trigger if there is more than one primary contact than i should display an error..i have a requirement now like if there is only one primary contact ,than the trigger should copy the contact email address to custom email address field on account tab.Can someone gimme a heads up?

the trigger that i have written below ensure that only one primary contact must be there but it does not copy that primary's contacts email address to custom email address on account tab

below is my trigger:

   trigger checkPrimaryField on Contact (before insert,before update) 
{
    list <contact> listcon= new list<contact>();
        String accID;
        integer count=0;

    for(contact c:Trigger.new )
    {
        accid = c.accountid;

     }

     listcon = [SELECT AccountId,Primary_Contact__c FROM Contact WHERE Primary_Contact__c = true AND AccountId = :accid];

     for(contact c : listcon)
     {
         if(c.Primary_Contact__c)
         {    
             count = count+1;
             }
    }
     system.debug('count is ::'+count);
     if(count>1)
     trigger.new[0].adderror('................');

     if(count >=1)
     {
        list <contact> d = [SELECT AccountId,Primary_Contact__c FROM Contact WHERE Primary_Contact__c = true];
         Map<Id, Account> m = new Map<Id, Account>();
        for(Contact c : d)
        {
            if (Trigger.newMap.get(c.Id).Primary_Contact__c= true) 
            {
                m.put(c.AccountId, new Account(Id = c.AccountId, Account_Email__c = c.Email));
            }
        }
    update m.values();
    }


}
  • So what is the exact issue you're facing? Just a heads up, your first loop does nothing but ensure that only the last Contact's (of the batch) related Contacts are queried, since you set accid to c each iteration... – Brian Mansfield Jun 9 '15 at 15:34
  • @brianmansfield - please see my updated query and code above..above code does not copy the primary contacts email address to custom email field on account tab – Ankita Singh Jun 9 '15 at 15:37
3

You should not validate that there is only one Primary Contact in the trigger context, but rather that there is only one per Account. For this you will need a Map<Id, List<Contact>> accountToPrimaryContacts map, where you only add Primary Contacts to the map, and if the size of any of those lists is more than 1, you should add an error to any such contacts that are in the trigger context.

You should also collect all Account Ids as a Set and query on that so you check against every related account and not just the last one. Note that you use the last Contact in your first for loop, but validate the first Contact in the list. This code will lead to very buggy and inconsistent behavior.

Set<Id> accountIds = new Set<Id>();
for (Contact newContact : trigger.new)
{
    accountIds.add(newContact.AccountId);
}

Map<Id, List<Contact>> accountToPrimaryContacts = new Map<Id, List<Contact>>();
for (Contact primaryContact : [SELECT AccountId FROM Contact WHERE AccountId IN :accountIds])
{
    if (!accountToPrimaryContacts.containsKey(primaryContact.AccountId))
        accountToPrimaryContacts.put(primaryContact.AccountId, new List<Contact>());
    accountToPrimaryContacts.get(primaryContact.AccountId).add(primaryContact);
}

for (Id accountId : accountToPrimaryContacts.keySet())
{
    if (accountToPrimaryContacts.get(accountId).size() > 1)
    {
        for (Contact primaryContact : accountToPrimaryContacts.get(accountId))
        {
            if (trigger.newMap.containsKey(primaryContact.Id))
                primaryContact.addError('message');
        }
    }
}

As always, MOVE YOUR CODE OUT OF THE TRIGGER BODY. Here is a good primer on trigger best practices. Also if you are sticking to One Trigger Per Object, your trigger should be named Contact or ContactTrigger depending on your preference.

I would move the above code that gets you accountToPrimaryContacts into a static method that returns that map. You can then have two methods that accept this map, one for validation, one for Account updates. Really these are stateful operations, but showing you a more elegant way to manage that state seems like it would risk confusion.

public static void validateOnePrimary(Map<Id, List<Contact>> primaryContacts, Map<Id, Contact> newMap)
{
    for (Id accountId : primaryContacts.keySet())
    {
        List<Contact> primary = primaryContacts.get(accountId);
        if (primary.size() > 1)
        {
            for (Contact primaryContact : primary)
            {
                if (newMap.containsKey(primaryContact.Id))
                    newMap.get(primaryContact.Id).addError('message');
            }
        }
    }
}

public static void setAccountEmail(Map<Id, List<Contact>> primaryContacts)
{
    List<Account> accounts = new List<Account>();
    for (Id accountId : primaryContacts.keySet())
    {
        List<Contact> primary = primaryContacts.get(accountId);
        if (primary.size() == 1)
            accounts.add(new Account(Id=accountId, Account_Email__c=primary[0].Email));
    }
    try
    {
        update accounts;
    }
    catch (DmlException dmx)
    {
        // you can do this part!
    }
}
  • This link is awesome, very helpful for me!! Although I must say so far, I haven't come into a client org and seen something as beautiful as this "one trigger per object" implemented. Maybe someday, one can hope... – Brian Mansfield Jun 9 '15 at 15:54
  • Consolidating into one trigger body is a project that can certainly be daunting for some orgs. The trigger handler pattern makes it somewhat less so, as you can move each trigger into a static service, and then call that from your handler so the eventing is clear. For instance this trigger might become ContactServices.validatePrimary and then your handler would call it from its beforeInsert and beforeUpdate methods. – Adrian Larson Jun 9 '15 at 16:01
  • Also makes order of execution super easy to control. – Adrian Larson Jun 9 '15 at 16:02
  • i am getting the following error while using the trigger u mentioned above Error: Invalid Data. Review all error messages below to correct your data. Apex trigger checkPrimaryField caused an unexpected exception, contact your administrator: checkPrimaryField: execution of BeforeUpdate caused by: System.FinalException: SObject row does not allow errors: Trigger.checkPrimaryField: line 24, column 1 – Ankita Singh Jun 10 '15 at 5:28
  • Sorry can only add errors to the trigger new reference. Add the map as a parameter. Again, you're managing a lot of state and a helper class would clean this up. Kind of outside the scope of your question. – Adrian Larson Jun 10 '15 at 5:43

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.