0

Can somebody point what I'm missing or help me with code coverage here, I covered all methods written in class in test class and still getting 64% code coverage which is not letting me deploy changes in production.

Trigger is copying data from To, Cc, Bcc of email and putting in object to make it searchable from global search. and in second part we have flag system, if new email has arrived in system, it will show flag to users in list view.

Trigger:

trigger EmailDataCopy on EmailMessage (after insert, after update) {

        //Copy emailMessage data
        EmailDataRetrive.EmailCopier(Trigger.new);

}

Class:

public class EmailDataRetrive {


    public static void EmailCopier (list<EmailMessage> emList){
        string emailmessages;
        set<string> Data = new set<String>();
        list<Case> UpdateCases = new list<Case>(); 
        map<id,list<EmailMessage>> emailMap = new map<id,list<EmailMessage>>();

        for(EmailMessage em : emList)
        {
            if(emailMap.containsKey(em.ParentId)){
                emailMap.get(em.ParentId).add(em);
            }
            else{
                list<EmailMessage> templist = new list<EmailMessage>();
                templist.add(em);
                emailMap.put(em.ParentId,templist);
            }
        }

       for(case c: [select id, Email_Data__c from Case where id in :emailMap.keySet()]){

           if(c.Email_Data__c != null && c.Email_Data__c.contains(' ')) Data = new set<String>(c.Email_Data__c.split(' '));
           else if (c.Email_Data__c != null && !c.Email_Data__c.contains(' ')){ Data = new set<String>(); data.add(c.Email_Data__c);}
            emailmessages = null;           
            for(EmailMessage copyEm : emailMap.get(c.Id)){
                if(copyEm.ToAddress != null && (!data.contains(copyEm.ToAddress))) emailmessages = copyEm.ToAddress;
                if(copyEm.CcAddress != null && (!data.contains(copyEm.CcAddress))) emailmessages = emailmessages + ' ' +copyEm.CcAddress;
                if(copyEm.BccAddress != null && (!data.contains(copyEm.BccAddress))) emailmessages = emailmessages + ' ' +copyEm.BccAddress;
            }

           c.Email_Data__c = c.Email_Data__c + ' ' + emailmessages;
           updateCases.add(c);
        }

        list<Database.SaveResult> Result = database.update(updateCases,false);
    }
}

Test Class:

@isTest(seeAllData = true)

private class EmailDataRetriveTest {
    static testMethod void testCaseFromEmail() {

        Test.starttest();

        Group testGroup = new Group(Name='test group', Type='Queue');
        insert testGroup;

        System.runAs(new User(Id=UserInfo.getUserId()))
        {
            QueuesObject testQueue = new QueueSObject(QueueID = testGroup.id, SObjectType = 'Case');
            insert testQueue;
        }

        Case aCase = new Case(OwnerId = testGroup.Id);
        aCase.Email_Data__c = 'test'; //To test delete function of trigger
        insert aCase;   

        //Trigger will hit after this line
        list <CaseComment> Comment = new list <CaseComment>();
        Comment = [Select Id, ParentId from CaseComment where ParentId = :aCase.Id];
        for(Integer i=0; i<Comment.size(); i++)
        {
            String parentID = Comment[i].ParentID; 
            Contact newcontact = [Select Email from Contact Where ID = :aCase.ContactID];

            EmailMessage em = new EmailMessage(
            Subject = 'A new External Case Comment has been posted',
            TextBody = Comment[i].CommentBody, 
            ToAddress = 'test@zimmerbiomet.com',
            CcAddress = 'test@zimmerbiomet.com',
            BccAddress = 'test@zimmerbiomet.com',
            Status = 'new',
            Incoming = true,
            ParentId = parentID 
            );

            insert em;
        }           

        update acase; 

        Test.stoptest();
    }
}
  • I'd be more worried about the fact that if you get this into production, you will break the ability to mass upload data, given that you have all those queries inside loops. – sfdcfox Dec 9 '15 at 16:01
  • We don't use mass upload for cases or emailMessages as this functionality is solely being used for email to case where there's no requirement of mass upload so there won't be any impact on that area, but I understand consequences and appreciate your point of view on that. – Dave Dec 9 '15 at 16:02
  • Ankz, it would be good practice to take a few minutes and update it. Won't be hard, and you will save aggravation to the next dev (it could be you!) who has to deal with this if you ever need to scale. – Sebastian Kessel Dec 9 '15 at 16:08
  • I wouldn't worry about that yet as per our org architecture, and as per approval process, this code has been already approved based on sandbox demo and have to deploy this without any modifications except test class as that can't impact any functionality otherwise I'll have to got for UAT and approval process again. I can update this code in next release but as of now I can't process-test this trigger without following full procedure again. – Dave Dec 9 '15 at 16:16
  • 2
    the for(Integer i=0; i<Comment.size(); i++) will loop 0 times as there are no caseComments – cropredy Dec 16 '15 at 21:09
1

There are some design issues in your provided class, with all those queries in loops, but I'll get to that later.

The reason you're not getting coverage here is because calling Messaging.SendEmail() doesn't create an EmailMessage record on its own. Everything I can dig up says that the EmailMessage SObject is tied to the Email-to-Case functionality. You might be able to get this to work if your test email message gets sent to one of the emails set up in your org for email-to-case, but I'm not certain of this (I'm not sure of the behavior of Messaging.SendEmail() inside of a test method).

To test your class, you'll need to manually create an EmailMessage record, and insert it.

Constructing the EmailMessage record should be fairly straightforward.

EmailMessage em = new EmailMessage(
    Subject = 'A new External Case Comment has been posted',
    TextBody = Comment[i].CommentBody,
    ToAddress = newcontact.Email,
    CcAddress = newcontact.Email,
    BccAddress = newcontact.Email,
    Status = 0,
    Incoming = true,
    ParentId = parentID 
);

After that, it's a plain, old DML Insert

ParentId appears to be a required field, and I imagine that some of the other fields are required as well, though I can't find which ones are. I'd imagine that Subject and ToAddress would be required.

The Status field is the interesting one here. Going through the schema explorer in Eclipse, the labels for the picklist values are New, Read, Replied, Sent, Forwarded, and Draft, but the actual values are integers, 0 - 5 inclusive. 0 = 'New', 1 = 'Read', 2 = 'Replied', 3 = 'Sent', 4 = 'Forwarded', and 5 = 'Draft'

Another, smaller issue with your code is that you're sending Case records to your helper class before insert. The Case records won't have an Id at this point, so your copyEmail class may behave unexpectedly on inserting Cases. You might get an error when you try to run your SQOL queries, or your queries may just return no rows.

From what I can see, EmailMessage records are always associated to a Case. This would suggest that Email-to-Case is being triggered on an incoming email, but creates and inserts the Case first, and then creates the EmailMessage record. If this is true (can't find any documentation to confirm this), it means that you will never be able to test this functionality on a Case Before Insert.

Another glaring issue with your test class is that you're not actually testing anything. Coverage !== Tested. For proper testing, you need some assertions. You're doing your org's release management procedure a disservice if you do any less.

I get the feeling that you know quite well what to do, but for the benefit of others, the bare minimum that you should be asserting in your unit tests are the following

system.assertEquals(newcontact.Email + ' '  + newcontact.Email + ' '  + newcontact.Email + ' ', newCase.Email_Data__c);
system.assertEquals(true, newCase.Indicator__c)

Better unit tests would cover not only the happy path, but also cases where no new email addresses are added, there end up being no email addresses added, indicator__c doesn't become true, etc...

Now, to address the design issues. As I see in the comments on your question, I understand that you have an involved process to make code changes. I'll start by saying that whoever approved this code in the first place should be removed from your code approval board.

This code isn't solely contained to Email-to-Case, it will affect every integration that you have with the Case object. If you have more code like this that touches the Case object, or if Case`s trigger(s) touches any other object, you will quickly run into the query limit.

Yes, you will need to go through your process again, but the cheapest time to fix an issue you know about is five minutes ago.

A significantly better structure for your copyEmail class would be this

public with sharing class copyEmail {

public static void emailCopier (list<Case> caseList){
    Map<Id, EmailMessage> messages = new Map<Id, EmailMessage>();
    for(EmailMessage msg : [SELECT ToAddress, CcAddress, BccAddress, ParentId FROM EmailMessage WHERE ParentId IN :caseList)
    {
        messages.add(msg.ParentId, msg);
    }

    for(Case each : caseList)
    {
        EmailMessage msg = messages.get(each.Id);
        List<String> additionalEmails;
        if(msg != null)
        {
            if(msg.ToAddress != null)
            {
                additionalEmails.add(msg.ToAddress);
            }

            if(msg.CcAddress != null)
            {
                additionalEmails.add(msg.CcAddress);
            }

            if(msg.BccAddress != null)
            {
                additionalEmails.add(msg.BccAddress);
            }

            each.Email_Data__c = String.Join(additionalEmails, ' ');
        }
    }
}

public static void emailupdater (list<Case> cases){
    Map<Id, EmailMessage> messages = new Map<Id, EmailMessage>();
    for(EmailMessage msg : [SELECT Status ParentId FROM EmailMessage WHERE ParentId IN :caseList)
    {
        messages.add(msg.ParentId, msg);
    }
    for(Case each : cases)
    {
        EmailMessage msg = messages.get(each.Id);
        if(msg != null && msg.Status.contains('New'))
        {
            each.Indicator__c = true;
        }
    }
}
}
| improve this answer | |
  • That did not help and gave me same coverage, neither solution for bulkification is correct as it will not insert correct data or update it. I was the one who approved code in tight schedule just by checking function not code and developer left after that without deploying. I have updated new code, check for bulkification of the trigger. – Dave Dec 15 '15 at 21:36
0

Final code with 100% code coverage

Trigger:

trigger EmailDataCopy on EmailMessage (after insert, after update) {

//Copy emailMessage data
EmailDataRetrive.EmailCopier(Trigger.new);

}

Class:

public class EmailDataRetrive {



public static void EmailCopier (list<EmailMessage> emList){
    string emailmessages;
    set<string> Data = new set<String>();
    list<Case> UpdateCases = new list<Case>(); 
    map<id,list<EmailMessage>> emailMap = new map<id,list<EmailMessage>>();

    for(EmailMessage em : emList)
    {
        if(emailMap.containsKey(em.ParentId)){
            emailMap.get(em.ParentId).add(em);
        }
        else{
            list<EmailMessage> templist = new list<EmailMessage>();
            templist.add(em);
            emailMap.put(em.ParentId,templist);
        }
    }

   for(case c: [select id, Email_Data__c from Case where id in :emailMap.keySet()]){

       if(c.Email_Data__c != null && c.Email_Data__c.contains(' ')) Data = new set<String>(c.Email_Data__c.split(' '));
       else if (c.Email_Data__c != null && !c.Email_Data__c.contains(' ')){ Data = new set<String>(); data.add(c.Email_Data__c);}
        emailmessages = null;           
        for(EmailMessage copyEm : emailMap.get(c.Id)){
            if(copyEm.ToAddress != null && (!data.contains(copyEm.ToAddress))) emailmessages = copyEm.ToAddress;
            if(copyEm.CcAddress != null && (!data.contains(copyEm.CcAddress))) emailmessages = emailmessages + ' ' +copyEm.CcAddress;
            if(copyEm.BccAddress != null && (!data.contains(copyEm.BccAddress))) emailmessages = emailmessages + ' ' +copyEm.BccAddress;
        }

       c.Email_Data__c = c.Email_Data__c + ' ' + emailmessages;
       updateCases.add(c);
    }

    list<Database.SaveResult> Result = database.update(updateCases,false);
   }
}

Test Class:

@isTest //(seeAllData = true)

private class EmailDataRetriveTest {
    static testMethod void testCaseFromEmail() {
        Group testGroup = new Group(Name='test group', Type='Queue');
        insert testGroup;

        System.runAs(new User(Id=UserInfo.getUserId()))
        {
            QueuesObject testQueue = new QueueSObject(QueueID = testGroup.id, SObjectType = 'Case');
            insert testQueue;
        }

        Case aCase = new Case(OwnerId = testGroup.Id);
        aCase.Email_Data__c = 'Test'; //To test delete function of trigger
        insert aCase;   

        list<EmailMessage> emaillist = new list<EmailMessage>();
        for(integer i =0; i<3; i++){
            EmailMessage em = new EmailMessage();
            em.Subject = 'A new External Case Comment has been posted';
            em.TextBody = 'Test Email';
            em.ToAddress = 'Test' + string.valueOf(i) + '@test.com';
            em.CcAddress =  'Test' + string.valueOf(i+1) + '@test.com';
            em.BccAddress = 'Test' + string.valueOf(i+2) + '@test.com';
            //Status = 'New',
            em.Incoming = true;
            em.ParentId = aCase.ID;
            emaillist.add(em); 
        }
        Test.startTest();
            Insert emaillist;
        Test.stoptest();
    }
}
| improve this answer | |

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.