function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
Jack InsJack Ins 

Need help with code covereage

Hi all,

I need some help with covering a trigger.  My test calss gets me 58% of the way there but I would relly like to get closer to 100%.

My trigger is not big and should be easy help.  Just need pointed in the right direction.

 

Trigger that I am trying to cover:

OpportunityContactAssoc (Code Covered: 58%)
 line  source 
 1   trigger OpportunityContactAssoc on Opportunity (before insert, before update) { 
 2   List<Contact> cntList = new List<Contact>(); 
 3   List<OpportunityContactRole> oppCntRole = new List<OpportunityContactRole>(); 
 4   public String cntID {get;set;} 
 5    
 6    for (Opportunity o: Trigger.new) { 
 7    if (o.Customer_Number__c != null) 
 8    { 
 9    try {Contact cnts = [select Id, cnt_plcsc_CustNmbr_ExtID__c, cnt_plcsc_Customer_Number__c 
 10    from Contact 
 11    where cnt_plcsc_Customer_Number__c =: o.Customer_Number__c 
 12    And cnt_plcsc_CustNmbr_ExtID__c =: o.Customer_Number__c + 'Ins01' limit 1]; 
 13    cntID = cnts.Id; 
 14    cntList.add(cnts);} catch (Exception e) {System.debug('Contact Record Does Not Exist.' + cntID);} 
 15    
 16    if(cntID != null) 
 17    { 
 18    OpportunityContactRole ocr = new OpportunityContactRole(ContactId=cntID, OpportunityID = o.Id, 
 19    Role = 'Primary Insured', IsPrimary = true); 
 20    upsert new List<OpportunityContactRole>{ocr}; 
 21    
 22    for(OpportunityContactRole oppcnts : [select Id, ContactId, OpportunityId, Role, IsPrimary 
 23    from OpportunityContactRole 
 24    where IsPrimary =: false And Role =: 'Primary Insured' And OpportunityId =: o.Id]) 
 25    { 
 26    delete oppcnts; 
 27    } 
 28    
 29    } 
 30    } 
 31    else { 
 32    try { Contact cnts = [select Id, MailingStreet, MailingState, cnt_plcsc_Customer_Number__c, 
 33    MailingPostalCode, MailingCity, LastName, FirstName, Email 
 34    from Contact 
 35    where Email =: o.Consumer_Email__c And MailingPostalCode =: o.Zip__c And RecordTypeId =: '01240000000DVfa' limit 1]; 
 36    cntID = cnts.Id; 
 37    o.Customer_Number__c = cnts.cnt_plcsc_Customer_Number__c; 
 38    cntList.add(cnts);} catch (Exception e) {System.debug('Contact Record Does Not Exist.');} 
 39    
 40    if(cntID != null){ 
 41    OpportunityContactRole ocr = new OpportunityContactRole(ContactId=cntID, OpportunityID = o.Id, 
 42    Role = 'Primary Insured', IsPrimary = true); 
 43    upsert new List<OpportunityContactRole>{ocr}; 
 44    } 
 45    } 
 46    } 
 47   } 

 test class (please don't laugh:smileywink:):

@isTest
private class TestOpportunityContactRole {

    static testMethod void TestmyOpportunityContactRoleTrigger() {
        
// TO DO: implement unit test
        Test.startTest();
        
		Contact cntExisting = new Contact(
        FirstName = 'Test', LastName = 'Tester', RecordTypeId = '01240000000DVfaAAG', Email='test@hanover.com', 
        cnt_plcsc_CustNmbr_ExtID__c = '123456789INS01', cnt_plcsc_Customer_Number__c='123456789');
    	insert cntExisting;
    	
    	Contact cntExisting2 = new Contact(
        FirstName = 'Test', LastName = 'Tester2', RecordTypeId = '01240000000DVfaAAG', Email='test2@hanover.com', 
        cnt_plcsc_CustNmbr_ExtID__c = '2123456789INS01', cnt_plcsc_Customer_Number__c='2123456789');
    	insert cntExisting;
    	
    	Opportunity myOppPolicyTest = new Opportunity(StageName='Quoted', CloseDate=Date.today(), Name='Tester1', 
    	Policy_Id__c='ABC1234567', Pol_Sym__c='ABC', Pol_No__c='1234567',LOB__c='Auto', 
    	opp_plcsc_Integration_ID__c='A1234567', Customer_Number__c = '123456789');
		insert myOppPolicyTest;
    	                
        OpportunityContactRole ocr = new OpportunityContactRole(OpportunityId = myOppPolicyTest.Id,
		ContactId = cntExisting.Id, Role = 'Primary Insured', IsPrimary = true);
		upsert new List<OpportunityContactRole> {ocr};
		
		System.assert(myOppPolicyTest.Customer_Number__c == '2123456789');
        Test.stopTest();
    }
}

 

 

Best Answer chosen by Admin (Salesforce Developers) 
Starz26Starz26

@JackIns

 

To cover any lines that were not covered, you will need to ensure that the records you are working with meet the conditions in the if statements as well as the parent FOR SOQL statements.

 

Takle each missing line one at a time. Get them covered, then go back and look at your test class and consolidate where necessary.

 

Also, you do not have to cover all the lines in one method. You can have multiple tests methods and when run together they cover the whole code. (i.e. method A covers the first Positive sceniero, Method 2 covers the negative, etc.)

 

The previous poster is technically correct, but it will not solve your immediate issue. With the trigger as you currently have it, you will run into governor limits after a small amount of records. In order to handle bulk updates/inserts you must ensure that you SOQL and DML statement are outside of all loops. This will require the use of Sets and Maps and is one of those milestones in your learning process of Apex. Doing a search for "Bulkify" will yield a lot of information on these boards.

 

 

All Answers

Starz26Starz26

Just a quick glance,

 

Your second insert should be giving you an error as you already inserted it.

 

change

 

insert cntExisting;

to

 

insert cntExisting2;

 

What lines are not being covered?

 

Lines 31 to 43 will only be covered if the Customer_Number__c is = Null and I do not see you inserting a record without a customer number.

 

vishal@forcevishal@force

queries within a for loop?

 

That's a really bad practice, what if I insert bulk records, here I will be hitting the limits!

 

First thing you need to do here is avoid any queries within a for loop.

 

Same thing applies for DML statements, you have all the dml's within the for loop.

Jack InsJack Ins

Starz26,

Being a new developer trying to learn to code and based on the sarcasm that I recieved from visha@force it seems that I may have taken the wrong direction in my trigger. 

 

I did add code to my test class based on your suggestions and was able to increase my coverage to 70% but I  am still not covering lines , 24, 26, 36, 37, 41 and 43.

 

Should I be looking to rewrite my trigger for better optimization?

Thanks for your help.

 

 

Starz26Starz26

@JackIns

 

To cover any lines that were not covered, you will need to ensure that the records you are working with meet the conditions in the if statements as well as the parent FOR SOQL statements.

 

Takle each missing line one at a time. Get them covered, then go back and look at your test class and consolidate where necessary.

 

Also, you do not have to cover all the lines in one method. You can have multiple tests methods and when run together they cover the whole code. (i.e. method A covers the first Positive sceniero, Method 2 covers the negative, etc.)

 

The previous poster is technically correct, but it will not solve your immediate issue. With the trigger as you currently have it, you will run into governor limits after a small amount of records. In order to handle bulk updates/inserts you must ensure that you SOQL and DML statement are outside of all loops. This will require the use of Sets and Maps and is one of those milestones in your learning process of Apex. Doing a search for "Bulkify" will yield a lot of information on these boards.

 

 

This was selected as the best answer
Jack InsJack Ins

Thank you very much.  I will start with your recommendations and see where I can improve my code coverage and learning curve. 

 

I appreciate your patience and tact with a beginner developer.

vishal@forcevishal@force

hey Jack,

 

that wasn't sarcastic. I just wanted you to know that it's a bad practice and being a new developer, it will help you a lot if you start avoiding such practices as early as possible. I may have sounded sarcastic but I only intended to make sure you know your code needs to be changed :)

 

* Whenever you are writing a trigger, you should bulkify it, reason being if data is inserted/updated/deleted through data loader, your trigger.new or trigger.old list can have thousands of records. In that case when you have dml and queries inside those trigger.new/trigger.old for loops, you are bound to hit the limits. So you need to avoid these things and always make sure your triggers are bulkified.

Jack InsJack Ins

Thanks Vishal,

 

Thank you for the follow up.  It's difficult to learn a new language when for so many years coding in a completely different language.  The concepts are very much different.  I will be looking into the Bulkify process and start learning how this works.

 

Thanks again for your response.

Jack InsJack Ins

Vishal,

I took a crack at bulkifying my code and this is where I am at.  It does not seem to being loading records as I would have expected.  Can you look at it and help me identify where I am going wrong?

Thanks Dwayne

 

trigger OpportunityContactAssoc on Opportunity (after insert) {

List<Opportunity> oppsList = new List<Opportunity>();
List<String> OppIdList = new List<String>();
map<String,Id> mapCntToOpp = new map<String,String>();//Mapping Contact to Opportinty
String wrkText = ''; // Temporary, working variable 
String replaceText = 'dup-'; // Text to replace. This is included for ISV partners who want to remove the "-dup" string that is included for duplicate AppExchange Opportunity Submissions

        for (Opportunity o: Trigger.new)  {
        	oppsList.add(o);//Add Opps to the main list!
            if (o.Customer_Number__c != null){   
            	wrkText = o.Customer_Number__c;
            	wrkText = wrkText.replace(replaceText,'');
            	OppIdList.add(wrkText);
            	mapCntToOpp.put(o.Id, wrkText);
            }
        }

/*Creates a map containing an association of contact customer number to contact id's.*/      
List<Contact> cntList = [select Id, cnt_plcsc_CustNmbr_ExtID__c, cnt_plcsc_Customer_Number__c 
		            	from Contact 
		            	where cnt_plcsc_Customer_Number__c =: OppIdList 
		            	And cnt_plcsc_CustNmbr_ExtID__c =: OppIdList + 'Ins01' limit 1];
map<String,ID> mapoppIDToCntID = new map<String, ID>();//Maps customer number to cntid.
		
		for(Contact c:cntList){
			mapoppIDToCntID.put(c.cnt_plcsc_Customer_Number__c, c.Id);
		}  

/*Loop through the main list of Opps*/
List<OpportunityContactRole> oppCntRole = new List<OpportunityContactRole>();

		for(Opportunity o:oppsList){
			if(mapoppIDToCntID.get(mapCntToOpp.get(o.Customer_Number__c)) != null){
				OpportunityContactRole ocr = new OpportunityContactRole();
				ocr.ContactId = mapoppIDToCntID.get(mapCntToOpp.get(o.Id));
				ocr.OpportunityId = o.Id;
				ocr.IsPrimary = true;
				ocr.Role = 'Primary Insured';
				oppCntRole.add(ocr);
				
			}
		}

/*Insert the new list of OCR's*/
	if(!oppCntRole.isEmpty()){
		insert oppCntRole;
	}
}

 

 

Jack InsJack Ins

Starz26,

 

I have spent some time to get my trigger Bulkified and wondered if you can look at it to let me know if you see any issues.  It is working great and I am so thankful for all the help.  What a great little mile stone.  :)

 

Thanks Dwayne

 

trigger OpportunityContactAssoc on Opportunity (after insert, after update) {

set<String> custnmbr = new set<String>();
		for (Opportunity o: Trigger.new){
            if (o.Customer_Number__c != null){
            	custnmbr.add(o.Customer_Number__c +'INS01');
            	System.debug('DLB932 - Number of Opp Recs - ' + custnmbr);
            }
        }
        
Map<String,Id> mapCnt = new Map<String,Id>();
/*List all Contacts that have the customer number the same as the loaded custnmbr set*/      
		for(Contact con: [Select Id, cnt_plcsc_Customer_Number__c 
							From Contact 
							Where cnt_plcsc_CustNmbr_ExtID__c IN: custnmbr]){
			mapCnt.put(con.cnt_plcsc_Customer_Number__c , con.Id);
			System.debug('DLB932 - Number of Cnt Recs - ' + mapCnt);
		}  

set<String> oppid = new set<String>();
		for (Opportunity o: Trigger.new){
            if (o.Customer_Number__c != null){
            	if(o.Id != null)
            	oppid.add(o.Id);
            	System.debug('DLB932 - Id of Opp Recs - ' + oppid);
            }
        }
        
Map<String,Id> mapExistingOCR = new Map<String,Id>();
/*Loop through OpportunityContactRole based on oppid to find existing OCR's*/
		for(OpportunityContactRole ocr1:[Select Id, OpportunityId From OpportunityContactRole 
										Where OpportunityId IN: oppid]){
			mapExistingOCR.put(ocr1.OpportunityId, ocr1.Id);
			System.debug('DLB932 - Id of OCR1 Recs - ' + mapExistingOCR);
		}
		
/*Loop through the main list of Opps*/
List<OpportunityContactRole> ocr = new List<OpportunityContactRole>();

		for(Opportunity o: Trigger.new){
			if(mapCnt.containsKey(o.Customer_Number__c)){
				if(!mapExistingOCR.containsKey(o.Id)){
				OpportunityContactRole ocr2 = new OpportunityContactRole();
				System.debug('Contact' + mapCnt);
				ocr2.ContactId = mapCnt.get(o.Customer_Number__c);
				ocr2.OpportunityId = o.Id;
				ocr2.IsPrimary = true;
				ocr2.Role = 'Primary Insured';
				ocr.add(ocr2);
				System.debug('DLB932 - CNT - ' + ocr);
				}	
			}
		}

/*Insert the new list of OCR's*/
	if(!ocr.isEmpty()){
		insert ocr;
	}
}

 

Starz26Starz26

looks good at first glance....

 

congrats on the milestone