function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
ChickenOrBeefChickenOrBeef 

Trigger that updates Opp with Account value (not working)

Hey everyone,

I wrote a trigger that's not getting any errors, but it's simply not working. Here is what I'm trying to do:

-We created a multi-select picklist on Opportunities called 'Opportunity Industries'
-We did this because if the account is an Agency, then the opp may have multiple industries
-But if the account is not an Agency? We simply want the Opportunity Industries field to be populated with the account's industry

Here is the trigger I wrote, which is not getting errors, but it's not updating the Opportunity Industries field:


trigger OppIndustry on Opportunity (before insert, before update) {
   
    Set<String> relevantAccountIds = new Set<String>();
     for (Opportunity o : Trigger.new) {
            IF(o.Account.Industry != NULL){
            relevantAccountIds.add(o.AccountId);
            }
        }
   
    List<account> accountsWithIndustry = [SELECT Id, Industry FROM Account WHERE Id IN :relevantAccountIds];
   
    map<string,string> accountsMap = new map<string,string>();
     for(Account a : accountsWithIndustry) {
        accountsMap.put(a.Id,a.Industry);
        }
   
    for(Opportunity oppInTrigger : Trigger.new) {
        IF(oppInTrigger.Account.Industry != NULL && oppInTrigger.Account.Industry != 'Agency') {
            String oppAccountIndustry = accountsMap.get(oppInTrigger.AccountId);
            oppInTrigger.Opportunity_Industries__c = oppAccountIndustry;
        }
    }
}



Any idea why that might not be working? I tried adding and removing the IF statement in the first 'for' statement, but that didn't do anything. I also tried changing the map to an 'account,string' map (and making the other necessary changes throughout the trigger accordingly), but that didn't help either.

Anything else I should try?

Thanks!
-Greg
Best Answer chosen by ChickenOrBeef
MJ Kahn / OpFocusMJ Kahn / OpFocus
Hi Greg,

You very carefully (and correctly) create a Set of Ids of Accounts you care about, and then you query the database to build a map (indexed by Account Id) of the Industry for each Account. You do that because (I assume) you know that Trigger.new knows what each Opp's Account Id is, but doesn't know any of the values of the Account's fields. In other words, the object in Trigger.new has all of the fields for that object populated, but doesn't have values for any of the fields in parent records for the object.

But then, in the "for Opportunity" loop, you reference the Trigger.new's Opportunity's Account's Industry field. As we just said, Trigger.new doesn't have the Opportunity's Account's field's values, so oppInTrigger.Account.Industry will always be null, and your trigger won't work.

Change this:

    for(Opportunity oppInTrigger : Trigger.new) {
        IF(oppInTrigger.Account.Industry != NULL && oppInTrigger.Account.Industry != 'Agency') {
            String oppAccountIndustry = accountsMap.get(oppInTrigger.AccountId);
            oppInTrigger.Opportunity_Industries__c = oppAccountIndustry;
        }
    }


to this:

    for(Opportunity oppInTrigger : Trigger.new) {
        String oppAccountIndustry = accountsMap.get(oppInTrigger.AccountId);
        IF(oppAccountIndustry != NULL && oppAccountIndustry != 'Agency') {
            oppInTrigger.Opportunity_Industries__c = oppAccountIndustry;
        }
    }


That should do the trick!

All Answers

MJ Kahn / OpFocusMJ Kahn / OpFocus
Hi Greg,

You very carefully (and correctly) create a Set of Ids of Accounts you care about, and then you query the database to build a map (indexed by Account Id) of the Industry for each Account. You do that because (I assume) you know that Trigger.new knows what each Opp's Account Id is, but doesn't know any of the values of the Account's fields. In other words, the object in Trigger.new has all of the fields for that object populated, but doesn't have values for any of the fields in parent records for the object.

But then, in the "for Opportunity" loop, you reference the Trigger.new's Opportunity's Account's Industry field. As we just said, Trigger.new doesn't have the Opportunity's Account's field's values, so oppInTrigger.Account.Industry will always be null, and your trigger won't work.

Change this:

    for(Opportunity oppInTrigger : Trigger.new) {
        IF(oppInTrigger.Account.Industry != NULL && oppInTrigger.Account.Industry != 'Agency') {
            String oppAccountIndustry = accountsMap.get(oppInTrigger.AccountId);
            oppInTrigger.Opportunity_Industries__c = oppAccountIndustry;
        }
    }


to this:

    for(Opportunity oppInTrigger : Trigger.new) {
        String oppAccountIndustry = accountsMap.get(oppInTrigger.AccountId);
        IF(oppAccountIndustry != NULL && oppAccountIndustry != 'Agency') {
            oppInTrigger.Opportunity_Industries__c = oppAccountIndustry;
        }
    }


That should do the trick!
This was selected as the best answer
ChickenOrBeefChickenOrBeef
Thanks, MJ!

I had to do that, and I also had to remove the IF statement I had in my first FOR loop.

Definitely useful to know that FOR loops can't reference related record fields.
ChickenOrBeefChickenOrBeef
*I meant Trigger.New loops, not FOR loops
MJ Kahn / OpFocusMJ Kahn / OpFocus
Oh good - you had me worried there for a minute!

Of course, you're right, you had to make a similar change in the loop where you figure out which Accounts to get. I was so focused on the bottom part of the trigger that I didn't go back and check the top.

I gather it's working now! (Just don't forget to write the unit tests for this trigger!)