2

I'm trying to get a list (and ultimately the ContactIds) of Contact Roles associated with an Opportunity in an after-update trigger, but the size of this list is always 0, even when there are Contact Roles in the Opportunity:

List<OpportunityContactRole> cRoles = [SELECT Id, Contactid FROM OpportunityContactRole WHERE OpportunityId = :opp.Id];

"opp" is the Opportunity that sets off the trigger. Am I doing anything wrong here?

Edit: this is all the code that precedes this line:

trigger OppStageChange on Opportunity (after update) {
    for (Opportunity opp : Trigger.new) {
        List<OpportunityContactRole> cRoles = [SELECT Id, Contactid FROM OpportunityContactRole WHERE OpportunityId = :opp.Id];
  • 1
    We would have to see the code where opp is derived and if this is for a unit test, we'd probably need to see that too. – greenstork Oct 30 '14 at 15:02
  • stupid question - could it be because of roles/permissions of the user under which this code is running? – Anup Oct 30 '14 at 15:12
  • Easiest way to make sure you trigger is working is to write a unit test. – greenstork Oct 30 '14 at 15:16
  • 2
    you have a soql query within a for loop in a trigger - this is not best practice and will fail when trigger set size increases such as through data loader. You need to fetch all OCR for all triggered Oppos in one SOQL - saving in a map by OppoId – cropredy Oct 30 '14 at 23:23
1

1) Can you test running this code in an asynchronous method called by the trigger? I seem to recall Opportunity Contact Roles not being populated til Workflows normally would (after after update triggers fire).

2) I would strongly recommend querying the OpportunityContactRoles using trigger.newMap.keySet() outside of the loop instead, otherwise the trigger is not going to be bulk-safe.

1

just like Anup and greenstork suggested - it's not best practice to have a soql query inside of a for loop and also since we can't tell for sure if the opportunity records it might be best for you to create a test class and then create an oppotunity as well as an OpportunityContactRole and associate the OpportunityContactRole to the opportunity and run an update on your opportunity record. I have edited your code to follow best practice however:

trigger OppStageChange on Opportunity (after update) {
    List<OpportunityContactRole> cRoles = [SELECT Id, Contactid FROM OpportunityContactRole WHERE OpportunityId = :Trigger.newMap.Keyset()];
    System.debug('cRoles list : '+cRoles);
}
  • your query is wrong, it should be [SELECT Id, Contactid FROM OpportunityContactRole WHERE OpportunityId IN :Trigger.newMap.Keyset()]; – Torsen Jul 3 '15 at 7:05
-1

You never should do a query inside the loop. When you are programing a trigger you should do it bulk-safe.

Example:

trigger OppStageChange on Opportunity (after update) {

    for(OpportunityContactRole ocr : [SELECT Id, Contactid,OpportunityId FROM OpportunityContactRole WHERE OpportunityId in :Trigger.newMap.keySet()]){
        //DO SOME WHIT OPPORTUNITY CONTACT ROLE
    }
}

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.