Return to Revisions

It's rubbish :)

I'm not sure how complex is the thing you've anonymised here:

newRegionDirector= userMap.get('//the user');

Is it somehow dependent on the Opportunity Owner / other Opp. fields or is it always going to be "John Doe"? Even if it's always "John Doe" - in ideal world you shouldn't have him hardcoded like that but identified by Role for example... or just some kind of checkbox / picklist on his record that only SysAdmins can modify...

And remember that in different locales the User.Name != User.FirstName + ' ' + User.LastName. Hungarians and Chinese prefer the "Doe John" notation so if user has set a hungarian locale preference - this trigger won't find the manager :D

But I digress.

Roughly speaking you should have:

1. "learning loop"
2. query only the stuff you have to
3. "fixing loop"

pattern.

"Learning loop" - collect from trigger.new info which data you have to fetch. Most of the time it's some kind of Set<String> of values you need to check against reference object. In this particular scenario - it can be as stupid as a boolean flag.

I've commented out an example here that would add Opp. owners' managers to the set to show how it could look like. You can skip all the commented out lines if you're reading it for the first time.

// Set<Id> usersToCheck = new Set<Id>();
Boolean needToFetchTheUser = false;

for(Opportunity o : trigger.new){
    if(o.Design_Region__c == 'EUROPE' && o.Design_Territory__c == 'NORTH' ){
        // usersToCheck.add(o.OwnerId);
        needToFetchTheUser = true; // you can even add "break;" here ;)
    }
}

Query - if the set is empty / boolean is false then we don't have to do anything, no need to waste the query. And certainly no need to retrieve info about 900 users.

if(needToFetchTheUser){
    User regionalManager = [SELECT Id FROM User WHERE Name = 'John Doe'];
    // Map<Id, User> oppOwners = new Map<Id, User>([SELECT Id, ManagerId FROM User WHERE Id IN :usersToCheck );
    // (last part should be embedded in this if statement)
}

"Fixing loop" - one more pass through trigger.new, this time setting the stuff we know. If the reference data was added to a Map it's very fast & effective.

for(Opportunity o : trigger.new){
    if(o.Design_Region__c == 'EUROPE' && o.Design_Territory__c == 'NORTH' ){
        o.Regional_Director__c = regionalManager.id ;
        // o.Regional_Director__c = oppOwners.get(o.OwnerId).ManagerId;
    }
}