You need to sign in to do that
Don't have an account?
Jodi Spitler
My very first Trigger: updating a child record from a value on a parent record. Could anyone help me optimize?
Hi All,
I'm very proud of my first trigger :), but I'm sure that it could be improved considerably. Could anyone assist? It works, but I will still need another trigger on the Opportunity Product (as it only works on update and not on insert). ...and I'm sure I missed out on some basics as this is my first shot.
What the code does: Updates a field (Duration) on the opportunity products based on a field on the opportunity line item (phase) and 3 fields on the opportunity (dur_nnCMRR_1, _2,or _3)
trigger UpdateDurations on Opportunity (after update)
//removed after insert as opps don’t have any product line when they are first created//
{for(Opportunity opp: Trigger.new)
{Opportunity oldOpp = Trigger.oldMap.get(Opp.Id);
if(oldOpp.Dur_2_nnCMRR__c != Opp.Dur_2_nnCMRR__c )
{ List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Evaluation'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: children)
{if(LI.Duration__c != Opp.Dur_2_nnCMRR__c)
{LI.Duration__c = Opp.Dur_2_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}
if(oldOpp.Dur_1_nnCMRR__c != Opp.Dur_1_nnCMRR__c )
{ List<OpportunityLineItem> Install = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Installation'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: Install)
{if(LI.Duration__c != Opp.Dur_1_nnCMRR__c)
{LI.Duration__c = Opp.Dur_1_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}
if(oldOpp.Dur_3_nnCMRR__c != Opp.Dur_3_nnCMRR__c )
{ List<OpportunityLineItem> Use = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Usage'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: Use)
{if(LI.Duration__c != Opp.Dur_3_nnCMRR__c)
{LI.Duration__c = Opp.Dur_3_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}}}
I'm very proud of my first trigger :), but I'm sure that it could be improved considerably. Could anyone assist? It works, but I will still need another trigger on the Opportunity Product (as it only works on update and not on insert). ...and I'm sure I missed out on some basics as this is my first shot.
What the code does: Updates a field (Duration) on the opportunity products based on a field on the opportunity line item (phase) and 3 fields on the opportunity (dur_nnCMRR_1, _2,or _3)
trigger UpdateDurations on Opportunity (after update)
//removed after insert as opps don’t have any product line when they are first created//
{for(Opportunity opp: Trigger.new)
{Opportunity oldOpp = Trigger.oldMap.get(Opp.Id);
if(oldOpp.Dur_2_nnCMRR__c != Opp.Dur_2_nnCMRR__c )
{ List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Evaluation'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: children)
{if(LI.Duration__c != Opp.Dur_2_nnCMRR__c)
{LI.Duration__c = Opp.Dur_2_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}
if(oldOpp.Dur_1_nnCMRR__c != Opp.Dur_1_nnCMRR__c )
{ List<OpportunityLineItem> Install = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Installation'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: Install)
{if(LI.Duration__c != Opp.Dur_1_nnCMRR__c)
{LI.Duration__c = Opp.Dur_1_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}
if(oldOpp.Dur_3_nnCMRR__c != Opp.Dur_3_nnCMRR__c )
{ List<OpportunityLineItem> Use = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Usage'];
List<OpportunityLineItem> newids = new List<OpportunityLineItem>();
for(OpportunityLineItem LI: Use)
{if(LI.Duration__c != Opp.Dur_3_nnCMRR__c)
{LI.Duration__c = Opp.Dur_3_nnCMRR__c;newids.add(LI);}}
if (newids.isEmpty()== false)
{update newids;}}}}
All Answers
{for(Opportunity opp: Trigger.new)
{Opportunity oldOpp = Trigger.oldMap.get(Opp.Id);
if(oldOpp.Dur_2_nnCMRR__c != Opp.Dur_2_nnCMRR__c )
with {for(Opportunity opp: Trigger.new){
if(opp.Dur_2_nnCMRR__c != Trigger.oldmap.get(opp.Id).Dur_2_nnCMRR__c )
thats one statement vs 2 statements in
2. Do not use SOQL inside for loops as you wilt hit governor limits in mass update
{ List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Opp.Id AND Phase__c='Evaluation'];
This should be outside for loop
List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Trigger.NewMAp.keyset() AND Phase__c='Evaluation'
and since you need three different lists you can use
List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Trigger.NewMAp.keyset() AND (Phase__c='Evaluation' OR Phase__c='Usage' OR Phase__c='Installation')
Now loop through and do check like
for(OpportunityLineItem child : children){
// do logic like check for phase etc a
}
trigger UpdateDurations on Opportunity (after update) {
List<OpportunityLineItem> childrentoupdate = new List<OpportunityLineItem>();
List<OpportunityLineItem> children = [ SELECT Id, OpportunityId, Duration__c,Phase__c from
OpportunityLineItem where OpportunityId = :Trigger.NewMap.Keyset() AND (Phase__c='Evaluation' OR Phase__c = 'Installation' OR Phase__c = 'Usage')];
for(OpportunityLineItem child : children){
if(child.Phase__C == 'Evaluation' && child.Duration__c != Trigger.NewMap.get(child.OpportunityId).Dur_2_nnCMRR__c ){
child.Duration__c = Trigger.NewMap.get(child.OpportunityId).Dur_2_nnCMRR__c;
childrentoupdate .add(child);
}
else if(child.Phase__C == 'Installation' && child.Duration__c != Trigger.NewMap.get(child.OpportunityId).Dur_2_nnCMRR__c ){
child.Duration__c = Trigger.NewMap.get(child.OpportunityId).Dur_2_nnCMRR__c;
childrentoupdate .add(child);
}
else if(child.Phase__C == 'Usage' && child.Duration__c != Trigger.NewMap.get(child.OpportunityId).Dur_3_nnCMRR__c){
child.Duration__c = Trigger.NewMap.get(child.OpportunityId).Dur_3_nnCMRR__c;
childrentoupdate .add(child);
}
}
if(childrentoupdate .size()>0){
update childrentoupdate ;
}}
Also a shout out to ebsta that started me on the right track :)
https://www.ebsta.com/blog/salesforce-apex-trigger/