2

I have a trigger in the contact (before update and insert) that when a boolean field is true (in insert) or when it changes from false to true (in the update) it goes and find all the opportunity contact roles that the contact is linked to and get some information of the opportunity. After that depending on some values in the opportunity it updates 2 fields in the contact.

My trigger is working fine when i do 1 at a time but i notice when i do a bulk update of the records, i get some unexpected results

my trigger looks like this

if (trigger.isInsert) {
    if (trigger.isBefore) {
        Contact_HDL.updateContactRolesInsert(trigger.new);
    }       
} else {
    if (trigger.isBefore) {
        Contact_HDL.updateContactRolesUpdate(Trigger.oldMap, Trigger.newMap);
    }
}   

this is my handler (Contact_HDL)

public static void updateContactRolesInsert(List<Contact> conTrigger){
    Set<Id> conSET = new Set<Id>();
    List<OpportunityContactRole> ocrLST = new List<OpportunityContactRole>();
    Map<Id, List<OpportunityContactRole>> conMAP = new Map<Id, List<OpportunityContactRole>>();
    for (Contact c:conTrigger) {
        if (c.Review_Stages__c) {
            conSET.add(c.Id);
        }
    }

    if (conSET.isEmpty()) {
        return;
    }

    for (OpportunityContactRole o:[SELECT Id, ContactId, Opportunity.StageName FROM OpportunityContactRole WHERE ContactId IN:conSET]){
        if (!conMAP.containsKey(o.ContactId)) {
            conMAP.put(o.ContactId,ocrLST);
        } 
        ocrLST = conMAP.get(o.ContactId);
        ocrLST.add(o);
        conMAP.put(o.ContactId, ocrLST);
    }

    if (!ocrLST.isEmpty()) {
        Contact_HDL.updateContactRoles(conTrigger, conMAP);
    }
}

public static void updateContactRolesUpdate(Map<Id, Contact> oldMap, Map<Id, Contact> newMap){
    Set<Id> conSET = new Set<Id>();
    List<OpportunityContactRole> ocrLST = new List<OpportunityContactRole>();
    Map<Id, List<OpportunityContactRole>> conMAP = new Map<Id, List<OpportunityContactRole>>();
    for ( Id oppID : newMap.keySet() ) {
        if (newMap.get( oppID ).Review_Stages__c != oldMap.get( oppID ).Review_Stages__c) {
            if (newMap.get( oppID ).Review_Stages__c) {
                conSET.add(oppID);
            }               
        }
    }

    if (conSET.isEmpty()) {
        return;
    }
    for (OpportunityContactRole o:[SELECT Id, ContactId, Opportunity.StageName FROM OpportunityContactRole WHERE ContactId IN:conSET]){
        if (!conMAP.containsKey(o.ContactId)) {
            conMAP.put(o.ContactId,ocrLST);
        } 
        ocrLST = conMAP.get(o.ContactId);
        ocrLST.add(o);
        conMAP.put(o.ContactId, ocrLST);
    }

    if (!ocrLST.isEmpty()) {
        Contact_HDL.updateContactRoles(newMap.values(), conMAP);
    }

}

public static void updateContactRoles(List<contact> conTrigger, Map<Id, List<OpportunityContactRole>> conMAP){
    Set<String> stages = new Set<String>();
    stages.add('Opp Lead');
    stages.add('Val 1');
    stages.add('Val 2');
    for (Contact c:conTrigger) {
        Boolean funded = false;
        Boolean progress = false;
        if (conMAP.containsKey(c.Id)) {
            for (OpportunityContactRole o:conMAP.get(c.Id)) {
                if (o.Opportunity.StageName == 'Loan Funded') {
                    funded = true;
                } else if (stages.contains(o.Opportunity.StageName)) {
                    progress = true;
                }
            }
        }
        c.Has_a_loan_in_funded__c = funded;
        c.Has_a_loan_in_progress__c = progress;
    }
}

So I am wondering something in the method updateContactRoles must be wrong, but I am not really sure what i need to change as some contacts the field Has_a_loan_in_funded__c was marked as true when it shouldnt

In case someone had presented the same, this was my solution based on sfdcfox answer:

I notice 2 mistakes, i was checking if a list is empty when i should have been checking the map and as answer i am reinitializing properly the list avoiding the spaghetti in the update

public static void updateContactRolesUpdate(Map<Id, Contact> oldMap, Map<Id, Contact> newMap){
    Set<Id> conSET = new Set<Id>();
    List<OpportunityContactRole> ocrLST = new List<OpportunityContactRole>();
    Map<Id, List<OpportunityContactRole>> conMAP = new Map<Id, List<OpportunityContactRole>>();
    for ( Id oppID : newMap.keySet() ) {
        if (newMap.get( oppID ).Review_Stages__c != oldMap.get( oppID ).Review_Stages__c) {
            if (newMap.get( oppID ).Review_Stages__c) {
                conSET.add(oppID);
            }               
        }
    }

    if (conSET.isEmpty()) {
        return;
    }
    for (OpportunityContactRole o:[SELECT Id, ContactId, Opportunity.StageName FROM OpportunityContactRole WHERE ContactId IN:conSET]){
        ocrLST = new List<OpportunityContactRole>();
        if (!conMAP.containsKey(o.ContactId)) {
            conMAP.put(o.ContactId,ocrLST);
        } 
        ocrLST = conMAP.get(o.ContactId);
        ocrLST.add(o);
        conMAP.put(o.ContactId, ocrLST);
    }

    if (!conMAP.isEmpty()) {
        Contact_HDL.updateContactRoles(newMap.values(), conMAP);
    }

}

1 Answer 1

3

You're re-using the same Set over and over again, which you do in several places. This means that you're actually getting your data all jumbled up. Create a new Set for each key you set:

if (!conMAP.containsKey(o.ContactId)) {
    conMAP.put(o.ContactId,new Set<Id>());
} 
ocrLST = conMAP.get(o.ContactId);

You'll want to read more about Passing Parameters By Reference and By Value in Apex for more information on why this happens.

1
  • Thank you it has sense, I added my fix to one of the methods in case it helps anyone
    – manza
    Commented Feb 7, 2019 at 22:59

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .