Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. Join them; it only takes a minute:

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

We've had a community deployment and my brain is fried and I just whipped up a quick class that is throwing the below error. What have I done wrong here?

Error: CANNOT_INSERT_UPDATE_ACTIVATE_ENTITY, MSI_UserTrigger: execution of AfterInsert caused by: System.StringException: Invalid id: () : [] () : []: ()

What I am trying to accomplish is:

  • When a new user is created and is active, if they are linked to a ContactId and part of a specific profile (by name), add their ContactId to a Set<ID>
  • Then loop through contacts where the Id is in the set, and update the Community_User__c field to TRUE

Trigger:

trigger MSI_UserTrigger on User (after insert) {

     if(Trigger.isAfter){
         if(Trigger.isInsert){
             MSIUserTriggerHandler.executeAfterInsert(Trigger.new);
         }
     }
}

Class:

public with sharing class MSIUserTriggerHandler {
    private static final String COMMUNITY_PROFILE = 'Community Knowledge User';
    private MSIUserTriggerHandler() {}

    public static void executeAfterInsert(List<User> newList){
        setCreatedCommunityUsers(newList);
    }

    private static void setCreatedCommunityUsers(List<User> newList){
        List<User> usersWithContacts = new List<User>();
        List<Contact> contactsToUpdate = new List<Contact>();
        Set<ID> contactIds = new Set<ID>();

        for(User u: newList){
            if(u.ContactId == null && u.ContactId == ''){
                continue;
            }
            if((u.ContactId != null && u.ContactId != '') && u.Profile.Name == COMMUNITY_PROFILE && u.isActive == true){
                contactIds.add(u.ContactId);
            }
        }

        for(Contact c : [Select Id,Name,Community_User_Created__c from Contact where Id in :contactIds]){
            if(!c.Community_User_Created__c){
                c.Community_User_Created__c = true;
                contactsToUpdate.add(c);
            }
        }
        update contactsToUpdate;
    }
}

Am I not adding the ID correctly to the Set<ID>, or am I querying wrong in my for(Contacts) loop?

share|improve this question
up vote 6 down vote accepted

When you perform a comparison between a String and an Id (e.g. ==, !=, or <>), then the other argument is "promoted" to an Id value. All Id values must be 15 or 18 characters long, and must contain only A-Z, a-z, and 0-9, so '' is not a valid Id.

Proof:

String a = '';
Id b;
System.assert(a != b);

Edit:

Also, there's a more efficient way you could write this trigger's function:

private static void setCreatedCommunityUsers(List<User> newList){ 
Contact[] contacts = [SELECT Id FROM Contact WHERE Id IN (SELECT ContactId FROM User WHERE Profile.Name = 'Community Knowledge User' AND IsActive = TRUE AND Id IN :newList) AND Community_User_Created__c = false]; 
for(Contact record: contacts) { 
record.Community_User_Created__c = true; 
} 
update contacts; 
}

Since we need to query for the Profile Name anyways, we can just perform all of the branch logic at once in the query, then update any records that are returned.

share|improve this answer
    
Might be worth mentioning a working alternative, such as String.isBlank(someId) or whatever you prefer. – Adrian Larson Mar 1 at 23:29
    
by adding the u.ContactId == '' condition in conjunction with the null I was attempting to be extra-super-careful to not grab empty/invalid records... and instead ended up creating my own invalid records. In the end I just removed u.ContactId == '' as per your proof and it worked flawlessly. Thank you! – Morgan Marchese Mar 1 at 23:29
    
@AdrianLarson I prefer to avoid calling functions when necessary, because of the overhead that library functions incur. In fact, I wrote up how I'd write the code: skip the initial loops and checks, and just make it all in to a single query. – sfdcfox Mar 1 at 23:41
    
Nice. I didn't realize there was already a null check. – Adrian Larson Mar 1 at 23:45
    
I see why you're an MVP this year @sfdcfox - You just cut out all of my code lines.. guess it's time to step my game up and remember that I can do cross object queries by public/foreign key relation. – Morgan Marchese Mar 1 at 23:53

One thing, you will never hit this continue statement. The condition should be an OR not an AND... if(u.ContactId == null && u.ContactId == ''){ Good looking code. Keep it up!

One optimization to consider here, in a similar vein to what Brian mentioned in his update as I was writing this, is that you may not need the query for the Contact list to do the operation you're performing. You can just new up the list of Contacts to be updated, add your Contact with the Id and the new field value and fire away - no need to get the current state of the record (if it does not matter).

That said, you will be updating all of the Contacts regardless of whether or not the previous value of Community_User_Created__c is true so, check to make sure this is acceptable in your org.

private static void setCreatedCommunityUsers(List<User> newList){
    List<Contact> contactsToUpdate = new List<Contact>();

    for (User u: newList) {
        if (u.ContactId == null || u.ContactId == ''){
            continue;
        }

        // create a Contact reference that we're going to use for update
        // all you need is the Id and the fields to set
        if (u.ContactId != null && u.ContactId != '' && u.Profile.Name == COMMUNITY_PROFILE && u.isActive == true){
            contactsToUpdate.add(new Contact(Id = u.ContactId, Community_User_Created__c = true));
        }
    }

    if (!contactsToUpdate.isEmpty()) {
        update contactsToUpdate;
    }
}
share|improve this answer
    
Hey Mark, thank you for the compliment on the code as always - I'm 100% self start so its awesome to hear from some pros that I'm heading the right way. Great suggestion about just doing the new contact inside the .add instead of the if check and individual set of the Boolean before update. You guys are always super helpful so I just wanted to make sure I came to thank you for adding your valuable input. Cheers! – Morgan Marchese Mar 1 at 23:57

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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