How to enforce CRUD and FLS in following situation (bulk trigger). Better idea is simply throw an exception or addError(errMsg) ?

trigger CaseCommentTrigger on CaseComment (after insert) {
   List<Case> toUpdate = new List<Case>();
   Map<Id, List<CaseComment>> caseComments = new Map<Id, List<CaseComment>>();

   for (CaseComment caseComment : Trigger.new) {
       if (caseComments.containsKey(caseComment.ParentId)) {
           caseComments.get(caseComment.ParentId).add(caseComment);
       } else {
           caseComments.put(caseComment.ParentId, new List<CaseComment> {caseComment});
       }
   }

   // Check FLS here and throw exception here?
   List<Case> cases = [SELECT  Id, Status FROM Case WHERE Id IN :caseComments.keySet()];
   for (Case c : cases ) {
       // Check FLS and addError() on CaseComment assigned to Case?
       // or check FLS here and throw exception here?
       c.Subject = 'Updated';
       toUpdate.add(c);
   }

   // Check FLS here and throw exception here?
   update toUpdate;
}

Update: Is it correct approach?

trigger CaseCommentTrigger on CaseComment (after insert) {
    List<Case> toUpdate = new List<Case>();
    Map<Id, List<CaseComment>> caseComments = new Map<Id, List<CaseComment>>();

    for (CaseComment caseComment : Trigger.new) {
        if (caseComments.containsKey(caseComment.ParentId)) {
            caseComments.get(caseComment.ParentId).add(caseComment);
        } else {
            caseComments.put(caseComment.ParentId, new List<CaseComment> {caseComment});
        }
    }

    List<Case> cases = [SELECT  Id, Status FROM Case WHERE Id IN :caseComments.keySet()];
    for (Case c : cases ) {

        try {
            SecurityUtils.checkFieldIsUpdateable(Case.SObjectType, Case.Subject);
            c.Subject = 'Updated';
            toUpdate.add(c);
        } catch (SecurityUtils.FlsException flsException) {
            for (CaseComment cc : caseComments.get(c.Id)) {
                cc.addError(flsException);
            }
        }
    }

    update toUpdate;
}
up vote 2 down vote accepted

Do not throw exceptions in a trigger. This can cause undesirable side effects. You should always use addError in a trigger, and never allow an exception to escape from a trigger. In recent releases, an uncaught exception would end up wrapping the exception in a DmlException, losing the original context, and in at least this release, doing so can can cause a fatal exception that aborts your entire transaction, even if you use try-catch blocks. FLS does not change from one record to the next, so if you need to check it, check it just once before trying to update anything, and add an error to all rows that would cause the error.

  • Thanks @sfdcfox but I wonder how can I use addError() when I update parent records on child object trigger. Is it possible, is it work as expected - rollback the transaction? – gpoluch Sep 23 at 8:24
  • @gpoluch No. Use partial update syntax, and report errors to the related record, as mentioned here and demonstrated here. Unless, of course, the transaction is meant to be an allOrNone update, but generally speaking, using the code in the related answer allows the caller to decide if they use allOrNone or not. – sfdcfox Sep 23 at 8:27
  • Nice example @sfdcfox but I wonder if I do not need to check CRUD and FLS for Case when I update it from CaseComment Trigger? Correct me if I am wrong? How to accomplish that? – gpoluch Sep 23 at 8:34
  • @gpoluch It depends on context. If the business rule requires an update for any comment, then FLS/CRUD checks might not be appropriate. You cannot blindly apply a rule either way. Each unique situation must be considered. – sfdcfox Sep 23 at 8:41
  • Thanks for explanation @sfdcfox, but I need to solve checkmarx violation similiar to provided exmple, so I need to check FLS/CRUD somehow anyway. – gpoluch Sep 23 at 8:48

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.