How to insert more than one record from visualforce page to custom object?

Question

I want to insert the records in a custom object. The code is working fine when I insert one record at a time but when I am trying to insert more than one record on a same run one at a time than it is giving exception INVALID_FIELD_FOR_INSERT_UPDATE, cannot specify Id in an insert call can anyone help?

<apex:page controller="myclass" docType="html-5.0">
<apex:form >


Product Name: <apex:input type="text" id="name" value="{!value}"/><br/>
Product Category: <apex:input type="text" id="cat" value="{!value2}"/><br/>
Product Status: <apex:input type="text" id="status" value="{!value3}"/><br/>

<apex:commandButton action="{!insertRecords}" value="INSERT RECORDS" rerender="theForm"/>   
    </apex:form>

</apex:page>



public class myclass {

    opportunityProduct__c op= new opportunityProduct__c();

    public string value { get; set;}
    public string value2 { get; set;}
    public string value3 { get; set;}
//String [] arrayOfProducts = new String[] {'Shavings','Bins/ Containers','others','fee'};


    public void insertRecords(){
       // value = apexpages.currentPage().getParameters().get('inpval1');
        //value2 = apexpages.currentPage().getParameters().get('inpval2');
        //value3 = apexpages.currentPage().getParameters().get('inpval3');
        system.debug(value + value2+ value3);
        op.Prod_name__c= value;
        op.Prod_type__c=value2;
         insert op;
        value='';
        value2='';
      system.debug('record inserted' + op); 
  //  for (Integer i=0; i<arrayOfProducts.size(); i++) {   
   //if(arrayOfProducts[i].equalsIgnoreCase(value2) && ((!value3.equalsIgnoreCase('Closed won') && (!value3.equalsIgnoreCase('Closed lost'))) ){
     // insert op;
      //system.debug('record inserted' + op); 
        //}                
        //else{
        //system.debug('Category not predefined. Record with a new type inserted. ' + op); 
        //arrayOfProducts[arrayOfProducts.size()]=value2;
        //insert op;
          // }
            //}    
      if ( value3.equalsIgnoreCase('Closed won') || value3.equalsIgnoreCase('Closed lost'))
      system.debug('Opportunity is in stage Closed Won or Closed Lost.');

    }

}

Answer 1

The problem with below code is:-

for (Integer i=0; i<arrayOfProducts.size(); i++) {   
    if(arrayOfProducts[i].equalsIgnoreCase(value2) && ((!value3.equalsIgnoreCase('Closed won') && (!value3.equalsIgnoreCase('Closed lost'))) ){
       insert op;
    }                
    else{
       arrayOfProducts[arrayOfProducts.size()]=value2;
       insert op;
    }
}

The above code will fail because:-

1. You have defined opportunityProduct__c instance at the beginning of your code as:-opportunityProduct__c op= new opportunityProduct__c(); You are running the loop and trying to insert the same instance every time. when loop run for the first row in array, op gets inserted and it will get assigned with a record id. when loop runs for second row, you are again inserting op which have an ID.

Note:- A record with an ID cannot be re-inserted and hence you are getting exception INVALID_FIELD_FOR_INSERT_UPDATE, cannot specify Id in an insert call

2. Do not write a dml statement inside for loop. You may hit the apex governor limit. You can take advantage of List to avoid it.

Your code should look something like this:-

List<opportunityProduct__c> oplist =new List<opportunityProduct__c>();
        for (Integer i=0; i<arrayOfProducts.size(); i++) {   
            if(arrayOfProducts[i].equalsIgnoreCase(value2) && ((!value3.equalsIgnoreCase('Closed won') && (!value3.equalsIgnoreCase('Closed lost'))) ){
                oplist.add(new opportunityProduct__c(Prod_name__c=value));
            }
        }  
        insert oplist;

Answer 2

When you do insert op; the system populates the Id of the new OpportunityProduct in that sObject variable.

If you'd like to reuse that variable as a container for another record to insert, you need to reinitialize it to remove that Id value and database identity:

opportunityProduct__c op= new opportunityProduct__c();

However, it's much better practice instead to accumulate a List<OpportunityProduct> and insert them all in a single operation, saving DML limits usage:

List<OpportunityProduct> opsToInsert = new List<OpportunityProduct>();

if (someLogic) {
    opsToInsert.add(
        new OpportunityProduct(
            Product2Id = ...,
            ...
        )
    );
}
// Potentially other logic to add more records to `opsToInsert`.
// ...
// Then, lastly,
insert opsToInsert;