Take the 2-minute tour ×
Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. It's 100% free, no registration required.

I am new to Salesforce . I am having a bit of problem wriiting a trigger, any help would be appriciated.

I want to Write a trigger that will set the name of an Opportunity record when it is created and when the fields below are updated. The opportunity name should contain the following, each separated by a space: Account Name,Owner Last Name, Opportunity Close Date.

trigger SetOpportunityName on Opportunity (before insert, before update) {
for (Opportunity op: Trigger.new) {
   op.Name = op.Account.Name+' '+Op.Owner.LastName+' '+op.ClosedDate;
   system.debug('*****************************' + op.Name);    
    }

But I am getting op.Account.Name and Op.Owner.LastName values as null.

I know there is a mistake, Please guide me.

share|improve this question
    
This sounds like a good use for Workflows. Is that an option for you? –  Mike Chale Jan 28 at 20:18
    
No I want to use Trigger :) –  SFDC Geek Jan 28 at 20:29
add comment

3 Answers

up vote 11 down vote accepted

You need to query for those values since they don't come down natively in the trigger. Anything that is related will be null (like op.Account.Name)

So I would gather up all the account and OwnerIds and then do separate queries outside of the for loop (otherwise you will have other issues), and then loop again to set the values... Also the owner could be a group so this may be tricky :-P

Set<Id> ownerIds = new Set<Id>();
Set<Id> AccountIds = new Set<Id>();
for (Opportunity op : Trigger.new) {
   ownerIds.add(op.OwnerId);
   accountIds.add(op.AccountId);
}

Map<Id, Account> accountMap = new Map<Id, Account>([SELECT Name FROM Account WHERE Id IN :accountIds]);
Map<Id, User> userMap = new Map<Id, User>([SELECT Name, LastName FROM User WHERE Id IN :OwnerIds]);
Map<Id, Group> groupMap = new Map<Id, Group>([SELECT Name FROM Group WHERE Id IN :OwnerIds]);

String sName;
for (Opportunity op : Trigger.new) {

  // add the account name
  sName = accountMap.get(op.AccountId).Name;

  // add either the group or user name
  if (userMap.get(op.OwnerId) != null) {
    sName += ' ' + userMap.get(op.OwnerId).LastName;
  } else if (groupMap.get(op.OwnerId) != null) {
    sName += ' ' + groupMap.get(op.OwnerId).Name;
  }

  // set the name :-)
  op.Name = sName;
}
share|improve this answer
    
I have faced the Error: Compile Error: Illegal assignment from LIST<Account> to MAP<Id,Account> –  SFDC Geek Jan 28 at 20:13
    
sorry let me edit the answer to fix it –  SalesForceGirl Jan 28 at 20:15
    
the problem was in the creation of the maps :-P its fixed now: Map<Id, Account> accountMap = new Map<Id, Account>([SELECT Name FROM Account WHERE Id IN :accountIds]); Map<Id, User> userMap = new Map<Id, User>([SELECT Name FROM User WHERE Id IN :OwnerIds]); Map<Id, Group> groupMap = new Map<Id, Group>([SELECT Name FROM Group WHERE Id IN :OwnerIds]); –  SalesForceGirl Jan 28 at 20:16
    
If I want to fetch only last name of the User then what I have to do? –  SFDC Geek Jan 28 at 20:30
    
since the owner could be a group this may not work on all, but change the query on user to get Lastname instead of name. I'll edit the post to show –  SalesForceGirl Jan 28 at 20:31
show 6 more comments

There's absolutely nothing wrong with SalesForceGirl's solution, but I don't see the advantage in creating maps when you can just query those fields directly from the opportunity. Simpler is always better imo. You can always refactor later if the need arises.

Obviously this is dependent on whether an owner is always a user:

for (Opportunity op : [select Account.Name, Owner.LastName from Opportunity where Id in :Trigger.new]) {
    Trigger.newMap.get(op.Id).Name = op.Account.Name + ' ' + op.Owner.LastName;
}
share|improve this answer
1  
this reduces a for loop :-) –  SalesForceGirl Jan 28 at 23:05
    
I don't think this actually works. When you set the fields here, you are setting it relative to that specific list. Not the trigger. Am I incorrect? –  dphil Feb 11 at 18:13
    
Actually -I think you're right dphil. I'm going to change my answer now –  willard Feb 12 at 0:30
add comment

If one knows about Salesforces limits and best coding practices you NEVER write a select statement in a loop.

Always use in memory maps in a trigger.

If you consult for a client that has numerous classes and triggers like that you'll never deploy to production without eventually hitting SOQL limits.

SalesforceGirl is 100% correct in the trigger code but I would go with a workflow where those are all standard objects and accessible. Managers and admins can edit those easily if need be.

share|improve this answer
    
You would not necessarily do a workflow, as a matter of fact, because of the potential echo effect caused by field updates causing recursive trigger calls, some of which may result in clients with "numerous classes and triggers" not being able to use opportunities. We have different tools for different purposes. It is true that I would also use workflow rules for this unless it were for a client that I knew had triggers I would have to potentially fix in order to do the workflow rule. –  sfdcfox Jan 29 at 2:58
    
Using workflow criteria to set an opportunity name you most certainly can. Evaluate the rule when a record is created, and every time it’s edited. Example... (Opportunity: Created By equals Admin User) or (Opportunity: Opportunity Name equals Opportunity Name).. We have this with a great amount of custom code for a specific client as they always want their opp names to stay the same. And I forgot the custom field used :) Update opportunity name with CustomerNiceName__c + Account + CloseDate –  user5553 Jan 29 at 3:03
    
sfdcfox I see what your getting at. In many cases we try to empower the client if possible and maybe wrap the trigger from firing. Many people want to manage their instance and remove triggers that are not necessary if you can help it. –  user5553 Jan 29 at 3:10
1  
"Configuration over code." That's my mantra. However, we've got both tools, and we have to remember that there are times one is appropriate over the other (and it's usually hard to convince a client of the correct tool to use...). –  sfdcfox Jan 29 at 3:13
1  
for(subject record: [select ... ]) is NOT a query in a for loop. It's actually a queryMore statement that reduces heap usage and allows the use of server cursors. This is actually preferred over maps if you only need the result once. A query in a for loop is for(...:...) { subject[] records = [...]; /* do stuff */ }, those are bad. –  sfdcfox Jan 29 at 3:17
show 5 more comments

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.