+ Start a Discussion
sudhirn@merunetworks.comsudhirn@merunetworks.com 

Trigger is not firing in opportuntiy

Hi,

  I wrote below trigger on opportuntiy to update account field for some reasons it is not workng please suggest me what is the issue in below trigger
trigger update_account_billing_email  on Opportunity (before insert, before update) {

 for(Opportunity opp:System.Trigger.new) 
  { 
    
   Opportunity op = [SELECT Billing_ContactEmail__c,AccountId from Opportunity 
                      where id in :Trigger.newMap.keySet()];

   Account ac = [SELECT Billing_Email__c from Account where id = :op.AccountId];
   
   If ( ac.Billing_Email__c == '')
   {
     ac.Billing_Email__c = op.Billing_ContactEmail__c;
     
     update ac;
    }
   

  } 



}

Thanks
Sudhir
Best Answer chosen by sudhirn@merunetworks.com
James LoghryJames Loghry
The trigger is firing, but you have to understand trigger context first.  Trigger.newMap (and Ids of records) are not available on a before insert operation.  See the following link for more info on that: https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_triggers_context_variables.htm

Further more, your trigger is performing an update or DML inside of a for loop, which will throw an exception in cases where you load several Opportunity records.

In other words, your trigger needs a bit of refactoring.  I've taken a stab at it for you below:
 
trigger update_account_billing_email  on Opportunity (before insert, before update) {
    Set<Id> accountIds = new Set<Id>();
    for(Opportunity opp:System.Trigger.new){ 
        accountIds.add(opp.AccountId);
    }

    Map<Id,Account> accountMap = new Map<Id,Account>([Select Billing_Email__c From Account Where Id in :accountIds And (Billing_Email__c = null Or Billing_Email__c = '')]);

    Map<Id,Account> accountsToUpdate = new Map<Id,Account>();
    for(Opportunity opp : Trigger.new){
        Account acct = accountMap.get(opp.AcountId);
        if(acct != null){
            acct.Billing_Email__c = opp.Billing_ContactEmail__c;
            accountsToUpdate.put(acct.Id,acct);
        }
    }

    update accountsToUpdate.values();
}

 

All Answers

James LoghryJames Loghry
The trigger is firing, but you have to understand trigger context first.  Trigger.newMap (and Ids of records) are not available on a before insert operation.  See the following link for more info on that: https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_triggers_context_variables.htm

Further more, your trigger is performing an update or DML inside of a for loop, which will throw an exception in cases where you load several Opportunity records.

In other words, your trigger needs a bit of refactoring.  I've taken a stab at it for you below:
 
trigger update_account_billing_email  on Opportunity (before insert, before update) {
    Set<Id> accountIds = new Set<Id>();
    for(Opportunity opp:System.Trigger.new){ 
        accountIds.add(opp.AccountId);
    }

    Map<Id,Account> accountMap = new Map<Id,Account>([Select Billing_Email__c From Account Where Id in :accountIds And (Billing_Email__c = null Or Billing_Email__c = '')]);

    Map<Id,Account> accountsToUpdate = new Map<Id,Account>();
    for(Opportunity opp : Trigger.new){
        Account acct = accountMap.get(opp.AcountId);
        if(acct != null){
            acct.Billing_Email__c = opp.Billing_ContactEmail__c;
            accountsToUpdate.put(acct.Id,acct);
        }
    }

    update accountsToUpdate.values();
}

 
This was selected as the best answer
Amit Chaudhary 8Amit Chaudhary 8
Pleae try below code. I hope that will help you
trigger update_account_billing_email  on Opportunity (before insert, before update) 
{
	
	Set<ID> setId = new Set<Id>();
	for(Opportunity opp: Trigger.new ) 
	{
		setId.add(opp.AccountId);
	}
	
	Map<Id,Account> mapAccount = new Map<Id,Account>([select id, Billing_Email__c from account where id in :setId ]);

	List<Account> lstAccount = new List<Account>();
	for(Opportunity opp: Trigger.new ) 
	{
		if(mapAccount.containsKey(opp.AccountId))
		{
			Account acc = mapAccount.get(opp.AccountId);
			acc.Billing_Email__c = opp.Billing_ContactEmail__c ;
			lstAccount.add(acc);
		}
	}

	if(lstAccount.size() > 0 )
	{
		update lstAccount;
	}
}
Please let us know if this will help you

Thanks
Amit Chaudhary
 
Arun Deepan LJArun Deepan LJ
Hi,
There are few mistakes in the code, such as
1. A SOQL query will return a List of Opportunity. But you are trying to collect as a single Opportunity.
2. Trigger.newMap.Keyset() will work only for "Before Update" and does not work for "Before Insert" Trigger. For "Before Insert" the value of Trigger.NewMap will be NULL
3. While querying the Account, do not use op.AccountID, which is single opportunity. Always use SET of all opportunity IDs
4. The most important error is, you have placed Two SOQL queries inside the FOR Loops. Remove that and keep the SOQL queries outside the FOR loop as shown in "Amit Chaudhary"'s example
 
sudhirn@merunetworks.comsudhirn@merunetworks.com
Thank you all I need last suggestion on writing a test class for below trigger
 
trigger update_account_billing_email  on Opportunity (before insert, before update) {
    Set<Id> accountIds = new Set<Id>();
    for(Opportunity opp:System.Trigger.new){ 
        accountIds.add(opp.AccountId);
    }

    Map<Id,Account> accountMap = new Map<Id,Account>([Select Billing_Email__c From Account Where Id in :accountIds And (Billing_Email__c = null Or Billing_Email__c = '')]);

    Map<Id,Account> accountsToUpdate = new Map<Id,Account>();
    for(Opportunity opp : Trigger.new){
        Account acct = accountMap.get(opp.AcountId);
        if(acct != null){
            acct.Billing_Email__c = opp.Billing_ContactEmail__c;
            accountsToUpdate.put(acct.Id,acct);
        }
    }

    update accountsToUpdate.values();
}

Thanks
Sudhir
Arun Deepan LJArun Deepan LJ
Hey, Create an account. Then create an opportunity and insert the same in the test class. Since it is a trigger, code will get covered when you perform DML operations in test class