0

I'm new to Apex and need to assign a User to a Contact via lookup field on the contact record based off of the user with the least amount of assignments and matching endpoints (account name custom user field) as the contact.

I am trying to avoid having a SOQL query inside of a for loop but can't exactly figure out how to do so as the query references a variable inside of the loop.

trigger newCoachAssignment on Contact (before update) {
    List<Contact> nc = Trigger.new;
    Set<contact> newMemberNames = new Set<contact>();
    newMemberNames.addAll(nc);        


    for (Contact myMember : nc){                            
        Contact oldMember = Trigger.Oldmap.get(myMember.Id);        
        Id oldMemberHasCoach = oldMember.Assigned_Coach__c ; 
        IF(oldMemberHasCoach == Null){
            //only assigning a coach to members who DIDNT previously have an assigned coach.
            IF(myMember.Membership_Status__c == 'Active' //creating if statement for eligible members
                && myMember.Portal_Registration_Date__c != Null 
                && myMember.Assigned_Coach__c           == Null 
                && myMember.Account_Name__c             != Null
                && myMember.LM_Available__c) {
                    string myAcc = myMember.Account_Name__c;        //varaible for member's account name.
                    id myNewCoach;                                  //finding user that has same account assignments as member with the least ammount of assignments.

                    myNewCoach = [select id from user where isactive = true and Eligible_coach__c = true and Eligible_Endpoints__c includes (:myAcc) order by Current_Assignments__c asc limit 1].id;
                    myMember.Assigned_Coach__c = myNewCoach;    //assigning the member a coach.

                }
            }
        }
   }
3

First, I think there maybe some measure of an architectural issue here. Generally speaking, I avoid multi-select lists like the plague. They are usually better off as a custom relationship. So in your case I would make an object called Account Assignments That has a lookup to a user and a lookup to an account.

Then you populate this object with all of your "Eligible_Endpoints__c" and kill that multi-select field.

Now to answer your question, The general pattern is to get a list of records before you query so your loop would look something like:

Map<Id, Id> membersToAssign = new Map<Id, Id>();

for (Contact myMember : nc){                            
    Contact oldMember = Trigger.Oldmap.get(myMember.Id);        
    Id oldMemberHasCoach = oldMember.Assigned_Coach__c ; 
    IF(oldMemberHasCoach == Null){
        //only assigning a coach to members who DIDNT previously have an assigned coach.
        IF(myMember.Membership_Status__c == 'Active' //creating if statement for eligible members
            && myMember.Portal_Registration_Date__c != Null 
            && myMember.Assigned_Coach__c           == Null 
            && myMember.Account_Name__c             != Null
            && myMember.LM_Available__c) {
                membersToAssign.put(myMember.Id, myMember.AccountId);
            }
        }
    }

Then you run your query after the loop:

Map<Id, Account> accountMap = new Map<Id, Account>([
    SELECT Id, (
        SELECT CoachId__c
        FROM AccountAssignments__r
    )
    FROM Account
    WHERE Id :membersToAssign.values()
]);

for (Contact myMember : nc) {
    if (membersToAssign.containsKey(myMember.Id)) {
        myMember.Assigned_Coach__c = 
            accountMap.get(membersToAssign.get(myMember.Id)).AccountAssignments__r[0];
    }
}

This is probably not 100% perfect for your use case (my code assumes that each account only has 1 coach assignment for instance) but it should show you what you can do to address your issue.

Bonus Tips:

the variable name oldMemberHasCoach implies a boolean value, not an Id value. Additionally, if you only use a variable once, declaring a variable is probably not a good idea as it adds to the limited heap size. I would change it to this:

IF(oldMember.Assigned_Coach__c == Null)

The same goes for the nc List. Trigger.new is already a list, there is little reason to create a second copy of the list; especially if you only use that copy once.

I don't see any reason for the extra if statement. I'm probably nit picking here but I would remove the if statement that I just mentioned above from your code altogether and add it as an && in the if statement below it. This is probably the way I would have written your for loop:

for (Contact myMember : trigger.new) {
    IF(
        oldMember.Assigned_Coach__c == Null
        && myMember.Membership_Status__c == 'Active'
        && myMember.Portal_Registration_Date__c != Null 
        && myMember.Assigned_Coach__c           == Null 
        && myMember.Account_Name__c             != Null
        && myMember.LM_Available__c
    ) {
        membersToAssign.put(myMember.Id, myMember.AccountId);
    }
}
  • 1
    I've had dozens of projects where they seem tempting, but it was never the best choice to use multi-select. – Adrian Larson Apr 16 at 18:33

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.