Roll Up Field of Open & Closed Activities on Account - Using an APEX class

Question

The use case is that I am trying to have a roll-up field on the account object that is a sum of the total # of Open and Closed activities on the account. This would be easy with a Trigger, but I want to limit the technical debt in my org.

What I have is just 1 Trigger on the Account. Here is the code for the trigger, AccountTrigger.

trigger AccountTrigger on Account (before insert, before update, before delete, after insert, after update, after delete, after undelete) {

if(Trigger_Settings__c.getInstance().Disable_All_Triggers__c){
    return;
}

AccountTriggerHandler handler = new AccountTriggerHandler();

if (Trigger.isBefore) {
    if(Trigger.isInsert){
        handler.OnBeforeInsert(Trigger.new);
    }
    else if(Trigger.isUpdate){
        handler.OnBeforeUpdate(Trigger.old,Trigger.new,trigger.newMap, Trigger.oldMap);
    }
    else if(Trigger.isDelete){
        handler.OnBeforeDelete(trigger.old,Trigger.newMap, Trigger.oldMap);
    }    
} 
else if (Trigger.isAfter) {
    if(Trigger.isInsert){
        handler.OnAfterInsert(Trigger.new,trigger.newMap);
    }
    else if(Trigger.isUpdate){
        handler.OnAfterUpdate(Trigger.old,Trigger.new,trigger.newMap, Trigger.oldMap);
    }
    else if(Trigger.isDelete){
        handler.OnAfterDelete(trigger.old, Trigger.oldMap);
    }    
}}

The AccountTrigger calls an APEX class called AccountTriggerHandler. This class has methods for beforeInsert, beforeUpdate, ect. so that way those methods just call other APEX classes. Here is the code for it.

public with sharing class AccountTriggerHandler {

public static boolean firstRunOnBeforeInsert = true;
public static boolean firstRunOnAfterInsert = true;
public static boolean firstRunOnBeforeUpdate = true;
public static boolean firstRunOnAfterUpdate = true;
public static boolean firstRunOnBeforeDelete = true;
public static boolean firstRunOnAfterDelete = true;
public static boolean firstRunOnUndelete = true;


public void OnBeforeInsert(Account[] newObjects){
    if(firstRunOnBeforeInsert){
        firstRunOnBeforeInsert = false;
    }
}

public void OnAfterInsert(Account[] newObjects, map<id,Account> MapNewMap){
    if(firstRunOnAfterInsert){
        firstRunOnAfterInsert = false;

        ActivityUtils au = new ActivityUtils.updateAccountActivityCount();
    }
}

public void OnBeforeUpdate(Account[] oldObjects, Account[] updatedObjects, map<id,Account> MapNewMap, map<id,Account> MapOldMap){
    if(firstRunOnBeforeUpdate){
        firstRunOnBeforeUpdate = false;
    }
}

public void OnAfterUpdate(Account[] oldObjects, Account[] updatedObjects, map<id,Account> MapNewMap, map<id,Account> MapOldMap){
    if(firstRunOnAfterUpdate){
        firstRunOnAfterUpdate = false;
        //Query accounts to be used in all of the future methods.
        Map<Id,Account> oppMap = new Map<Id,Account>([
            SELECT Id 
            FROM Account
            WHERE Id in :MapNewMap.keyset()
        ]);
        //SendemailController.sendInvoice(updatedObjects, MapOldMap);
    }
}

public void OnBeforeDelete(Account[] ObjectsToDelete, map<id,Account> MapNewMap, map<id,Account> MapOldMap){
    if(firstRunOnBeforeDelete){
        firstRunOnBeforeDelete = false;
    }
}

public void OnAfterDelete(Account[] deletedObjects, map<id,Account> MapOldMap){
    if(firstRunOnAfterDelete){
        firstRunOnAfterDelete = false;
    }
}

public void OnUndelete(Account[] restoredObjects){
    if(firstRunOnUndelete){
        firstRunOnUndelete = false;
    }
}}

I have a 3rd APEX class called ActivityUtils which has the code for pulling in the open and closed activities and pushing them to 2 different fields, on multiple objects. This was a solution I found online on this blog. Ideally, I would just want to pull in both the count of the open and closed activities only on the Account object, and push them to 1 field Count_of_Activities__c.

public class ActivityUtils {

//config

String fieldToUpdate = 'NVM_Activities__c'; //this field must be added to each object we're updating
String fieldOpenToUpdate = 'Count_of_Activities__c'; //this field must be added to each object we're updating

//state
set<id> accountIds;
set<id> contactIds;
set<id> opportunityIds;
set<id> leadIds;
set<id> caseIds;

public ActivityUtils(sObject[] records) {
    accountIds = new set<id>();
    contactIds = new set<id>();
    opportunityIds = new set<id>();
    leadIds = new set<id>();
    caseIds = new set<id>();
    captureWhatAndWhoIds(records);
    addAccountIdsFromRlatedObjects();
}

public void updateAccountActivityCount() {
    if(accountIds.size() == 0) return;
    updateActivityCount('Account','WhatId', getStringFromIdSet(accountIds));
     updateActivityHistory('Account','WhatId', getStringFromIdSet(accountIds));


}
public void updateContactActivityCount() {
    if(contactIds.size() == 0) return;
    updateActivityCount('Contact','WhoId', getStringFromIdSet(contactIds));
            updateActivityHistory('Contact','WhoId', getStringFromIdSet(contactIds));

}
public void updateOpportunityActivityCount() {
    if(opportunityIds.size() == 0) return;
    updateActivityCount('Opportunity','WhatId', getStringFromIdSet(opportunityIds));
            updateActivityHistory('Opportunity','WhatId', getStringFromIdSet(opportunityIds));

}
public void updateLeadActivityCount() {
    if(leadIds.size() == 0) return;
    updateActivityCount('Lead','WhoId', getStringFromIdSet(leadIds));
            updateActivityHistory('Lead','WhoId', getStringFromIdSet(leadIds));

}
public void updateCaseActivityCount() {
    if(caseIds.size() == 0) return;
    //updateActivityCount('Case','WhoId', getStringFromIdSet(caseIds));
    //updateActivityHistory('Case','WhoId', getStringFromIdSet(caseIds));

}
private void updateActivityCount(String objToUpdate, String queryFld, String updateIds) {
    string strQuery = 'SELECT Id, (SELECT Id FROM OpenActivities) FROM ' + objToUpdate + ' WHERE Id IN (' + updateIds + ')';
    sObject[] sobjects = new list<sobject>();
    for(sObject so : database.query(strQuery)) {
        OpenActivity[] oActivities = so.getSObjects('OpenActivities');
        Integer openActivityCount = oActivities == null ? 0 : oActivities.size();
        sObject obj = createObject(objToUpdate, so.Id);
        obj.put(fieldOpenToUpdate, openActivityCount);
        sobjects.add(obj);
        system.debug('openActivityCount: ' + openActivityCount);
    }
    update sobjects;
}

private void updateActivityHistory(String objToUpdate, String queryFld, String updateIds) {
    string strQuery = 'SELECT Id, (SELECT Id FROM ActivityHistories) FROM ' + objToUpdate + ' WHERE Id IN (' + updateIds + ')';        
    System.debug(strQuery);
    sObject[] sobjects = new list<sobject>();
    for(sObject so : database.query(strQuery)) {
        ActivityHistory[] oActivities = so.getSObjects('ActivityHistories');
        Integer closedActivityCount = oActivities == null ? 0 : oActivities.size();
        sObject obj = createObject(objToUpdate, so.Id);
        obj.put(fieldToUpdate, closedActivityCount);
        sobjects.add(obj);
        system.debug('ActivityHistoryCount: ' + closedActivityCount);
    }
    update sobjects;
}

private void captureWhatAndWhoIds(sObject[] objects) {
    for(sObject o : objects) {
        Id whatId = (Id)o.get('WhatId');
        Id whoId = (Id)o.get('WhoId');
        if(whatId != null) {
            String objectName = getObjectNameFromId(whatId);
            if(objectName == 'account') accountIds.add(whatId);
            if(objectName == 'opportunity') opportunityIds.add(whatId);
        }
        if(whoId != null) {
            String objectName = getObjectNameFromId(whoId);
            if(objectName == 'contact') contactIds.add(whoId);
            if(objectName == 'lead') leadIds.add(whoId);
        }
    }
}

private void addAccountIdsFromRlatedObjects() {
    for(Opportunity o : [SELECT AccountId FROM Opportunity WHERE Id IN :opportunityIds]) accountIds.add(o.AccountId);
    for(Contact c : [SELECT AccountId FROM Contact WHERE Id IN :contactIds]) accountIds.add(c.AccountId);
}

private String getObjectNameFromId(Id objId) {
    String preFix = String.valueOf(objId).left(3).toLowercase();
    if(prefix == '001') return 'account';
    if(prefix == '003') return 'contact';
    if(prefix == '006') return 'opportunity';
    if(prefix == '00q') return 'lead';
    //if(prefix == '500') return 'case';
    return '';
}

private String getStringFromIdSet(set<id> idSet) {
    string idString = '';
    for(Id i : idSet) idString+= '\'' + i + '\',';
    return idString == '' ? idString : idString.left(idString.length()-1); //If idString contains some ids we want to ensure we strip out the last comma
}

//The main part of the method below was taken from //Taken from http://www.salesforce.com/us/developer/docs/apexcode/Content/apex_dynamic_dml.htm
//However we've modified this to accept an object id
private sObject createObject(String typeName, Id objId) {
    Schema.SObjectType targetType = Schema.getGlobalDescribe().get(typeName);
    if (targetType == null) {
        // throw an exception
    }

    // Instantiate an sObject with the type passed in as an argument
    //  at run time.
    return targetType.newSObject(objId);
}
} 

The reason I'm going for this solution is becuase it seemed simple and scalable. The issue I'm running into now is whenever I try and edit an Account to test this functionality, I get this error.

Error:Apex trigger AccountTrigger caused an unexpected exception, contact your administrator: AccountTrigger: execution of AfterUpdate caused by: System.SObjectException: Invalid field WhatId for Account: ()

Why would the variable WhatId be causing this? Is there a simpler solution to what I'm trying to do, and if so what is it?

Answer 1

Looks like those records should be the list of activity history records. Could probably stand to remove them but Im not sure how they tie into the rest of the code. It kind of seems like this code was written to expect the raw activity history to be passed over, so it could get the related object ids from those records. It makes more sense for this use case to instead pass along whatever records are in your trigger context via caller or constructor.

Heres a total rewrite of your class. The primary difference it all the method are now static. Theres no instance values kept between methods. You pass your ids into the class using the same method, but this method has an Id list as a parameter. We keep passing along that list instead of storing it anywhere. The rest of the class is mostly the same, you query the data, iterate over the records, create new instances of the accounts, and then update those values.

public class ActivityUpdate {
    // If a constructor were used, putting these as parameters makes a lot of sense 
    private static String fieldToUpdate = 'NVM_Activities__c'; //this field must be added to each object we're updating
    private static String fieldOpenToUpdate = 'Count_of_Activities__c'; //this field must be added to each object we're updating

    // Could easily be an @future job, literally wouldn't have to change anything but adding the annotation 
    public static void updateAccountActivityCount(List<Id> accountIds) {
        if(accountIds.size() == 0) return;
        // Could easily add parameters for type, field, etc to make this method usable for other object types 
        updateActivityCount('Account','WhatId', getStringFromIdSet(accountIds));
        updateActivityHistory('Account','WhatId', getStringFromIdSet(accountIds));
    }

    private static void updateActivityCount(String objToUpdate, String queryFld, String updateIds) {
        string strQuery = 'SELECT Id, (SELECT Id FROM OpenActivities) FROM ' + objToUpdate + ' WHERE Id IN (' + updateIds + ')';

        sObject[] sobjects = new list<sobject>();

        for(sObject so : database.query(strQuery)) {
            OpenActivity[] oActivities = so.getSObjects('OpenActivities');

            Integer openActivityCount = oActivities == null ? 0 : oActivities.size();
            sObject obj = createObject(so.Id);

            obj.put(fieldOpenToUpdate, openActivityCount);

            sobjects.add(obj);
        }

        update sobjects;
    }

    private static void updateActivityHistory(String objToUpdate, String queryFld, String updateIds) {
        string strQuery = 'SELECT Id, (SELECT Id FROM ActivityHistories) FROM ' + objToUpdate + ' WHERE Id IN (' + updateIds + ')';

        sObject[] sobjects = new list<sobject>();

        for(sObject so : database.query(strQuery)) {
            ActivityHistory[] oActivities = so.getSObjects('ActivityHistories');

            Integer closedActivityCount = oActivities == null ? 0 : oActivities.size();
            sObject obj = createObject(so.Id);

            obj.put(fieldToUpdate, closedActivityCount);

            sobjects.add(obj);
        }

        update sobjects;
    }

    private static String getStringFromIdSet(set<id> idSet) {
        string idString = '';
        for(Id i : idSet) idString+= '\'' + i + '\',';
        return idString == '' ? idString : idString.left(idString.length()-1); //If idString contains some ids we want to ensure we strip out the last comma
    }

    private static sObject createObject(Id objId) {
        // Actually just need an Id, can access type & create new instance from id 
        return objId.getSObjectType().newInstance(objId);
    }
}

You should be able to call the first public static method like so:

ActivityUpdate.updateAccountActivityCount(MapNewMap.keySet());

I'd really recommend making it a bit more generic, since it seems like you were using this for a bunch of different types. Theres more info in the comments on how to do that. I'd do it, but I'm heading out to lunch!

You'll also want to keep recursion in mind (since you're updating the records which you are also currently updating in your trigger context), by maybe making this a @future job with a flag on the trigger handler to prevent recursion, or by moving this to a batch job instead of in the trigger.