trigger working..but not letting me edit any records

Question

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();
    }


}

Answer 1

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!
    }
}