1

I am create a custom object record on Opportunities called Recipe Sheets (based on the Opportunity Line Items) and this trigger is working properly. Then I have a second trigger firing on the insert of these Recipe Sheets to create Recipe Sheet Items based on the Product Items associated to the Products in the Opportunity Line Items.

If I deactivate Trigger 2, Trigger 1 works, but when I activate Trigger 2 I get this error:

"Error:Apex trigger NewRecipeSheets caused an unexpected exception, contact your administrator: NewRecipeSheets: execution of AfterUpdate caused by: System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, CreateRecipeItems: execution of AfterInsert caused by: System.DmlException: Insert failed. First exception on row 0 with id a07Z000000HdDsxIAF; first error: INVALID_FIELD_FOR_INSERT_UPDATE, cannot specify Id in an insert call: [Id] Trigger.CreateRecipeItems: line 22, column 1: []: Trigger.NewRecipeSheets: line 24, column 1"

Does anyone have any suggestions for what I am doing wrong in Trigger 2?

Trigger 1

trigger NewRecipeSheets on Opportunity (after update){

    List<Recipe_Sheet__c> RecipeList =new List<Recipe_Sheet__c>();
    List<Opportunitylineitem> opplinitemlist =new List<Opportunitylineitem>();
    List<Opportunity> Opportunitylist =new List<Opportunity>();

    for(Opportunity opp :trigger.new){
        if(opp.Stagename=='Closed Won'&&opp.Create_Recipe_Sheet_s__c==True){       
        Opplinitemlist =[select Id,PricebookEntry.Product2.Name,PricebookEntry.Product2.Do_Not_Discount__c,PricebookEntry.Product2.Record_Type_ID__c,Total_Drop__c,Finish_s__c,Notes__c, For_Vaulted_Ceiling__c,PricebookEntry.Product2.id from Opportunitylineitem where Opportunityid =: opp.id AND Product2.Do_Not_Discount__c=False];
        Opportunitylist  =[Select Accountid from Opportunity where Id =: opp.id];

        for(Opportunitylineitem Opplist :opplinitemlist ){
        Recipe_Sheet__c rec =new Recipe_Sheet__c();
        rec.Opportunity__c =Opportunitylist[0].Id;
        rec.product__c=opplist.PricebookEntry.Product2.Id;
        rec.Name=opplist.PricebookEntry.Product2.Name;
        rec.For_Vaulted_Ceiling__c=opplist.For_Vaulted_Ceiling__c;
        rec.Finish_s__c=opplist.Finish_s__c;
        rec.Notes__c=opplist.Notes__c;
        rec.Total_Drop__c=opplist.Total_Drop__c;
        rec.RecordTypeID=opplist.PricebookEntry.Product2.Record_Type_ID__c;
        RecipeList.add(rec);
        }
        insert RecipeList;
        } 
        }       
}

Trigger 2

trigger CreateRecipeItems on Recipe_Sheet__c (after insert) {

    List<Recipe_Sheet_Item__c> RecipeItemList =new List<Recipe_Sheet_Item__c>();
    List<Recipe_Sheet__c> RecipeList =new List<Recipe_Sheet__c>();
    List<Product_Item__c> ProductItemlist =new List<Product_Item__c>();
    List<Product2> ProductList =new List<Product2>();

    for(Recipe_Sheet__c rec :trigger.new){
        if(rec.Name !=''){       
        ProductList = [select Id,Name,Do_Not_Discount__c,Record_Type_Id__c from Product2 where Id =: rec.Product__c];
        ProductItemList =[select Id,Product__c,Quantity__c,Group__c,Item__c,Length_in_inches__c,External_Wall_Mount_Vaulted_Ceiling__c from Product_Item__c where Product__c =: rec.Product__c];
        Recipelist  =[Select Id,Opportunity__c,Product__c from Recipe_Sheet__c where Id =: rec.id];

        for(Product_Item__c pilist :ProductItemlist ){
        Recipe_Sheet_Item__c ritem = new Recipe_Sheet_Item__c();
        ritem.Recipe_Sheet__c = Recipelist[0].Id;
        ritem.Group__c = pilist.Group__c;
        ritem.Item__c = pilist.Item__c;
        ritem.Quantity__c = pilist.Quantity__c;
        RecipeItemList.add(ritem);
        }
        if (!RecipeItemList.isEmpty()) insert RecipeItemList ;
        } 
        }       
}
1

The problem is with the scope of your variables. RecipeItemList is declared outside the for(Recipe_Sheet__c rec :trigger.new) loop so the items added to the list the first time round the loop and then inserted are still in the list the second time round the loop and so are inserted again which is an error because they already have an ID assigned.

Narrowing the scope of RecipeItemList so that it is declared and initialised within the loop will solve the immediate problem because each time round the loop a new empty list will be created as the starting point.

However, your code in both triggers does contain the anti-patterns of querying and doing DML inside a loop and so really also needs bulkifying.

  • Thanks @Keith C moving the RecipeItemList worked. In regards to your comment about "both triggers does contain the anti-patterns of querying and doing DML inside a loop and so really also needs bulkifying" I am not quite advanced enough of a developer to understand how to resolve this. – Grace Jun 12 '14 at 17:49
  • @Grace I wish there was a really good tutorial on the subject of bulkifying triggers because it makes trigger writing quite a bit harder. It amounts to making sure that there are no queries or inserts/updates inside the trigger's loop over the objects by making use of sets and maps. Then the trigger will not hit governor limits whether 1 or 1000 objects are inserted or updated. – Keith C Jun 12 '14 at 17:55
  • If the use case is to only have these triggers fire for one off opportunities ( triggered by a user) rather than a mass data job, is it critical to bulkify these? – Grace Jun 12 '14 at 18:01
  • Even if only one Opportunity is updated if there are many Opportunitylineitem your second trigger could hit a governor limit. By assuming no-one will ever bulk update you are storing up trouble for your org in the future: who is to say that a year from now the owner of 500 Opportunity objects won't need changing and most people would do a bulk update for that. (Your current triggers look like they will keep adding Recipe_Sheet__c/Recipe_Sheet_Item__c objects for any change to Opportunity - is that really what you want?) In my experience unbulkified code always causes trouble down the line. – Keith C Jun 12 '14 at 18:24
  • I will see if I can bulkify these, but the only time this trigger will fire is when the Opportunity is set to Closed Won and the Create Recipe Sheet check box is set to true (I have a Workflow deselecting it immediately after). The user should never check this box more than once per opportunity and this check box should never be checked via a data job. – Grace Jun 12 '14 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.