1

I'm writing a very simple trigger. At this point I have 100% coverage, but I don't feel like the tests are robust enough.

High Level Opportunity Contact Role Trigger that fires on Delete.

The trigger then updates the Contact Roles' parent Opportunity.

However, in my test class, I'm not able to verify that the opportunity has been updated. Despite the fact that trigger does successfully update in the opp through the user interface.

Code Sample Trigger

trigger OpportunityContactRoleTrigger on OpportunityContactRole (after delete) {
    String role;
    String contactId;
    String parentOppId;

    if(Trigger.isDelete && Trigger.size == 1){
        for(OpportunityContactRole ocr : Trigger.old ){
            role = ocr.role;
            contactId = ocr.contactId;
            parentOppId = ocr.OpportunityId;
        }

        Opportunity parentOpp = [SELECT Id, ContactId, Delivery_Contact__c, Pickup_Contact__c FROM Opportunity WHERE Id =: parentOppId];
            switch on role {
                when 'Delivery Contact' {
                    parentOpp.Delivery_Contact__c = parentOpp.ContactId;
                }
                when 'Pickup Contact' {
                    parentOpp.Pickup_Contact__c = parentOpp.ContactId;
                } when else{
                    System.debug('Do Nothing');
                }
            }
        update parentOpp;
    }
}

Test Class

    @isTest
private without sharing class OpportunityContactRoleTrigger_Test {
    @TestSetup
    static void makeData(){
        //Test Data Creation
        //Create Account
        List<Account> accts = new List<Account>();
        List<Contact> conts = new List<Contact>();
        List<OpportunityContactRole> ocrs = new List<OpportunityContactRole>();
        Account acct1 = new Account(Name = 'Test Account1');
        accts.add(acct1);
        Account acct2 = new Account(Name = 'Test Account2');
        accts.add(acct2);
        Account acct3 = new Account(Name = 'Test Account3');
        accts.add(acct3);
        insert accts;
        //Create Contact
        Contact cont1 = new Contact(
            FirstName = 'Test',
            LastName = 'Role1',
            AccountId = acct1.Id
        );
        conts.add(cont1);
        Contact cont2 = new Contact(
            FirstName = 'Test',
            LastName = 'Role2',
            AccountId = acct2.Id
        );
        conts.add(cont2);
        Contact cont3 = new Contact(
            FirstName = 'Test',
            LastName = 'Role3',
            AccountId = acct3.Id
        );
        conts.add(cont3);
        insert conts;
        //Create Opportunity
        Opportunity opp = new Opportunity(
            Name = 'Test Role Delete',
            AccountId = acct1.Id,
            StageName = 'Quoting',
            CloseDate = Date.Today() + 10,
            Customer_Contact__c  = cont1.Id,
            Pickup_Contact__c = cont2.Id,
            Delivery_Contact__c = cont3.Id
        );

        insert opp;

        //Create Opportunity Contact Role
        OpportunityContactRole ocrPrimaryContact = new OpportunityContactRole(
            ContactId = cont1.Id,
            OpportunityId = opp.Id,
            Role = 'Primary Contact',
            IsPrimary = true
        );
        ocrs.add(ocrPrimaryContact);

        OpportunityContactRole ocrPickupContact = new OpportunityContactRole(
            ContactId = cont2.Id,
            OpportunityId = opp.Id,
            Role = 'Pickup Contact',
            IsPrimary = false
        );
        ocrs.add(ocrPickupContact);

        OpportunityContactRole ocrDeliveryContact = new OpportunityContactRole(
            ContactId = cont3.Id,
            OpportunityId = opp.Id,
            Role = 'Delivery Contact',
            IsPrimary = false
        );
        ocrs.add(ocrDeliveryContact);

        insert ocrs;
    }

    @isTest
    static void TestDeleteDeliveryContactRole() {
        Opportunity parentOpp = [
            SELECT
            Id,
            Customer_Contact__c,
            Pickup_Contact__c,
            Delivery_Contact__c,
            ContactId
            FROM Opportunity
            WHERE Name = 'Test Role Delete'
            LIMIT 1
        ];

        OpportunityContactRole deliveryContactRole_Del = [
            SELECT Id, ContactId
            FROM OpportunityContactRole
            WHERE OpportunityId =: parentOpp.Id
            AND Role = 'Delivery Contact' LIMIT 1
        ];
        System.assertEquals(parentOpp.Delivery_Contact__c, deliveryContactRole_Del.ContactId, 'Delivery does not match');
        Test.startTest();
        Database.DeleteResult result  = Database.delete(deliveryContactRole_Del, false);
        // System.assertEquals(parentOpp.Delivery_Contact__c, parentOpp.Customer_Contact__c);
        Test.stopTest();

        System.assertEquals(true, result.isSuccess());
        // System.assertEquals(parentOpp.Delivery_Contact__c, parentOpp.Customer_Contact__c);

    }

    @isTest
    static void TestDeletePickupContactRole() {
        Opportunity parentOpp = [
            SELECT
            Id,
            Customer_Contact__c,
            Pickup_Contact__c,
            Delivery_Contact__c,
            ContactId
            FROM Opportunity
            WHERE Name = 'Test Role Delete'
            LIMIT 1
        ];

        OpportunityContactRole pickupContactRole_Del = [
            SELECT Id, ContactId
            FROM OpportunityContactRole
            WHERE OpportunityId =: parentOpp.Id
            AND Role = 'Pickup Contact' LIMIT 1
        ];
        System.assertEquals(parentOpp.Pickup_Contact__c, pickupContactRole_Del.ContactId, 'Delivery does not match');
        Test.startTest();
        Database.DeleteResult result  = Database.delete(pickupContactRole_Del, false);
        // System.assertEquals(parentOpp.Delivery_Contact__c, parentOpp.Customer_Contact__c);
        Test.stopTest();

        System.assertEquals(true, result.isSuccess());


    }
}
  • 1
    Then when else bit is not strictly necessary. In addition, why not bulkify the code? There's a lot of room for improvement. Your test itself looks fine to me (at least, it's better than most beginning developers' tests). – sfdcfox Feb 23 '20 at 21:20
3

Two things I see here:

  1. If you want to evaluate changes to the opportunity, then you need to requery it after the OCR trigger has fired. ie, after your Test.stopTest(), requery the Oppportunity and then make your asserts.

  2. I don't see how your commented out System.assertEquals() statements about the contact lookups will ever be true, even after you requery. Your trigger populates either the Delivery_Contact__c or Pickup_Contact__c lookup on Opportunity, with the contact from the deleted OCR, when the OCR being deleted is of the appropriate role.

    Your test data puts the same contact in the lookups and the corresponding OCRS: cont2 is assigned to the Pickup Contact lookup and given an Pickup Contact OCR. cont3 gets the Delivery Contact lookup and OCR. So when either of those OCRs is deleted, the Opportunity record will get a new LastModifiedDate, but the lookup values will be the same as before.

    You need to either cross the lookups and OCRs (maybe assign cont1 to all three lookups?), or leave the lookups blank, in order for there to be a change to the record that you can assert after re-querying it.

  • Thanks. Regarding 2. Since the Opp ContactId is equal to the Primary Contact Opportunity Contact Role. When a user deletes a non-Primary user the ContactIds are different. – thinker Feb 24 '20 at 13:11

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.