+ Start a Discussion
frasuyetfrasuyet 

Contact Trigger Send Emails - Approach

With some help, I've developed the trigger below that is fully funtional and operates as designed. In a nut shell, it performs two primary functions.

 

  1. Sends an email to the contact - assuming that the criteria is satisfied
  2. Updates the contact to indicate that an email has been sent. (This could be satisifed w/ activity reporting but deferred to a tracking within the contact since direct contact reporting is simplier than the often inferred reporting that occurs with activities)

With the update of epc_invite_sent__c at the end of the statement, this trigger fires twice (assuming the criteria is met initially) once to send the email and again to make the update to epc_invite_sent__c. The trigger then exits if the IF statement is not satisfied.

 

Being a neophyte developer, I am wondering if this is the best approach for the trigger to fire twice. Though it works, I am thinking that there might be a better approach that I am not aware of that might be more efficient and or less "expensive" from a query stand point.

 

Welcome the thoughts and best practice pointers. Thanks.

 

 

trigger EPC_Invite_EmailMessage on Contact (after insert, after update) {

  for (Contact c : Trigger.new){
    Account a = [Select id, ESR_or_GDS__c, Parent_Chain__c From Account Where Id = :c.AccountId];
    if(c.HasOptedOutofEmail == FALSE &&
       c.RecordTypeId == '012500000009AB1' && //Standard Contact
       c.EPC_Invite_Sent__c == FALSE &&
       c.Opt_out_acme_Partner_Central__c == FALSE &&
       c.No_Longer_There_Do_not_email__c == FALSE &&
       c.EPC_Invite_Sent__c == FALSE &&
       a.ESR_or_GDS__c == 'ESR' &&
       a.Parent_Chain__c != Null &&
      (a.Parent_Chain__c != 'a0L50000000LQWw' || 
       a.Parent_Chain__c != 'a0L70000002eIb3' || 
       a.Parent_Chain__c != 'a0L50000000LQYL')) {

       String template = ''; // initialize variable to hold template name filled in below
       if(c.Language__c == 'Arabic'||   
          c.Language__c == 'Armenian'|| 
          c.Language__c == 'Bengali'||
          c.Language__c == 'English'||
          c.Language__c == 'Hindi (urdu)'||
          c.Language__c == 'Lithuanian'||
          c.Language__c == 'Malay (Malaysia)'||
          c.Language__c == 'Romanian'||
          c.Language__c == 'Serbian'||
          c.Language__c == 'Slovak'||
          c.Language__c == 'Ukrainian') {
         //set to english template
         template = 'EPC Invite - English';
       }
       else {
         //set to english template 
         template = 'EPC Invite - ' + c.Language__c;
       }
       
       Messaging.SingleEmailMessage message = new Messaging.SingleEmailMessage();
       message.setTemplateId([select id from EmailTemplate where Name = :template].id);
       
       message.setTargetObjectId(c.id);
       
       // String to hold the email addresses to which the email is being sent. 
       message.setToAddresses (new String[] {c.Email});

       // Send message under cover of org wide email address xreply@acme.com instead of user that executed trigger
       message.setOrgWideEmailAddressId('0D2T00000004CL9');

       // Send the email that has been created.  
       Messaging.sendEmail(new Messaging.Email[] {message});
       
       Contact contact = [Select ID
                           From Contact
                          Where Id = :c.Id];
                          
       contact.EPC_Invite_Sent__c = true;       
       update contact;
         
    }
  }
}

 

 

 

 

rtuttlertuttle

I see two problems here.  The first is you're querying the account inside a for loop.  You'll hit governor limits if your trigger ever handles a bulk insert or update.  To get around this, add the account ids to a set, and query for all of the accounts at once into a map, then grab them by the id off the contact record from a map.

 

The second problem is you're sending the e-mails in the loop.  Add them to a list then send them all at once at the end like so:

 

 List<Messaging.SingleEmailMessage> messages = new List<Messaging.SingleEmailMessage>();
Set<Id> accountSet = new Set<Id>();

for(Contact c : trigger.new) {
	accountSet.add(c.AccountId);
}

Map<Id,Account> accountMap = new Map<Id,Account>( [select Id, ESR_or_GDS__c, Parent_Chain__c  from Account where Id in :accountSet] );

for(Contact c : trigger.new) {
Account a = accountMap.get(c.Id);


// your processing goes here


messages.add(message);

}

Messaging.sendEmail(messages);

 

 

Hope that helps.

 

 

Edit:  Fixed variable name in messaging.sendemail

 

 

 

-Richard

rtuttlertuttle

Okay, so after some caffeine and fully re-reading the code and requirements I see a couple other issues and have a suggestion for your approach.  I had one open question, is this only for new contacts being inserted?

 

Also one thing to be cautious of is this trigger is built with some hard coded Ids.  I see it might not be possible to change some of those though, just be warned when writing test code this is going to be a pain because those Ids are org specific.  You might consider using a query at the top to pull in each of the parent chains that you want to skip sending this e-mail; the same goes for the record type id.

 

The only thing I'm unsure about with this approach is whether it'll send the e-mail even if there is an error somewhere in the step (workflow, validation, or another trigger).  Maybe someone else has some experience in that area as to whether the trigger will complete its work even in the event of dml error.

 

 List<Messaging.SingleEmailMessage> messages = new List<Messaging.SingleEmailMessage>();
et<Id> accountSet = new Set<Id>();
r(Contact c : trigger.new) {
accountSet.add(c.AccountId);
trigger EPC_Invite_EmailMessage on Contact (before insert, before update) { List<Messaging.SingleEmailMessage> messages = new List<Messaging.SingleEmailMessage>(); Set<Id> accountSet = new Set<Id>(); for(Contact c : trigger.new) { accountSet.add(c.AccountId); } Map<Id,Account> accountMap = new Map<Id,Account>( [select Id, ESR_or_GDS__c, Parent_Chain__c from Account where Id in :accountSet] ); for(Contact c : trigger.new) { Account a = accountMap.get(c.Id); if(c.HasOptedOutofEmail == FALSE && c.RecordTypeId == '012500000009AB1' && //Standard Contact c.EPC_Invite_Sent__c == FALSE && c.Opt_out_acme_Partner_Central__c == FALSE && c.No_Longer_There_Do_not_email__c == FALSE && c.EPC_Invite_Sent__c == FALSE && a.ESR_or_GDS__c == 'ESR' && a.Parent_Chain__c != Null && (a.Parent_Chain__c != 'a0L50000000LQWw' || a.Parent_Chain__c != 'a0L70000002eIb3' || a.Parent_Chain__c != 'a0L50000000LQYL') ) { String template = ''; // initialize variable to hold template name filled in below if(c.Language__c == 'Arabic'|| c.Language__c == 'Armenian'|| c.Language__c == 'Bengali'|| c.Language__c == 'English'|| c.Language__c == 'Hindi (urdu)'|| c.Language__c == 'Lithuanian'|| c.Language__c == 'Malay (Malaysia)'|| c.Language__c == 'Romanian'|| c.Language__c == 'Serbian'|| c.Language__c == 'Slovak'|| c.Language__c == 'Ukrainian') { //set to english template template = 'EPC Invite - English'; } else { //set to non english template template = 'EPC Invite - ' + c.Language__c; } Messaging.SingleEmailMessage message = new Messaging.SingleEmailMessage(); message.setTemplateId([select id from EmailTemplate where Name = :template].id); message.setTargetObjectId(c.id); // String to hold the email addresses to which the email is being sent. message.setToAddresses (new String[] {c.Email}); // Send message under cover of org wide email address xreply@acme.com instead of user that executed trigger message.setOrgWideEmailAddressId('0D2T00000004CL9'); // Add the e-mail to the list to be sent messages.add(message); c.EPC_Invite_Sent__c = true; } } Messaging.sendEmail(messages); }

 

 

 

 

-Richard

David SchachDavid Schach

I agree with rtuttle's approach, with some additions:

 

1. That second query at the bottom may be unnecessary, as you've got the field information in Trigger.New, but I can't think that through because I have a feeling that this whole approach may be unnecessary.

 

2. You're creating two separate processes (send email and update field) in one trigger - why not combine them and update the field AND send the email?  Is it because you want to be sure the email was sent?  You're not going to get that information back in the trigger.

 

3. Why use an after trigger?  I'd use a before trigger if I used a trigger at all.  Or I could use two triggers:

 

Update the boolean.

If Trigger.Old == false and Trigger.New == true (i.e. you updated the field in the before trigger), then send the email.

 

(Of course, the second action needs to be filtered: Happen every time the booelan is true and it's an insert, and every time !Old && New and it's an update.)

 

That said, there's a bigger question here:

 

Why aren't you using workflow?  It seems to be just as good. 

 

Summary:

Either use a before trigger and combine both actions into one loop, adding to lists and executing at the end,

or use two triggers (before and after),

or use workflow.

 

This is a good example of a question that would generate a super discussion at a developer meet-up!

David SchachDavid Schach

Richard,

 

To answer your question, the actions are independent.  Your trigger (a good one) first generates the emails, commits a field change to the update queue, sends the emails, and then attempts a DML update.

 

Therefore, the DML error happens last and the emails will be sent anyway.

 

If the boolean update violates a validation rule, I think it will still send the emails.

 

David

David SchachDavid Schach

Replying to myself:

 

If we want to do a real check on this, update the boolean in a before trigger, then use an after trigger and send the email after checking if Trigger.OldMap != Trigger.NewMap. 

 

OH - I may be wrong in my last post.  Note the final entry at http://www.x2od.com/2008/11/09/salesforce-order-of-execution.html - I don't know what "sending email" means, exactly, as related to emails sent within triggers.  Interesting. 

 

The plot thickens.

rtuttlertuttle

Ahh, thanks for sharing that.  I was hoping they held off on committing the email send.  This could be tested by adding an error into this very trigger, or another trigger.

 

If I get some time tonight I'll test this.

 

 

-Richard

frasuyetfrasuyet

 

Thanks for the banter and constructive criticism. Very helpful. To answer the workflow question.

 

Correct, this is a great use case for workflow but chose to use a trigger instead because:

 

  1. I would have 26 different workflow rules - one (email alert) for each language. It seemed like a lot of over head to maintain and would be more streamlined bundled within a trigger.
  2. With a trigger I can use SetSaveasActivity which includes the body of the email sent where as with workflow you have to create a dummy task to represent the email which does not include the body of the email. (This current contrainst blows)
  3. Emails also need to be sent to qualified contacts based on updates to the account so I figured it would be eaiser to maintain 2 triggers instead of 1(account) trigger and 26 different workflow rules. I.e. Have one common solution framework instead of 2 different approaches.

Now time to digest and make some modifications. :smileyvery-happy:

David SchachDavid Schach

@frasuyet Those are excellent reasons to use a trigger.  Well thought-through.

 

I'd love to see the final trigger when you've got a version 2.0.