0

The code below is responsible for assigning Leads utilizing Round Robin distribution. The process is defined as follows:

  • Lead is created and meets predefined criteria within a automated process (Process Builder) which "calls" a flow to assign the Lead to a Sales Queue
  • When the Lead is assigned to the Sales Queue we "call" a APEX Class to handle the distribution
  • We pass the class two @InvocableVariables: 1) The Queue ID which owns the record 2) The Lead ID which represents the record we are interested in

When testing, the process works as expected! However, I want to clean up the class, and most importantly, prepare and pass data with efficiency. Lines 1 - 51 are where they need to be, but the breakdown begins at Line 53. Here is the "big picture" outcome I would like to see, but I'm having trouble:

  • I would like to leverage a Map of both Queue IDs and Lead IDs which goes through a method that contains Lead ID and Queue ID parameters. The method will get the Lead (including the ID & OwnerID) followed by retrieving a list of users (GroupMembers) who are members of the Queue ID we pass it. From there, I will be able to use what is returned within Lines 87 - 104.

Hopefully all of that made sense, I'm still new to APEX development, thank you in advance for helping me clean up the class. Feel free to ignore my personal comments littered throughout the class.

public with sharing class leadRoundRobinAssignment{

    // -------- Constants --------
    // Store exceptions for recording
    private static List<Exception__c> createExceptions = new List<Exception__c>();  

    // Store Sales Representative List
    private static List<User> repList = new List<User>();

    //Store Lead Queue ID
    private static Set<Id> queueIdsSet = new Set<Id>();

    // Set of Lead ID
    private static Set<Id> leadIdsSet = new Set<Id>();

    // Set of Group Member IDs
    private static Set<Id> userIdsSet = new Set<Id>();

    // -------- Properties --------
    //Used in Round Robin Assignment (Use a private, and set to 0)
    public static Integer index;
    public static Integer leadNumber;
    public static Integer repSize;

    @InvocableMethod 
    public static List<AssignLeadActionResult> assignLeads(List<AssignLeadActionRequest> requests){ 
        //List<assignLeadsRoundRobin> results = new List<assignLeadsRoundRobin>
        List<AssignLeadActionResult> results = new List<AssignLeadActionResult> ();

       // Id Queueid = data[0].Queueid;
       for(AssignLeadActionRequest request : requests) 
       {
           //results.add(assignLead(request));
           queueIdsSet.addAll(request.queueID);
           leadIdsSet.addAll(request.leadID);
       }

       // THIS IS WHERE THINGS ARE NOT CLICKING FOR ME!
       Map<ID, QueueSObject> queueIdMap = new Map<ID, Set<Id>>([SELECT QueueId, SobjectType FROM QueueSObject WHERE ID .....]);
       Map<ID, Lead> leadIdMap = new Map<ID, Set<Id>>([SELECT ID, OwnerId, Lead_Round_Robin__c FROM Lead WHERE ID .....]);


       System.debug('This is our queueIdsSet: ' + queueIdsSet);
       System.debug('This is our leadIdsSet: ' + leadIdsSet); 
       System.debug('This is our queueIdMap: ' + queueIdMap);
       System.debug('This is our leadIdMap: ' + leadIdMap);

       return results;
    } 

    private static AssignLeadActionResult assignLead(AssignLeadActionRequest request){ 
        /* get list of all the leads (this should break because we are loading all Leads, change to Map) */
        List<Lead> leadList = [Select Id, OwnerId, Lead_Round_Robin__c FROM Lead Where Id IN:request.leadID]; 

        if(request.QueueID != null){

            // Set<Id> userIdsSet = new Set<Id>();

            // Fetch Ids of the users
            For(GroupMember gm : [Select Id, UserOrGroupId FROM GROUPMEMBER WHERE GroupId IN : request.QueueID]){
                userIdsSet.add(gm.UserOrGroupId);
            }

            /* fetch the total number of active users available for round robin */
            repList = [Select Id, Name,  Profile.Name From User Where Id In : userIdsSet AND ISACTIVE = true];
            System.debug('This is our repList: ' + repList);


        }

        // return if there are no active users 
        if(repList != null || repList.size() != 0){

            For(Lead leadRecord : leadList){
                if(leadRecord.Lead_Round_Robin__c != null){
                    System.debug('This is our Lead Round Robin: ' + leadRecord.Lead_Round_Robin__c);
                    leadNumber = Integer.valueOf(leadRecord.Lead_Round_Robin__c);
                    //repSize = repList.size();
                    repSize = 2;
                    System.debug('This is our repSize: ' + repSize);
                    index = Math.MOD(leadNumber ,repSize); // 
                    System.debug('This is our index:' + index);
                    leadRecord.OwnerId = repList[index].id;

                Future_LeadRoundRobinAssignment.notifyLeadOwner(leadRecord.id,leadRecord.OwnerId);

                }
            }
        }

        if(leadList!=null && leadList.size()>0){

            update leadList;

        }

        else{

            createExceptions.add(EvolveException.buildException('leadRoundRobinAssignment', 'assignLead', 'This is an error'));
            System.debug('AssignAction error: ' + 'This is an error'); 

            insert createExceptions;

        }

        return null; 
    }

    public class AssignLeadActionRequest {
        @InvocableVariable(required = false)
        public List<ID> queueID; 

        @InvocableVariable(required = false)
        public List<Id> leadID;         

    }

    public class AssignLeadActionResult {

        @InvocableVariable(required = false)
        public List<Id> leadID;

    }
}
  • Hi Larry, welcome to SFSE! We look for specific questions here more than generalized code reviews (there is a Code Review Stack Exchange, however). Could you edit your question to focus on a specific issue you're struggling with in revising this class? – David Reed May 13 at 23:10
  • Welcome to the community! You might want to post this in the code review stack exchange instead, or as @DavidReed said refine your question. codereview.stackexchange.com – PartOfTheOhana May 14 at 0:35
  • @DavidReed Read the code again--it has problems, and thus is not suitable for CRSE. It definitely belongs here as is. – sfdcfox May 14 at 3:36
3

Your code has a number of problems. We'll walk through those, then get back to the main question of bulkification.


// -------- Constants --------

No, these are not constants. That would imply that they are unchanging, such as Math.PI. I realize that you're new, so now is the time to get your terminology correct. Those are simply static variables.


private ...

The default access mode for Apex is private. There's no need to declare this every time. While not strictly a problem, it's rather unusual for this to actually be written out.


private static ...

This is a bad idea. This becomes memory that cannot be deallocated until the end of the transaction, unless you manually do so, which could cause problems later. Prefer local variables when possible. Also, these variables show exactly why you're having a problem; you can't distinguish between processing one lead and another with the way your data is structured. There is a better way.


private static AssignLeadActionResult assignLead(AssignLeadActionRequest request){ 

This method can be called in a loop, so you can end up with too many SOQL queries, etc.


if(repList != null ...
if(leadList!=null ...

A query will never be null.


            if(leadRecord.Lead_Round_Robin__c != null){

Is this an auto-number field? I presume it also would never be null.


Future_LeadRoundRobinAssignment.notifyLeadOwner(leadRecord.id,leadRecord.OwnerId);

Future methods are limited to 50/transaction. Just to be safe, I'd bulkify this.


So, to bulkify our code, we use a design pattern I've coined the "Aggregate Query Result" pattern. This is a very common theme in Apex, so you'll want to learn it. To do this, we first aggregate the data we want to query on, then we query for that data, and finally we do any updates.


Here's my trimmed down version, untested, but should be very close to what you're looking for.

public with sharing class leadRoundRobinAssignment{
    @InvocableMethod 
    public static List<AssignLeadActionResult> assignLeads(List<AssignLeadActionRequest> requests){ 
        List<AssignLeadActionResult> results = new List<AssignLeadActionResult>();
        Set<Id> queueIds = new Set<Id>(), leadIds = new Set<Id>(), userIds = new Set<Id>();
        Lead[] updatedRecords = new Lead[0];
        Map<Id, Id[]> queueUsers = new Map<Id, Id[]>();

        // --- AGGREGATE DATA PHASE --- //
        // Get all queue Ids and lead Ids.
        for(AssignLeadActionRequest request: requests) {
           queueIds.addAll(request.queueIDs);
           leadIds.addAll(request.leadIDs);
        }
        // Initialize queue->user Map
        for(Id queueId: queueIDs) {
            queueUsers.put(queueId, new Id[0]);
        }
        // --- QUERY DATA PHASE --- //
        // Get all leads present
        Map<Id, Lead> leads = new Map<Id, Lead>([SELECT Lead_Round_Robin__c FROM Lead WHERE Id = :leadIds]);
        // Get all queue members
        for(GroupMember member: [SELECT UserOrGroupId, GroupId FROM GroupMember WHERE GroupId = :queueIds]) {
            if(member.UserOrGroupId.getSObjectType() == User.sobjectType) {
                userIds.add(member.UserOrGroupId);
                queueUsers.get(member.GroupId).add(member.UserOrGroupId);
            }
        }
        // Eliminate inactive users
        for(User record: [SELECT Id FROM User WHERE IsActive = false AND Id = :userIds]) {
            userIds.remove(record.Id);
        }
        // --- UPDATE DATA PHASE --- //
        // Rebuild Queue users
        for(Id queueId: queueUsers.keySet()) {
            Id[] tempUserIds = queueUsers.get(queueId);
            Set<Id> activeUsers = new Set<Id>(tempUserIds);
            activeUsers.retainAll(userIds);
            tempUserIds.clear();
            tempUserIds.addAll(activeUsers);
        }
        // For each request, figure out possible assignees
        for(AssignLeadActionRequest request: requests) {
            Set<Id> leadQueueUsers = new Set<Id>();
            for(Id queueId: request.queueIds) {
                leadQueueUsers.addAll(queueUsers.get(queueId));
            }
            Id[] repAssignList = new List<Id>(leadQueueUsers);
            repAssignList.sort(); // For consistency
            // For each lead, assign active user
            for(Id leadId: request.leadIds) {
                Lead tempLead = leads.remove(leadId);
                if(tempLead != null) {
                    if(repAssignList.isEmpty()) {
                        // No possible assignments, report error.
                    } else {
                        tempLead.OwnerId = repAssignList[Math.mod( Integer.valueOf(tempLead.Lead_Round_Robin__c),repAssignList.size())];
                        updatedRecords.add(tempLead);
                    }
                } else {
                    // Oops, duplicate lead in same transaction, report error.
                }
            }
        }
        update updatedRecords;
        // Don't forget to notify users...

        return results;
    } 

    public class AssignLeadActionRequest {
        @InvocableVariable(required = false)
        public List<ID> queueIDs;

        @InvocableVariable(required = false)
        public List<Id> leadIDs;

    }

    public class AssignLeadActionResult {
        @InvocableVariable(required = false)
        public List<Id> leadIDs;
    }
}

Note: Some features intentionally left out; the main point was to illustrate bulkification in a relatively simple way.

  • Thank you for the amazing feedback! To answer your questions: 1) Yes, the Lead Round Robin field is an auto number which is referenced when evenly distributing Lead to users within the queue 2) The notification is housed in the future method which creates an AgentWork (Omni-Channel) item which worked before I started the "clean up". However, I will leverage what you provided here to get me going. Lastly, what features have you intentionally left out? Just so I'm aware of where I'm headed. – Larry Caper May 14 at 14:53
  • @LarryCaper Mostly regarding the error reporting bits (they're commented as such). It shouldn't be much, just letting you know this code is definitely not "production quality." – sfdcfox May 14 at 15:24
  • Gotcha! Thank you. I've created a test class for the previous version of the code which passed the following Lead Assignment scenarios: 1 Lead + 1 Queue + 1 User, 1 Queue + 2 Users + 2 Leads (both assigned to Queue 1), 2 Queue + 2 Users + 2 Leads (1 assigned to Queue 2 and 1 assigned to Queue 2) and Bulk Test. I should be able to let you know if it works shortly! – Larry Caper May 14 at 15:29
  • @LarryCaper What was the error? – sfdcfox May 14 at 18:00
  • I fixed the error, in the code you have "ketSet" vs "keySet". However, no big deal. However, the issue now is the class is not showing up in my list of Apex Actions (within Flow). – Larry Caper May 14 at 20:05

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.