I have a custom lightning component button with trigger on the Opportunity object, which clones the existing Opportunity and associated Quote record. At the moment, we cannot clone Opportunities where the PricebookEntry is Inactive. This means everytime I need to clone an Opportunity, I need to go into the Opportunity > Quote > PricebookEntry > and manually change IsActive to TRUE. I would like to change this so you can clone Opportunities even if the PricebookEntry is Inactive, however I'm unsure where to insert the code to change this as it seems it's a standard Salesforce configuration.

I would greatly appreciate some assistance in editing the code!

My apex trigger code snippet is per the below:

                //Clone Quote Item
                List<QuoteLineItem> items = new List<QuoteLineItem>();
                for (Quote objQuote : lstQuoteResult){
                    for (QuoteLineItem pi : [Select p.QuoteId, p.Product2Id, p.Subtotal, p.ListPrice, p.Quantity,
                                             From QuoteLineItem p where QuoteId = :objQuote.Old_Quote__c]) {
                                                 QuoteLineItem newPI = pi.clone(false);
                                                 newPI.QuoteId = objQuote.id;
                insert items;
            List<OpportunityLineItem> mOpplineList = new List<OpportunityLineItem>();
            List<OpportunityLineItem> mOppLineItemList = [SELECT Id, OpportunityId, SortOrder, PricebookEntryId, Product2Id, ProductCode, Name, CurrencyIsoCode, Quantity, UnitPrice, 
                                                          ListPrice, ServiceDate, Description
                                                          FROM OpportunityLineItem WHERE OpportunityId = :objOppty.id];
                for(integer x = 0; x < lstResult.size(); x++)
                    for (OpportunityLineItem mOppItem : mOppLineItemList)
                        OpportunityLineItem mNewOppItem = mOppItem.clone(false,false,false,false);
                        mNewOppItem.id = null;
                        mNewOppItem.OpportunityId = lstResult[x].Id;
                insert mOpplineList;
  • 1
    You'll get more help if you reduce the size of the posted code to only the parts that really matter like the innumerable fields in the select and clone assignments; removing commented out code. Note also that lstOppty will never be null and you can fetch the OpportunityLineItems in a subselect of the first query. Please read How to Ask.
    – cropredy
    Commented Apr 30 at 3:48

1 Answer 1


I have also built a lightning component to clone opportunities so I know what you're going through, there are three things that stand out to me right away:

  1. It feels like you're doing too much into one method. It's a lot easier to test when things are broken up into smaller chunks. It looks like from your snip you're doing queries logic and inserts all in the same method.
  2. It seems like you should only need to loop once, you shouldn't need to do a check if the there are olis to clone because as it loops if there is nothing to do it won't do anything.
  3. You need to actually check if the pbe is inactive then activate it before inserting the olis.

Something like this:

List<OpportunitylineItem> getOLIsToClone(Opportunity objOppty)
    return [SELECT Id, 
              FROM OpportunityLineItem 
              WHERE OpportunityId = :objOppty.id]

void makeClonedOLIs(List<OpportunityLineItem> originalOppyLines
  List<OpportunityLIneItem> olisToInsert = new List<OpportunityLineItem>();
  List<PricebookEntry> pbesToUpdate = new List<PriceBookEntry>():

  for(OpportunityLineItem oli : originalOppyLines)
      if(oli.PricebookEntry.Isactive = false)

      OpportunityLineItem mNewOppItem = oli.clone(false,false,false,false);
      mNewOppItem.Id = null;
      mNewOppItem.OpportunityId = lstResult[x].Id;
      olisToInsert .add(mNewOppItem);

   update pbesToUpdate;
   insert olisToInsert;

List<PriceBookEntry> makePBEActive(Id pbeId)
     List<PriceBookEntry> pbeList= new List<PriceBookEntry>();
     PriceBookEntry pbe = new PriceBookEntry();
        pbe.Id = pbeId;
        pbe.IsActive = true;
     return pbeList;

I know I said we should separate things out, but since I'm typing this out right in the browser I've opted to just put the dml into the logic. For a full production code I would have made a custom class to hold the two lists and then just return the class and do the dml outside the make method.

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .