0

Write a trigger on the Account when the Account is updated check all opportunities related to the account. Update all Opportunities Stage to close lost if an opportunity created date is greater than 30 days from today and stage not equal to close won. I have no idea why it is not working.

trigger AccountTrigger on Account (after update) {
   
 if(Trigger.isAfter && Trigger.isUpdate){
      List<Opportunity> oppList= new List<Opportunity>();
      List<Opportunity> listAccountIds=[SELECT stageName, createdDate, accountId FROM Opportunity WHERE AccountId !=null];
       for(Account acc:Trigger.new){
        for(Opportunity opp: listAccountIds){
            if(acc.id==opp.AccountId){
                if(opp.CreatedDate<System.now()-30 && opp.stageName !='Close Won'){
                    opp.StageName = 'Close Lost';
                     oppList.add(opp);
                }
         }
       }
     }
        update oppList;
            
    }
}
1
  • 2
    There are a whole bunch of issues here: unfiltered SOQL, nested for loops to perform ID matching, logic directly in trigger, subtraction of 30 from datetime (which goes wrong across DST transitions). Take a look at this Q&A for help in querying and matching the data you need for this logic.
    – Phil W
    Sep 17, 2022 at 20:45

1 Answer 1

3
    List<Opportunity> listAccountIds=[SELECT stageName, createdDate, accountId FROM Opportunity WHERE AccountId !=null];

You're essentially querying every record in the database. Instead, use an appropriate filter:

Opportunity[] opportunities = [
  SELECT StageName, CreatedDate 
  FROM Opportunity 
  WHERE AccountId = :Trigger.new
];

This eliminates the following check:

if(acc.id==opp.AccountId){

Similarly, we can apply this logic for three other conditions:

Opportunity[] opportunities = [
  SELECT Id
  FROM Opportunity
  WHERE AccountId = :Trigger.new AND
    CreatedDate < LAST_N_DAYS:30 AND
    StageName NOT IN ('Closed Won','Closed Lost')
];

The LAST_N_DAYS:30 targets the last 30 days, so using CreatedDate < LAST_N_DAYS:30 covers dates earlier than the last 30 days.

In addition, we know that we're updating to Closed Lost, so we don't need to update records that are already Closed Lost.

We don't need to copy the values to a new list, we can just assign the new values, and save. That brings the final code down to just:

trigger AccountTrigger on Account (after update) {
  Opportunity[] opportunities = [
    SELECT Id
    FROM Opportunity
    WHERE AccountId = :Trigger.new AND
      CreatedDate < LAST_N_DAYS:30 AND
      StageName NOT IN ('Closed Won', Closed Lost')
  ];
  for(Opportunity record: opportunities) {
    record.StageName = 'Closed Lost';
  }
  update opportunities;
}

As far as the initial if statement:

if(Trigger.isAfter && Trigger.isUpdate){

Never write a condition that will always be true, or always be false. If you need it later, add it later.


Finally, consider using a trigger framework. It's considered a best practice in Salesforce to minimize the amount of logic the actual trigger does. This makes it easier to test the trigger and add enhancements in a centralized manner.

I have my own personal preference as to what a trigger framework looks like, but there are many to choose from. Find one that is no more powerful than what you think you'll need. It's easy to fall into a trap of overengineering.

You must log in to answer this question.

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