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
LiorGLiorG 

Need help bulkifying my trigger

I'm a newbie to apex and not quite sure how to bulkify my trigger. Help would be appreciated!

 

 

 

 

trigger opp_to_acct_update on Opportunity (before insert, before update) {
   

List<Opportunity> oppupdate= new List<Opportunity>();      

for(Opportunity oppRec: trigger.new){           

Id acctId = oppRec.AccountId;       

Account acctRec = [select Optimus_User_ID__c, Username__c , OwnerId from Account where Id = :acctId];                        

if (oppRec.Account_Owner__c == null) {           

oppRec.Account_Owner__c = acctRec.OwnerId;       

} else {           

acctRec.OwnerId = oppRec.Account_Owner__c;       

}               

 

if (acctRec.Optimus_User_ID__c == null || acctRec.Username__c == null) {           acctRec.Optimus_User_ID__c = oppRec.Optimus_User_ID__c;           

acctRec.Username__c = oppRec.Username__c;       

} else {           

oppRec.Optimus_User_ID__c = acctRec.Optimus_User_ID__c;           

oppRec.Username__c = acctRec.Username__c;       

}               

 

oppupdate.add(oppRec);               

 

update acctRec;       

}      

 }

 

Best Answer chosen by Admin (Salesforce Developers) 
Rahul SharmaRahul Sharma

you have to insert the records in test methods witth all the required fields like:

 

@istest   
private class TestAccountOppUpdateTrigger    {              
static testMethod void TestAccountOppUpdate()           
{
Account objAccount = new Account(Name = 'Test Account 1');                              
insert objAccount;
Opportunity Opp = new Opportunity(Name = 'Test Opportunity', AccountID = objAccount.Id);
insert Opp;               
                                  
}                                    
}

 Also give additional required fields needed for Account and Opportunity.

All Answers

Rahul SharmaRahul Sharma

Hi LiorG,

 

The problem in your code is due to query and dml statement (update) inside a for loop.

Query problem can be solved using maps, while for dml, collect the list inside for loop and update that list outside the loop.

I have optimized the code for you, check if it helps:

 

trigger opp_to_acct_update on Opportunity (before insert, before update)
{
  List<Opportunity> oppupdate= new List<Opportunity>();  
  Set<Id> SetAccId = new Set<Id>();
  for(Opportunity oppRec: trigger.new)
  SetAccId.add(oppRec.AccountId);
  // Use map to store the data
  Map<Id, Account> MapAccount = new Map<Id, Account>([select Optimus_User_ID__c, Username__c , OwnerId from Account where Id IN :SetAccId]);
  for(Opportunity oppRec: trigger.new)
  {                      
    if (oppRec.Account_Owner__c == null && MapAccount.get(oppRec.AccountId).OwnerId != null) 
    {           
      oppRec.Account_Owner__c = MapAccount.get(oppRec.AccountId).OwnerId;     
      // fetch data from Map, using map.get method
    }
    else 
    {           
      MapAccount.get(oppRec.AccountId).OwnerId = oppRec.Account_Owner__c;       
    }                
    
    if (MapAccount.get(oppRec.AccountId).Optimus_User_ID__c == null || MapAccount.get(oppRec.AccountId).Username__c == null) 
    {           
      MapAccount.get(oppRec.AccountId).Optimus_User_ID__c = oppRec.Optimus_User_ID__c;           
      MapAccount.get(oppRec.AccountId).Username__c = oppRec.Username__c;       
    } 
    else 
    {           
      oppRec.Optimus_User_ID__c = MapAccount.get(oppRec.AccountId).Optimus_User_ID__c;           
      oppRec.Username__c = MapAccount.get(oppRec.AccountId).Username__c;       
    }               
    oppupdate.add(oppRec);      
  }
  // Update statements outside for loop
  if(!MapAccount.values().isEmpty())
    update MapAccount.values();
  if(!oppupdate.isEmpty())
    update oppupdate;
}

 

LiorGLiorG

Thanks for the help, but I'm getting this message when I create an opportunity:

 

Apex trigger opp_to_acct_update caused an unexpected exception, contact your administrator: opp_to_acct_update: execution of BeforeInsert caused by: System.SObjectException: DML statment cannot operate on trigger.new or trigger.old: Trigger.opp_to_acct_update: line 37, column 5

 

Then, when I take this part out:

 

// Update statements outside for loop  if(!MapAccount.values().isEmpty())    update MapAccount.values();  if(!oppupdate.isEmpty())    update oppupdate;

 

I am able to insert an opp, but when I run the test, I get only 46% converage and this message:

 

System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, opp_to_acct_update: execution of BeforeInsert caused by: System.NullPointerException: Attempt to de-reference a null object Trigger.opp_to_acct_update: line 11, column 77: []

Rahul SharmaRahul Sharma

Its an before trigger so you wont need update statements, as it would be executed before saving.

Does the trigger working properly after removing the update statements?

LiorGLiorG

Yes, it works properly when I remove the update statements. However, I am still getting only 46% converage in my test.

 

 

My test is:

 

@istest   

private class TestAccountOppUpdateTrigger    {              

static testMethod void TestAccountOppUpdate()           

{                              

Opportunity Opp = new Opportunity();               

insert Opp;               
                                  

}                                    

}

 

 

And the errors are 

 

System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, opp_to_acct_update: execution of BeforeInsert caused by: System.NullPointerException: Attempt to de-reference a null object Trigger.opp_to_acct_update: line 11, column 77: []

 

 

 

My Debug log looks like:

 

Trigger.opp_to_acct_update: line 11, column 77: []?22.0 APEX_CODE,FINE;APEX_PROFILING,FINE;DB,INFO;VALIDATION,INFO;WORKFLOW,FINEST23:26:44.360 (360048000)|EXECUTION_STARTED23:26:44.360 (360139000)|CODE_UNIT_STARTED|[EXTERNAL]|01pS00000001wuY|TestAccountOppUpdateTrigger.TestAccountOppUpdate23:26:44.360 (360221000)|METHOD_ENTRY|[2]|01pS00000001wuY|TestAccountOppUpdateTrigger.TestAccountOppUpdateTrigger()23:26:44.360 (360276000)|METHOD_EXIT|[2]|TestAccountOppUpdateTrigger23:26:44.360 (360408000)|DML_BEGIN|[9]|Op:Insert|Type:Opportunity|Rows:123:26:44.379 (379053000)|CODE_UNIT_STARTED|[EXTERNAL]|01qS00000000a6j|opp_to_acct_update on Opportunity trigger event BeforeInsert for [new]23:26:44.379 (379561000)|SOQL_EXECUTE_BEGIN|[8]|Aggregations:0|select Optimus_User_ID__c, Username__c , OwnerId from Account where Id IN :SetAccId23:26:44.390 (390096000)|SOQL_EXECUTE_END|[8]|Rows:023:26:44.390 (390364000)|EXCEPTION_THROWN|[11]|System.NullPointerException: Attempt to de-reference a null object23:26:44.390 (390455000)|FATAL_ERROR|System.NullPointerException: Attempt to de-reference a null object
Trigger.opp_to_acct_update: line 11, column 7723:26:44.835 (390567000)|CUMULATIVE_LIMIT_USAGE23:26:44.835|LIMIT_USAGE_FOR_NS|(default)|Number of SOQL queries: 1 out of 100Number of query rows: 0 out of 50000Number of SOSL queries: 0 out of 20Number of DML statements: 1 out of 150Number of DML rows: 1 out of 10000Number of script statements: 6 out of 200000Maximum heap size: 0 out of 3000000Number of callouts: 0 out of 10Number of Email Invocations: 0 out of 10Number of fields describes: 0 out of 100Number of record type describes: 0 out of 100Number of child relationships describes: 0 out of 100Number of picklist describes: 0 out of 100Number of future calls: 0 out of 10
23:26:44.835|TOTAL_EMAIL_RECIPIENTS_QUEUED|023:26:44.835|STATIC_VARIABLE_LIST|opp_to_acct_update:MapAccount:4opp_to_acct_update:SetAccId:8opp_to_acct_update:oppupdate:4
23:26:44.835 (390567000)|CUMULATIVE_LIMIT_USAGE_END
23:26:44.390 (390741000)|CODE_UNIT_FINISHED|opp_to_acct_update on Opportunity trigger event BeforeInsert for [new]23:26:44.391 (391806000)|DML_END|[9]23:26:44.391 (391936000)|EXCEPTION_THROWN|[9]|System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, opp_to_acct_update: execution of BeforeInsert
caused by: System.NullPointerException: Attempt to de-reference a null object
Trigger.opp_to_acct_update: line 11, column 77: []23:26:44.396 (396395000)|FATAL_ERROR|System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, opp_to_acct_update: execution of BeforeInsert
caused by: System.NullPointerException: Attempt to de-reference a null object


Rahul SharmaRahul Sharma

you have to insert the records in test methods witth all the required fields like:

 

@istest   
private class TestAccountOppUpdateTrigger    {              
static testMethod void TestAccountOppUpdate()           
{
Account objAccount = new Account(Name = 'Test Account 1');                              
insert objAccount;
Opportunity Opp = new Opportunity(Name = 'Test Opportunity', AccountID = objAccount.Id);
insert Opp;               
                                  
}                                    
}

 Also give additional required fields needed for Account and Opportunity.

This was selected as the best answer
LiorGLiorG

Great, works.

 

Thanks for the help.

Rahul SharmaRahul Sharma

Nice to hear :)

OyeCodeOyeCode

I pretty much had the same problem, i am trying to bulk insert the opportunity and have this error

 

 System.DmlException: Insert failed. First exception on row 0; first error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, OpportunityBeforeInsert: execution of BeforeInsert caused by: System.ListException: List index out of bounds: 0 Trigger.OpportunityBeforeInsert: line 14, column 33: []

 

public class testOpportunityBeforeUpdate{
    /*****************************************************
    * Test class to Test Opportunity Before Insert Trigger
    ******************************************************/
    static testMethod void test_OpportunityBeforeInsert(){
        // Bulkifying the Opportunity
        List<Opportunity> Oppty = new List<Opportunity>();
        Profile p = [select id from profile where name='Standard User'];
        //Bulk insert Opportunity here
        for (Integer i=0;i<20;i++) {
            Opportunity samp_opp= new Opportunity ( Name = 'Test Account');
            Oppty.add(samp_opp);
        }
        insert Oppty ;
    } // end of the test method
}// end of the test class

 

 

and my BeforeInsert trigger is here

 

trigger OpportunityBeforeInsert on Opportunity (before insert) 
{
    User sysAdmin = new User();
    Map<Id, User> ownMgr = new Map<Id, User>([Select u.Id, u.ManagerId, u.Name From User u where IsActive = true]);
    for (User u :ownMgr.values())
    {
        if(u.Name == 'System Administrator')sysAdmin = u;
    }
    
    for (Opportunity oppy : Trigger.new) {
        //assign Opportunity Commission Salesperson Id  
        List<Account> ownId = [Select a.OwnerId, a.Id From Account a where a.Id = :oppy.AccountId]; 
        
        if(sysAdmin.Id == ownId[0].OwnerId)
        {
            //since sys admin is account owner, just use Opp Owner
            oppy.Commission_Salesperson__c = oppy.OwnerId; 
        }
        else
        {
            //use Account Owner
            oppy.Commission_Salesperson__c = ownId[0].OwnerId; 
        }
        
        oppy.OpportunityOwner__c = oppy.OwnerId;
        if(ownMgr.containsKey(oppy.OwnerId) == true)oppy.Owner_Manager__c = ownMgr.get(oppy.OwnerId).ManagerId;
        
        //--ADJUSTMENTS-- If either Original Opportunity fields are set, do a lookup and validate it exists
        if(oppy.OriginalOpportunity__c != null || (oppy.Original_Transaction_ID__c != null && oppy.Original_Transaction_ID__c != ''))
        {
            Opportunity origOpp = OpportunityUtil.setOriginalOpp(oppy);  
            if(origOpp == null) oppy.addError('Original Opportunity fields must match an existing Opportunity that is in stage "Closed Booked".');
        }
    }

}

 

 

Rahul SharmaRahul Sharma

Hi ForceLabs,

 

In your trigger line #12, you are querying the list Account in ownId.

The error you are facing is due to use of ownId[0].OwnerId.

It will throw error when you list is empty and you try to access its field.

Also in line #12 your querying inside a for loop, avoid it first.

then before using ownId[0].OwnerId, do a check if list is not empty.