0

i have a object Job__c where one multi select picklist(person names) is there.Whenever i will select value a mail should trigger to that person which i am linking with custom setting(users__c).In users__c 2 fields are there Name and Email__c.if this Name is there in that selected multi select picklist value a mail should go to the respective email id. For that i have written one trigger with handler class and i am getting the mail also.But my problem is whenever i am updating anything in the record i am getting the mails.So how can i restrict this?
I am posting here my code also.

Trigger:

trigger notify_users on Job__c (before insert, before update) {


    notify_users_Trigger t=new notify_users_Trigger();
    if(Trigger.isBefore && (Trigger.isInsert || Trigger.isUpdate )){

        try{

            t.notify_users(Trigger.new);

        }
        catch (exception e){

            system.debug('**'+e.getMessage() );

        }
    }

}

Handler Class

public class notify_users_Trigger{
    public integer i {get; set;}

    public void notify_users(List<Job__c> jobs)
    {

        List<users__c> us1=new List<users__c>();//custom settings


        EmailTemplate et=[Select id from EmailTemplate where id=:'00X3D000000QcZA'];
        List<Messaging.SingleEmailMessage> emails = new List<Messaging.SingleEmailMessage>();


        for(Job__c j:jobs)
        {
          system.debug('j.multyselect__c' +j.multyselect__c); 
          String[] m = j.multyselect__c.split(';');

            system.debug('m '+m);
            for ( i = 0; i<m.size(); i++ )
            {
                us1=[select Name,Email__c from users__c where Name=:m[i]];
                system.debug('us1: '+us1);
                for(users__c use:us1)
                {
                    system.debug('use.Email__c '+use.Email__c);
                    Messaging.SingleEmailMessage singleMail = new Messaging.SingleEmailMessage();
                    singleMail.toAddresses=new String[] { use.Email__c};
                    singleMail.setTargetObjectId(j.ownerID);                              
                    singleMail.setTemplateId(et.Id);
                    singleMail.setSaveAsActivity(false);
                    emails.add(singleMail);

                    system.debug('successfully submitted ');


                }
                system.debug('------'+Messaging.sendEmail(emails));

            }
        }
    }
}
2
  • There are many ways to improve the notifications... the first is what you say, that if you touch something else than the multi-picklist, not to get a notification. another improvement could be that if the field has User1;User2 and changes to User1;User3, you only notify User3 (and even User2 releasing him). And there are some things that you could improve on the code as well, but let me know first this and I'll keep you posted ;) A simple way (far from best) is to compare the trigger.new against the trigger.old (only on update) to check if the field changed and process only those records. Nov 3, 2017 at 8:00
  • @SergioAlcocer sir how to compare trigger.old an trigger.new in handler class? this is the 1st time i have written a handler class
    – force1
    Nov 3, 2017 at 8:14

1 Answer 1

0

The simplest way is replacing...

try{
    t.notify_users(Trigger.new);
}

by

try{
    List<Job__c> jobs;
    if (Trigger.isInsert) {
        jobs = trigger.new;
    } else {
        jobs = new List<Job__c>();
        for(Job__c forJob : trigger.new) {
            if (forJob.multyselect__c != trigger.oldMap.get(forJob.Id).multyselect__c) {
                jobs.add(forJob);
            }
        }
    }
    t.notify_users(jobs);
}

You can also change it to

t.notify_users(Trigger.new, Trigger.oldMap);

and check inside your code if its different, but by having the class like this...

public class notify_users_Trigger{
    public integer i {get; set;}
    /* unless you are going to use it in more than one method, don't declare it here */

    public void notify_users(List<Job__c> jobs, Map<Id, Job__c> oldJobsMap)
    {

        List<users__c> us1=new List<users__c>();//custom settings
        /* if you are going to make a us1 = [query], you don't need to initialize this. 
        Even if the query returns 0 records, it will return an Empty list. 
        Initialise it if you expect to have an exception and you don't want to check against null afterwards.*/

        EmailTemplate et=[Select id from EmailTemplate where id=:'00X3D000000QcZA'];
        /* Don't hard-code Ids in a query, worse case scenario, you could hard-code it in the singleMail.setTemplateId('00X....'); 
            It is better to use a custom setting or use developerNames, etc. If someone deletes the template by mistake, you are in trouble
        */
        List<Messaging.SingleEmailMessage> emails = new List<Messaging.SingleEmailMessage>();


        for(Job__c j:jobs)
        {
            if (oldJobsMap != null && j.multyselect__c != oldJobsMap.get(j.Id).multyselect__c)
            {
                system.debug('j.multyselect__c' +j.multyselect__c); 
                String[] m = j.multyselect__c.split(';');

                system.debug('m '+m);
                for ( i = 0; i<m.size(); i++ ) /* here you could do    for (String forName : m) // and use forName instead of m[i] */
                {
                    us1=[select Name,Email__c from users__c where Name=:m[i]]; 
                    /* Multiple warnings: 
                    *  "Never" make queries inside a loop 
                    * Since its a CustomSetting, you could use us1 = Users__c.getInstance(m[i]);
                    * If you assign a query to a single record (not a list of them) and the query returns 0 records, it will throw LIST_HAS_NO_ROWS_FOR_ASSIGNMENT exception
                    * If you assign a query to a single record (not a list of them) and the query returns 0 records, it will throw LIST_HAS_MORE_THAN_ONE_ROW_FOR_ASSIGNMENT exception
                    */
                    system.debug('us1: '+us1);
                    for(users__c use:us1)
                    {
                        system.debug('use.Email__c '+use.Email__c);
                        Messaging.SingleEmailMessage singleMail = new Messaging.SingleEmailMessage();
                        singleMail.toAddresses=new String[] { use.Email__c};
                        singleMail.setTargetObjectId(j.ownerID);
                        singleMail.setTemplateId(et.Id);
                        singleMail.setSaveAsActivity(false);
                        emails.add(singleMail);

                        system.debug('successfully submitted ');


                    }
                    system.debug('------'+Messaging.sendEmail(emails));
                    /* consider moving it out of the inner loop to outside the loops */
                }
            }
        }
    }
}

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.