1

I have created an apex trigger that will update the accountId on the case. I don't think this will work for bulk inserts and was wondering if someone could glance at this and suggest an edit or to. Thank in advance.

trigger CaseReassignQueueBasedOnSubject on Case (after insert) {
    //Get All Accounts
    List<Account> account = [SELECT Id, CNUM__c FROM Account];
    //Variables for later use
    List<Case> casesToUpdate = new List<Case>();
    String CNUM = null;
    String AccountId = null;
    String CaseId = null;

    //For each case set variable -- Not working for bulk inserts
    for (Case c: Trigger.New) {
        //validate that the parsing won't fail
        if (c.Subject.contains('[')) {
            //get CNUM
            Integer start_pos = c.Subject.indexOf('[') + 1;
            Integer end_pos = c.Subject.indexOf(']',start_pos);
            CNUM = c.Subject.substring(start_pos,end_pos);
            CaseId = c.Id;
            System.debug('Parsed_CNUM: ' + CNUM);
            casesToUpdate.add(c);
        }
    }

    //find the accountId and set local variable
    for (Account la: account) {
       if (la.CNUM__c == CNUM) {
          System.debug('Account.Id: ' + la.Id);
          System.debug('Account.CNUM__c: ' + la.CNUM__c);
          AccountId = la.Id;
        }
    }

    //Update Case Values
    System.debug('Case.Id: ' + CaseId);
    List<Case> updateCases = [select AccountId, Id from Case where Id = :CaseId];
    for (Case updC: updateCases) {
          System.debug('AccountId_NEW_SETTING_VALUE: ' + AccountId);
          updC.AccountId = AccountId;
          System.debug('updC.AccountId: ' + updC.AccountId);
    }

    //Update Cases
    System.debug('updateCases: ' + updateCases.size());
    if (updateCases.size() > 0) {
        update updateCases;
    }
}

Thanks in advance.

4

This is probably more appropriate for codereview.stackexchange.com (I am making a point to check the [salesforce-apex] tagged questions over there), but here's what I see.

You don't have a WHERE clause in your query on Account

It doesn't appear that you're interested in returning all accounts, you just need to know if there's an Account with a matching CNUM. I'd suggest putting your query off until after you parse the case subjects

Case subject parsing is a bit verbose

substring(), contains(), and indexof() will work, but I get the feeling that this would be better off as a regular expression.

// '[' and ']' are special characters in regex, and probably need to be escaped
// Normally, you would only need a single backslash, but Apex requires backslashes to
//   be escaped themselves
// \[(\w+)\] = matches a string that contains '[]' with one or more (that's the '+') 
//   word characters (a-z, A-Z, 0-9, and punctuation. The '\w' is the shortcut for that) inside
// The '(' and ')' denote a capture group, which is how we'll get the result
// It makes sense to compile the regex, because we'll likely be using it many times per transaction
Pattern myPattern = Pattern.compile('\\[(\\w+)\\]');

for(Case c :trigger.new){
    // A Matcher is what actually runs given input against the compiled regex
    Matcher myMatcher = myPattern.matcher(c.Subject);

    // There is also a matches() method, but what we want to do here is use find()
    //   because we don't care if the entire subject matches our pattern, we
    //   just want to extract a substring
    if(myMatcher.find()){
        myCNUMs.add(myMatcher.group(1));
    }
}

Maps (and collection types in general) are your friend

There is a common pattern used by more inexperienced developers that looks like this

for(MyObject1__c obj1 :obj1List){
    for(MyObject2__c obj2 :obj2List){
        if(obj1.field_a__c == obj2.field_b__c){
            // do something
        }
    }
}

You haven't quite fallen into this trap, but you're close. Instead of using that, using a Map makes things a lot faster (at the cost of requiring some setup).

When you query your accounts, I'd suggest iterating over the results and storing them in a map keyed on CNUM__c

Map<String, Account> cnumToAccountMap = new Map<String, Account>();

for(Account acct :[SELECT Id, CNUM__c FROM Account WHERE <your conditions here>]){
    cnumToAccountMap.put(acct.CNUM__c, acct);
}

After populating your map, if you have the CNUM from the case subject, you can simply ask your map for an account with that CNUM. If you only need the Account Id, you could use a Map<String, Id> instead.

You're parsing your case subjects, but you're not associating them to a specific case

This is where your bulk handling falls apart. You iterate over all the cases in Trigger.new, but you aren't storing your parsed CNUM in a way that you can retrieve any of them at a later time. You simply have a single CNUM variable, and that will hold the last CNUM that you encounter.

In the end, I'll suggest having two loops over trigger.new. The first will gather the CNUMs to limit the number of Accounts you need to query, and the second will prepare the update you want to perform (which I'll get to next)

You're using a query to pull records that you already have access to

Yes, I know that you're in an after insert context. You might not need to be, but that's for another section.

Records in trigger context variables are read-only in after trigger contexts, but there are a couple of other ways to do this.

  • Use the clone() method of the SObject class
  • Use the SObject constructor to create a separate instance

Both achieve the same thing, decoupling the instance of a particular Case that you want to update from the trigger context variables which are read-only.

for(Case c :trigger.new){
    Case decoupled = c.clone();

    // Now that we're not working on the same in-memory copy of the Case that is in 
    //   trigger.new, we can edit fields
    decoupled.Status = 'Closed';

    // Another method is to use the sobject constructor to pass in the Id, and any
    //   fields that you want to set.
    // Use one or the other
    decoupled = new Case(
        Id = c.Id,
        Status = 'Closed'
    );
}

Either method will work (and avoid the need for that query on Case), but I prefer the SObject constructor method where possible.

As it turns out, you can use beforeInsert

before trigger contexts are useful when you want to modify the same records that are going through the trigger (which you are doing here).

before insert triggers can be a bit tricky, because a record being inserted does not have an Id available for use until after insert. If you need to query, or otherwise find a record in trigger.new and use an Id, after insert is what you need to use.

In your particular case, however, you aren't actually using the Id of the newly inserted cases for anything (or, you don't need to use the Id).

The following trigger is probably the endgame for this particular trigger (until you get into trigger frameworks, that is). I won't spell everything out for you, but I should give you enough hints to finish it for yourself.

// This is a case for a before insert trigger, not an after insert one
trigger CaseReassignQueueBasedOnSubject on Case (before insert) {
    // First up, iterate over your cases to find CNUMs.
    // A Set<String> will do nicely to store them all for your query later on
    Set<String> subjectCNUMSet = new Set<String>();

    // Compile the regex here
    Pattern myPattern = Pattern.compile('\\[(\\w+)\\]');

    for(Case c :trigger.new){
        // Set up the matcher
        Matcher myMatcher = myPattern.matcher(c.Subject);

        if(myMatcher.find()){
            subjectCNUMSet.add(myMatcher.group(1));
        }
    }

    // Now that you have your CNUMs, you can query the matching accounts
    //   and build your map from CNUM to account (or accountId)
    Map<String, Account> cnumToAccountMap = new Map<String, Account>();

    // perform a soql for-loop to query your accounts, and build the map appropriately

    // Now, we can iterate over trigger.new one last time, and set the appropriate accounts
    for(Case c :Trigger.new){
        // You're going to need to either use the regex again (set up a new matcher),
        //   store the CNUM in the Cases as you find them in the first loop,
        //   or build a map from CNUM to Case

        // Once you have your CNUM for this particular case, find the corresponding account
        //  in the cnumToAccountMap
        // You can then directly set the AccountId for this case (since we're before insert, 
        //   and records in trigger.new are still editable)
        // It's a good idea to check that you actually have a CNUM, and that
        //   the CNUM you have actually maps to an account.
        // If you don't do this, you risk running into a Null Pointer Exception
        if(cnumToAccountMap.containsKey(<your cnum here>)){
            c.AccountId = cnumToAccountMap.get(<your cnum here>).Id;
        }
    }

    // Because we're doing this before insert, there's no need for any other logic
    //   or queries after this point
}

Just in case, the <your cnum here> in my example is not meant to literally be that value. It's simply a placeholder. You need to somehow find the CNUM value for the particular case you're working on, and insert that value in place of the placeholder.

  • I missed the before insert one, good catch – gNerb Aug 24 '18 at 17:27
  • @Derek F, I guess there is a lot to learn about triggers. I am very impressed with your write up. I thank you very much. I will go through the before insert and see if I can get that squared away. – Gunner1714 Aug 24 '18 at 20:08
  • @Gunner1714 You're welcome. I'd say that rather than just triggers though, there's a lot of depth to the Salesforce platform as a whole. In the end, I think a lot of it ends up being about: -- Using collections (lists, sets, and maps) in a lot of places to work on more than just one record at a time. -- Using as few SOQL/SOSL queries as you can get away with (and not running over the other governor limits). -- Knowing when to use "clicks" (validation rules, workflow, approval processes, etc...) instead of code. – Derek F Aug 24 '18 at 20:26
  • Derek, Even with your hints and comments. I struggled through trying to get the before insert to work. I ended up completed the after insert which seems to work. Thank you again for your help – Gunner1714 Aug 27 '18 at 15:26
2

From what I can tell you're trying to take a case that has a value CNUM in it's subject. You're trying to find the account with a matching CNUM value and associate the case to the account.

First Create a map linking the cases to the CNUM value.

Map<String, List<Id>> CNUMToCaseIds = new Map<String, List<Id>>();

for (Case c : trigger.new) {
    if (c.Subject.contains('[')) {
        Integer start_pos = c.Subject.indexOf('[') + 1;
        Integer end_pos = c.Subject.indexOf(']',start_pos);
        CNUM = c.Subject.substring(start_pos,end_pos);
        CaseId = c.Id;

        if (!CNUMToCaseId.containsKey(CNUM)) {
            CNUMToCaseId.put(CNUM, new List<Id>());
        }
        System.debug('Parsed_CNUM: ' + CNUM);
        CNUMToCaseId.get(CNUM).add(caseId);
    }
}

Then query for your accounts:

List<Account> accounts = [
    SELECT Id, CNUM__C
    FROM Account
    WHERE CNUM__c IN :CNUMToCaseId.keySet()
];

Then you should be able to do what you want:

List<Case> casesToUpdate = new List<Case>();

for (Account a : accounts) {
    for (id i : CNUMToCaseId.get(a.CNUM__c)) {
        Case cToUpdate = new Case(
            Id = i,
            AccountId = a.Id
        );

        casesToUpdate.add(cToUpdate);
    }
}

update casesToUpdate;
  • Issue with for (Case c : trigger.new) { accountIds.add(c.AccountId); } Is I don't know the AccountId at this time so I have to pull all of the accounts Thanks for the mapping. I've been looking at that but just couldn't wrap my head around how to do it properly – Gunner1714 Aug 24 '18 at 16:35
  • Yea I'm sorry, to be hones the code is incredibly hard to follow so I was going step by step but I think I understand what you need, I'm updating it now. – gNerb Aug 24 '18 at 16:37
  • Updated, that should get you pretty close – gNerb Aug 24 '18 at 16:44
  • Sorry about the code. Illegal assignment from List to Id Case cToUpdate = new Case( Id = CNUMToCaseId.get(a.CNUM__c), AccountId = a.Id ); – Gunner1714 Aug 24 '18 at 17:06
  • Yup ill fix it, sorry was trying to get it done quickly – gNerb Aug 24 '18 at 17:07

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.