0

I'm new in Saleforce world. I need some help.

Scenario: The user creates an Opportunity with an empty Best Opportunity Because (Best_opp_because__c) field for the account. When the user chooses the best opportunity, the remaining existing opportunities belonging to the account should change their Page Layout (RecordTypeId) so that they no longer have the Best_opp_because__c field.

My Trigger:

trigger OpportunityTrigger on Opportunity (before update) {
    List<Opportunity> listOfOpp1 = [SELECT Id, Name, RecordTypeId, Best_opp_because__c FROM Opportunity WHERE Best_opp_because__c != null AND AccountId =: Trigger.new[0].AccountId];
    List<Opportunity> listOfOpp2 = [SELECT Id, Name, RecordTypeId, Best_opp_because__c FROM Opportunity WHERE Best_opp_because__c = null AND AccountId =: Trigger.new[0].AccountId];

    if(listOfOpp1.size() != 0){
        for(Opportunity opp : listOfOpp2){
            opp.RecordTypeId = '000...';
        }
    }
}

But unfortunately it doesn't work. Thank you for your help.

1
  • Please clarify your specific problem or provide additional details to highlight exactly what you need. As it's currently written, it's hard to tell exactly what you're asking.
    – Community Bot
    Feb 16, 2022 at 15:44

1 Answer 1

1

I'd suggest a completely different approach from what your provided trigger shows, but before that I have a quick observation.

On avoiding DML in before triggers

The behavior of before trigger contexts that allows you to avoid needing to perform a DML update only applies to changes made to the instances of records contained in trigger.new or trigger.newMap. The records you're trying to update aren't contained in one of those trigger context variables, so DML is required.

Even if you were working on records contained in a trigger context variable, when you run a query for those records you get a different in-memory instance of that record.

tl;dr: If you want to avoid DML, you need to work on the records stored in trigger.new or trigger.newMap

On a better approach for your trigger

You don't need to query for both the records with Best_opp_because__c set and those without it set. Your plain-language description of what you want to do should serve as a good foundation for what your trigger should look like.

In a nutshell, you need to do 4 things:

  1. Figure out which (if any) Opportunities being updated make a change to Best_opp_because__c
  2. Gather the Ids of the related Accounts
  3. Query to find all of the other Opportunities related to those Accounts
  4. Update the record type of those other Opportunities

In an update trigger, you have access to trigger.new, trigger.old, trigger.newMap, and trigger.oldMap. So if you want to check to see if a field is changing, you should compare the old record values to the new record values.

// The easiest way to get the corresponding old record is to use the oldMap
// You could use trigger.old, but then you'd need to loop from 0 to trigger.new.size()
//   and trigger.new[index] and trigger.old[index]
// Using the oldMap when it's available just makes things a bit easier (and saves
//   some typing)
for(SomeObject newRec :trigger.new){
    SomeObject oldRec = trigger.oldMap.get(newRec.Id);

    if(oldRec.Some_Field__c != newRec.Some_Field__c){
        // do something
    }
}

A lot of working in Apex comes down to operating on collections of records instead of operating on individual records. If you ever find yourself writing something like trigger.new[0], then you need to take a step back and figure out how to work on the entire collection instead.

Queries and DML are two things that should essentially never be inside of a loop. Use a loop to gather the data you need into a collection, and then perform the operation outside of the loop. In your case, inside of that // Do something if block in my example is where you would add an Account Id to a List or a Set (which you'd define and initialize before and outside of the for loop).

The general pattern is

Set<Id> myIdSet = new Set<Id>();

for(SomeObject__c record :records){
    if(some condtion){
        myIdSet.add(record.Some_Id__c);
    }
}

List<OtherObject__c> relatedRecords = [SELECT Id FROM OtherObject__c WHERE Id IN :myIdSet];

Using negative filters in a query (NOT IN, Field__c != value, etc...) is something to generally avoid if you can. Negative filters aren't the end of the world in most cases, but they do tend to make query performance and selectivity worse (which Salesforce will complain about if it gets bad enough).

The approach I'd suggest here would be to:

  • Query all Opportunities related to the Account Ids you've gathered
  • Put the result into a Map
  • Remove the records that have a change to Best_Opp_Because__c
// Provided that you have another, separate Set of Ids that you add the Opportunity Id to
//  when Best_Opp_Because__c changes to a non-null value...
Map<Id, Opportunity> oppsToUpdate = new Map<Id, Opportunity>([query for Opps matching our target account ids here]);

// Maps don't allow us to remove items in bulk, but Sets do.
// So we grab the map's keyset, and remove items from that.
// The end result is that all of those items are removed from the map in one fell
//   swoop
oppsToUpdate.keySet().removeAll(oppIdsSetWhereTheTargetFieldIsChanging);

That should leave you with the list of records (and only those records) that need their recordtype changed. Make the change in a loop, perform the DML update outside of the loop, and you should be done!

A few last suggestions

  • You really only want one trigger per object (the order that triggers run in if you have two or more on an object is not predictable)
  • Triggers shouldn't have logic inside of them. It's fine for getting your feet wet, but you really should use a Trigger Framework (like Kevin O'Hara's). Keep the logic in a separate class, and call that class from your trigger.
  • Hard-coding Ids is a red flag, and should be avoided. For getting your target record type, you can obtain that information through the SObject Describe Information (Schema.SObjectType.Opportunity.getRecordTypeInfosByName().get('My record type name').getRecordTypeId()). This helps avoid issues when Ids change between environments (if you create a recordtype in a sandbox, then deploy it to production, that recordtype will have a different Id in production)

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .