6

Description: When I deactivate multiple users, say a,b,c, the following code is triggered to send emails to their respective managers. but in my case the manager receives the email with only the username of the last user for all emails i.e for a, b, c.

the expected scenario is that the manager should receive email for all 3 users mentioning their corresponding names in the mail as opposed to a single user name in all emails.

Could you please help me with what changes need to be done to make this piece of code work as expected

Apex Code:

public without sharing class ChangeOpportunityOwner {       
    @Future
    public static void changeOwner(List<Id> usrIds){
        Boolean isUpdate=false;
        List<Messaging.MassEmailMessage> messages=new list<Messaging.SingleEmailMessage>();
        Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();
        Map<Id, Boolean> mapUserToActive = new Map<Id, Boolean>();
        try{
            for (User u1 : [SELECT Id, isActive FROM User]) { 
                mapUserToActive.put(u1.Id, u1.isActive);
            }
            List <user>usrs= [Select Id,Name,ManagerId,IsActive from user where Id=:usrIds];
            List<Opportunity> finalOptList=new List<Opportunity>();
            for(User u:usrs){   
                List<Opportunity>optys=[Select Id,OwnerId,StageName from Opportunity where OwnerId=:u.Id]; 
                for(Opportunity opt:optys){
                    if(opt.StageName!='Closed Won' && opt.StageName!='Closed Lost')
                    {
                        if (mapUserToActive.get(u.ManagerId)== true && mapUserToActive.get(u.ManagerId)!=null) {
                            opt.OwnerId=u.ManagerId;
                        }
                        else{ 
                            User usr= [Select Id,Name,ManagerId from user where Name='Test Manager'];
                            opt.OwnerId=usr.Id;
                        }
                        finalOptList.add(opt);
                        // Set list of people who should get the email
                        List<String> sendTo = new List<String>();
                        sendTo.add(u.ManagerId);
                        mail.setToAddresses(sendTo);
                        //  Set who the email is sent from
                        mail.setReplyTo('privacy@test.com');
                        mail.setSenderDisplayName('Customer Service');
                        // Set email contents - you can use variables!
                        mail.setSubject('Open Opportunities transferred to you');
                        String body = 'Dear Manager,<br> ';
                        body +='<br>';
                        body += 'Please note that '+u.name+'  has been deactivated as a Salesforce User hence all of its <br> Open Opportunities has been assigned to you if any.<br>';
                        body +='<br>';
                        body +='Kind regards,<br>';
                        body +='<br>';
                        body +='SFDC Support';
                        mail.setHtmlBody(body);

                        isUpdate=true;
                    }
                }
                messages.add(mail);
                //In the below debugs I can see all 3 users inserted properly in the email template body
                system.debug('messages:'+messages);
                system.debug('Mail:'+mail);
            }
            if(isUpdate)
                system.debug('message1:'+messages);//in this debug when I receive an email,it always displays user c's information.which is last user.
            Messaging.sendEmail(messages,false);

            if(isUpdate)
            {
                update finalOptList;
            }
        }
        catch(Exception e)
        {
            system.debug('An Exception Occured: '+e.getMessage());
        }
    }


    @InvocableMethod(label='Change Opportunity Owner' description='It Change the owner of opportunity when owner is inactive')
    public static void changeOwnerFuture(List<Id> usrIds)
    {
        changeOwner(usrIds);
    }
}

Updating the code below:

        public without sharing class ChangeOpportunityOwner {       
        @Future
        public static void changeOwner(List<Id> usrIds){
        Boolean isUpdate=false;
        List<Messaging.SingleEmailMessage> messages=new list<Messaging.SingleEmailMessage>();
        Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();
        Map<Id, Boolean> mapUserToActive = new Map<Id, Boolean>();
        map<Id,user>idToUser= new map<Id,User>([Select Id,Name,ManagerId,IsActive from user where Id IN :usrIds]);
        List<Opportunity> finalOptList=new List<Opportunity>();
        set<Id>mgrIds = new set<Id>();
           for (User u1 : idToUser.values()) { 
                    mgrIds.add(u1.ManagerId);
                    // collect the ids of the managers
              }
        /*for (User u1 : [SELECT Id, isActive FROM User]) { 
        mapUserToActive.put(u1.Id, u1.isActive);
        }*/

        map<Id,user>idToManager = new map<Id,User>([Select Id,Name,ManagerId,IsActive from user where Id IN :mgrIds]);

              for (User u2 : idToManager.values()) { 
                    mapUserToActive.put(u2.Id, u2.isActive);
                    // you can add them to your map if you need the map
              }
        //List <user>usrs= [Select Id,Name,ManagerId,IsActive from user where Id=:usrIds];

        for(User u: idToUser.values()){   
        List<Opportunity>optys=[Select Id,OwnerId,StageName from Opportunity where OwnerId=:u.Id]; 
        for(Opportunity opt:optys){
        if(opt.StageName!='Closed Won' && opt.StageName!='Closed Lost')
        {
        if (mapUserToActive.get(u.ManagerId)== true && mapUserToActive.get(u.ManagerId)!=null) {
        opt.OwnerId=u.ManagerId;
        }
        else{ 
        User usr= [Select Id,Name,ManagerId from user where Name='Test Manager'];
        opt.OwnerId=usr.Id;
        }
        finalOptList.add(opt);
        list<string> userStore=new List<String>();
        userStore.add(u.Name);
        system.debug('userStore:'+userStore);
        // Set list of people who should get the email
        List<String> sendTo = new List<String>();
        sendTo.add(u.ManagerId);
        mail.setToAddresses(sendTo);
        //  Set who the email is sent from
        mail.setReplyTo('privacy@test.com');
        mail.setSenderDisplayName('Customer Service');
        // Set email contents - you can use variables!
        mail.setSubject('Open Opportunities transferred to you');
        String body = 'Dear Manager,<br> ';
        body +='<br>';
        body += 'Please note that '+userStore+'  has been deactivated as a Salesforce User hence all of its <br> Open Opportunities has been assigned to you if any.<br>';
        body +='<br>';
        body +='Kind regards,<br>';
        body +='<br>';
        body +='SFDC Support';
        mail.setHtmlBody(body);

        //isUpdate=true;
        }
        }
        messages.add(mail);
        system.debug('messages:'+messages);
        system.debug('Mail:'+mail);
        }
        //if(isUpdate)
        //messages1 = new List<Messaging.SingleEmailMessage>(messages);
        system.debug('message1:'+messages);
        Messaging.sendEmail(messages,true);

        //if(isUpdate)
        //{
        update finalOptList;
        //}


        }


        @InvocableMethod(label='Change Opportunity Owner' description='It Change the owner of opportunity when owner is inactive')
        public static void changeOwnerFuture(List<Id> usrIds)
        {
        changeOwner(usrIds);
        }
        } 

It still displays the Latest users info i.e.User C. Please suggest what is wrong in the updated code..

16
  • 3
    If you put a little time into indentation, it would make your code a lot more readable. – Adrian Larson Jan 6 '17 at 15:55
  • @AdrianLarson But I have already put the code in proper way? Is it not readable? – Dhananjay Patil Jan 6 '17 at 16:01
  • When you properly indent your code, we can visually see the structure of it and get an intuitive grasp of your level of nested complexity. When your code is uniformly indented (no variation at all), there's no way to do that. – Adrian Larson Jan 6 '17 at 16:09
  • @AdrianLarson Noted.I will use Code beautification site to arrange the code in proper format.Apologies for inconvenience. – Dhananjay Patil Jan 6 '17 at 16:16
  • 1
    Or just paste it into the Developer Console (the Execute Anonymous window) and hit Shift + Tab. If you're pasting it into SFSE, you can just select all and tab it out one extra tab before copying so that it formats it as code here. – Adrian Larson Jan 6 '17 at 16:17
2

First, it doesn't appear to me that you need your try-catch block at all. Instead, simply query for the users you need (not everyone in your org).

      map<Id,user>idToUser= new map([Select Id,Name,ManagerId,IsActive from user where Id IN :usrIds]);
      set<Id>mgrIds = new set();
      for (User u1 : idToUser.values()) { 
            mapUserToActive.put(u1.Id, u1.isActive);
            // you can create this map if you need it, not certain that it's necessary since you have this above in the map you queried
            mgrIds.add(u1.ManagerId);
            // collect the ids of the managers
      }

      // now query for the managers
      map<Id,user>idToManager = new map([Select Id,Name,ManagerId,IsActive from user where Id IN :mgrIds]);

      for (User u1 : idToManager.values) { 
            mapUserToActive.put(u1.Id, u1.isActive);
            // you can add them to your map if you need the map
      }

      list<user>allUsers = new list();
      // get a combined list of users and managers
      allUsers.addAll(usrs);
      allUsers.addAll(managers);          

      // query for all of the opportunities at one time
      list<Opportunity> finalOptList=new List<Opportunity>();
      map<Id,Opportunity>idToOpportunity=new map([Select Id,OwnerId,StageName from Opportunity where OwnerId in :userIds]);
      // you can optionally group this query by ownerId

      // create a map of opportunity Ids to OwnerId
      for(Opportunity opt:optys){
          if(opt.StageName!='Closed Won' && opt.StageName!='Closed Lost')
          {
              oppIdToOwnerId.put(opt.Id,opt.OwnerId);
          }

Now you should have everything you need to operate in a single for loop on user (or userID) to create your emails. You may want to create a map though of <Id,list<string>> to hold your ManagerId compared to the userIds (or names) that report to the manager. that's the only thing else I can think of that might be helpful to you. You may also want to do something similar for userId to list<OpporutnityId> or similar. I'll leave it to you to work out those details as that should be simple enough with what you have here.

EDIT:

Based on your revised code, here's what I'd recommend you do to solve your issues:

    public without sharing class ChangeOpportunityOwner {       
    @Future
    public static void changeOwner(List<Id> usrIds){
    Boolean isUpdate=false;
    List<Messaging.SingleEmailMessage> messages=new list<Messaging.SingleEmailMessage>();
    Messaging.SingleEmailMessage mail = new Messaging.SingleEmailMessage();
    Map<Id, Boolean> mapUserToActive = new Map<Id, Boolean>();

    map<Id,user>idToUser= new map<Id,User>([Select Id,Name,ManagerId,IsActive from user where Id IN :usrIds]);

    List<Opportunity> finalOptList=new List<Opportunity>();
    set<Id>mgrIds = new set<Id>();

    for (User u1 : idToUser.values()) { 
                mgrIds.add(u1.ManagerId);
                // collect the ids of the managers
    }

    map<Id,user>idToManager = new map<Id,User>([Select Id,Name,ManagerId,IsActive from user where Id IN :mgrIds]);

    for (User u2 : idToManager.values()) { 
        mapUserToActive.put(u2.Id, u2.isActive);
    }

    map<Id,Opportunity>idToOpportunity=new map([Select Id,OwnerId,StageName from Opportunity where OwnerId in :userIds groupBy OwnerId]);


    // create a map of opportunity Ids to OwnerId
    map<Id,Id>oppIdToOwnerId = new map<Id,Id>();

    for(Id opId:idToOpportunity.keyset()){
        if(idToOpportunity.get(opId).StageName!='Closed Won' && idToOpportunity.get(opId).StageName!='Closed Lost')
        {
            oppIdToOwnerId.put(opId,idToOpportunity.get(opId).OwnerId);
            // create map of OppIds to OwnerIds for Opps of Interest
        } 
    }

    //get list of opp Ids for each ownerId & put into map
    string OppIdStr = '';
    list<Id>UserId = new list<Id>();
    integer i = 0;
    map<Id,string>usrIdToOppIdString = new map<Id,string>();
    for(Id oId : oppIdToOwnerId.keyset()){            
        if(i=0){
            UserId[0] = oppIdToOwnerId.get(oId);
            OppIdStr = oId + ', ';
            usrIdToOppIdString.put(UserId[0],OppIdStr);
            if(mapUserToActive.get(UserId[0]
        }                 
        else if(i>0 && (oppIdToOwnerId.get(oId) == UserId[0]){
            UserId[i] = oppIdToOwnerId.get(oId).OwnerId;
            OppIdStr +=  ', '+ oId;
            usrIdToOppIdString.put(UserId[i],OppIdStr);
        }
        else{
            UserId.clear();
            i=0;
            UserId[0]=oppIdToOwnerId.get(oId);
            OppIdStr = oId + ', ';
            usrIdToOppIdString.put(UserId[0],OppIdStr);
       }
       i++;

    }


    //Run for loop on oppIdToOwnerId map         
    String UserNames = '';
    for(Id oId:oppIdToOwnerId.keyset()){   
        id managerId = idToUser.get(idToOpportunity.get(oId).OwnerId).ManagerId
        if (mapUserToActive.get(ManagerId)== true ) {
        // the above cannot be null. Would get null pointer exception 
        // when attempting to put into map if value from query was null
            idToOpportunity.get(oId).OwnerId=ManagerId;
            if(!managerIdToUserName.contains(ManagerId)){
                userNames += idToUser.get(idToOpportunity.get(oId).OwnerId).Name + ', ';
                managerIdToUserName.put(ManagerId,userNames);
            }
            else if(managerIdToUserName.contains(ManagerId) &&  !managerIdToUserName.get(ManagerId)userNames.contains(idToUser.get(idToOpportunity.get(oId).OwnerId).Name){
                userNames = managerIdToUserName.get(managerId);
                userNames += idToUser.get(idToOpportunity.get(oId).OwnerId).Name + ', ';
                managerIdToUserName.put(ManagerId,userNames);

            }

        }
        else{ 
            User usr= [Select Id,Name,ManagerId from user where Name='Test Manager'];
            idToOpportunity.get(oId).OwnerId=usr.Id;
            // are you certain this shouldn't be usr.ManagerId ?
            // this scenario seems unlikely, but you know your org best

            if(managerIdToUserName.contains(usr.Id) && !managerIdToUserName.get(usr.Id).contains(idToUser.get(idToOpportunity.get(oId).OwnerId).Name){
                userNames = managerIdToUserName.get(usr.Id);
                userNames += idToUser.get(idToOpportunity .get(oId).OwnerId).Name + ', ';
                managerIdToUserName.put(usrId,userNames);


            }
            else if(!managerIdToUserName.contains(usr.Id)){
                userNames = idToUser.get(idToOpportunity.get(oId).OwnerId).Name + ', ';
                managerIdToUserName.put(usrId,userNames);
            }
        }

    finalOptList.add(idToOpportunity.get(oId));
    }

    // Now create the emails using the maps in a for loop either on ManagerId 
    // or on Opportunity Id if you want them combined into one per manager

Will leave the rest to you.

AVOID ITERATING ON MAP VALUES as you were on this line: for(User u: idToUser.values()){.

I do not recommend using nested SOQL queries as you do in your current code. Instead, take them outside as I've shown above in both examples to create your maps.

These are deeply nested maps and you must keep them straight which I know can be difficult to do. There are probably cleaner ways to do some of this, but I don't have time to write all of your code for you. Use this as a basis for finishing writing your code. All you need do now is finish the loop for your email.

3
  • I'd created list of SingleEmailMessages List<Messaging.SingleEmailMessage> messages=new list<Messaging.SingleEmailMessage>(); the issue that I am facing here is when I try to add the messages to this list,the latest message overrides the previous one resulting in list containing only the latest message.Could You please suggest a solution in order to obtain a list of all the messages.. – Dhananjay Patil Jan 9 '17 at 7:14
  • I have taken Map as suggested by you but still it is not working.I am updating the code on question.Can you please look at the updated code and let me know.. – Dhananjay Patil Jan 9 '17 at 7:48
  • I have modified the code suggested by u and it works perfectly fine.Thank You So much for the suggestion. – Dhananjay Patil Jan 11 '17 at 18:18
0

I believe you are getting the correct debug and wrong output because the object reference is maintained which results in same name in all emails.

I believe it can be fixed by adding a line mail = new Messaging.SingleEmailMessage(); before mail.setToAddresses(sendTo);

finalOptList.add(opt);
// Set list of people who should get the email
List<String> sendTo = new List<String>();
sendTo.add(u.ManagerId);
/************ADD THIS LINE****************/
mail = new Messaging.SingleEmailMessage();
/*****************************************/

mail.setToAddresses(sendTo);
//  Set who the email is sent from
mail.setReplyTo('privacy@test.com');
mail.setSenderDisplayName('BD Customer Service');
0

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.