0

Let me begin by saying, I know that this could be significantly better code if using the Trigger.newMap and Trigger.oldMap I have run into a situation though where I need to reuse a class within a trigger that uses a contstructor to pass in the maps from the trigger --> triggerHandler --> Class w/ Contstructor

I have created a void method to handle a simple update of subscription preferences. I was able to get the Insert version to work and have full coverage there, however I am not able to get coverage on the comparison between old values and new values in the trigger. It will hit the conditionals but not actually enter into the conditional. When I attempt to do an assert on the subscription values, it fails. I'm not sure what I'm missing.

ContactProcess.cls

// Constructor called in trigger handler. 
public class ContactProcesses  {
public enum Operation_Mod { BEFORE_OP, AFTER_OP }

    private List<Contact> givenContactList;
    private Map<Id, Contact> oldContactMap;
    private Map<Id, Contact> newContactMap;
    private List<Contact> updateContactList;
    private Contact_Trigger_Setting__mdt triggerSettings; 


    Operation_Mod mod;
    public ContactProcesses( Map<ID, Contact> newCons, Map<ID, Contact> oldCons, Contact_Trigger_Setting__mdt settings, Operation_Mod mod) {
        updateContactList = new List<Contact>();
        toCreateTaskLst = new List<Task>();
        updateAccountList = new List<Account>();
        updateMemDetails = new List<CA_Membership_Details__c>();
        classificationsToCreate = new List<Classification_Type_Contact__c>();
        givenContactList = newCons.values();
        oldContactMap = oldCons;
        newContactMap = newCons;
        triggerSettings = settings; 
        this.mod = mod;
        
    }

//my Method that I'm trying to cover: 
public void subscriptionPrefsUpdate( Map<String, Contact_Subscription_Recipes__mdt> mapOfRecipes){
        System.debug('SubscriptionMethod Active');

        
        for (Contact c : newContactMap.values()) {
            Contact oldc = oldContactMap.get(c.Id);
            System.debug('NewContact: ' + String.valueOf(c.Recipe__c)); 
            System.debug('OldContact: ' + String.valueOf(oldc.Recipe__c));  
            if (c.Recipe__c  != null && oldc.Recipe__c != c.Recipe__c) {
                Contact_Subscription_Recipes__mdt subRecipe = mapOfRecipes.get('Recipe_' + c.Recipe__c); //subscription recipe
                
                c.AMFEMs_Opt_Out__c = subRecipe.meta_MFEMsOptOut__c; 
                c.Appeals_Opt_Out__c = subRecipe.meta_MonthlyAppealLetter_ML_OptOut__c; 
                c.Do_Not_Mail__c = subRecipe.meta_DoNotMail__c; 
                
            } else if(c.Recipe__c == null && oldc.Recipe__c != c.Recipe__c) {
                Contact_Subscription_Recipes__mdt subRecipe = mapOfRecipes.get('Recipe_None');
                
                c.AMFEMs_Opt_Out__c = subRecipe.meta_MFEMsOptOut__c; 
                c.Appeals_Opt_Out__c = subRecipe.meta_MonthlyAppealLetter_ML_OptOut__c; 
                c.Do_Not_Mail__c = subRecipe.meta_DoNotMail__c; 
                
            } else if(oldc.Recipe__c == c.Recipe__c && c.Recipe__c != null) {
                validateSubscriptionValues(oldc, c, triggerSettings);
            }

UnitTestClass

@isTest
private static void testBeforeUpdateContactRecipesPreferences(){
        //Test Data
        Contact_Trigger_Setting__mdt triggerSettings = [SELECT Id,TurnOnContactTriggerErrors__c FROM Contact_Trigger_Setting__mdt WHERE DeveloperName = 'Contact_Trigger_Settings'];
        Map<String, Contact_Subscription_Recipes__mdt> mapOfRecipes = ContactGateWay_AC.getRecipesMap();  
        Map<Id, Contact> newContacts = new Map<Id,Contact>(); 
        Map<Id,Contact> oldContacts = new Map<Id, Contact>(); 
        Contact cOld = new Contact(
            FirstName = 'TestFirst', 
            LastName = 'TestLast', 
            Recipe__c ='1', 
            Email = '[email protected]',
           
        );             
        insert cOld; 
        System.assertEquals('1', cOld.Recipe__c); 
        System.assertEquals(true, cOld.Do_Not_Mail__c);
        oldContacts.put(cOld.Id, cOld);   
        Contact cNew = new Contact();
        cNew = cOld;  
        cNew.Recipe__c = '2'; 
        newContacts.put(cNew.Id, cNew); 
        update cNew; 
        ContactProcesses cp = new ContactProcesses(newContacts, oldContacts, triggerSettings, ContactProcesses.Operation_Mod.BEFORE_OP);
        cp.subscriptionPrefsUpdate(mapOfRecipes); 

        
        Contact savedCon = [
            SELECT 
                Id,
                Recipe__c, 
                ADF_Foundation_Mail_Opt_Out__c, 
                AMFEMs_Opt_Out__c, 
                Appeals_Opt_Out__c, 
                Do_Not_Mail__c, 
                Faith_Justice__c, 
                Newsletter_NL_Opt_Out__c, 
                Number_of_MLs__c, 
                Number_of_Newsletters_NL__c, 
                Receipts_Only_No_Other_Mail__c 
            FROM Contact 
            WHERE 
               Id =: cNew.Id
        ];
        //Expected Results: 
        System.assertEquals('2',  savedCon.Recipe__c); 
        System.assertEquals(true, savedCon.Do_Not_Mail__c);
         
    }

1 Answer 1

4

There seem to be a few fundamental mistakes here. The main one seems to be that you aren't thinking about object instances correctly, but let me start with another one first.

Looking at the following fragment

Contact cOld = new Contact(
    FirstName = 'TestFirst', 
    LastName = 'TestLast', 
    Recipe__c ='1', 
    Email = '[email protected]',
);
insert cOld; 
System.assertEquals('1', cOld.Recipe__c); 
System.assertEquals(true, cOld.Do_Not_Mail__c);

When you insert cOld, the only update that is made to that in-memory instance of Contact is that it is given an Id. If you want to check any other fields, you need to query for it (using the Id that you just obtained).

As is, System.assertEquals('1', cOld.Recipe__c); is a useless assertion. You're explicitly setting that field before you insert, so there is no way this assertion could possibly fail. If you don't expect that inserting the record will change that, this assertion should be removed entirely.

This would be the bare minimum changes I'd make to that section

Contact cOld = new Contact(
    FirstName = 'TestFirst', 
    LastName = 'TestLast', 
    Recipe__c ='1',
    // Removed a comma at the end of this line, which would be a syntax error
    //   since it is the last provided name-value pair
    Email = '[email protected]' 
);
insert cOld;

// You need to re-query the record to get additional or updated values
cOld = [SELECT Id, Recipe__c, Do_Not_Mail__c FROM Contact WHERE Id = :cOld.Id];

// You should strongly consider providing the optional 3rd parameter to your assertEquals()
//   and assertNotEquals() calls (which is a string that gets printed if the
//   assertion fails).
// It is immensely helpful to have additional information beyond "value was not the same
//   (expected '1', actual null)"
System.assertEquals('1', cOld.Recipe__c, 'Recipe__c was changed after inserting'); 
System.assertEquals(true, cOld.Do_Not_Mail__c, 'value of Do_Not_Mail__c is wrong');

As is, I'd be surprised if your test is not failing at that second assertion (against Do_Not_Mail__c).

The main issue

The following fragment is fundamentally wrong

Contact cNew = new Contact();
cNew = cOld;  
cNew.Recipe__c = '2'; 
newContacts.put(cNew.Id, cNew); 
update cNew;

I can see that you're trying to make an independent copy of cOld, but that is not what this code does. When you execute cNew = cOld;, you are saying "cNew is just another name for cOld." They are both pointing to the same Contact instance in-memory.

Because of that, when you execute cNew.Recipe__c = '2';, you're also updating the Recipe__c value of cOld (again, because cOld and cNew point to the same object instance). You can verify this yourself with a simple debug.

So if you do actually get into the code you want to test, oldC.Recipe__c will always be equal to c.Recipe__c (and you'd gain coverage only for the contents of the final else-if).

The update cNew; is unnecessary and provides no value to you (since you aren't querying for any records in the code you're trying to cover with this test), but as long as it isn't causing you to run into governor limits it is harmless.

What you want to do instead

is to create a completely independent instance of your test Contact.

You need to have two separate instances of your Contact in memory so that when you make changes to one, you're only making changes to one instance.

The easiest way to do that is to use the SObject clone() method.

// clone() gives you a copy of a record that is completely independent of the source
// It can take up to 4 arguments, the first one being whether or not to copy the Id of
//   the source record (default is false = do not copy the Id from the source)
Contact cNew = cOld.clone(true);

// This line is bad, and should be removed
//cNew = cOld;

// Now that cNew is an independent Contact instance, this line will not cause
//   the value on cOld to be updated.
cNew.Recipe__c = '2'; 
newContacts.put(cNew.Id, cNew); 

// No benefit to running an update here
//update cNew;

As usual with testing

If you want to cover multiple code paths (i.e. multiple branches of an if-else), you should have one test per code path.

One test to cover when Recipe__c is changing from null to non-null
One test to cover when Recipe__c is changing from non-null to null
One test to cover when Recipe__c is not changing and is not null

1
  • Very helpful @Derek F. I did infact have to use the update cnew though. The method from the cp class does not automatically update. I'm not sure if this is the best approach but my system asserts for all the values are now passing without the hardcodes as you say.
    – thinker
    Commented Sep 15, 2021 at 16:06

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .