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
SFDCDevQASFDCDevQA 

Need help with system.limitexception too many soql queries

I'm running into a system limit issue with a trigger that calls a class if I activate it via the dataloader.  I'm not sure if it's the trigger or the class causing the exception.  Hopefully it is the trigger since it's so much smaller and should be easier to fix.  Can someone please take a quick look and let me know what is causing this to hit limits and how I can fix it?  It works fine on an individual record edit basis and I even tested it with a small upload and it worked fine but when I went to do the actual upload it froze up with all these errors.

 

Thanks,

Amanda

 

Trigger

trigger CloneOpportunityRollover on Opportunity (after Insert, after Update) {
for(Opportunity opp : Trigger.New){

ApexPages.StandardController controller = new ApexPages.standardController(opp );

OpportunityCloneWithItemsController clone = new OpportunityCloneWithItemsController(controller);

// load the current record
opp = (Opportunity)controller.getRecord();

if (opp.Hidden_New_Class_Start_Date__c != null) {
clone.clonewithitems();

}}}

Best Answer chosen by Admin (Salesforce Developers) 
JHayes SDJHayes SD

There are a number of SOQL and DML statements that get invoked when cloneWithItems() is called.  That method will need to be refactored to support running via a trigger.  A couple of suggestions:

 

1. You can eliminate the need for SOQL queries inside your class by creating a new method with arguments corresponding to a List of each of the child elements you are querying.  The logic should be similar to what is in cloneWithItems, however the DML should operate on the Lists that are the arguments.

 

Ex: cloneWithItems(Opportunity o, List<OpportunityLineItem> oli, List<OpportunityContactRole> ocr)

 

2. In your trigger, initialize the Lists before iterating over the records in Trigger.new.  Use a bind variable to pass in the IDs to the SOQL query.

 

Ex: oli = [select Id, unitprice, Quantity, PricebookEntryID, Description From OpportunityLineItem where OpportunityId in :Trigger.newMap().keySet()]

 

3. After the Lists are initialized in your trigger, pass them into your new method.

 

Ex:  cloneWithItems(o, oli, ocr)

 

Hope this helps!!

 

Jeremy

All Answers

JHayes SDJHayes SD

Since there aren't any SOQL queries in your trigger, the error is coming from the OpportunityCloneWithItemsController constructor (or some function called by it).  Is the constructor setting the value for Hidden_New_Class_Start_Date__c?  If not, do you have to cast the Opportunity?

SFDCDevQASFDCDevQA

I was really hoping it wouldn't be the class.  Here's the class code.  The class does reset the hidden class start date field back to null just after it inserts the new opportunity.  I think I have everything listed first and then set to query so I'm not sure what is going on...

 

Thanks so much,

Amanda

 

public class OpportunityCloneWithItemsController {
 
    //added an instance variable for the standard controller
    private ApexPages.StandardController controller {get; set;}
     // add the instance for the variables being passed by id on the url
    private Opportunity opp {get;set;}
    // set the id of the record that is created -- ONLY USED BY THE TEST CLASS
    public ID newRecordId {get;set;}
 
    // initialize the controller
    public OpportunityCloneWithItemsController(ApexPages.StandardController controller) {
 
        //initialize the stanrdard controller
        this.controller = controller;
        // load the current record
        opp = (Opportunity)controller.getRecord();
        
 
    }
 
    // method called from the VF's action attribute to clone the opp
    public PageReference cloneWithItems() {
 
         // setup the save point for rollback
         Savepoint sp = Database.setSavepoint();
         Opportunity newOpp;
 
         try {
 
              //copy the opportunity - ONLY INCLUDE THE FIELDS YOU WANT TO CLONE
             opp = [select Id, Name, Type, AccountID, Estimated_of_Students__c, StageName, Hidden_New_Class_Start_Date__c,Competitor__c, Competitors_in_Consideration__c from Opportunity where id = :opp.id];
             newOpp = opp.clone(false);
             if (opp.Hidden_New_Class_Start_Date__c != NULL) {
                 integer Month=opp.Hidden_New_Class_Start_Date__c.Month();
 
             if ( Month==11 || Month==12 || Month==1 || Month==2 || Month==3 || Month==4) {
                 newOpp.Academic_Term__c = 'Spring';
                 } else if ( Month==5 || Month==6 || Month==7)  {
                 newOpp.Academic_Term__c = 'Summer';
                 } else if ( Month==8 || Month==9 || Month==10)  {
                 newOpp.Academic_Term__c = 'Fall';
                 } else {
                 newOpp.Academic_Term__c = NULL;
             }}
             if (opp.Hidden_New_Class_Start_Date__c == NULL) {
                 newOpp.CloseDate = date.Today();
                 } else {
                 newOpp.CloseDate = opp.Hidden_New_Class_Start_Date__c;
             }    
             if (opp.StageName == 'Students Registering') {
                 newOpp.StageName = 'Prior Sapling User';
                 } else {
                 newOpp.StageName = 'Confirmed Teaching/Class Schedule';
            }
            if (opp.StageName == 'Students Registering') {
                         newOpp.Type = 'Renewal';
                         } else {
                         newOpp.Type = NULL;
            }
            newOpp.Hidden_New_Class_Start_Date__c = NULL;
             insert newOpp;
            opp.Hidden_New_Class_Start_Date__c = NULL;
             update Opp;
             
            
            
             // set the id of the new Opp created for testing
               newRecordId = newOpp.id;
 
             // copy over the line items - ONLY INCLUDE THE FIELDS YOU WANT TO CLONE
             List<OpportunityLineItem> items = new List<OpportunityLineItem>();
             for (OpportunityLineItem oli : [Select o.Id, o.unitprice, o.Quantity, o.PricebookEntryID, o.Description From OpportunityLineItem o where OpportunityID = :opp.id]) {
                  OpportunityLineItem newoli = oli.clone(false);
                  newoli.OpportunityID = newOpp.id;
                  items.add(newoli);
             }
             insert items;
             
              // copy over the Contact Roles - ONLY INCLUDE THE FIELDS YOU WANT TO CLONE
             List<OpportunityContactRole> Roles = new List<OpportunityContactRole>();
             for (OpportunityContactRole ocr : [Select c.Id, c.ContactID, c.IsPrimary, c.Role From OpportunityContactRole c where OpportunityID = :opp.id]) {
                  OpportunityContactRole newocr = ocr.clone(false);
                  newocr.OpportunityID = newOpp.id;
                  roles.add(newocr);
             }
             insert Roles;

         } catch (Exception e){
             // roll everything back in case of error
            Database.rollback(sp);
            ApexPages.addMessages(e);
            return null;
         }
 
        return new PageReference('/'+newOpp.id+'/e?retURL=%2F'+newOpp.id);
        
    }

}

JHayes SDJHayes SD

There are a number of SOQL and DML statements that get invoked when cloneWithItems() is called.  That method will need to be refactored to support running via a trigger.  A couple of suggestions:

 

1. You can eliminate the need for SOQL queries inside your class by creating a new method with arguments corresponding to a List of each of the child elements you are querying.  The logic should be similar to what is in cloneWithItems, however the DML should operate on the Lists that are the arguments.

 

Ex: cloneWithItems(Opportunity o, List<OpportunityLineItem> oli, List<OpportunityContactRole> ocr)

 

2. In your trigger, initialize the Lists before iterating over the records in Trigger.new.  Use a bind variable to pass in the IDs to the SOQL query.

 

Ex: oli = [select Id, unitprice, Quantity, PricebookEntryID, Description From OpportunityLineItem where OpportunityId in :Trigger.newMap().keySet()]

 

3. After the Lists are initialized in your trigger, pass them into your new method.

 

Ex:  cloneWithItems(o, oli, ocr)

 

Hope this helps!!

 

Jeremy

This was selected as the best answer
SFDCDevQASFDCDevQA

I started with your suggestions on the trigger since the trigger seems less daunting.  I'm now getting a Constructor not defined: [ApexPages.StandardController].<Constructor>(LIST<Opportunity>) at line 18 column 43 error so I'm sure I'm doing something wrong but I'm not sure what.  I was trying to follow the code best practices here: http://wiki.developerforce.com/page/Apex_Code_Best_Practices based on what I thought you were saying.

 

trigger CloneOpportunityRollover on Opportunity (after Insert, after Update) {

List <Opportunity> opp = [select Id, Name, Type, AccountID, Estimated_of_Students__c, StageName, Hidden_New_Class_Start_Date__c,Competitor__c, Competitors_in_Consideration__c, (select Id, unitprice, Quantity, PricebookEntryID, Description From OpportunityLineItems), (Select Id, ContactID, IsPrimary, Role From OpportunityContactRoles) from Opportunity where ID IN :Trigger.newMap.keySet()];
List<OpportunityLineItem> opportunitylineitems = new List<OpportunityLineItem>{};
List<OpportunityContactRole> opportunityContactRoles = new List<OpportunityContactRole>{};

for(Opportunity o : opp){
     // Use the child relationships dot syntax to access the related Contacts
     for(OpportunityLineItem oli: o.OpportunityLineItems){
      opportunitylineitems.add(oli);
     }
     for(Opportunitycontactrole ocr: o.OpportunityContactRoles){
      opportunitycontactroles.add(ocr);
     }
   }


ApexPages.StandardController controller = new ApexPages.standardController(opp );


OpportunityCloneWithItemsController clone = new OpportunityCloneWithItemsController(controller);

// load the current record
opp = (Opportunity)controller.getRecord();

if (opp.Hidden_New_Class_Start_Date__c != null) {
clone.clonewithitems(opp,opportunitylineitems,opportunitycontactroles);

}}

SFDCDevQASFDCDevQA

I tried the following along with many other changes but I still keep getting errors....now I am getting Compile Error: Method does not exist or incorrect signature: controller.getRecord() at line 29 column 20

 

trigger CloneOpportunityRollover on Opportunity (after Insert, after Update) {

List <Opportunity> opp = [select Id, Name, Type, AccountID, Estimated_of_Students__c, StageName, Hidden_New_Class_Start_Date__c,Competitor__c, Competitors_in_Consideration__c, (select Id, unitprice, Quantity, PricebookEntryID, Description From OpportunityLineItems), (Select Id, ContactID, IsPrimary, Role From OpportunityContactRoles) from Opportunity where ID IN :Trigger.newMap.keySet()];
List<OpportunityLineItem> opportunitylineitems = new List<OpportunityLineItem>{};
List<OpportunityContactRole> opportunityContactRoles = new List<OpportunityContactRole>{};


      
for(Opportunity o : opp){

     if (o.Hidden_New_Class_Start_Date__c != null){
     // Use the child relationships dot syntax to access the related Contacts
     for(OpportunityLineItem oli: o.OpportunityLineItems){
      opportunitylineitems.add(oli);
     }
     for(Opportunitycontactrole ocr: o.OpportunityContactRoles){
      opportunitycontactroles.add(ocr);
     }
     ApexPages.StandardController controller = new ApexPages.standardController(o);
OpportunityCloneWithItemsController clone = new OpportunityCloneWithItemsController(controller);
   }
}





// load the current record
opp = (Opportunity)controller.getRecord();

if (opp.Hidden_New_Class_Start_Date__c != null) {
clone.clonewithitems(opp,opportunitylineitems,opportunitycontactroles);

}}

SFDCDevQASFDCDevQA

Looks back through and tried this now....I'm too new to this to tell if it's really bulkified or not.  It's still giving an error of Compile Error: Method does not exist or incorrect signature: clone.clonewithitems(LIST<Opportunity>, LIST<OpportunityLineItem>, LIST<OpportunityContactRole>) at line 26 column 1 but is that maybe because I haven't changed the class method that it is trying to call yet?

 

trigger CloneOpportunityRollover on Opportunity (after Insert, after Update) {

List <Opportunity> opp = [select Id, Name, Type, AccountID, Estimated_of_Students__c, StageName, Hidden_New_Class_Start_Date__c,Competitor__c, Competitors_in_Consideration__c, (select Id, unitprice, Quantity, PricebookEntryID, Description From OpportunityLineItems), (Select Id, ContactID, IsPrimary, Role From OpportunityContactRoles) from Opportunity where ID IN :Trigger.newMap.keySet()];
List<OpportunityLineItem> opportunitylineitems = new List<OpportunityLineItem>{};
List<OpportunityContactRole> opportunityContactRoles = new List<OpportunityContactRole>{};


      
for(Opportunity o : opp){

     if (o.Hidden_New_Class_Start_Date__c != null){
     // Use the child relationships dot syntax to access the related Contacts
     for(OpportunityLineItem oli: o.OpportunityLineItems){
      opportunitylineitems.add(oli);
     }
     for(Opportunitycontactrole ocr: o.OpportunityContactRoles){
      opportunitycontactroles.add(ocr);
     }
     ApexPages.StandardController controller = new ApexPages.standardController(o);
OpportunityCloneWithItemsController clone = new OpportunityCloneWithItemsController(controller);
   }
}

clone.clonewithitems(opp, opportunitylineitems, opportunitycontactroles);

}

SFDCDevQASFDCDevQA

Ended up combining it all into one trigger, rather than a trigger plus class and doing as you said - pulling everything out into lists first and then pushing the info into the lists and then calling it into the method.  Took awhile but I got it!

 

Thanks,

Amanda

JHayes SDJHayes SD

Awesome, great to see you got it solved!  You'll be doing this sort of thing alot :)