0

As a part of a larger project surrounding Apex triggers, I need to move what was functional code from a before update trigger on the lead object to a trigger handler called by a trigger on the Lead object.

While working through this I am struggling with how to best update OwnerID and Reassignment_Status__c (see the two if statements in the code below, I know that the LD variable does not exist).

In addition, I would like to get feedback regarding the overall structure of how this is written because:

  1. I'm concerned that the way that I am using the SELECT statements here may be inefficient

  2. I have another project upcoming in which the plan is to leverage after an after delete trigger handler to grab values from merged lead records and insert them into a custom object. How/whether I am able to address the issues with the handler I am posting about here will inform how I approach that task as well.

Below is my current code.

public void OnBeforeUpdate( List<Lead> newLead, List<Lead> oldLead, Map<ID, Lead> newLeadMap , Map<ID, Lead> oldLeadMap )
{
    system.debug('Lead Trigger On Before Update ');

    try
    {
        Set<Id> ldIdSet = new Set<Id>();    //Initialize set of LeadIDs
        List<Lead> ldList = new List<Lead>(); //Initialize list of lead records for update
        List<ProcessInstance> piList = new List<ProcessInstance>();   //Initialize list of approval process items


        ldList = [SELECT Id, OwnerID, Reassignment_Status__c FROM Lead WHERE Id IN :ldIdSet];
        piList = [SELECT SubmittedById,SystemModstamp,TargetObjectId, Status,  (SELECT ID, ProcessNodeId, StepStatus, Comments,TargetObjectId,ActorId,CreatedById,IsDeleted,IsPending, OriginalActorId,ProcessInstanceId,RemindersSent,CreatedDate 
        FROM StepsAndWorkitems ) FROM ProcessInstance pi WHERE pi.TargetObjectId IN :ldIdSet order by createddate DESC limit 1];

            if(ldList.size() > 0) {
                for(Lead l : ldList)
                {
                    ldIdSet.Add(l.Id);   //Add Ids from ldList to the LeadID set list
                }

                        If (ldList[0].Reassignment_Status__c == 'Pending' && piList[0].Status == 'Approved')   //Update Owner to the approval process requestor
                        {

                        LD.OwnerId = piList[0].SubmittedById;
                        LD.Reassignment_Status__c = '';
                        system.debug('Reassignment Approved - Owner Changed from '+ldList[0].OwnerId+'to ' +piList[0].SubmittedById);

                        }


                        If (ldList[0].Reassignment_Status__c == 'Pending' && piList[0].Status == 'Rejected') //Clear the reassignment status indicator, make no change to Owner


                            LD.Reassignment_Status__c = '';


                                system.debug('Reassignment Rejected - Owner not changed');

                                  }            
    }

    catch(Exception e)
    {
        system.debug('The following exception has occured:' +e.getMessage());
        system.debug('The following Line:' + e.getLineNumber());
    }   

}
5

(Please, excuse the indentation in the code snippets)

Because you are passing the Lists of sObjects to the handler, they are passed by reference and the only thing you need to do to update them is modify the fields you are interested in (just like you are currently doing using the equal sign for assignment). The Salesforce execution framework and the before update trigger will reference the new values. Your code has some issues which I will point out in the following code snippet.

See code comments:

public void OnBeforeUpdate( List<Lead> newLead, List<Lead> oldLead, Map<ID, Lead> newLeadMap , Map<ID, Lead> oldLeadMap )
{

    system.debug('Lead Trigger On Before Update ');

    try
    {
    Set<Id> ldIdSet = new Set<Id>();  //Why do you need a Set? pass the newLead List directly. You are missing the reference to the records coming from the trigger!!!!
    List<Lead> ldList = new List<Lead>(); // call this leadList, cleaner to read
    List<ProcessInstance> piList = new List<ProcessInstance>(); 


    ldList = [SELECT Id, OwnerID, Reassignment_Status__c FROM Lead WHERE Id IN :ldIdSet]; //ldIdSet IS EMPTY!!! This query will retun 0 records
    piList = [SELECT SubmittedById,SystemModstamp,TargetObjectId, Status,  (SELECT ID, ProcessNodeId, StepStatus, Comments,TargetObjectId,ActorId,CreatedById,IsDeleted,IsPending, OriginalActorId,ProcessInstanceId,RemindersSent,CreatedDate 
    FROM StepsAndWorkitems ) FROM ProcessInstance pi WHERE pi.TargetObjectId IN :ldIdSet order by createddate DESC limit 1]; //ldIdSet IS EMPTY!!! This query will retun 0 records

        if(ldList.size() > 0) { //Because of the query before, this will never evaluate to true (END OF TRY BLOCK)
            for(Lead l : ldList) // don't name a variable with a single letter, not good practice in long code classes
            {
                ldIdSet.Add(l.Id);   //Too late, your set was already referenced in the queries when it was empty
            }

                    If (ldList[0].Reassignment_Status__c == 'Pending' && piList[0].Status == 'Approved')   // Huge issue, all your helper logic assumes only one record. If 20 records come through they will update that same record 20 times. Everything from here on has this same issue
                    {

                    LD.OwnerId = piList[0].SubmittedById;
                    LD.Reassignment_Status__c = '';
                    system.debug('Reassignment Approved - Owner Changed from '+ldList[0].OwnerId+'to ' +piList[0].SubmittedById);

                    }


                    If (ldList[0].Reassignment_Status__c == 'Pending' && piList[0].Status == 'Rejected') 


                    LD.Reassignment_Status__c = '';


            system.debug('Reassignment Rejected - Owner not changed');

    }            
}

catch(Exception e)
{
    system.debug('The following exception has occured:' +e.getMessage());
    system.debug('The following Line:' + e.getLineNumber());
}   

}

Here is my version of the same code leveraging maps to tie Lead to ProcessInstance.

public void OnBeforeUpdate( List<Lead> newLead, List<Lead> oldLead, Map<ID, Lead> newLeadMap , Map<ID, Lead> oldLeadMap )
{

    system.debug('Lead Trigger On Before Update ');

    try
    {
    Map<Id,ProcessInstance> leadIdToPi = new Map<Id,ProcessInstance>(); 


    Map<Id,Lead> leadMap = new Map<Id,Lead>([SELECT Id, OwnerID, Reassignment_Status__c FROM Lead WHERE Id IN :newLead]); 
    List<ProcessInstance> piList = [SELECT SubmittedById,SystemModstamp,TargetObjectId, Status,  (SELECT ID, ProcessNodeId, StepStatus, Comments,TargetObjectId,ActorId,CreatedById,IsDeleted,IsPending, OriginalActorId,ProcessInstanceId,RemindersSent,CreatedDate 
    FROM StepsAndWorkitems ) FROM ProcessInstance pi WHERE pi.TargetObjectId IN :leadMap.keySet() order by createddate DESC limit 1]; 

    for(ProcessInstance processInstance : piList){
        leadIdToPi.put(processInstance.TargetObjectId,processInstance);
    }

    for(Lead lead : leadList){
        ProcessInstance processInstance = leadIdToPi.get(lead.Id);
        if (lead.Reassignment_Status__c == 'Pending' && processInstance.Status == 'Approved')   
        {

        lead.OwnerId = processInstance.SubmittedById;
        lead.Reassignment_Status__c = '';
        system.debug('Reassignment Approved - Owner Changed from '+lead.OwnerId+'to ' +processInstance.SubmittedById);

        }


        if (lead.Reassignment_Status__c == 'Pending' && processInstance.Status == 'Rejected') 


        lead.Reassignment_Status__c = '';


        system.debug('Reassignment Rejected - Owner not changed');
    }




    }            
}

catch(Exception e)
{
    system.debug('The following exception has occured:' +e.getMessage());
    system.debug('The following Line:' + e.getLineNumber());
}   

}

Hope this helps.

  • Nice. I'd add that you could remove the try/catch block because you WANT to find out when things don't work! Otherwise they'll fail silently and you'll never know unless you happen to have the developer console open. – DavidSchach Sep 6 '19 at 2:22
  • Thank you very much Glen for the feedback. I see now how badly I had butchered this. I had to make a few tweaks to get it working exactly the way I wanted but your feedback was a life saver. And thank you David re: the try/catch, that is a good call. When it comes to iterating through records in these lists, am I understanding correctly that it is essentially the same process? In this instance I will typically only be updating a single record at a time when an approval process is completed, but I have a need to do some more complex work with an after delete trigger. – Brandon Stacy Sep 6 '19 at 2:36

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.