1

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?

  • Check out DLRS. its a well known package that does roll-up summaries on related objects out of the box. You might be able to use just this package, and not have to write any code. – battery.cord Jul 10 '18 at 15:18
  • As for your code, it looks like somewhere in this code you're putting WhatId to an Account object, or querying a WhatId on an account object. Im digging into it now but Im pretty busy today. – battery.cord Jul 10 '18 at 15:19
  • I'll check out the package, and thanks for looking into this. I really appreciate it a lot! – Kruse in Salesforce Jul 10 '18 at 15:25
  • So its weird that you're getting this message on After Update when you don't actually call the activity check on after update, you call it on after insert. This line (ActivityUtils au = new ActivityUtils.updateAccountActivityCount();) also strikes me as odd - how does this compile? You don't have a no-arg constructor in your class. I don't even think thats a legal syntax (Id expect ActivityUtils() or ActivityUtils(records).) – battery.cord Jul 10 '18 at 15:28
  • Small nitpick Schema.getGlobalDescribe() is slow & bloated since it returns all values. Id either "cache" the result, or use Type.forName(typeName).newInstance() and set the id of the returned object before returning it as the result. – battery.cord Jul 10 '18 at 15:29
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.

  • Note: I did not try to compile this - there might be a few mistakes my IDE didnt catch. – battery.cord Jul 10 '18 at 16:27

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.