Flow with invocable apex

Question

Apex beginner here so I'm open to any and all suggestions. I'm working on a flow + invocable apex combination to mass reassign records. Basically trying to take variables obtained by the flow and pass them along to apex. I have the flow querying and storing a couple thousand account records in a record collection and then apex is updating this collection of accounts with a new owner. It's working when a small number of records need to be reassigned, but I get the following error when trying to update a couple of thousand records - "An Apex error occurred: System.LimitException: Apex CPU time limit exceeded". I'm looking to see where I'm going wrong. I believe it has to do with the nested loop and I understand a Map might be my solution, but I'm unsure exactly how to make the adjustment.

Here's the class with the Invocable Variables that are getting set via the flow:

public class TerritoryAssignmentVariables {

@InvocableVariable
public List<Account> accounts;

@InvocableVariable
public String newOwnerId;}

And here's another class I've setup for updating the records based on the above variables:

public class TerritoryAssignmentClass {

@InvocableMethod(label = 'Update Records')
public static void updateAccounts(List<TerritoryAssignmentVariables> oldValues){
    
List<Account> Updatedaccounts = new List<Account>();
    
    for(TerritoryAssignmentVariables value : oldValues){
        for(Account acc : value.accounts){
            acc.OwnerId = value.newOwnerId;
            Updatedaccounts.add(acc);
        }
    }
    update Updatedaccounts;
}

}

Does anyone have any tips or suggestions on this? I'd appreciate any assistance!

Answer 1

There's nothing inherently wrong with this code as it stands. You may have heard somewhere that "nested loops" are a Bad Thing, and that you Absolutely Need a Map, but this just Bad Advice. You know you need a map when you specifically have the following pattern:

SomeObject1[] list1 = "list of records here";
SomeObject2[] list2 = "list of other records here";
for(SomeObject1 item1: list1) {
  for(SomeObject2 item2: list2) {
    if(item1.someField == item2.someOtherField) {
      doSomethingWith(item1, item2);
    }
  }
}

This is the only time when you Absolutely Need a Map.

What you need is more time. You may have workflow rules, triggers, processes, etc that are triggered by updating those records, and you simply can't do it all in the 10,000ms (10 seconds) of CPU time you have available to you.

Either reconstruct your flow to not try and do everything in one pass (e.g. limit it to just X number of accounts/owners) with a screen in between each update, or consider adding Queueable to your code so you can offload the process to a separate process:

public class TerritoryAssignmentClass extends Queueable{
    @InvocableMethod(label = 'Update Records')
    public static void updateAccounts(List<TerritoryAssignmentVariables> oldValues){
        List<Account> Updatedaccounts = new List<Account>();
        for(TerritoryAssignmentVariables value : oldValues){
            for(Account acc : value.accounts){
                acc.OwnerId = value.newOwnerId;
                Updatedaccounts.add(acc);
            }
        }
        System.enqueueJob(new TerritoryAssignmentClass(Updatedaccounts));
    }
    // State to update records //
    Account[] scope;
    TerritoryAssignmentClass(Account[] scope) {
        this.scope = scope;
    }
    // Queueable Context execution //
    public void execute(QueueableContext ctx) {
        update scope;
    }
}

This will give your code 60,000ms/one minute to update the records, which should probably be plenty. If not, you can adjust your code to chain itself based on the load:

    public void execute(QueueableContext ctx) {
        Account[] accountsToUpdate = new Account[0];
        // Adjust 100 as needed //
        while(accountsToUpdate.size()<100 && !scope.isEmpty()) {
            accountsToUpdate.add(scope.remove(0));
        }
        update accountsToUpdate;
        // We have more work to do, so enqueue again //
        if(!scope.isEmpty()) {
            System.enqueueJob(this);
        }
    }

Answer 2

Updated class, including the 'adjustable' block. You'll see I had to use 'Implements' when declaring queueable in the first line. Using 'extends' was not letting me save the class.

public class TerritoryAssignmentClass Implements queueable{
    @InvocableMethod(label = 'Update Records')
    public static void updateAccounts(List<TerritoryAssignmentVariables> oldValues){
        List<Account> Updatedaccounts = new List<Account>();
        for(TerritoryAssignmentVariables value : oldValues){
            for(Account acc : value.accounts){
                acc.OwnerId = value.newOwnerId;
                Updatedaccounts.add(acc);
            }
        }
        System.enqueueJob(new TerritoryAssignmentClass(Updatedaccounts));
    }
    // State to update records //
    Account[] scope;
    TerritoryAssignmentClass(Account[] scope) {
        this.scope = scope;
    }
    // Queueable Context execution //
    public void execute(QueueableContext ctx) {
        Account[] accountsToUpdate = new Account[0];
        // Adjust 100 as needed //
        while(accountsToUpdate.size()<10 && !scope.isEmpty()) {
            accountsToUpdate.add(scope.remove(0));
        }
        update accountsToUpdate;
        // We have more work to do, so enqueue again //
        if(!scope.isEmpty()) {
            System.enqueueJob(this);
        }
    }
}