function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
bohemianguy100bohemianguy100 

help bulkifying a trigger

I've written a trigger that is working fine for single record updates and inserts:

 

trigger OpportunityUpdateFromOppProduct on OpportunityLineItem (after insert, after update) {
    	
    	List<OpportunityLineItem> olis = [Select Id, OpportunityId, Service_Code__c, Is_Full_Service__c From OpportunityLineItem Where OpportunityId =: Trigger.new[0].OpportunityId];
    
    	Set<String> serviceCodeNames = new Set<String>();
    	for (OpportunityLineItem oli : olis) {
        	serviceCodeNames.add(oli.Service_Code__c);
    	}
    
    	Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: serviceCodeNames]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}
    
    	Map<String, OpportunityLineItem> oliMap = new Map<String, OpportunityLineItem>();
    
    	Opportunity tmpOpp = [Select Id, Type From Opportunity Where Id =: Trigger.new[0].OpportunityId];
    
    	Map<Id, Opportunity> oppsToUpdate = new Map<Id, Opportunity>();
    
    	String oppServiceFieldName = null;
    
    	string serviceFieldValue = null;
    	
    	Boolean alreadyExists;
    	
    	Boolean isFull = true;
    	
    	String str;
    
    	if (tmpOpp.Type.contains('Syndicated')) {
    	
    		for (OpportunityLineItem oli : olis) {
    			
    			if (oliMap.containsKey(oli.Service_Code__c)) {
    				alreadyExists = true;
    			}
    			else {
    				alreadyExists = false;
    			}
    			
    			oliMap.put(oli.Service_Code__c, oli);

    			if (alreadyExists) {
    				
    				if (isFull != false) {
        
        				oppServiceFieldName = scMap.get(oli.Service_Code__c);

        				if (oli.Is_Full_Service__c == 'True') {
            				serviceFieldValue = 'Full';
        				}
        				else {
            				serviceFieldValue = 'Partial';
        				}
        				tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    				}
    			}
    			else {
    				
    				str = oliMap.get(oli.Service_Code__c).Is_Full_Service__c;
    				
    				
    				if (str == 'Full') {
    					isFull = true;
    				}
    				else {
    					isFull = false;
    				}

    				oppServiceFieldName = scMap.get(oli.Service_Code__c);

        				if (oli.Is_Full_Service__c == 'True') {
            				serviceFieldValue = 'Full';
        				}
        				else {
            				serviceFieldValue = 'Partial';
        				}
        				tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    				}
    			oppsToUpdate.put(tmpOpp.Id, tmpOpp);
    		}
    	}
    	update oppsToUpdate.values();
}

 I was trying to bulkify the trigger so it would handle bulk updates using data loader.  I modified the trigger to the following:

 

trigger OpportunityUpdateFromOppProduct on OpportunityLineItem (after insert, after update) {
    	
    	Set<Id> opIds = new Set<Id>();
    	for (OpportunityLineItem oli : Trigger.new) {
    		opIds.add(oli.OpportunityId);
    	}
    
    	List<OpportunityLineItem> olis = [Select Id, OpportunityId, Service_Code__c, Is_Full_Service__c From OpportunityLineItem Where OpportunityId in: opIds];
    
    	Set<String> serviceCodeNames = new Set<String>();
    	for (OpportunityLineItem oli : olis) {
        	serviceCodeNames.add(oli.Service_Code__c);
    	}
    
    	Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: serviceCodeNames]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}
    
    	Map<String, OpportunityLineItem> oliMap = new Map<String, OpportunityLineItem>();
    	
    	Map<Id, Opportunity> mapOpp = new Map<Id, Opportunity>();
	
		for (Opportunity o : [Select Id, Type From Opportunity Where Id in: opIds]) {
			mapOpp.put(o.Id, o);
		}
    
    	Map<Id, Opportunity> oppsToUpdate = new Map<Id, Opportunity>();
    
    	String oppServiceFieldName = null;
    
    	string serviceFieldValue = null;
    	
    	Boolean alreadyExists;
    	
    	Boolean isFull = true;
    	
    	String str;
    	
    	for (Id i : mapOpp.keyset()) {
    
    		if (mapOpp.get(i).Type.contains('Syndicated')) {
    		
    			Opportunity tmpOpp = mapOpp.get(i);
    	
    			for (OpportunityLineItem oli : olis) {
    			
    				if (oliMap.containsKey(oli.Service_Code__c)) {
    					alreadyExists = true;
    				}
    				else {
    					alreadyExists = false;
    				}
    			
    				oliMap.put(oli.Service_Code__c, oli);

    				if (alreadyExists) {
    				
    					if (isFull != false) {
        
        					oppServiceFieldName = scMap.get(oli.Service_Code__c);

        					if (oli.Is_Full_Service__c == 'True') {
            					serviceFieldValue = 'Full';
        					}
        					else {
            					serviceFieldValue = 'Partial';
        					}
        					tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    					}
    				}
    				else {
    				
    						str = oliMap.get(oli.Service_Code__c).Is_Full_Service__c;
    				
    						if (str == 'Full') {
    							isFull = true;
    						}
    						else {
    							isFull = false;
    						}

    						oppServiceFieldName = scMap.get(oli.Service_Code__c);

        					if (oli.Is_Full_Service__c == 'True') {
            					serviceFieldValue = 'Full';
        					}
        					else {
            					serviceFieldValue = 'Partial';
        					}
        					tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    					}
    				oppsToUpdate.put(tmpOpp.Id, tmpOpp);
    			}
    		}
    	}
    	update oppsToUpdate.values();
}

 

It processes correctly for one record, but fails to update correctly for subsequent records.

 

I think the problem is occuring within this section of the code:

 

Set<String> serviceCodeNames = new Set<String>();
    	for (OpportunityLineItem oli : olis) {
        	serviceCodeNames.add(oli.Service_Code__c);
    	}
    
    	Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: serviceCodeNames]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}

 

 Can anyone help with how I can correctly get this bulkified?

Best Answer chosen by Admin (Salesforce Developers) 
Jeremy.NottinghJeremy.Nottingh

It looks like it's something small to me. If the service code already exists, you should only replace it if you're changing from "Partial" to "Full". You shouldn't ever update "Full" to "Partial". Here's the code section:

 

 

if (alreadyExists) {
    				
   if (isFull != false) {
        
       oppServiceFieldName = scMap.get(oli.Service_Code__c);

        if (oli.Is_Full_Service__c == 'True') {
            serviceFieldValue = 'Full';
        }
        else {
//** this will replace "Full" in some cases. 
		serviceFieldValue = 'Partial'; 
        }
        tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    }
 }

If you remove the line I noted above, it will only put "Partial" in there if alreadyExists is false, which I believe is the behavior you're looking for.

 

By the way, I don't understand what IsFull is meant to indicate. You're setting it at the end of the oli loop, which means it's going to apply to the next oli, and will not necessarily be accurate. Maybe you don't really need it?

 

Jeremy

 

 

 

All Answers

Jeremy.NottinghJeremy.Nottingh

Hey bohemianguy,

Really, what you need to do is keep track of what service codes are on each opportunity separately. Which means you need to know which olis are on which Opportunity. One way to keep Opportunities and olis together is to do a subquery on Opportunity.

 

 

list<OpportunityLineItem> olis = Trigger.new;
map<id, set<String>() oppidtoservicecodesmap = new map<id, set<String>>();

for (OpportunityLineItem oli : olis)  {

   if (oppidtoservicecodesmap.get(oli.OpportunityID) == null) {
      oppidtoservicecodesmap.put(oli.OpportunityID, new set<String> { oli.Service_Code__c };
   } else {
      set<String> tempset = oppidtoservicecodesmap.get(oli.OpportunityID);
      tempset.add(oli.Service_Code__c);
      oppidtoservicecodesmap.put(oli.OpportunityID, tempset);
   }
}


list<Opportunity> opps = [select id, [fields], (select id, [fields] from OpportunityLineItem) from Opportunity where id in :oppidtoservicecodesmap.keyset()];

//Now you can loop through the Opportunities and olis together
//Any other querying that needs doing, do it here, before the loop.

for (Opportunity o : opps) {
   //do all your Opportunity-level stuff here
   //like mess with oppidtoservicecodesmap.get(o.id)
   for (OpportunityLineItem oli : o.OpportunityLineItems) {
      //do any oli-level stuff here

   } //for OpportunityLineItem oli
   
   //conclude Opportunity stuff

}//for Opportunity o

 

 

I left out the parts of your code that didn't seem like they would change much. Basically, though, you have to figure out how to index everything off of the Opportunity, which is a little complicated because it's not an Opportunity Trigger. See if what I said makes sense.

 

Jeremy
bohemianguy100bohemianguy100

Hi Jeremy,

 

I think I understand your example, but I'm still not clear how I correctly get my service field codes from my lookup table.

 

I have to query the Service_Code__c table with the set of strings in the oppidtoservicecodesmap for each opportunity, so I have the following:

 

trigger OpportunityUpdateFromOppProduct on OpportunityLineItem (after insert, after update, after delete) {
    	
    	list<OpportunityLineItem> olis = Trigger.new;

		map<id, set<String>> oppidtoservicecodesmap = new map<id, set<String>>();
		
		for (OpportunityLineItem oli : olis)  {   
			if (oppidtoservicecodesmap.get(oli.OpportunityID) == null) {      
				oppidtoservicecodesmap.put(oli.OpportunityID, new set<String> { oli.Service_Code__c });
			}
			else {      
				set<String> tempset = oppidtoservicecodesmap.get(oli.OpportunityID);      
				tempset.add(oli.Service_Code__c);      
				oppidtoservicecodesmap.put(oli.OpportunityID, tempset);   
			}
		}
					
		list<Opportunity> opps = [select Id, Type, (select Id, OpportunityId, Service_Code__c, Is_Full_Service__c from OpportunityLineItems) from Opportunity where id in :oppidtoservicecodesmap.keyset()];

		Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: oppidtoservicecodesmap.values()]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}

		for (Opportunity o : opps) {
   			//do all your Opportunity-level stuff here
   			//like mess with oppidtoservicecodesmap.get(o.id)   
   			
   			for (OpportunityLineItem oli : o.OpportunityLineItems) {
     		//do any oli-level stuff here

   			} //for OpportunityLineItem oli
   
   				//conclude Opportunity stuff

		}//for Opportunity o
}

 

Now, I'm getting an error: Invalid bind expression type of SET<String> for column of type string, which is where I was trying to query all Service_Code__c object to get all the service field names  I'm not clear on that part?

 

Thanks for the help.  I appreciate it.
Jeremy.NottinghJeremy.Nottingh

This part is your problem, because oppidtoservicecodesmap.values() is a list of sets, rather than one big set:

 

 

for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: oppidtoservicecodesmap.values()]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}

 I can see a couple of ways to handle this. First, how many service codes do you have total? If it's not more than a couple of hundred, how about you just query all of them? That way you'll definitely have the ones you need.

 

for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c ]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}

 

 

Another way would be to carry a set of Service Codes through the loops:

 

 

		map<id, set<String>> oppidtoservicecodesmap = new map<id, set<String>>();
		set<String> applicableservicecodeset = new set<String>(); //added
		for (OpportunityLineItem oli : olis)  {   
			applicableservicecodeset.add(oli.Service_Code__c); //added
                        if (oppidtoservicecodesmap.get(oli.OpportunityID) == null) {      
				oppidtoservicecodesmap.put(oli.OpportunityID, new set<String> { oli.Service_Code__c });
			}
			else {      
				set<String> tempset = oppidtoservicecodesmap.get(oli.OpportunityID);      
				tempset.add(oli.Service_Code__c);      
				oppidtoservicecodesmap.put(oli.OpportunityID, tempset);   
			}
		}
					
		list<Opportunity> opps = [select Id, Type, (select Id, OpportunityId, Service_Code__c, Is_Full_Service__c from OpportunityLineItems) from Opportunity where id in :oppidtoservicecodesmap.keyset()

		Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in :applicableservicecodeset]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}

It seems like either way would work. Let us know how it goes!

 

Jeremy

 
bohemianguy100bohemianguy100

Hi Jeremy,

 

I only have about 50 different service codes (more will be added down the road), but it still isn't that many, so I queried all the service codes as you suggested.  Here is the updated code:

 

trigger OpportunityUpdateFromOppProduct on OpportunityLineItem (after insert, after update, after delete) {
    
    if (Trigger.isInsert || Trigger.isUpdate) {
    	
    	list<OpportunityLineItem> olis = Trigger.new;

		map<id, set<String>> oppidtoservicecodesmap = new map<id, set<String>>();
		
		for (OpportunityLineItem oli : olis)  {   
			if (oppidtoservicecodesmap.get(oli.OpportunityID) == null) {      
				oppidtoservicecodesmap.put(oli.OpportunityID, new set<String> { oli.Service_Code__c });
			}
			else {      
				set<String> tempset = oppidtoservicecodesmap.get(oli.OpportunityID);      
				tempset.add(oli.Service_Code__c);      
				oppidtoservicecodesmap.put(oli.OpportunityID, tempset);   
			}
		}
					
		list<Opportunity> opps = [select Id, Type, (select Id, OpportunityId, Service_Code__c, Is_Full_Service__c from OpportunityLineItems) from Opportunity where id in :oppidtoservicecodesmap.keyset()];

		Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}
    	
    	Map<String, OpportunityLineItem> oliMap = new Map<String, OpportunityLineItem>();
    	
    	Map<Id, Opportunity> oppsToUpdate = new Map<Id, Opportunity>();
    
    	String oppServiceFieldName = null;
    
    	string serviceFieldValue = null;
    	
    	Boolean alreadyExists;
    	
    	Boolean isFull = true;
    	
    	String str;
		
		for (Opportunity o : opps) {
 
   			if (o.Type.Contains('Syndicated')) {
   				
   				Opportunity tmpOpp = o;
   				
   				for (OpportunityLineItem oli : o.OpportunityLineItems) {
     			
     				if (oliMap.containsKey(oli.Service_Code__c)) {
     					alreadyExists = true;
     				}
     				else {
     					alreadyExists = false;
     				}
					oliMap.put(oli.Service_Code__c, oli);
					
					if (alreadyExists) {
						
						if (isFull == false) {
							
							oppServiceFieldName = scMap.get(oli.Service_Code__c);
							
							if (oli.Is_Full_Service__c == 'True') {
								serviceFieldValue = 'Full';
							}
							else {
								serviceFieldValue = 'Partial';
							}
							if (oppServiceFieldName != null) {
								tmpOpp.put(oppServiceFieldName, serviceFieldValue);
							}
						}
					}
					else {
						str = oliMap.get(oli.Service_Code__c).Is_Full_Service__c;
						
						if (str == 'Full') {
							isFull = true;
						}
						else {
							isFull = false;
						}
						
						oppServiceFieldName = scMap.get(oli.Service_Code__c);
						
						if (oli.Is_Full_Service__c == 'True') {
							serviceFieldValue = 'Full';
						}
						else {
							serviceFieldValue = 'Partial';
						}
						if (oppServiceFieldName != null) {
							tmpOpp.put(oppServiceFieldName, serviceFieldValue);
						}
					}
					oppsToUpdate.put(tmpOpp.Id, tmpOpp);
   				}
   			} 
		}
		update oppsToUpdate.values();
    }
    
    if (Trigger.isDelete) {
    
    	List<OpportunityLineItem> olis = Trigger.old;
    	
    	Set<Id> opIds = new Set<Id>();
    	for (OpportunityLineItem oli : Trigger.old) {
    		opIds.add(oli.OpportunityId);
    	}
    	
    	Set<String> serviceCodeNames = new Set<String>();
    	for (OpportunityLineItem oli : olis) {
        	serviceCodeNames.add(oli.Service_Code__c);
    	}
    
    	Map<String, String> scMap = new Map<String, String>();
    
    	for (Service_Code__c sc : [Select Name, Opportunity_Service_Field_Name__c From Service_Code__c Where Name in: serviceCodeNames]) {
        	scMap.put(sc.Name, sc.Opportunity_Service_Field_Name__c);
    	}
    
    	Map<String, OpportunityLineItem> oliMap = new Map<String, OpportunityLineItem>();
    	
    	Map<Id, Opportunity> mapOpp = new Map<Id, Opportunity>();
	
		for (Opportunity o : [Select ID, Type, (Select Id, OpportunityId, Service_Code__c, Is_Full_Service__c From OpportunityLineItems where Service_Code__c in :serviceCodeNames) from Opportunity where Id in: opIds]) {
			mapOpp.put(o.Id, o);
		}
    
    	Map<Id, Opportunity> oppsToUpdate = new Map<Id, Opportunity>();
    
    	String oppServiceFieldName = null;
    
    	string serviceFieldValue = null;
    	
    	Boolean alreadyExists;
    	
    	Boolean isFull = true;
    	
    	String str;
    	
    	for (Id i : mapOpp.keyset()) {
    
    		if (mapOpp.get(i).Type.contains('Syndicated')) {
    		
    			Opportunity tmpOpp = mapOpp.get(i);
    		
    			for (String s : serviceCodeNames) {
    				oppServiceFieldName = scMap.get(s);
    				tmpOpp.put(oppServiceFieldName, null);
    				oppsToUpdate.put(tmpOpp.Id, tmpOpp);
    			}
    		
    			for (OpportunityLineItem oli : tmpOpp.OpportunityLineItems) {
    			
    				if (oliMap.containsKey(oli.Service_Code__c)) {
    					alreadyExists = true;
    				}
    				else {
    					alreadyExists = false;
    				}
    			
    				oliMap.put(oli.Service_Code__c, oli);

    				if (alreadyExists) {
    				
    					if (isFull != false) {
        
        					oppServiceFieldName = scMap.get(oli.Service_Code__c);

        					if (oli.Is_Full_Service__c == 'True') {
            					serviceFieldValue = 'Full';
        					}
        					else {
            					serviceFieldValue = 'Partial';
        					}
        					tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    					}
    				}
    				else {
    				
    					str = oliMap.get(oli.Service_Code__c).Is_Full_Service__c;
    				
    				
    					if (str == 'Full') {
    						isFull = true;
    					}
    					else {
    						isFull = false;
    					}

    					oppServiceFieldName = scMap.get(oli.Service_Code__c);

        					if (oli.Is_Full_Service__c == 'True') {
            					serviceFieldValue = 'Full';
        					}
        					else {
            					serviceFieldValue = 'Partial';
        					}
        					tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    					}
    				oppsToUpdate.put(tmpOpp.Id, tmpOpp);
    			}
    		}
    	}
    	update oppsToUpdate.values();
    }
}

After running several tests using dataloader with a batch size of 200, it looks to be processing correctly.  I did notice one strange annomaly which occurs with deletes.  If I have multiple opportunity products that have the same service code, but different values, the delete works on the first delete, but doesn't process the second delete correctly.

 

For example, say I have the following 4 opportunity product records for the same opportunity:

 

--------------Name---------------------                           -Service code-   -Is Full Service-
Pharmacy Management Trends and Profiles   -      PBM             -      True
Pharmacy Management Trends and Profiles   -      PBM             -      False
Pharmacy Management Trends and Profiles   -      PBM             -      False
Pharmacy Management Trends and Profiles   -      PBM             -      False

 

The opportunity field service name "PBM" will contain the value of "Full" because there is one record where the service code is True.

 

If I delete one of the opportunity products where "Is Full Service" is equal to false, PBM still remains Full on the opportunity, which is correct.  Now, if I delete another of the opportunity products where "Is Full Service" is equal to false, PBM updates to partial, which is incorrect.  There is still a an opportunity product where "Is Full Service" is equal to true and PBM should remain "Full" on the opportunity.  If I edit the opportunity product, it will fix itself, but the delete doesn't seem to be working quite right.  It only occurs if I delete more than one opportunity product that has the same service code.

 

Any idea what might be causing that?

 

Thanks for all your help!
Jeremy.NottinghJeremy.Nottingh

It looks like it's something small to me. If the service code already exists, you should only replace it if you're changing from "Partial" to "Full". You shouldn't ever update "Full" to "Partial". Here's the code section:

 

 

if (alreadyExists) {
    				
   if (isFull != false) {
        
       oppServiceFieldName = scMap.get(oli.Service_Code__c);

        if (oli.Is_Full_Service__c == 'True') {
            serviceFieldValue = 'Full';
        }
        else {
//** this will replace "Full" in some cases. 
		serviceFieldValue = 'Partial'; 
        }
        tmpOpp.put(oppServiceFieldName, serviceFieldValue);
    }
 }

If you remove the line I noted above, it will only put "Partial" in there if alreadyExists is false, which I believe is the behavior you're looking for.

 

By the way, I don't understand what IsFull is meant to indicate. You're setting it at the end of the oli loop, which means it's going to apply to the next oli, and will not necessarily be accurate. Maybe you don't really need it?

 

Jeremy

 

 

 

This was selected as the best answer
bohemianguy100bohemianguy100

Thank you Jeremy...that was it.  After removing that line and changing if(isFull != false) to if(isFull == false) it corrected the problem.  It is working as expected now.

 

Thanks again for the help!