1

I am still very new to Apex coding and am working on a Trigger that creates or updates a record when a custom object (Support Comment) record is created or edited. The trigger works as intended except when doing bulk updates or creation.

I believe my issue is due to a lack of bulkification, but I am still learning this concept. Any help would be much appreciated.

Edited Trigger:

trigger createCaseComment2 on Support_Comment__c (after insert, after update) {

Id Id;
Id ccId;
string commentBody = null;

List<Support_Comment__c> supportCommentsToUpdate = new List<Support_Comment__c>();
List<CaseComment> caseCommentsToInsert = new List<CaseComment>();
List<CaseComment> caseCommentsToUpdate = new List<CaseComment>();
List<CaseComment> caseCommentsToDelete = new List<CaseComment>();

Map<Id, CaseComment> supportCommentIdToCaseComments = new Map<Id, CaseComment>();

if(Trigger.isInsert){
    for (Support_Comment__c newSupportComment : Trigger.new) {
        Id = newSupportComment.Id;

        System.debug('The newSupportComment Id = ' + Id);
        if (newSupportComment.Comment__c != null && newSupportComment.Community_User__c == 'Standard User' && newSupportComment.Public__c == true){
            if (stoprecurssion.runonce()){

                CaseComment cComment = new CaseComment(ParentId = newSupportComment.Case__c, CommentBody = newSupportComment.Comment__c, IsPublished = TRUE);
                caseCommentsToInsert.add(cComment);

                supportCommentIdToCaseComments.put(newSupportComment.Id, cComment);
            }
        }
    }
    insert caseCommentsToInsert;

    for (Support_Comment__c newSupportComment : Trigger.new) {
        if (newSupportComment.Comment__c != null && newSupportComment.Community_User__c == 'Standard User' && newSupportComment.Public__c == true){

            ccId = supportCommentIdToCaseComments.get(newSupportComment.Id).Id;
            System.debug('The new standard Comment Id = ' + ccId);

            Support_Comment__c supComsToUpdate = new Support_Comment__c(Id = newSupportComment.Id, Standard_Case_Comment_Id__c = ccId);
            supportCommentsToUpdate.add(supComsToUpdate);
        }
    }

    update supportCommentsToUpdate;
}
if(Trigger.isUpdate) {
    for (Support_Comment__c updatedSupComPrivate : Trigger.new) {
        Id = updatedSupComPrivate.Id;
        Support_Comment__c oldSupportComment = Trigger.oldMap.get(Id);
        String oldComBody = oldSupportComment.Comment__c;
        Boolean isPub = oldSupportComment.Public__c;
        String cComId = oldSupportComment.Standard_Case_Comment_Id__c;

            if (oldSupportComment.Public__c == FALSE && updatedSupComPrivate.Public__c == TRUE && updatedSupComPrivate.Standard_Case_Comment_Id__c == null) {
                if (stoprecurssion.runonce()) {

                CaseComment cCommentAdd = new CaseComment(ParentId = updatedSupComPrivate.Case__c, CommentBody = updatedSupComPrivate.Comment__c, IsPublished = TRUE);
                caseCommentsToInsert.add(cCommentAdd);

                supportCommentIdToCaseComments.put(updatedSupComPrivate.Id, cCommentAdd);
                }
            }
        }
        insert caseCommentsToInsert;

        for (Support_Comment__c updatedSupComPrivate : Trigger.new) {
            if (updatedSupComPrivate.Public__c == TRUE && updatedSupComPrivate.Standard_Case_Comment_Id__c == null) {

                ccId = supportCommentIdToCaseComments.get(updatedSupComPrivate.Id).Id;
                System.debug('The new standard Comment Id = ' + ccId);

                Support_Comment__c supComsToUpdate = new Support_Comment__c(Id = updatedSupComPrivate.Id, Standard_Case_Comment_Id__c = ccId);
                supportCommentsToUpdate.add(supComsToUpdate);
            }
        }
        update supportCommentsToUpdate;

    for (Support_Comment__c updatedSupportCommentPublic : Trigger.new) {
        Id = updatedSupportCommentPublic.Id;
        Support_Comment__c oldSupportComment = Trigger.oldMap.get(Id);
        String oldComBody = oldSupportComment.Comment__c;
        Boolean isPub = oldSupportComment.Public__c;
        String cComId = oldSupportComment.Standard_Case_Comment_Id__c;

        Support_Comment__c updatedComment = [SELECT Id, Comment__c, Standard_Case_Comment_Id__c, public__c FROM Support_Comment__c WHERE Id =:oldSupportComment.Id];

            if (updatedComment.Public__c == TRUE && oldSupportComment.Comment__c == oldComBody && oldSupportComment.Standard_Case_Comment_Id__c != null) { 

                CaseComment cCommentUpdate = [SELECT Id, ParentId, CommentBody FROM CaseComment WHERE ParentId =:oldSupportComment.Case__c AND Id =:cComId];
                if (cCommentUpdate.Id == cComId) {

                    cCommentUpdate.CommentBody = updatedComment.Comment__c;
                    caseCommentsToUpdate.add(cCommentUpdate);
                }
            }
        }
        update caseCommentsToUpdate;

    for (Support_Comment__c supportCommentNowPrivate : Trigger.new) {
        Id = supportCommentNowPrivate.Id;
        Support_Comment__c oldSupportComment = Trigger.oldMap.get(Id);
        String oldComBody = oldSupportComment.Comment__c;
        Boolean isPub = oldSupportComment.Public__c;
        String cComId = oldSupportComment.Standard_Case_Comment_Id__c;

        Support_Comment__c privateComment = [SELECT Id, Comment__c, Standard_Case_Comment_Id__c, public__c FROM Support_Comment__c WHERE Id =:oldSupportComment.Id];

        if (privateComment.Public__c == FALSE && oldSupportComment.Public__c == TRUE && oldSupportComment.Standard_Case_Comment_Id__c != null) {                
            CaseComment cCommentRemove = [SELECT Id, ParentId, CommentBody FROM CaseComment WHERE ParentId =:oldSupportComment.Case__c AND Id =:cComId];

                if (cCommentRemove.Id == cComId) {

                    caseCommentsToDelete.add(cCommentRemove);
                }

            delete caseCommentsToDelete;
        }
    }
}

}

4
  • Please edit your post to be more clear about what you mean when you say it doesn't work as intended. If you get any error messages, please include them verbatim.
    – Adrian Larson
    Jan 17 '17 at 19:50
  • 4
    It'll probably take a while for someone to type up a proper answer, but the key to making salesforce code 'bulk-friendly' is to make sure that you do not have any DML (update, insert, upsert, delete, or undelete) nor any SOQL queries inside of loops (both of which I see in your provided code).
    – Derek F
    Jan 17 '17 at 19:52
  • This link would help you to get the idea of what you should implement and modify in your code: salesforce.stackexchange.com/questions/47469/…
    – Ashwani
    Jan 17 '17 at 19:59
  • @AdrianLarson I have updated as you have asked. The only error I am receiving is via email giving me an Apex governor limit warning.
    – user25814
    Jan 18 '17 at 21:08
5

Like I said in my comment, the key to bulkifying code in Salesforce is to remove all DML and SOQL from the inside of loops.

The reason why this is important is because you're only allowed 100 SOQL queries and 150 DML statements per (synchronous) transaction, and a trigger (or rather, the trigger context variables) can easily contain more than 100 records. The limits are higher for asynchronous transactions (@future, scheduled apex, etc...), but not anywhere close to high enough where you can forego bulkifying code.

The general strategy to use is to make use of collections (that is, Lists, Sets, and Maps). Ashwani's comment links to a very good question/answer on trigger best practices, but I think it bears repeating (because while the linked question/answer is excellent, it probably contains other content that would just confuse you more at this point in your development).

trigger MyTrigger on MyObject__c(before update){
    // Declare a collection to hold things
    Set<Id> myIds = new Set<Id>();
    List<OtherObject__c> recordsToUpdate = new List<OtherObject__c>();

    // Then loop over trigger.new to gather the ids in your collection
    for(MyObject__c myRec :trigger.new){
        myIds.add(myRec.Id);
    }

    // And finally, run your query
    for(OtherObject__c otherRec :[SELECT Id, my_field__c FROM OtherObject__c WHERE lookup_field__c IN :myIds]){
        // Do some work, and add the result to a collection to avoid DML in a loop
        recordsToUpdate.add(otherRec);
    }

    // Now that we're out of the loop, it's safe to do DML
    update recordsToUpdate;
}

Taking one portion of your trigger, it looks like you'll need to augment the general strategy a bit because this portion of your trigger depends on knowing which Support_Comment__c was used to generate a given CaseComment

// define collections
List<Support_Comment__c> supportCommentsToUpdate = new List<Support_Comment__c>();
List<CaseComment> caseCommentsToInsert = new List<CaseComment>();

// This last collection is how we'll be able to keep track of which Support_Comment__c
//   corresponds to which CaseComment
Map<Id, CaseComment> supportCommentIdToCaseComment = new Map<Id, CaseComment>();

// My personal taste is to keep checks on which trigger context we're in
//   as high-level in the code as possible.
// In your particular case, there is nothing in this loop that needs to be run in
//   an 'update' context.
if(Trigger.isInsert){
    for (Support_Comment__c newSupportComment : Trigger.new) {
        Id = newSupportComment.Id;

        System.debug('The newSupportComment Id = ' + Id);
        if (newSupportComment.Comment__c != null && newSupportComment.Community_User__c == 'Standard User' && newSupportComment.Public__c == true){
            if (stoprecurssion.runonce()){

                commentBody = newSupportComment.Comment__c;

                CaseComment cComment = new CaseComment(ParentId = newSupportComment.Case__c, CommentBody = newSupportComment.Comment__c, IsPublished = TRUE);
                // Instead of DML inside of a loop, put the record into the collection
                caseCommentsToInsert.add(cComment);

                // Also need to keep track of which Support Comment we used
                supportCommentIdToCaseComments.put(newSupportComment.Id, cComment);
            }
        }
    }

    // outside of loop, safe to perform dml
    insert caseCommentsToInsert;

    // Now we run another loop to take care of the update to support comments
    for (Support_Comment__c newSupportComment : Trigger.new) {
        // Since the in-memory instance of each CaseComment in our map is
        //   the exact same instance that went through the DML insert,
        //   and we're now past the DML,
        //   the CaseComments in our map will contain the Id of the CaseComments.
        ccId = supportCommentIdToCaseComments.get(newSupportComment.Id).Id;
        System.debug('The new standard Comment Id = ' + ccId);

        Support_Comment__c supComsToUpdate = new Support_Comment__c(Id = newSupportComment.Id, Standard_Case_Comment_Id__c = ccId);
        supportCommentsToUpdate.add(supComsToUpdate);
    }

    // outside of loop, safe to perform dml
    update supportCommentsToUpdate;
}
7
  • @AdrianLarson Whoops, forgot the closing square brace. Thanks for pointing that out.
    – Derek F
    Jan 17 '17 at 20:38
  • Derek, thank you for the comments, and the above example. I will take this and see what I can do to clean up my code. Thanks again!
    – user25814
    Jan 18 '17 at 21:10
  • I have updated my code above and believe I did it justice, but now I am receiving an Script Error. "Sandbox: Developer script exception from ... : createCaseComment2 : createCaseComment2: execution of AfterInsert caused by: System.NullPointerException: Attempt to de-reference a null object Trigger.createCaseComment2: line 34, column 1'" The thing that I don't understand is when I went to do a bulk update, one of the records updated correctly, but the others did not. Thanks again for all your help.
    – user25814
    Jan 19 '17 at 0:34
  • @user25814 In this case, that means that not all of the records in Trigger.new that passed through your other 'if' statement (the one on line 19) were placed in the map (on line 25). It must have something to do with your stoprecurssion.runOnce() check (on line 20), as that is the only meaningful difference between the first two loops up to that point.
    – Derek F
    Jan 19 '17 at 1:27
  • @user25814 Also, don't forget to take your queries (and DML) out of the loops later on in your trigger (lines 83, 87, 95, 104, 107, and 114 in the current iteration of your posted trigger).
    – Derek F
    Jan 19 '17 at 1:30

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.