function readOnly(count){ }
Sign Up ›
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
Learn More

Browse by Topic
ShowAll Questionssorted byDate Posted
+ Start a Discussion
John AthitakisJohn Athitakis 

Trigger Fires in some instances and not others

Here is a simple trigger. There are two objects with look ups to one another, Project and Opportunity.
Goal here is that when a Project is created with a lookup populated to Opportunity, that the code goes out to that Opportunity and populates the lookup withe the id to the project. This should work on create/change and if the lookup on Project is nulled out, that the lookup that may have exited pointing back to it from Opportunity is also nulled.

The code is working in the first instance (when a project is created, it finds the opp and fills out the lookup) but it is not doing any actions on 'update' (nulling or updating) which is confusing me given the simplicty of the code.

Thoughts?

 

trigger PSA_Project_Automation on pse__Proj__c (before insert, before update) {
List<Id> OpportunityIds = new List<Id>();
List<Id> NullOpportunityIds = new List<Id>();   
    for (pse__Proj__c proj : Trigger.new){
       
            if(proj.pse__Opportunity__c != null){
            OpportunityIds.add(proj.pse__Opportunity__c);
            }  
           
        if(Trigger.IsUpdate){
            pse__Proj__c oldProj = Trigger.oldMap.get(proj.Id);
            
            if(proj.pse__Opportunity__c == null && oldProj.pse__Opportunity__c !=null)
             NullOpportunityIds.add(oldProj.pse__Opportunity__c); 
        }
        
    }

    List<Opportunity> oppList = [SELECT id, pse__Primary_Project__c FROM Opportunity Where id in :OpportunityIds];
    List<Opportunity> NullOppList = [SELECT id, pse__Primary_Project__c FROM Opportunity Where id in :NullOpportunityIds];
    
    for(pse__Proj__c p : Trigger.new){
    for(integer i=0; i<oppList.size(); i++){
      oppList[i].pse__Primary_Project__c = p.id;
        }
    for(integer i=0; i<NullOppList.size(); i++){
      NullOppList[i].pse__Primary_Project__c = null;      
        }
    }
Best Answer chosen by John Athitakis
JeffreyStevensJeffreyStevens
I assume you're doing DML after the code shown here?  In this case - you're probably doing DML twice to the same object?  (opportunity).

Here's another way of writing the trigger you might think about...
  
trigger PSA_Project_Automation on pse__Proj__c (before insert, before update) {
  
  list<id> opportunityIds = new set<id>();
  map<id,id> mOpptyProjIds = new map<id,id>();

  for(pse__Proj__c proj : trigger.new) {
    if(proj.pse__Opportunity__c != null) {
      opportunityIds.add(proj.pse__Opportunity__c);
      mOpptyProjIds.put(proj.pse__Opportunity__c,proj.id);
    }
    if(trigger.isUpdate) {
      if(trigger.oldmap.get(proj.id).pse__opportunity__c != null
          && proj.pse__Opportunity__c == null
         ) {
        opportunityIds.add(trigger.oldmap.get(proj.id).pse__opportunity__c);
        mOpptyProjIds.put(trigger.oldmap.get(proj.id).pse__opportunity__c,proj.id);
      }
    }
  }

  // Get the Opportunities and update them
  list<opportunity> opportunitiesToUpdate = new list[SELECT id,pse__Primary_Project__c FROM Opportunity WHERE id IN :opportunityIds];

  for(opportunity o :opportunitiesToUpdate) {
    if(mOpptyProjIds.containsKey(o.id)) {
      o.pse_Primary_Project__c = mOpptyProjIds.get(o.id);
    }
  }

  if(opportunitiesToUpdate.size()>0) {
    update opportunitiesToUpdate;
  }
}

On second thought - not sure that helps that much - it does save you two nested loops, and a soql.  

I'm wondering if the fact that YOUR lines 6,7,8 are executed on both insert and update - that might be a problem also. 

All Answers

JeffreyStevensJeffreyStevens
I assume you're doing DML after the code shown here?  In this case - you're probably doing DML twice to the same object?  (opportunity).

Here's another way of writing the trigger you might think about...
  
trigger PSA_Project_Automation on pse__Proj__c (before insert, before update) {
  
  list<id> opportunityIds = new set<id>();
  map<id,id> mOpptyProjIds = new map<id,id>();

  for(pse__Proj__c proj : trigger.new) {
    if(proj.pse__Opportunity__c != null) {
      opportunityIds.add(proj.pse__Opportunity__c);
      mOpptyProjIds.put(proj.pse__Opportunity__c,proj.id);
    }
    if(trigger.isUpdate) {
      if(trigger.oldmap.get(proj.id).pse__opportunity__c != null
          && proj.pse__Opportunity__c == null
         ) {
        opportunityIds.add(trigger.oldmap.get(proj.id).pse__opportunity__c);
        mOpptyProjIds.put(trigger.oldmap.get(proj.id).pse__opportunity__c,proj.id);
      }
    }
  }

  // Get the Opportunities and update them
  list<opportunity> opportunitiesToUpdate = new list[SELECT id,pse__Primary_Project__c FROM Opportunity WHERE id IN :opportunityIds];

  for(opportunity o :opportunitiesToUpdate) {
    if(mOpptyProjIds.containsKey(o.id)) {
      o.pse_Primary_Project__c = mOpptyProjIds.get(o.id);
    }
  }

  if(opportunitiesToUpdate.size()>0) {
    update opportunitiesToUpdate;
  }
}

On second thought - not sure that helps that much - it does save you two nested loops, and a soql.  

I'm wondering if the fact that YOUR lines 6,7,8 are executed on both insert and update - that might be a problem also. 
This was selected as the best answer
John AthitakisJohn Athitakis
Hello Jeffrey, 

Thank you! This is definately worked with a few little modifications! Thank you for this alternate way of doing this! It's very efficient.