1

I have a trigger that is supposed to see if a field is empty and, if it is empty, it is supposed to check if there is an Account Team Member with a certain role. If that is true then it is supposed to delete that team member. I can only get 60% code coverage on it and I've been wondering why. I am new to writing code. My trigger and test class are below.

TRIGGER

trigger DeleteATM on Account(after insert, after update) {
    List<Id> lstId = new List<Id>();

   for(Account acc: Trigger.new){
       if (acc.Customer_Success_Manager__c== null) {
           List<AccountTeamMember> atm = [Select UserId from AccountTeamMember where      TeamMemberRole =: 'Customer Success Manager' AND AccountID=:acc.id AND UserID!=:acc.Customer_Success_Manager__c];
           delete atm;
       }
   }
}

TEST CLASS

@isTest
Public class RemoveCSMTest {
    Static testMethod void insertNewAccount() {
        Account Cre = new Account();
        Cre.Market__c = 'Houston';Cre.Business_Unit__c = '334';
        Cre.Name = 'Test CSM2';
        Cre.Customer_Success_Manager__c = '005A0000000SAC0';
        Insert cre;
        List<Account> a = [SELECT Id, Name, Customer_Success_Manager__c FROM Account WHERE Name = 'TestCSM2'];
        for (Account accs : a){
             accs.Customer_Success_Manager__c=NULL;
             Update accs;
        }
    }
}
  • Do you use an IDE like Eclipse ? With the Force.com plugin you can run your test and see which lines are not covered. In your example, I think the 60% represent the 6 lines of code and the other 40% the { and } lines. Try to put the { and } in the end of a line with some code. – SF_user Nov 6 '14 at 14:21
  • 5
    See the "Avoid SOQL Queries or DML statements inside FOR Loops" section of Apex Code Best Practices for an explanation and an example of how to fix. (This addresses the question in the title. The body of your question asks something else.) – Keith C Nov 6 '14 at 14:21
  • I used the developer console to test it an the last two lines aren't covered I can't figure out why though. I have another trigger just like it that checks if the field value has changed and I get 100% on that one. – Blair Kristin Nov 6 '14 at 14:25
  • You need to create a AccountTeamMember record so that the SOQL query in your trigger returns at least one record. – BarCotter Nov 6 '14 at 14:56
8

You are running a SOQL statement within a loop. This is not best practice and will usually end up having governor limit issues like you are seeing.

You need to move your query outside of the loop and try to leverage a collection. See below I am using a map to hold the account team members.

EDIT

Changed code to delete ALL team members, not just a single team member.

trigger DeleteATM on Account(after insert, after update) {

    map<Id,list<AccountTeamMember>> account2TeamMemberMap = new map<Id,list<AccountTeamMember>>();
    for(AccountTeamMember mem : [Select Id, AccountId From AccountTeamMember Where TeamMemberRole = 'Customer Success Manager' and AccountId In : trigger.new]){
        if(account2TeamMemberMap.containsKey(mem.AccountId)){
             account2TeamMemberMap.put(mem.AccountId, new list<AccountTeamMember>());
        }
        account2TeamMemberMap.get(mem.AccountId).add(mem);
    }

    list<AccountTeamMember> members2Delete = new list<AccountTeamMember>();
    for(Account acc: Trigger.new){
        if (account2TeamMemberMap.containsKey(acc.Id) && acc.Customer_Success_Manager__c == null) {
            members2Delete.addAll(account2TeamMemberMap.get(acc.Id));
        }
    }
    delete members2Delete;
}

As @KeithC suggested, I would take a look at the Apex Coding Best Practices

| improve this answer | |
  • An Account may have multiple AccountTeamMember records so your map should be a map from AccountId to a List: Map<Id, List<AccountTeamMember>>. – BarCotter Nov 6 '14 at 15:19
  • 1
    You are right, there can be multiple, but he is looking for a certain role. I know they may be able to have multiple members with a certain role, but none of this was given as details so I simplified. If there are multiple team members with that role, then yes, using a list of members would work. – Chris Duncombe Nov 6 '14 at 15:50
  • 1
    AccountID In: trigger.new...absolutely brilliant! That solves a problem I had posted about yesterday. – eyedar Nov 6 '14 at 19:40
  • I only want one person to be a Customer Success Manager, so I need to delete any old members listed as Customer Success Manager in the Account Team object. – Blair Kristin Nov 6 '14 at 20:08
  • 1
    Hi Blair, it would be pretty difficult to debug your problem in the comments section of this site. If you create a new question and add the code then we can take a look. – BarCotter Nov 7 '14 at 11:23

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.