Can I have multiple transactions within 1 Schedulable Class?

Question

I have a Schedulable class that does multiple rollups from my Opportunities to their respective Accounts. This seems to work fine until recently I noticed some issues.

Essentially my Schedulable class is laid out like this:

Updated Added Complete Class

global class ScheduledOpportunityToAccountRollup implements Schedulable {

    /* Unique list of accounts to be rolled up */
    Set<String> account_list = new Set<String>();
    /* Unique list of unpaid invoice opportunities to be rolled up */
    Set<AggregateResult> unpaid_invoices = new Set<AggregateResult>();

    global void execute(SchedulableContext sc) {
        scheduleAccountsRevenueRollup();
        scheduleAveragePastDueInvoiceRollup();
    }

    private void scheduleAccountsRevenueRollup(){
        getAccountRevenueList();
        if(!account_list.isEmpty()) OpportunityUtils.rollupAccountRevenues(account_list);
    }

    private void scheduleAveragePastDueInvoiceRollup(){
        getAccountUnpaidInvoiceSet();
        if(!unpaid_invoices.isEmpty()) OpportunityUtils.rollupAverageDaysPastDue(unpaid_invoices);
    }

    /**
     * GET ACCOUNT REVENUE LIST
     * Opportunities must have Labor Services and either be paid or past due in order to be included in this scheduled batch
     */
    private void getAccountRevenueList(){
        List<Account> accounts = new List<Account>([SELECT Id FROM Account WHERE Total_Balance2__c != 0 OR Current_or_Previous_Customer__c != null]);
        for(Account a : accounts){
            account_list.add(a.Id);
        }
    }

    /**
     * GET ACCOUNT UNPAID INVOICE SET
     * Opportunities must have Labor Services past due in order to be included in this scheduled batch
     */
    private void getAccountUnpaidInvoiceSet(){
        unpaid_invoices = new Set<AggregateResult>([
            SELECT AccountId, COUNT(Id) Invoices, AVG(Days_Past_Due__c) Average, MAX(Days_Past_Due__c) Max
            FROM Opportunity
            WHERE IsDeleted = FALSE 
            AND Invoice_Num__c != '' 
            AND Is_Paid__c = FALSE
            GROUP BY AccountId 
        ]);
    }

}

When this runs I have issue with some accounts not being rolled up in the 2nd method scheduleAveragePastDueInvoiceRollup(). So when troubleshooting I removed the first one scheduleAccountsRevenueRollup() and this time everything in scheduleAveragePastDueInvoiceRollup() worked exactly as expected.

SO, question is, am I not allowed to run multiple methods that each have their own transactions? At the moment I'm not seeing script errors from any governing limits, just finding issues on accounts that aren't being rolled up.

Answer 1

A single schedulable function is a single transaction. If you really wanted to isolate the functions from each other, you could use a schedulable batchable class. Here's an implementation:

public class ScheduledOpportunityToAccountRollup implements Schedulable, Database.Batchable<Integer> {
    public void execute(SchedulableContext sc) {
        Database.executeBatch(this, 1);
    }

    public Integer[] start(Database.BatchableContext context) {
        return new Integer[] { 1, 2 };
    }

    public void execute(Database.BatchableContext context, Integer[] scope) {
        if(scope[0] == 1) {
            scheduleAccountsRevenueRollup();
        } else {
            scheduleAveragePastDueInvoiceRollup();
        }
    }

    public void finish(Database.BatchableContext context) {
    }
    // Don't forget to include your other functions here
}

General Optimizations

Don't use class-level variables when they belong in a function. Use return values if you really want to split your function up into smaller parts. For example, you could write:

Set<Id> getAccountRevenueList(){
    return new Map<Id, Account>(
        [SELECT Id FROM Account WHERE Total_Balance2__c != 0 OR Current_or_Previous_Customer__c != null]
    ).keySet();
}

We can use the Map(SObject[]) constructor to automatically get the ID values from the query without a loop or even a temporary variable.

From there, you can simply:

void scheduleAccountsRevenueRollup() {
    OpportunityUtils.rollupAccountRevenues(
        getAccountRevenueList()
    );
}

This method shouldn't be responsible for seeing if the list is empty. Your utility class should be willing to accept an empty collection and return early if it has nothing to do.

Now, we've doubled up on your governor limits (because each phase runs in a separate transaction, and guaranteed to be in order), and we've prevented exceptions from one function harming the other-- this is no longer an all or nothing transaction.

Finally, note that I've dropped the global values to public (because there's no reason for this class to be global), and dropped the "private" keyword on the functions that are used locally, since the language automatically makes unmarked functions and variables private.

---

As far as why "some things may not be working" correctly, the AggregateResult query only returns rows that match at least one record. If there are zero values, they won't be included, so you necessarily need two parameters to your utility class: the list of all ID values that you expect, and the AggregateResult collection from the query.