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
brozinickrbrozinickr 

Bulkifying Helper Class and Trigger

Hello,

 

I was wondering if someone would be able to help me make this helper apex class and trigger more bulk friendly.  I highlighted the lines that seem failing with the bulk load:

 

public class GooalAndActuals {

 public Opportunity opportunityNew
 {
    get;set;
 }
 
 Actual__c l_oActual;
 AL_Goal__c l_oTarget;
 
 
 public String getGoalType(String aOpportunityType){
    if(aOpportunityType=='New Ad Sales') return 'NASR';
    else if(aOpportunityType=='Acct. Mgmt. Renewal') return 'Renewal';
    else if((aOpportunityType=='Acct. Mgmt. New Ad Only') || (aOpportunityType=='Acct. Mgmt. Late Renewal') || (aOpportunityType=='Acct. Mgmt. TER') || (aOpportunityType=='Acct. Mgmt. Supercede'))return 'Upsell';
    else if(aOpportunityType=='Big Deal') return 'Big Deal';
    else if(aOpportunityType=='Storefront') return 'Storefront';
    else if(aOpportunityType=='HR / FS') return 'HR / FS';
    else return '';
 }
 
 
 
 public ID CreateGoal(Id OwnerID)
 {
    System.debug('****Goal Method Call*****');
    ID GoalID;
    if(opportunityNew!=null)
    {       
        String GoalType = getGoalType(opportunityNew.Type);
        
        if((GoalType=='Renewal') || (GoalType=='Upsell'))
            return null;
        System.debug('Goal Type:'+GoalType);
            System.debug('Select id From Goal__c   Where  Year_Month__c=:'+opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month() + ' And   Goal_Owner__c=:'+OwnerID);
            l_oTarget=null;
            for(AL_Goal__c l_oLoopTarget : [Select id, Year_Month__c, Goal_Owner__c,Goal_Type__c From AL_Goal__c 
                                                       Where 
                                                       Year_Month__c=:opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month()
                                                       And 
                                                       Goal_Owner__c=:OwnerID]
                            )
                            {
                                l_oTarget=l_oLoopTarget;
                                break;
                            }
                        
                                            
            if(l_oTarget==null)
            {
                Date l_oNewTargetDate = Date.newInstance(opportunityNew.CloseDate.Year(), opportunityNew.CloseDate.Month(), 1);
                l_oTarget= new AL_Goal__c();
                l_oTarget.Goal_Amount__c=0;
                l_oTarget.Goal_Date__c=l_oNewTargetDate;
                l_oTarget.Goal_Owner__c=OwnerID;
                l_oTarget.Goal_Type__c = GoalType;
                insert l_oTarget;
            }
            GoalID=l_oTarget.Id;
                
    }   
    return GoalID;  
 }
 
public String getPaymentType()
 {
    String paymentType = null;
    if(opportunityNew!=null)
    {
        List<Recon_Detail__c> reconDetails = [Select id, Payment_Type__c From Recon_Detail__c Where Contract_Id__c=:opportunityNew.Contract_ID__c limit 1];
        if (reconDetails.size() > 0) {
            paymentType = reconDetails[0].Payment_Type__c;
        }
    }
    return paymentType;
 }
 
 public void CreateActual(Id OwnderID,ID GoalID)
 {
          
    if((opportunityNew!=null) && (GoalID!= null))
    {
            
            String getPaymentType = getPaymentType();
            
            l_oActual = new Actual__c();
            l_oActual.Opportunity__c=opportunityNew.Id;
            l_oActual.Opportunity_Amount__c=opportunityNew.Amount;
            l_oActual.AccountName__c=opportunityNew.AccountId;
            l_oActual.Goal__c=GoalID;
            l_oActual.Actual_Owner__c=OwnderID;
            l_oActual.Payment_Type__c=getPaymentType();
            insert l_oActual;
    }
 }
 
 public void DeleteActual(Id OpportunityID)
 {
    if(opportunityNew!=null)
    {
        List<Actual__c> l_olstActual = [Select id From Actual__c Where Opportunity__c=:opportunityNew.Id];
        delete l_olstActual;            
    }
 }
}

 

trigger CreateActualsAndTargets 
on 
Opportunity (after insert, after update) 
{
 if(Trigger.isUpdate || Trigger.isInsert)   
    {
      Map<ID,Opportunity> l_OpportunityOwners = new Map<ID,Opportunity> ([Select id, Opportunity_Owner_Manager__c, Contract_ID__c,StageName,Amount,AccountId,CloseDate, o.OwnerId, Type From Opportunity o where id in:trigger.newMap.keySet()]);
      GooalAndActuals l_oGooalAndActuals = new GooalAndActuals ();
        
      for(Opportunity l_oOpportunityNew:l_OpportunityOwners.values())
      {
        System.debug('********************Trigger Start***********************');
        if(l_oOpportunityNew.StageName=='Closed/Won')
        {
          ID SalesRepGoalID,ManagerGoalID;
          //modified
          l_oGooalAndActuals = new GooalAndActuals ();
          l_oGooalAndActuals.opportunityNew=l_oOpportunityNew;
          
          
          //Goal Creation
          SalesRepGoalID=l_oGooalAndActuals.CreateGoal(l_oOpportunityNew.OwnerId);//SalesRep Goal Creation
          if(l_oOpportunityNew.Opportunity_Owner_Manager__c!= null)
          {
            ManagerGoalID=l_oGooalAndActuals.CreateGoal(l_oOpportunityNew.Opportunity_Owner_Manager__c);//Manager Goal Creation
          }
          
          
          //Actual Deletion
          if(Trigger.isUpdate)
          {
            l_oGooalAndActuals.DeleteActual(l_oOpportunityNew.Id);
            
          }
          
          //Actual Creation
          l_oGooalAndActuals.CreateActual(l_oOpportunityNew.OwnerId,SalesRepGoalID);//SalesRep Actual Creation
          if(l_oOpportunityNew.Opportunity_Owner_Manager__c!= null)
          {
            l_oGooalAndActuals.CreateActual(l_oOpportunityNew.Opportunity_Owner_Manager__c,ManagerGoalID);//Manager Actual Creation
          }
          
          
          
        }
        
        /*
        Start:  Project
        *   Author          |Author-Email            |Date       |Comment
        *   ----------------|------------------------|-----------|--------------------------------------------------
        *   Sakonent Admin  |abrar.haq@sakonent.com  |02.16.2012 |Initial code 
        *   Purpose: It will remove Actual record(s) associated to Opportunity if Stage goes from 'Closed/Won' to some other stage.
        */        
        else if(Trigger.isUpdate)
        {
            if(Trigger.oldMap.get(l_oOpportunityNew.Id).StageName == 'Closed/Won' && l_oOpportunityNew.StageName != 'Closed/Won')
            {
                l_oGooalAndActuals = new GooalAndActuals ();
                l_oGooalAndActuals.opportunityNew=l_oOpportunityNew;
                l_oGooalAndActuals.DeleteActual(l_oOpportunityNew.Id);
            }
        }
        /*End: Project */        
        
      }
    }
   
}

 

 

 

EnthEnth

The issue is you're basically making SOQL calls from inside the loop in your trigger. You need to query ReconDetails once and hold the records in a Map, indexed by ContractId and then you can retrieve from the Map the appropriate record in getPaymentType.

 

You can either pre-query the ReconDetails and pass the map in as a parameter to getPaymentType or you can hold the map as a Static variable and access it.

brozinickrbrozinickr

Does this look better?  The only problem is that I am now getting this error: Initial term of field expression must be a concrete SObject: String at line 75 column 64 (line commented out)

 

public String getPaymentType()
 {
    String paymentType = null;
    Map<Id,String> rdmap = new Map<Id,String>();
    List<Recon_Detail__c> rds = [SELECT Id, Contract_ID__c, Opportunity__c, Payment_Type__c from Recon_Detail__c WHERE Contract_Id__c=:opportunityNew.Contract_ID__c];
    
    for(Recon_Detail__c rd : rds) {
        rdmap.put(rd.Contract_ID__c, rd.Payment_Type__c);
        }
        
        //paymentType = rdmap.get(opportunityNew.Contract_ID__c).Payment_Type__c;
        
        return paymentType;

    }

 

brozinickrbrozinickr

Never mind, I'm stupid.  I put Id, String in my map, it should have been String, String.  I tested it manually and works fine.

 

public String getPaymentType()
 {
    String paymentType = null;
    Map<String,String> rdmap = new Map<String,String>();
    List<Recon_Detail__c> rds = [SELECT Id, Contract_ID__c, Opportunity__c, Payment_Type__c from Recon_Detail__c WHERE Contract_Id__c=:opportunityNew.Contract_ID__c];
    
    for(Recon_Detail__c rd : rds) {
        rdmap.put(rd.Contract_ID__c, rd.Payment_Type__c);
        }
        
        paymentType = rdmap.get(opportunityNew.Contract_ID__c);
        
        return paymentType;

    }
brozinickrbrozinickr

Can anyone help me better bulkify this part of my method?  Whenever I bulk test it, it fails on the line with SOQL for loop.  Is there a better way I could write this so it's more bulk safe?

 

 

 public ID CreateGoal(Id OwnerID)
 {
    System.debug('****Goal Method Call*****');
    ID GoalID;
    if(opportunityNew!=null)
    {       
        String GoalType = getGoalType(opportunityNew.Type);
        
        if((GoalType=='Renewal') || (GoalType=='Upsell'))
            return null;
        System.debug('Goal Type:'+GoalType);
            System.debug('Select id From Goal__c   Where  Year_Month__c=:'+opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month() + ' And   Goal_Owner__c=:'+OwnerID);
            l_oTarget=null;
            
            for(AL_Goal__c l_oLoopTarget : [Select id, Year_Month__c, Goal_Owner__c,Goal_Type__c From AL_Goal__c 
                                                       Where 
                                                       Year_Month__c=:opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month()
                                                       And 
                                                       Goal_Owner__c=:OwnerID]
                            )
                            {
                                l_oTarget=l_oLoopTarget;
                                break;
                            }
                        
                                            
            if(l_oTarget==null)
            {
                Date l_oNewTargetDate = Date.newInstance(opportunityNew.CloseDate.Year(), opportunityNew.CloseDate.Month(), 1);
                l_oTarget= new AL_Goal__c();
                l_oTarget.Goal_Amount__c=0;
                l_oTarget.Goal_Date__c=l_oNewTargetDate;
                l_oTarget.Goal_Owner__c=OwnerID;
                l_oTarget.Goal_Type__c = GoalType;
                insert l_oTarget;
            }
            GoalID=l_oTarget.Id;
                
    }   
    return GoalID;  
 }

 

EnthEnth

You should never have an insert statement in a loop. Create a new List before your for loop, then replace your insert with a mylist.add(l_oTarget). After your for-loop put in the insert (for extra credit you can check the list size > 0 to avoid trying to insert an empty list.

            List<AL_Goal__c> recordsToInsert = new List<AL_Goal__c>();

            for(AL_Goal__c l_oLoopTarget : [Select id, Year_Month__c, Goal_Owner__c,Goal_Type__c From AL_Goal__c 
                                                       Where 
                                                       Year_Month__c=:opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month()
                                                       And 
                                                       Goal_Owner__c=:OwnerID]
                            )
                            {
                                l_oTarget=l_oLoopTarget;
                                break;
                            }
                        
                                            
            if(l_oTarget==null)
            {
                Date l_oNewTargetDate = Date.newInstance(opportunityNew.CloseDate.Year(), opportunityNew.CloseDate.Month(), 1);
                l_oTarget= new AL_Goal__c();
                l_oTarget.Goal_Amount__c=0;
                l_oTarget.Goal_Date__c=l_oNewTargetDate;
                l_oTarget.Goal_Owner__c=OwnerID;
                l_oTarget.Goal_Type__c = GoalType;
                recordsToInsert.add(l_oTarget);
                //insert l_oTarget;

            }
            GoalID=l_oTarget.Id;

. . .
} // End of For loop if (recordsToInsert.size() > 0) insert recordsToInsert;

 

 

brozinickrbrozinickr

Thanks for your help!  I put in the new logic but I'm still hitting the limits even with the insert outside of the loop.  I'm wondering if I need to figure out a new way to evaluate if there is a goal already added.  In our process, an opportunity is Closed/Won then two goals and two actuals could potentially be written (usually it will be the two actuals that will be written, one for the rep, one of the manager).  I'm thinking it may be smart to maybe separate out the rep and manager creation of goals and actuals so it doesn't trip off the limits.

 

public ID CreateGoal(Id OwnerID)
 {
    System.debug('****Goal Method Call*****');
    ID GoalID;
    if(opportunityNew!=null)
    {       
        String GoalType = getGoalType(opportunityNew.Type);
        
        if((GoalType=='Renewal') || (GoalType=='Upsell'))
            return null;

        System.debug('Goal Type:'+GoalType);
            System.debug('Select id From Goal__c   Where  Year_Month__c=:'+opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month() + ' And     Goal_Owner__c=:'+OwnerID);

            l_oTarget=null;

            List<AL_Goal__c> recordsToInsert = new List<AL_Goal__c>();

            for(AL_Goal__c l_oLoopTarget : [Select id, Year_Month__c, Goal_Owner__c,Goal_Type__c From AL_Goal__c 
                                                       Where 
                                                       Year_Month__c=:opportunityNew.CloseDate.Year()+'-'+opportunityNew.CloseDate.Month()
                                                       And 
                                                       Goal_Owner__c=:OwnerID]
                            )
                            {
                                l_oTarget=l_oLoopTarget;
                                break;
                            }
                        
                                            
            if(l_oTarget==null)
            {
                Date l_oNewTargetDate = Date.newInstance(opportunityNew.CloseDate.Year(), opportunityNew.CloseDate.Month(), 1);
                l_oTarget= new AL_Goal__c();
                l_oTarget.Goal_Amount__c=0;
                l_oTarget.Goal_Date__c=l_oNewTargetDate;
                l_oTarget.Goal_Owner__c=OwnerID;
                l_oTarget.Goal_Type__c = GoalType;
                recordsToInsert.add(l_oTarget);
                //insert l_oTarget;
            }
            GoalID=l_oTarget.Id;
    
            if (recordsToInsert.size() > 0){
            insert recordsToInsert;}
                
    }   
    return GoalID;  
 }