You need to sign in to do that
Don't have an account?
Claire Sunderland 1
Apex Trigger for calculating business hours only 55% Code Coverage
Hi - I am an admin that is trying to code something into our instance to calculate business hours between 2 date/time fields based on owner location to account for time zone changes. I was able to find and tweak the code from this website (https://www.tech-vision.us/salesforce-the-number-of-business-hours-between-two-date-time-fields/). I then wrote a Test Class but am only able to get to 55% code coverage and cannot figure out why.
Trigger:
Test Class:
I've checked the developer console and the sections that aren't covered are:
if (LocationName_BusinessHours_Map.containsKey(leadObj.Owner_Location__c)) {
decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get(leadObj.Owner_Location__c).Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c);
Decimal resultingHours = result / (60 * 60 * 1000);
leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1);
Does anyone have any ideas on what I'm missing? Thank you!
Trigger:
trigger Demo_Response_Biz_Hours_Time_Zones on Lead (before insert, before update) { //Selecting all active BusinessHours records List<BusinessHours> BusinessHours_List = [SELECT Id, Name FROM BusinessHours WHERE IsActive = true]; //Adding map where BusinessHours Name will be the key Map<String, BusinessHours> LocationName_BusinessHours_Map = new Map<String, BusinessHours> (); //Filling the map | Location Name, BusinessHours Object for (BusinessHours b : BusinessHours_List) { LocationName_BusinessHours_Map.put(b.Name, b); } for (Lead leadObj : Trigger.new) { //This part of a trigger only works when we add new records if (Trigger.isInsert) { //We check if both Start Time & Completed Time fields are not empty if (leadObj.Demo_Form_Submission_Date__c != NULL && leadObj.First_Activity_Most_Recent_Demo_Request__c != NULL) { //Then we do another check if current record is tied to a location that has business hours record if (LocationName_BusinessHours_Map.containsKey(leadObj.Owner_Location__c)) { decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get(leadObj.Owner_Location__c).Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c); Decimal resultingHours = result / (60 * 60 * 1000); leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1); } else{ //Just as a safety net, let's use default business hours in case there is no match between Location name and BH name if(LocationName_BusinessHours_Map.get('Default').Id != NULL){ decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get('Default').Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c); Decimal resultingHours = result / (60 * 60 * 1000); leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1); } } } } else if (Trigger.isUpdate) { //This part of a trigger only works when we perform update //Checking if Start Time or Completed Time has been changed & if they are not empty if ((leadObj.Demo_Form_Submission_Date__c != NULL && leadObj.First_Activity_Most_Recent_Demo_Request__c != NULL) && ((leadObj.Demo_Form_Submission_Date__c != Trigger.oldMap.get(leadObj.id).Demo_Form_Submission_Date__c) || (leadObj.First_Activity_Most_Recent_Demo_Request__c != Trigger.oldMap.get(leadObj.id).First_Activity_Most_Recent_Demo_Request__c))) { if (LocationName_BusinessHours_Map.containsKey(leadObj.Owner_Location__c)) { decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get(leadObj.Owner_Location__c).Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c); Decimal resultingHours = result / (60 * 60 * 1000); leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1); } else{ if(LocationName_BusinessHours_Map.get('Default').Id != NULL){ decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get('Default').Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c); Decimal resultingHours = result / (60 * 60 * 1000); leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1); } } } else if (leadObj.Demo_Form_Submission_Date__c == NULL || leadObj.First_Activity_Most_Recent_Demo_Request__c == NULL) { //Another safety net that will reset Hours spent in case Start Time OR Completed time becomes empty after update. //That way you won't have old & inaccurate data from past calculations. leadObj.APEXTEST_Demo_Response_Time_Zones__c = NULL; } } } }
Test Class:
@isTest public class TestDemoResponseTime { Static testmethod void LeadTest() { //create a lead Lead leadObj = new Lead(); leadObj.LastName = 'Test'; leadObj.Company = 'Test ABC'; leadObj.LeadSource = 'Sales Prospecting'; leadObj.Lead_Source_Detail__c = 'Apex Test Class'; leadObj.OwnerID = '0054G000008UaurQAC'; leadObj.Demo_Form_Submission_Date__c = datetime.newInstance(2020, 04, 15, 14, 00, 00); insert leadObj; //business hours BusinessHours b = [SELECT Id FROM BusinessHours WHERE Name = 'Denver']; //update the lead leadObj = [SELECT Id, LastName, Company, LeadSource, Lead_Source_Detail__c, OwnerID, Owner_Location__c, Demo_Form_Submission_Date__c, First_Activity_Most_Recent_Demo_Request__c FROM Lead WHERE Id != NULL]; leadObj.First_Activity_Most_Recent_Demo_Request__c = datetime.newInstance(2020, 04, 16, 19, 00, 00); update leadObj; } }
I've checked the developer console and the sections that aren't covered are:
if (LocationName_BusinessHours_Map.containsKey(leadObj.Owner_Location__c)) {
decimal result = BusinessHours.diff(LocationName_BusinessHours_Map.get(leadObj.Owner_Location__c).Id, leadObj.Demo_Form_Submission_Date__c, leadObj.First_Activity_Most_Recent_Demo_Request__c);
Decimal resultingHours = result / (60 * 60 * 1000);
leadObj.APEXTEST_Demo_Response_Time_Zones__c = resultingHours.setScale(1);
Does anyone have any ideas on what I'm missing? Thank you!
It's all good, we all started somewhere. I was lucky as I had a very good mentor for the early part of my SF career who helped refine my coding skills. They are not perfect, but they have work for me over the years.
Ok. Using the formula field is fine. I should have worked that one out myself.
Now to handle the repetitive code components, if we were to write a Trigger Handler, we could go with something like: This borrows heavily on the code that you had written. But as we check the code structure, at any point where there is repetitive code, we have moved it to it's own method.
By breaking it out into separate methods, we can invoke them as required. For example, if you wanted to have the code also run on insert, then you could add something like: rather than writing out all the code again.
Now, since i have written a Trigger Hanlder, best we do a demo of how to invoke the Handler class: As we check the Trigger code, we notice that there is minimal work being done here. It's all being passed to the Handler to do the heavy lifting.
There are ways to reduce this code even further, but I like this level as it still makes it obvious what is happening.
And now, we need to consider where this question started, it was with the Test coverage. Here is a sample using the TestSetup to create the base line test data. Once we have those test records, we can tweak them for each test we run. Remembering that when each test method completes, the test data reverts back to the base line. By using TestSetup, we actually reduce the run time of our Test code, as the records are only written once in the setup, rather than over and over for each test method. This becomes more relevant when we have more complex code which is using alot more records.
Notice that we have 2 x test methods, one where we will use the Default Business Hours, and one where we have set the Location.
This would give you the extra coverage we spoke about when we said we needed to test each branch of the IF statement.
However, if we run only one of these tests, we will still get good coverage because we have the method setResponseTime in our code only the once. And one test will cover the majority of that method. By using the second test method, we can cover the IF branch within this method.
Other things to consider would be the creation of a "Running User" and the use of the System.runAs() command. The need for this depends mostly on the use of the object we are testing and whether we would expect different behaviours for the code based on the permissions of the user running the code. In this case, since we are not using a "Running User", the code will be executing as a System Admin. You could use the User that we have created, or you could create a second User to be "Running User" and so test behaviors such as a different profile to the owner running the code. I will leave that decision up to you.
Now the test code should give you around 95%. The code not covered would the exception i've thrown if we get no business hours returned. If you are confident that situation will never arise, then you could remove those code lines to get 100% coverage. I had them in as I started with no business hours set in my developer org.
All code is provided as-is and is written in my developer org, so field names may not match, and validation rules may be different. However, it should give you a good idea of how to solve your question.
I hope that the above is helpful.
Regards
Andrew G
All Answers
You would need to fulfill the requirements of each branch.
And looking at your code which is doing a test against leadObj.Owner_Location__c I cannot see that value being set in your test class.
I would suggest that you set up at least 2 x test methods, one to test each branch.
For some feedback on your code, i found it difficult to read. And also for a Trigger it is starting to get somewhat long. Best practice is to minimise the amount of code in the Trigger, usually by building a Trigger Handler class which does the heavy lifting.
Also, you use this chunk of code repeatedly: I would recommend busting that out into a separate method to call as a single line each time.
example: Whilst it appears to read longer, it means every time that you want the business Hours calculated, it will be just one line in the code moving forward.
Another thing to consider, the use of hardcoded strings in Code, (test or live) is not really best practice.
I would update my test code to create a User record with the values and profile required.
Please also consider the use of System.runas() and Test.testStart() & Test.testStop();
And Asserts. Coverage is only part of the battle. Good test code will have asserts to ensure the code is behaving the way it is intended and to detect if there are any future changes in the system that affeact the function of this code.
If you are considering using a Trigger Handler, here is a post i did a while back on Handlers. It also covers some considerations for test code.
https://developer.salesforce.com/forums/ForumsMain?id=9062I000000IKZFQA4
Regards
Andrew
I don't have Office_Location__c in my Test Class because it's a formula field so I can't set a value for it. The value is set based on who the Owner of the record is which is why I pulled in OwnerID - is that a workaround or do I need to do something else here?
I understand what you mean about the code being repetitive - I'm unsure where to separate out and put the method you mentioned. Can you specify? I did end up editing out the 'before insert' informaton because I realized we will only need this to run on update.
Also working on adding asserts to the apex class and creating user records instead of using hard coded values.
It's all good, we all started somewhere. I was lucky as I had a very good mentor for the early part of my SF career who helped refine my coding skills. They are not perfect, but they have work for me over the years.
Ok. Using the formula field is fine. I should have worked that one out myself.
Now to handle the repetitive code components, if we were to write a Trigger Handler, we could go with something like: This borrows heavily on the code that you had written. But as we check the code structure, at any point where there is repetitive code, we have moved it to it's own method.
By breaking it out into separate methods, we can invoke them as required. For example, if you wanted to have the code also run on insert, then you could add something like: rather than writing out all the code again.
Now, since i have written a Trigger Hanlder, best we do a demo of how to invoke the Handler class: As we check the Trigger code, we notice that there is minimal work being done here. It's all being passed to the Handler to do the heavy lifting.
There are ways to reduce this code even further, but I like this level as it still makes it obvious what is happening.
And now, we need to consider where this question started, it was with the Test coverage. Here is a sample using the TestSetup to create the base line test data. Once we have those test records, we can tweak them for each test we run. Remembering that when each test method completes, the test data reverts back to the base line. By using TestSetup, we actually reduce the run time of our Test code, as the records are only written once in the setup, rather than over and over for each test method. This becomes more relevant when we have more complex code which is using alot more records.
Notice that we have 2 x test methods, one where we will use the Default Business Hours, and one where we have set the Location.
This would give you the extra coverage we spoke about when we said we needed to test each branch of the IF statement.
However, if we run only one of these tests, we will still get good coverage because we have the method setResponseTime in our code only the once. And one test will cover the majority of that method. By using the second test method, we can cover the IF branch within this method.
Other things to consider would be the creation of a "Running User" and the use of the System.runAs() command. The need for this depends mostly on the use of the object we are testing and whether we would expect different behaviours for the code based on the permissions of the user running the code. In this case, since we are not using a "Running User", the code will be executing as a System Admin. You could use the User that we have created, or you could create a second User to be "Running User" and so test behaviors such as a different profile to the owner running the code. I will leave that decision up to you.
Now the test code should give you around 95%. The code not covered would the exception i've thrown if we get no business hours returned. If you are confident that situation will never arise, then you could remove those code lines to get 100% coverage. I had them in as I started with no business hours set in my developer org.
All code is provided as-is and is written in my developer org, so field names may not match, and validation rules may be different. However, it should give you a good idea of how to solve your question.
I hope that the above is helpful.
Regards
Andrew G