Recursive Before Update Trigger Overwriting Saved Value with Null

Question

UPDATE: Okay, so I've identified the issue. A workflow rule that checks for a specific opportunity stage and updates a datetime field with NOW() if true. Why would this field update undo other field updates by the trigger? Also, I tested the same functionality as a process builder instead of a workflow rule and it doesn't interfere with the trigger. Any idea why?

---

I have written apex code so that when a new opportunity contact role is added to an opportunity and the contact has certain data, then two fields are updated on the opportunity, a datetime field and a contact role count field.

My problem is that the before update trigger on opportunity is called two more times and it appears to overwrite the two fields back to null. See screenshot at the end. I have recursive checks in place, but it appears to not be working for some reason. Any suggestions?

Opportunity trigger:

trigger OpportunityTrigger on Opportunity (before insert, before update) {

    if (Trigger.isBefore) {
        if(Trigger.isInsert){
            OpportunityTriggerHandler.OnBeforeInsert(Trigger.New, Trigger.Old, Trigger.NewMap);
        }
        if(Trigger.isUpdate){
            OpportunityTriggerHandler.OnBeforeUpdate(Trigger.New, Trigger.Old, Trigger.NewMap, Trigger.OldMap);
        }
    }
}

Opportunity trigger handler:

public class OpportunityTriggerHandler {
    public static List<String> conversionLocationList = UtilityMethods.GetPicklistValues(Opportunity.Conversion_Location__c.getDescribe());
    public static Map<String, Conversion_Location_Map__c> conversionLocationMap = UtilityMethods.GetConversionLocationMap();
    public static void OnBeforeInsert(List<Opportunity> newOpportunity, List<Opportunity> oldOpportunity, Map<ID, Opportunity> newOpportunityMap){
        List<Opportunity> newAEHandoffOpportunities = new List<Opportunity>();
        List<Opportunity> newCSMHandoffOpportunities = new List<Opportunity>();
        for(Opportunity opp : newOpportunity) {
            if(opp.Handoff_Trigger_Datetime__c != Null) {
                newAEHandoffOpportunities.add(opp);
            }
            if(conversionLocationMap.keySet().contains(opp.Conversion_Location__c)){
                opp.Conversion_Bucket__c = conversionLocationMap.get(opp.Conversion_Location__c).Conversion_Bucket__c;
                opp.Communication_Channel__c = conversionLocationMap.get(opp.Conversion_Location__c).Communication_Channel__c;
            }
        }
        if(newAEHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.ChangeOpportunityOwner(newAEHandoffOpportunities);
        }
    }
    public static void OnBeforeUpdate(List<Opportunity> newOpportunity, List<Opportunity> oldOpportunity, Map<ID, Opportunity> newOpportunityMap, Map<ID, Opportunity> oldOpportunityMap){
        List<Opportunity> newAEHandoffOpportunities = new List<Opportunity>();
        List<Opportunity> newCSMHandoffOpportunities = new List<Opportunity>();

        for(Id oppId : newOpportunityMap.keySet()) {
            if(!RecursionCheck.IsRecordIdInSet(oppId, 'Before')){
                if(newOpportunityMap.get(oppId).Handoff_Trigger_Datetime__c != oldOpportunityMap.get(oppId).Handoff_Trigger_Datetime__c) {
                    newAEHandoffOpportunities.add(newOpportunityMap.get(oppId));
                }
                if(newOpportunityMap.get(oppId).StageName == 'Evaluate' && newOpportunityMap.get(oppId).Amount >= Opportunity_Custom_Settings__c.getOrgDefaults().CSM_Handoff_Threshold__c && newOpportunityMap.get(oppId).First_Order_Date__c!= null && newOpportunityMap.get(oppId).Customer_Success_Manager__c == null && newOpportunityMap.get(oppId).No_CSM_Override__c == False){
                    newCSMHandoffOpportunities.add(newOpportunityMap.get(oppId));
                }
                if(conversionLocationMap.keySet().contains(newOpportunityMap.get(oppId).Conversion_Location__c)){
                    newOpportunityMap.get(oppId).Conversion_Bucket__c = conversionLocationMap.get(newOpportunityMap.get(oppId).Conversion_Location__c).Conversion_Bucket__c;
                    newOpportunityMap.get(oppId).Communication_Channel__c = conversionLocationMap.get(newOpportunityMap.get(oppId).Conversion_Location__c).Communication_Channel__c;
                }
                RecursionCheck.AddRecordIdToSet(oppId, 'Before');
            }
        }
        if(newAEHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.ChangeOpportunityOwner(newAEHandoffOpportunities);
        }
        if(newCSMHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.RoundRobinCustomerSuccessManager(newCSMHandoffOpportunities);
        }
    }
}

Opportunity Contact Role Trigger:

trigger OpportunityContactRoleTrigger on OpportunityContactRole (before insert, after insert, after update, after delete) {  
    if (Trigger.isBefore) {
        if(Trigger.isInsert){                OpportunityContactRoleTriggerHandler.OnBeforeInsert(Trigger.New, Trigger.Old, Trigger.NewMap);
        }
    }
    if(Trigger.isAfter){
        if(Trigger.isInsert){                OpportunityContactRoleTriggerHandler.OnAfterInsert(Trigger.New, Trigger.Old, Trigger.NewMap, Trigger.OldMap);
        }
        if(Trigger.isUpdate){                OpportunityContactRoleTriggerHandler.OnAfterUpdate(Trigger.New, Trigger.Old, Trigger.NewMap, Trigger.OldMap);
        }
        if(Trigger.isDelete){                OpportunityContactRoleTriggerHandler.OnAfterDelete(Trigger.Old, Trigger.OldMap);
        }
    }
}

Opportunity Contact Role Trigger Handler:

public class OpportunityContactRoleTriggerHandler {
    public static void OnBeforeInsert(List<OpportunityContactRole> newOpportunityContactRole, List<OpportunityContactRole> oldOpportunityContactRole, Map<ID, OpportunityContactRole> newOpportunityContactRoleMap){
        Set<String> oppContCompIdSet = new Set<String>();
        Set<Id> contactIdSet = new Set<Id>();
        for(OpportunityContactRole ocr : newOpportunityContactRole) {
            contactIdSet.add(ocr.ContactId);
            ocr.Opportunity_Contact_Composite_ID__c = String.Join(new List<String>{ocr.OpportunityId, ocr.ContactId}, '');
            if(!oppContCompIdSet.contains(ocr.Opportunity_Contact_Composite_ID__c)){
                oppContCompIdSet.add(ocr.Opportunity_Contact_Composite_ID__c);
            }
            else{
                ocr.Opportunity_Contact_Composite_ID__c = null;
            }
        }
        List<OpportunityContactRole> ocrList = [SELECT Id, Opportunity_Contact_Composite_ID__c
        FROM OpportunityContactRole
        WHERE Opportunity_Contact_Composite_ID__c IN :oppContCompIdSet];
        Set<String> oppContCompIdList = new Set<String>();
        for(OpportunityContactRole ocr : ocrList){
            oppContCompIdList.add(ocr.Opportunity_Contact_Composite_ID__c );
        }

        Map<Id, Contact> contactMap = new Map<Id, Contact>([SELECT Id, Rev_User_ID__c, BillingAccountID_From_Rev_DB__c, Rev_Billing_Account_Created_Datetime__c 
        FROM Contact 
        WHERE Id IN :ContactIdSet 
        AND Rev_User_ID__c != null]);
        for(OpportunityContactRole ocr : newOpportunityContactRole) {
            if(ocr.Opportunity_Contact_Composite_ID__c == null || oppContCompIdList.contains(ocr.Opportunity_Contact_Composite_ID__c)){
                ocr.AddError('Duplicate Opportunity Contact Role. Prevented Insertion of record.');
            }
            if(contactMap.containsKey(ocr.ContactId)){
                ocr.Rev_User_ID__c = contactMap.get(ocr.ContactId).Rev_User_ID__c;
                ocr.Rev_Billing_Account_ID__c = contactMap.get(ocr.ContactId).BillingAccountID_From_Rev_DB__c;
                ocr.Rev_Billing_Account_Created_Datetime__c = contactMap.get(ocr.ContactId).Rev_Billing_Account_Created_Datetime__c;
            }
        }
    }
    public static void OnAfterInsert(List<OpportunityContactRole> newOpportunityContactRole, List<OpportunityContactRole> oldOpportunityContactRole, Map<ID, OpportunityContactRole> newOpportunityContactRoleMap, Map<ID, OpportunityContactRole> oldOpportunityContactRoleMap){
        Map<Id, Opportunity> opportunityNewDataMap = new Map<Id, Opportunity>();
        for(Id ocrId : newOpportunityContactRoleMap.keySet()){
            if(!RecursionCheck.IsRecordIdInSet(ocrId, 'After')){
                if(!opportunityNewDataMap.containsKey(newOpportunityContactRoleMap.get(ocrId).OpportunityId)){
                    opportunityNewDataMap.put(newOpportunityContactRoleMap.get(ocrId).OpportunityId, new Opportunity(Id=newOpportunityContactRoleMap.get(ocrId).OpportunityId, Opportunity_Contact_Role_Count__c = 1));
                }
                else{                        opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Opportunity_Contact_Role_Count__c += 1;
                }
                if(newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c != null){                        if(newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c < opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c || opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c == null){
                        opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c = newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c;
                    }
                }
                RecursionCheck.AddRecordIdToSet(ocrId, 'After');
            }
        }
        Map<Id, Opportunity> opportunityMap = new Map<Id, Opportunity>([SELECT Id, Opportunity_Contact_Role_Count__c, Rev_Billing_Account_Created_Datetime__c FROM Opportunity WHERE Id IN :OpportunityNewDataMap.values()]);
        List<Opportunity> opportunityUpdateList = new List<Opportunity>();
        for(Id oppId : opportunityMap.keySet()){
            opportunityMap.get(oppId).Opportunity_Contact_Role_Count__c += opportunityNewDataMap.get(oppId).Opportunity_Contact_Role_Count__c;
            if(opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c == null || opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c > opportunityNewDataMap.get(oppId).Rev_Billing_Account_Created_Datetime__c){                    
               opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c = opportunityNewDataMap.get(oppId).Rev_Billing_Account_Created_Datetime__c;
                opportunityUpdateList.add(opportunityMap.get(oppId));
            }
        }
        Database.SaveResult[] srList = Database.update(opportunityUpdateList, False);
    }
    public static void OnAfterUpdate(List<OpportunityContactRole> newOpportunityContactRole, List<OpportunityContactRole> oldOpportunityContactRole, Map<ID, OpportunityContactRole> newOpportunityContactRoleMap, Map<ID, OpportunityContactRole> oldOpportunityContactRoleMap){


        Map<Id, Opportunity> opportunityNewDataMap = new Map<Id, Opportunity>();
        for(Id ocrId : newOpportunityContactRoleMap.keySet()){
            if(!RecursionCheck.IsRecordIdInSet(ocrId, 'After')){

                if(newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c != null){
                    if(!opportunityNewDataMap.containsKey(newOpportunityContactRoleMap.get(ocrId).OpportunityId)){
                        opportunityNewDataMap.put(newOpportunityContactRoleMap.get(ocrId).OpportunityId, new Opportunity(Id=newOpportunityContactRoleMap.get(ocrId).OpportunityId, Rev_Billing_Account_Created_Datetime__c = newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c));
                    }
                    else{
                        if(newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c < opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c){
                            opportunityNewDataMap.get(newOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c = newOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c;
                        }
                    }
                }
                RecursionCheck.AddRecordIdToSet(ocrId, 'After');
            }
        }
        Map<Id, Opportunity> opportunityMap = new Map<Id, Opportunity>([SELECT Id, Rev_Billing_Account_Created_Datetime__c FROM Opportunity WHERE Id IN :OpportunityNewDataMap.values()]);
        List<Opportunity> opportunityUpdateList = new List<Opportunity>();
        for(Id oppId : opportunityMap.keySet()){

            if(opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c == null || opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c > opportunityNewDataMap.get(oppId).Rev_Billing_Account_Created_Datetime__c){
                opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c = opportunityNewDataMap.get(oppId).Rev_Billing_Account_Created_Datetime__c;
                opportunityUpdateList.add(opportunityMap.get(oppId));
            }
        }
                Database.SaveResult[] srList = Database.update(opportunityUpdateList, False);
    }
    public static void OnAfterDelete(List<OpportunityContactRole> oldOpportunityContactRole, Map<ID, OpportunityContactRole> oldOpportunityContactRoleMap){
        Map<Id, Opportunity> opportunityNewDataMap = new Map<Id, Opportunity>();
        for(Id ocrId : oldOpportunityContactRoleMap.keySet()){
            if(!RecursionCheck.IsRecordIdInSet(ocrId, 'After')){
                if(!opportunityNewDataMap.containsKey(oldOpportunityContactRoleMap.get(ocrId).OpportunityId)){
                    opportunityNewDataMap.put(oldOpportunityContactRoleMap.get(ocrId).OpportunityId, new Opportunity(Id=oldOpportunityContactRoleMap.get(ocrId).OpportunityId, Opportunity_Contact_Role_Count__c = 1));
                }
                else{
                    opportunityNewDataMap.get(oldOpportunityContactRoleMap.get(ocrId).OpportunityId).Opportunity_Contact_Role_Count__c += 1;
                }
                if(oldOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c != null){
                    if((opportunityNewDataMap.get(oldOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c == null) || (oldOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c < opportunityNewDataMap.get(oldOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c)){
                        opportunityNewDataMap.get(oldOpportunityContactRoleMap.get(ocrId).OpportunityId).Rev_Billing_Account_Created_Datetime__c = oldOpportunityContactRoleMap.get(ocrId).Rev_Billing_Account_Created_Datetime__c;

                    }
                }
                RecursionCheck.AddRecordIdToSet(ocrId, 'After');
            }
        }
        Map<Id, Opportunity> opportunityMap = new Map<Id, Opportunity>([SELECT Id, Opportunity_Contact_Role_Count__c, Rev_Billing_Account_Created_Datetime__c FROM Opportunity WHERE Id IN :OpportunityNewDataMap.values()]);
        List<Opportunity> opportunityUpdateList = new List<Opportunity>();
        Map<Id, OpportunityContactRole> remainingOCRMap = new Map<Id, OpportunityContactRole>([Select Id, OpportunityId, Rev_Billing_Account_Created_Datetime__c
        FROM OpportunityContactRole
        WHERE OpportunityId IN :OpportunityMap.keySet()
        AND Id NOT IN :oldOpportunityContactRoleMap.keySet()
        AND Rev_Billing_Account_Created_Datetime__c != null
        ORDER BY Rev_Billing_Account_Created_Datetime__c DESC NULLS FIRST]);
        Map<Id, OpportunityContactRole> ocrOppIdMap = new Map<Id, OpportunityContactRole>();
        for(Id ocrId: remainingOCRMap.keySet()){

            ocrOppIdMap.put(remainingOCRMap.get(ocrId).OpportunityId, remainingOCRMap.get(ocrId));
        }
        for(Id oppId : opportunityMap.keySet()){
            opportunityMap.get(oppId).Opportunity_Contact_Role_Count__c -= opportunityNewDataMap.get(oppId).Opportunity_Contact_Role_Count__c;
            if(ocrOppIdMap.containskey(oppId)){
                opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c = ocrOppIdMap.get(oppId).Rev_Billing_Account_Created_Datetime__c;
            }
            else{
                opportunityMap.get(oppId).Rev_Billing_Account_Created_Datetime__c = null;
            }
            opportunityUpdateList.add(opportunityMap.get(oppId));
        }
        Database.SaveResult[] srList = Database.update(opportunityUpdateList, False);
    }
}

Here is what I am seeing when I create a new opportunity/contact role from the New Opportunity button on the contact's opportunity related list.

Answer 1

As Kris is saying, you should be looking to the processes, workflows and any other declarative operation that may be firing on the same transaction.

You can also try a couple of things to see if you can control a bit the transactions that are operating on this classes. I will suggest to make a clear cut between before update and before insert operation.

trigger OpportunityTrigger on Opportunity (before insert, before update) {

    if (Trigger.operationType == TriggerOperation.BEFORE_INSERT) {
        OpportunityTriggerHandler.getInstance().OnBeforeInsert(Trigger.New, Trigger.Old, Trigger.NewMap);

    }

    if (Trigger.operationType == TriggerOperation.BEFORE_UPDATE) {
        OpportunityTriggerHandler.getInstance().OnBeforeUpdate(Trigger.New, Trigger.Old, Trigger.NewMap, Trigger.OldMap);
    }

}

Try this approach to make sure that you only start the transaction you need. As you can see I added the getInstance() method that is part of the next piece of code.

public class OpportunityTriggerHandler {

    @TestVisible private static OpportunityTriggerHandler instance;

    public static List<String> conversionLocationList = UtilityMethods.GetPicklistValues(Opportunity.Conversion_Location__c.getDescribe());
    public static Map<String, Conversion_Location_Map__c> conversionLocationMap = UtilityMethods.GetConversionLocationMap();

    public static OpportunityTriggerHandler getInstance() {
        if(instance == null) {
           instance = new OpportunityTriggerHandler();
        }
        return instance;
    }

    public static void OnBeforeInsert(List<Opportunity> newOpportunity, List<Opportunity> oldOpportunity, Map<ID, Opportunity> newOpportunityMap){
        List<Opportunity> newAEHandoffOpportunities = new List<Opportunity>();
        List<Opportunity> newCSMHandoffOpportunities = new List<Opportunity>();

        for(Opportunity opp : newOpportunity) {
            if(opp.Handoff_Trigger_Datetime__c != Null) {
                newAEHandoffOpportunities.add(opp);
            }
            if(conversionLocationMap.keySet().contains(opp.Conversion_Location__c)){
                opp.Conversion_Bucket__c = conversionLocationMap.get(opp.Conversion_Location__c).Conversion_Bucket__c;
                opp.Communication_Channel__c = conversionLocationMap.get(opp.Conversion_Location__c).Communication_Channel__c;
            }
        }
        if(newAEHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.ChangeOpportunityOwner(newAEHandoffOpportunities);
        }
    }

    public static void OnBeforeUpdate(List<Opportunity> newOpportunity, List<Opportunity> oldOpportunity, Map<ID, Opportunity> newOpportunityMap, Map<ID, Opportunity> oldOpportunityMap){
        List<Opportunity> newAEHandoffOpportunities = new List<Opportunity>();
        List<Opportunity> newCSMHandoffOpportunities = new List<Opportunity>();

        for(Id oppId : newOpportunityMap.keySet()) {
            if(!RecursionCheck.IsRecordIdInSet(oppId, 'Before')){
                if(newOpportunityMap.get(oppId).Handoff_Trigger_Datetime__c != oldOpportunityMap.get(oppId).Handoff_Trigger_Datetime__c) {
                    newAEHandoffOpportunities.add(newOpportunityMap.get(oppId));
                }
                if(newOpportunityMap.get(oppId).StageName == 'Evaluate' && newOpportunityMap.get(oppId).Amount >= Opportunity_Custom_Settings__c.getOrgDefaults().CSM_Handoff_Threshold__c && newOpportunityMap.get(oppId).First_Order_Date__c!= null && newOpportunityMap.get(oppId).Customer_Success_Manager__c == null && newOpportunityMap.get(oppId).No_CSM_Override__c == False){
                    newCSMHandoffOpportunities.add(newOpportunityMap.get(oppId));
                }
                if(conversionLocationMap.keySet().contains(newOpportunityMap.get(oppId).Conversion_Location__c)){
                    newOpportunityMap.get(oppId).Conversion_Bucket__c = conversionLocationMap.get(newOpportunityMap.get(oppId).Conversion_Location__c).Conversion_Bucket__c;
                    newOpportunityMap.get(oppId).Communication_Channel__c = conversionLocationMap.get(newOpportunityMap.get(oppId).Conversion_Location__c).Communication_Channel__c;
                }
                RecursionCheck.AddRecordIdToSet(oppId, 'Before');
            }
        }
        if(newAEHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.ChangeOpportunityOwner(newAEHandoffOpportunities);
        }
        if(newCSMHandoffOpportunities.size()>0){
            OpportunityRoundRobinHandoff.RoundRobinCustomerSuccessManager(newCSMHandoffOpportunities);
        }
    }
}

The code is exactly the same you had before with the small modification of inserting a Singletone pattern. The idea is to make sure that there is only one instance of this class in each transaction. There are a lot of moving parts on this code but perhaps this can help you to reduce some of the issues you are facing. I think that in a code like this you need to deactivate parts of the code until you can find what is that causing this problem.

Try to comment your code and leave only the basic flow for one update to make sure that it works as you are expecting. Slowly start de-commenting other parts of your code and run the code every time you do so. I think you get the idea.