function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
bohemianguy100bohemianguy100 

Need help refactoring my trigger to make bulk safe

I have a trigger that loops thru all Opportunity Products and updates the related opportunity.  The trigger works fine if there isn't more than 3 or 4 Opportunity Products, but it bombs if there are more than 5 Opportunity Products.  I'm trying to make my trigger bulk safe.  Can someone provide some help on how to refactor my trigger to make it bulk safe?

 

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];
	
	for (OpportunityLineItem oli : olis) {
		
		Service_Code__c sc = [Select Opportunity_Service_Field_Name__c From Service_Code__c Where Name =: oli.Service_Code__c];

		String soql = 'Select ' + sc.Opportunity_Service_Field_Name__c + ' From Opportunity Where Id = \'' + oli.OpportunityId + '\'';

		List<Opportunity> opps = Database.query(soql);
		
		string serviceFieldValue;
		
		if (oli.Is_Full_Service__c == 'True') {
			serviceFieldValue = 'Full';
		}
		else {
			serviceFieldValue = 'Partial';
		}
		
		if (opps[0].get(sc.Opportunity_Service_Field_Name__c) != 'Full') {
			opps[0].put(sc.Opportunity_Service_Field_Name__c, serviceFieldValue);
		}
		update opps;
	}
}

 

Thanks.

dke01dke01

Read this http://sfdc.arrowpointe.com/2008/09/13/bulkifying-a-trigger-an-example/

and this http://wiki.developerforce.com/index.php/Best_Practice:_Bulkify_Your_Code

 

Two notes on your specific code:

 

Using 

Trigger.new[0]

Will give you problems if/when you try to insert more then on row.  You should just use Trigger.new

 

The line 

update opps;

Should should not be within a For loop.

Instead make

List<Opportunity> oppsToUpdate = new List<Opportunity>();
For ()
{
.... your code
oppsToUpdate.add(opps);

}
update(oppsToUpdate);

 

Doing it this way mean you only make 1 update stament instead of 1 for each olis.

Shanky77Shanky77

Hi,

  Iis pretty  simple. The best way would be the update to the database should be out of the for loop.

  Populate the list opps within the for loop and come out of it and finally update it to the DB. Let me know

  if issue still persists.

bohemianguy100bohemianguy100

Thanks for the responses.

 

I moved the update statement outside of the for loop.  The problem is that I'm looping thru OpportunityLineItems for the same opportunity and I'm getting a duplicate id in list error:

 

Duplicate id in list: 006S0000003fwWVIAY: Trigger.OpportunityUpdateFromOppProduct: line 32, column 2

 

Each iteration of the loop updates a different field in the same opportunity based on the field returned from the dynamic soql.

 

I think I need to use a map here, but I can't quite get the structure right.

 

Here is the modified code for the trigger with the update statement moved outside of the for loop.  Oh, I am going to change Trigger.new[0] to just Trigger.new as well.

 

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];
	
	List<Opportunity> oppsToUpdate = new List<Opportunity>();
	
	for (OpportunityLineItem oli : olis) {
		
		Service_Code__c sc = [Select Opportunity_Service_Field_Name__c From Service_Code__c Where Name =: oli.Service_Code__c];

		String soql = 'Select ' + sc.Opportunity_Service_Field_Name__c + ' From Opportunity Where Id = \'' + oli.OpportunityId + '\'';

		List<Opportunity> opps = Database.query(soql);
		
		string serviceFieldValue;
		
		if (oli.Is_Full_Service__c == 'True') {
			serviceFieldValue = 'Full';
		}
		else {
			serviceFieldValue = 'Partial';
		}
		
		if (opps[0].get(sc.Opportunity_Service_Field_Name__c) != 'Full') {
			opps[0].put(sc.Opportunity_Service_Field_Name__c, serviceFieldValue);
		}
		for (Opportunity o : opps) {
			oppsToUpdate.add(o);
		}
		//update opps;
	}
	update(oppsToUpdate);
}