+ Start a Discussion
SFTerrSFTerr 

Trigger returns Contact ID instead of name

Hi, trying to modify below apex trigget to generate contact role name instead of ID
I would appreciate suggestions what I'm doing wrong here
 
trigger PrimaryContactRole on Opportunity (before update) {

   for (Opportunity o : Trigger.new) {

       Integer i = [select count() from OpportunityContactRole where OpportunityId = :o.id and IsPrimary = true];
      
       if(i==1) {
       OpportunityContactRole contactRole =
            [select Contact.Name from OpportunityContactRole where OpportunityId = :o.id and IsPrimary = true];

       o.Primary_Contact_Role__c = contactRole.ContactID;
       } 

      }

   }

thanks
Best Answer chosen by SFTerr
JayantJayant
This is not bulkified. Can you see if the below code helps you ? I have also included comments to explain what a particular statement does. There may be text alignment issues as it has been copied over from Notepad++ rather than Eclipse.

trigger PrimaryContactRole on Opportunity (before update) {
    //list to hold related primary Contact Roles
    List<OpportunityContactRole> cRoles = new List<OpportunityContactRole>();
    
    //query the Contact Roles object, count is not really required as there will be only 1 primary role
    //resulting list has all the Opportunity Ids from Trigger.New that do have an associated Contact Role
    cRoles = [SELECT OpportunityId, IsPrimary, Role FROM OpportunityContactRole WHERE OpportunityId IN :Trigger.newMap.keySet() AND IsPrimary = TRUE];
    
    //we can also iterate over Trigger.New but lets iterate over the queried Contact Roles instead
    //if batch size was 200 and only 10 Opportunities have an associated Contact Role, this collection would do the required stuff in just 10 statements rather //than 200 if we iterate over Trigger.New.
    //also if we iterate over Trigger.New, we would have needed to find the matching Contact Role record from queried result set explicitly using another inner for loop  
    for(OpportunityContactRole ocr : cRoles) {
        Trigger.newMap.get(ocr.OpportunityId).Primary_Contact_Role__c = ocr.Role;
    }
}

All Answers

JayantJayant
Modify line # 8 to 11 as - 

OpportunityContactRole contactRole = [select Contact.Name, Role from OpportunityContactRole where OpportunityId = :o.id and IsPrimary = true];
o.Primary_Contact_Role__c = contactRole.Role;
JayantJayant
By the way, your trigger is not bulkified properly. Please take out the SOQLs out of For loop.
SFTerrSFTerr
Hi Jayant, thank you for your help, works great
if you are not too busy, do you mind having a look at modified code if its correctly bulkified now?
 
trigger PrimaryContactRole on Opportunity (before update) {
List<String> names = new List<String>();
   for (Opportunity o : Trigger.new) {

       Integer i = [select count() from OpportunityContactRole where OpportunityId = :o.id and IsPrimary = true];
      
       if(i==1) {
OpportunityContactRole contactRole =
            [select Contact.Name from OpportunityContactRole where OpportunityId = :o.id and IsPrimary = true];

       o.Primary_Contact_Role__c = contactRole.Contact.Name;
       } 

      }

   }

thanks
JayantJayant
This is not bulkified. Can you see if the below code helps you ? I have also included comments to explain what a particular statement does. There may be text alignment issues as it has been copied over from Notepad++ rather than Eclipse.

trigger PrimaryContactRole on Opportunity (before update) {
    //list to hold related primary Contact Roles
    List<OpportunityContactRole> cRoles = new List<OpportunityContactRole>();
    
    //query the Contact Roles object, count is not really required as there will be only 1 primary role
    //resulting list has all the Opportunity Ids from Trigger.New that do have an associated Contact Role
    cRoles = [SELECT OpportunityId, IsPrimary, Role FROM OpportunityContactRole WHERE OpportunityId IN :Trigger.newMap.keySet() AND IsPrimary = TRUE];
    
    //we can also iterate over Trigger.New but lets iterate over the queried Contact Roles instead
    //if batch size was 200 and only 10 Opportunities have an associated Contact Role, this collection would do the required stuff in just 10 statements rather //than 200 if we iterate over Trigger.New.
    //also if we iterate over Trigger.New, we would have needed to find the matching Contact Role record from queried result set explicitly using another inner for loop  
    for(OpportunityContactRole ocr : cRoles) {
        Trigger.newMap.get(ocr.OpportunityId).Primary_Contact_Role__c = ocr.Role;
    }
}
This was selected as the best answer
JayantJayant
If its resolved, please do mark it as resolved and select the most helpful answer as best answer.
SFTerrSFTerr
I have a lot to learn:)
Thank you very much for your help, works great.