Apex Trigger on Account working properly in Sandbox but does nothing in Production

Question

I have written a few triggers for my company's org, all seems to be working correctly except one on Account... It was working for the last few months and just today, I have noticed that it stopped working. There is no error message, it just does nothing.

The code coverage for this code is 100% given the test class is in place.

This is an "After" trigger, in which I have switched to "Before" and again, it works in Sandbox, but does nothing in Production... So I have switched it back to the original state when it worked the first few months after I have deployed it into Production. I have also checked and made sure that all custom validation rules on Accounts and Contacts are mirroring each other between Sandbox and Production.

See code below. Any help or explanation is greatly appreciated!!! Thanks in advance!

trigger moveContactsOutOfView on Account (After Update) {
    if(checkRecursive.runOnce()){

        set<String> notAccId = New Set<String>();     
        set<String> accId = New Set<string>();
        List<Contact> conUpdate = new List<Contact>();
        List<Contact> con1Update = new list<Contact>();
        List<AccountTeamMember> d1 = new List<AccountTeamMember>();

        for (Account a1: trigger.new){
            if (a1.ID != null && a1.Move_out_of_view__c == true && a1.LastModifiedById != '00500000006xZUv' && a1.LastModifiedById != '00500000006wsIm' && a1.LastModifiedById != '00500000006xdMm' && a1.LastModifiedById != '00500000006wnoE'){
                accId.Add(a1.ID);
            }
            else if(a1.ID != null && a1.Move_out_of_view__c == false && a1.LastModifiedById != '00500000006xZUv' && a1.LastModifiedById != '00500000006wsIm' && a1.LastModifiedById != '00500000006xdMm' && a1.LastModifiedById != '00500000006wnoE'){
                notAccId.add(a1.ID);
            }
        }

        LIST<Contact> cY = [SELECT AccountID, LastModifiedById FROM Contact WHERE AccountID In: AccId];
        LIST<Contact> cN = [SELECT AccountID, LastModifiedById FROM Contact WHERE AccountID In: notAccId];
        List<AccountTeamMember> toBeDeleted =[SELECT ID, LastModifiedById FROM AccountTeamMember WHERE AccountID In: AccId];

        for (Account a1: trigger.new){
            for (Contact c1:cY){
                if (c1.LastModifiedById != '00500000006xdMm' && c1.LastModifiedById != '00500000006wnoE' && c1.LastModifiedById != '00500000006wsIm' && c1.LastModifiedById != '00500000006xZUv'){
                    c1.Move_out_of_view__c = True;
                    conUpdate.add(c1);
                }
            }
            for (Contact c2:cN){
                if (c2.LastModifiedById != '00500000006xdMm'&& c2.LastModifiedById != '00500000006wnoE' && c2.LastModifiedById != '00500000006wsIm' && c2.LastModifiedById != '00500000006xZUv'){
                    c2.Move_out_of_view__c = False;
                    con1Update.add(c2);
                }
            }
        }
        for (AccountTeamMember t1:toBeDeleted){
            if (t1.LastModifiedById != '00500000006xdMm' && t1.LastModifiedById != '00500000006wnoE' && t1.LastModifiedById != '00500000006wsIm' && t1.LastModifiedById != '00500000006xZUv'){
                d1.add(t1);
            }
        }

        try{ 
            update conUpdate;
        }
        catch (DMLException e) {
            for (Account con : trigger.new) {
                con.addError(e.getDmlMessage(0));
            }
        }
        try{
            update con1Update;
        }
        catch (DMLException a){
            for (Account acc:trigger.new){
            acc.addError(a.getDmlMessage(0));
        }
    }
    try{
        delete d1;
    }
    catch (DMLException b){
        for (Account acc1:trigger.new){
            acc1.addError(b.getDmlMessage(0));
        }
    }    
}
//}

Answer 1

I am not going to go into why this particular does not work in my answer, but rather point you toward some best practices that will help you isolate code behavior and make your code base more reliable.

MOVE YOUR CODE OUT OF THE TRIGGER BODY

It is very important that you do not write your trigger code in the body itself. At a bare minimum, you should move the code out into a services class that will say HOW to perform actions and specify with sharing. Additionally, I strongly prefer to implement a handler class that will say WHEN to act and WHICH records to act on.

Services:

public with sharing class AccountServices
{
    public static void updateSomeContacts(List<Account> accounts)
    {
        // manipulate contacts
        safeUpdate(contacts, new Map<Id, Account> ( accounts ));
    }
    static void safeUpdate(List<Contact> contacts, Map<Id, Account> accountMap)
    {
        //update your contacts and handle DML Exceptions
        //see below
    }
}

Handler:

public with sharing class AccountTriggerHandler
{
    final List<Account> newRecords;
    final Map<Id, Account> oldMap;
    public AccountTriggerHandler(List<Account> newRecords, Map<Id, Account> oldMap)
    {
        this.newRecords = newRecords;
        this.oldMap = oldMap;
    }

    public void beforeInsert()
    {
        AccountServices.updateSomeContacts(newRecords);
    }
    //add events as you need
}

Trigger:

trigger Account on Account (before insert)
{
    AccountTriggerHandler handle = new AccountTriggerHandler(trigger.new, trigger.oldMap);
    if (trigger.isBefore)
    {
        if (trigger.isInsert) handle.beforeInsert();
    }
    //add events as you need
}

Associate errors to the proper record

Aside from the fact that this type of error handling is more friendly to update DML Operations which are not all or none, you may have different errors per record and so you will get more helpful error messages this way. If you are a professional developer, it is probably worth your while to write a generic utility to handle this for you.

public with sharing class AccountServices
{
    static void safeUpdate(List<Contact> contacts, Map<Id, Account> accountMap)
    {
        try
        {
            update contacts;
        }
        catch (DmlException dmlEx)
        {
            for(Integer i = 0; i < dmlEx.getNumDML(); i++)
            {
                Id errorKey = contacts[i].AccountId
                for (Account errorRecord : accountMap.get(errorKey))
                {
                    errorRecord.addError( dmlEx.getMessage(i) );
                }
            }
        }
    }
}

QUERY ONCE

You should never have three different queries on the same object. That is wasteful from a governer limits consumption standpoint. Combine them all and write methods to filter out what you need.

public with sharing class ContactServices
{
    static List<Contact> hasAccount(List<Account> accounts, List<Contact> candidates)
    {
        Set<Id> accountIds = new Map<Id, Account> ( accounts ).keySet();
        List<Contact> matches = new List<Contact>();
        for (Contact candidate : candidates)
        {
            if (accountIds.contains(candidate.AccountId)
            {
                matches.add(candidate);
            }
        }
        return matches;
    }
}

DO NOT HARDCODE IDS

Instead, you should query for the records you need and contain their Ids in a Set (as in this case you are checking if an Id is among them). Although it looks like for the intents and purposes of your trigger, you could apply this filter in the where clause.

List<User> someImportantUsers = [SELECT Id FROM User]; // WHERE ...
Set<Id> importantUserIds = new Map<Id, User> ( someImportantUsers ).keySet();
// ...
if (importantUserIds.contains(contact.LastModifiedById))
{
    // do stuff
}
// OR
List<Contact> contacts = [SELECT Id FROM Contact WHERE LastModifiedById IN :someImportantUsers]

Answer 2

Issue with hard coded the UserId which differ in Sandbox and Production. Instead of hard coding the ids we can store in the custom setting and use in your code.