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
TheReportDoctorTheReportDoctor 

Using after update on Opportunity to add an OpportunityLineItem

I get the error:  maximum trigger depth exceeded .  I believe the system sees the OpportunityLineItem update as an update to the Opportunity object and creates an endless loop. This is the code from my trigger:

trigger OpportunityTest on Opportunity (after update) {
    for(Opportunity o : trigger.new) {
       List<OpportunityLineItem> newOrders = new List<OpportunityLineItem>();
       List<PricebookEntry> product = [SELECT ID, Name, UnitPrice FROM PricebookEntry WHERE ProductCode = 'GC1020' LIMIT 1];
       for (PricebookEntry pb: product) {
           //o.AddError('Product ' + pb.Name);
           newOrders.add ( new OpportunityLineItem (OpportunityID = o.ID, PricebookEntryID = pb.ID, Description = pb.Name, Quantity = 2, UnitPrice = pb.UnitPrice));
           insert newOrders;
       }
    }
}
Best Answer chosen by Admin (Salesforce Developers) 
TheReportDoctorTheReportDoctor

I understand that if the record existed than you would want to check, but on the first run without any records in the object why does it error?

 

TRD

 

================

 

trigger OpportunityTest on Opportunity (after update) {
    List<OpportunityLineItem> newOrders = new List<OpportunityLineItem>();
    String opID;
    String pbID;
    for(Opportunity o : trigger.new) {
       List<PricebookEntry> product = [SELECT ID, Name, UnitPrice FROM PricebookEntry WHERE ProductCode = 'GC1020' LIMIT 1];
       for (PricebookEntry pb: product) {
           opID = o.ID;
           pbID = pb.ID;
           List<OpportunityLineItem> existingOrders = [SELECT ID FROM OpportunityLineItem WHERE OpportunityID = :opID AND PricebookEntryID = :pbID LIMIT 1];
           if (existingOrders.isEmpty()) {
               newOrders.add ( new OpportunityLineItem (OpportunityID = o.ID, PricebookEntryID = pb.ID, Description = pb.Name, Quantity = 2, UnitPrice = pb.UnitPrice));
           }
       }
        try {
            insert newOrders;
        }
        catch (DmlException de) {
            for (Integer I = 0; I < de.getNumDml(); i++) {
                o.addError(de.getDmlMessage(i));
            }
        }
    }
}

All Answers

AvromAvrom

The classic way of doing this is to put in some sort of check before the action that triggers the recursive call. What about only adding an OpportunityLineItem to newOrders if one doesn't already exist for that Opportunity/pricebook combo?

TheReportDoctorTheReportDoctor
The OpportunityLineItem does not exists.  The Opportunity does exists.  Adding the OpportunityLineItem causes the Opportunity after update trigger to kick off.  The Opportunity after update trigger has the code that adds the OpportunityLineItem record... and there we go in an endless loop.
AvromAvrom

The line item still doesn't exist at the time it causes the after update trigger to be fired on its parent? That's a bit odd.

Message Edited by Avrom on 01-20-2010 01:46 PM
TheReportDoctorTheReportDoctor
That is my question, or is there something else going on?
AvromAvrom

And you've checked that a line item with the same opportunity/pricebook combo *doesn't* exist at any point when the new one is added? My suspicion is that you're ending up trying to create an infinite number of these line items.

 

TheReportDoctorTheReportDoctor

I have one opportunity with no products.  That's all.

 

Hugh

AvromAvrom

Can you post the code with the check in it?

 

TheReportDoctorTheReportDoctor
Not sure what you mean by the check in it?
AvromAvrom

It's not sufficient to check the number of OpportunityLineItems present after all your code fires--it's throwing an exception, so it'll all get rolled back. I mean something like this:

 

trigger OpportunityTest on Opportunity (after update) { for(Opportunity o : trigger.new) { List<OpportunityLineItem> newOrders = new List<OpportunityLineItem>(); List<PricebookEntry> product = [SELECT ID, Name, UnitPrice FROM PricebookEntry WHERE ProductCode = 'GC1020' LIMIT 1]; for (PricebookEntry pb: product) { //o.AddError('Product ' + pb.Name);

List<OpportunityLineItem> existingOrders = [SELECT ID FROM OpportunityLineItem WHERE OpportunityID = o.ID AND PricebookEntryID = pb.ID LIMIT 1];

if (existingOrders.isEmpty()) { newOrders.add ( new OpportunityLineItem (OpportunityID = o.ID, PricebookEntryID = pb.ID, Description = pb.Name, Quantity = 2, UnitPrice = pb.UnitPrice));

} insert newOrders; } } }

Best,

Avrom

 

Message Edited by Avrom on 01-20-2010 03:01 PM
Message Edited by Avrom on 01-20-2010 03:02 PM
TheReportDoctorTheReportDoctor
In the select statement you suggested for the existingOrders list you are using o.ID and pb.ID which are not interrupted in the execution of the statement.  I am looking up how to do a dynamic query.  I don't see this solution working but I am giving it a try.
TheReportDoctorTheReportDoctor

I understand that if the record existed than you would want to check, but on the first run without any records in the object why does it error?

 

TRD

 

================

 

trigger OpportunityTest on Opportunity (after update) {
    List<OpportunityLineItem> newOrders = new List<OpportunityLineItem>();
    String opID;
    String pbID;
    for(Opportunity o : trigger.new) {
       List<PricebookEntry> product = [SELECT ID, Name, UnitPrice FROM PricebookEntry WHERE ProductCode = 'GC1020' LIMIT 1];
       for (PricebookEntry pb: product) {
           opID = o.ID;
           pbID = pb.ID;
           List<OpportunityLineItem> existingOrders = [SELECT ID FROM OpportunityLineItem WHERE OpportunityID = :opID AND PricebookEntryID = :pbID LIMIT 1];
           if (existingOrders.isEmpty()) {
               newOrders.add ( new OpportunityLineItem (OpportunityID = o.ID, PricebookEntryID = pb.ID, Description = pb.Name, Quantity = 2, UnitPrice = pb.UnitPrice));
           }
       }
        try {
            insert newOrders;
        }
        catch (DmlException de) {
            for (Integer I = 0; I < de.getNumDml(); i++) {
                o.addError(de.getDmlMessage(i));
            }
        }
    }
}

This was selected as the best answer
AvromAvrom

Here's the flow that I think might be going on:

 

1. The Opportunity record is updated.

2. The after update trigger begins executing.

3. A new OpportunityLineItem gets added. (This is still in the middle of the trigger.)

4. This updates the Opportunity record.

5. The after update trigger gets recursively called.

6. A new OpportunityLineItem gets added (In the middle of the recursive call to the trigger). It shares the Opportunity and PricebookEntry of the old one, but is a new record with a new id.

7. 4-6 repeat, adding more and more instances of OpportunityLineItem, until the recursive nesting gets too deep.

8. An error is thrown.

9. The changes (including the many OpportunityLineItems are rolled back.

10. Your UI returns, displaying the error and no new OpportunityLineItems.

 

Best,

Avrom

 

Message Edited by Avrom on 01-21-2010 10:49 AM
TheReportDoctorTheReportDoctor
Thanks.  I see how it works now.  Is this true to all  child-master relations?
AvromAvrom

Depending on your org, you may actually be able to turn this off for Opportunities. Go to Setup>Critical Updates and look for the "New Opportunity Save Behavior" update. If it's there, and you have the option to deactivate it, that will stop child updates from updating the opportunity object.

 

If you can't, or decide not to, do that, don't try and use dynamic SOQL. Instead, look at the section, "Using Maps and Sets in Bulk Triggers" in the Force.com Apex Developer's guide.

 

Hope this helps,

Avrom