Fix this trigger: Account Rivals

// Automatically populate a lookup field to the Rival object
trigger GetRival on Account (after insert, after update) {

if(Trigger.isAfter){
Set rivelName = pluck.strings(“Rival_Picklist__c”,Trigger.New);
Rival__c accRivalName = [SELECT Id, Name
FROM Rival__c
WHERE Name = acc.Rival_Picklist__c];
for(Account acc : Trigger.New){
acc.Rival__c = accRivalName.Name;
}
}
}

Trigger getRival on Account(After insert,After update, before insert, before update){

Map accountperRivalpicklist = new Map();
for(Account acc : trigger.new){
accountperRivalpicklist.put(acc.rival_picklist__c,acc);
}
List rivalstobeupdated = new List();
for(Rival__c rival : [SELECT Id,Name from Rival__c where NAME IN: accountperRivalpicklist.put.keyset()]){
if(accountperRivalpicklist.put.keyset().contains(rival.Name)){
rival.Account__c = accountperRivalpicklist.put.keyset.get(rival.Name).Id;
rivalstobeupdated.add(rival);
}
}

if(!rivalstobeupdated.isEmpty()){
update rivalstobeupdated;
}
}

// First thing for update same object we need to write trigger using before event
// Automatically populate a lookup field to the Rival object
trigger GetRival on Account (before insert, before update) {
Map rivalCustomMap = new Map();
List rivalStringList = new List();
for (Account acc : Trigger.new) {
rivalStringList.add(acc.Rival_Picklist__c);
}

List rivalList = [SELECT Id, Name
FROM Rival__c
WHERE Name IN :rivalStringList
];
for(Rival__c rival : rivalList){
rivalCustomMap.put(rival.Name, rival.Id);
}

for (Account acc : Trigger.new) {
acc.Rival__c = rivalCustomMap.get(acc.Rival_Picklist__c);
}
}

trigger GetRival on Account (before insert, before update) {

Set r = new Set();
for (Account acc : Trigger.new) {
r.add(acc.Rival_Picklist__c);
}

List rList = new List([select Id, name from Rival__c where id in :r);

Map rMap = new Map();
for (Rival__c rr : rList){
rMap.put(rr.Id, rr);
}

for ( Account acc : Trigger.New){

Rival rRec = rMap.get(acc.Rival_Picklist__c) ;
acc.Rival__c = rRec.name;
}

}

// Automatically populate a lookup field to the Rival object
trigger GetRival on Account (before insert, before update) {
List comp = [SELECT Id, Name
FROM Rival__c
WHERE Name = acc.Rival_Picklist__c limit 1];
String rivalId=null;
if(comp !=null && comp.size()>0)
rivalId= comp.get(0).id;
for (Account acc : Trigger.new) {
acc.Rival__c = rivalId;
}
}

trigger GetRival on Account (before insert, before update) {

//Create a set of rival picklist values
set rivalPickList = new Set();
for (Account acc : Trigger.new) {
rivalPickList.add(a.rival_picklist__c);
}

List rivalObjList = [Select Id,Name from Rival__c where Name in: rivalPickList];
Map rivalNametoIdMap = new Map();

//Create a map to search
for(Rival__c r:rivalObjList) {
rivalNametoIdMap.put(r.Name, r.Id);
}

//Associate the account record with Rival object
for(Account acc:Trigger.New) {
Id rivalId = rivalNametoIdMap.get(a.rival_picklist__c);
acc.Rival__c = rivalId;
}

}

trigger GetRival on Account (before insert, before update) {

Map acctToRival = new Map();
for (Rival__c riv : [SELECT Id, Name FROM Rival__c]) {

acctToRival.put(riv.Name, riv.Id);

}

for(Account a : Trigger.new) {

a.Rival__c = acctToRival.get(a.Rival_Picklist__c);

}

}

Hello,

I have just started learning SF (about 3 weeks now!!) and below is my version of the solution:

// Automatically populate a lookup field to the Rival object
trigger GetRival on Account (before insert, before update) {
List acc = new List();
List rivalAcc = [SELECT Id, Name FROM Rival__c WHERE Name = :acc.Rival_Picklist__c];

Map accnRivals = new Map();
for(Rival__c rivalInLoop : rivalAcc) {
accnRivals.put(acc.Id,rivalAcc.Name);
}

for(Account accInTrigger : trigger.new) {
accInTrigger.Rival_Picklist__c = accnRivals.get(accInTrigger.Id);
}
}

Please do let me know in case there are any issues. I havent tried this in my Dev org. Just came up with it by reading till Chapter 5!

trigger GetRival on Account (before insert,before update) {
Set rivalSet= new Set();
List rivalList = new List();

for(Id key:Trigger.newMap.keySet())
rivalSet.add(Trigger.newMap.get(key).Rival_Picklist__c);
rivalList = [SELECT Id,Name FROM Rival__c WHERE Name IN: rivalSet];

for(Account a:Trigger.new){
if(a.Rival_Picklist__c!=null){
for(Rival__c r:rivalList ){
if(a.Rival_Picklist__c==r.Name)
a.Rival__c=r.Name;
}
}
}
}

Problems

1) The overall design could be better. For one, a company typically has one or more
rivals so the Rival custom object should be a junction object with 2 account
lookups. Rivals would then be tied to other accounts. With rival records for each account,
then each account would have a list of rivals. This requires no apex trigger to synchronize
the account lookup based on the selected rival picklist value and allows other rival attributes
to be tracked if needed.

2) Before Insert and Before update should be used since setting the Rival
lookup won’t actually update anything since “after insert” and “after update”
are used.

3) The list of rivals should be queried once with all the possible rivals being used
in the inserted or updated accounts.

4) If the Rival Picklist doesn’t have a value, errors will be thrown.

5) If the rival is updated to blank, the rival lookup isn’t cleared.

6) The soql query is missing the colon.

7) Trigger Name implies that one Account trigger is created for each Account feature.
Ideally, one account trigger is created and, this is more of taste, should
delegate to a helper apex class such as a “Trigger Handler”.

Solution

trigger AccountTrigger on Account (before insert, before update){

if (Trigger.isBefore){
if (Trigger.IsInsert){
AccountTriggerHandler.setRivalOnInsert(Trigger.new);
}
else if (Trigger.IsUpdate){
AccountTriggerHandler.setRivalOnUpdate(Trigger.oldMap, Trigger.newMap);
}
}
}

public class AccountTriggerHandler {

public static void setRivalOnInsert(List newAccounts){
List newAccountsWithRival = new List();

for (Account newAcct : newAccounts){
if (String.isBlank( newAcct.Rival_Picklist__c ) == false){
rivalNames.add( newAcct.Rival_Picklist__c );
newAccountsWithRival.add(newAcct);
}
}

// Only sync when necessary.
if (rivalNames.size() > 0){
syncRivalsOnAccounts(newAccountsWithRivals);
}
}

public static void setRivalOnUpdate(Map oldAccounts, Map newAccounts){
List updatedAccountRivals = new List();

for (Account newAcct : newAccounts){
Account oldAcct = oldAccounts.get(newAcct.Id);

// Rival didn’t change so nothing to do so continue on.
if (oldAcct.Rival__c == newAcct.Rival__c){
continue;
}

if (String.isBlank( newAcct.Rival_Picklist__c )){
newAcct.Rival__c = null;
}
else {
updatedAccountRivals.add(newAcct);
}
}

// Only sync when necessary.
if (updatedAccountRivals.size() > 0){
syncRivalsOnAccounts(updatedAccountRivals);
}
}

private static void syncRivalsOnAccounts(List accountsWithRivals){
Set rivalNames = new Set();

// Only query the rivals that we need.
for (Rival__c accountWithRival : accountsWithRivals){
rivalNames.add(accountWithRival.Rival_Picklist__c);
}

Map rivalAccountsMap = getRivalAccountsMap(rivalNames);

for (Account newAccountWithRival : newAccountsWithRival){
Rival__c rival = rivalAccountsMap.get(newAccountsWithRival.Rival_Picklist__c);

// Ensure the rival exists
if (rival != null){
newAccountWithRival.Rival__c = rival.Id;
}
}
}

private static Map getRivalAccountsMap(String rivalNames){
List rivals =
[select Id,
name
from Rival__c
where name in :rivalNames];

Map rivalsMap = new Map();

for (Rival__c rival : rivals){
rivalsMap.put(rival.Name, rival);
}

return rivalsMap;
}
}

trigger PopulateRival on account (before insert,before update)
{
set PickValues=new set();
for(account t:trigger.new)
{
//get all picklist values in trigger.new, add them to a set
PickValues.add(t.PickRival__c);
}

//get rival records whose name is matched with piclklistvalues
list r_list=[select id,name from Rival__c where name in:PickValues];

//map the rival name to id
map r_map=new map();
for(Rival__c t2 : r_list)
{
r_map.put(t2.name,t2.id);
}

//get the rival id from map using picklistvalue, and assign to account lookup field
for(account t3 : trigger.new)
{
t3.Rival__c=r_map.get(t2.PickRival__c);
}

}

1. Never put SOQL inside a Loop!
2. Better to populate a Lookup Field using IDs instead of Name
3. Better to go with before insert, before update instead of after insert, after update since Account Ids are not needed.

Best Practice: I think that the way to go would be forcing the user to select the Rival Record from the Lookup Field. This would avoid coding, one less field :), APEX Usage (3M characters limit), speed optimized, duplicated picklist values that might generate errors.

However for education purpose, this is how I planned to fix the Trigger:

1. Get a List of all Rivals
2. Create and fill a Rivals Map
3. Iterate over each inserted/updated account to set the Rivals ID based on the previous Map

trigger GetRival on Account (before insert, before update){

//Get a List of all Rivals
List allRivals = [SELECT Id, Name FROM Rival__c];

//Crate a map with Rivals Names and their Ids
Map RivalsMap = new Map();

//Fill the map
for(List allRivals){
RivalsMaps.put(allRivals.Name, allRivals.Id);
}

//For each account
for(Account acc : Trigger.new){

acc.Rival__c = RivalsMaps.get(acc.Rival_Picklist__c);

}
}

PS. Is anyone around here going to Dreamforce??

trigger GetRival on Account (before insert , before update) {

// Ceate picklist entries from schema.getDescribe()
List pkList = new List();
for(Schema.pickListEntry pe: Account.Rival_PickList__c.getDescribe().getPickListValues()){
pkList.add(pe.getValue());
}

//Create Map of Name,Id
Map rivalMap = new Map();
for(Rival__c rival : [SELECT id, name FROM Rival__c WHERE Name In :pkList]){
rivalMap.put(rival.Name, rival.Id);
}

// Check against newly inserted, updated Accounts
for(Account acc : Trigger.new){
if(rivalMap.containsKey(acc.Rival_PickList__c)){
acc.Rival__c = rivalMap.get(acc.Rival_PickList__c);
}

}
}