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
Team WorksTeam Works 

Better way to write this Trigger?

Hello All,

 

I have got a custom Object 'Customer Project' in the related list of an opportunity with the picklist field Status (Active, Inactive). Whenever a record on this object is inserted or updated wit status = Active-->a custom field 'Activate Customer Project' checkbox field on Opportunity must be set to True. 

Moreover, as soon as the Record with status= Active in inserted for custom object ''Customer Project'' , the other records with the Active status must become inactive for the opportunity i.e only one customer project can exist with an Active status for an opportunity.

I have written the following code(bulkified) which is working but any suggestions if there is a better way to do this :

(Note : I have used  If(Trigger.Isupdate && trigger.isAfter)  and used this List insted of string List<Id> setcp = new List<Id>();

for testing only)

Many Thanks!!

 ----------------------------

trigger updateOpportunityStatus on Customer_Project__c (after insert, after update) {
                List<opportunity> opps = new List<opportunity>();
                List<Id> setcp = new List<Id>();
                List<Customer_Project__c> cps = new List<Customer_Project__c>();
                List<Customer_Project__c> cpps = new List<Customer_Project__c>();              
                opportunity opp;  
    
    If(Trigger.Isinsert && trigger.isAfter){
     for(Customer_Project__c cp : trigger.new){
        If(cp.status__c=='Active' && setcp.size()==0 ){
            setcp.add(cp.Id);        
             opp = [Select Id, name,Active_Cust_Project__c from Opportunity where Id= :cp.Opportunity__c];
            
            If(opp != null){
               opp.Active_Cust_Project__c = True;
               opps.add(opp); }
               
            }
           cps = [select Id,status__c from Customer_Project__c where Opportunity__c=:opp.id];
           for(Customer_Project__c c:cps){
             //Using the List because there is no get or a similar method for set
             If(c.Id != setcp.get(0)){
             c.status__c = 'Inactive';
             cpps.add(c);}           
           }
       }
       
     update cpps;     
    }
    
    If(Trigger.Isupdate && trigger.isAfter){
     for(Customer_Project__c cp : trigger.new){
        If(cp.status__c=='Active' ){
            opp = [Select Id, name,Active_Cust_Project__c from Opportunity where Id= :cp.Opportunity__c];
            If(opp != null && opp.Active_Cust_Project__c != True){
               opp.Active_Cust_Project__c = True;
               opps.add(opp);             }
          }
         cps = [select Id,status__c from Customer_Project__c where Opportunity__c=:opp.id];
           for(Customer_Project__c c:cps){
             //Using the List because there is no get or a similar method for set
             If(c.Id != setcp.get(0)){
             c.status__c = 'Inactive';
             cpps.add(c);}           
           }

      }
    }
        

update opps;
        
        

     }

------------------------------------------------------------------------------------------------------------------------------------------

Best Answer chosen by Admin (Salesforce Developers) 
k_bentsenk_bentsen

Try out the below. Also you can get rid of the If(Trigger.Isinsert && trigger.isAfter) and If(Trigger.Isupdate && trigger.isAfter) statements and just have one block of code.

 

    for(Customer_Project__c cp : trigger.new){
        If(cp.status__c=='Active' )
            setcp.add(cp.Id);          
    }
    if(!setcp.isEmpty()){
        List<Opportunity> oppList = [Select Id, name,Active_Cust_Project__c from Opportunity where Id IN :setcp];
        List<Id> oppIds = new List <Id>();
        for(Opportunity o; oppList){
            o.Active_Cust_Project__c = True;
            opps.add(o);
            oppIds.add(o.Id);
        }
        List<Customer_Project__c> cps = [select Id,status__c from Customer_Project__c where Opportunity__c IN :oppIds AND Id NOT IN :setcp];
        for(Customer_Project__c cs: cps){
            cs.status__c = 'Inactive';
            cpps.add(cs);
        }
        update cpps;
    }

 

 

All Answers

k_bentsenk_bentsen

You'll definitely want to move your SOQL queries outside your loops..
Team WorksTeam Works

Hey Thanks!! I would like to but how can i move the query out of the loop in case it is using  the cp variable which gets populated in the loop itself? Clue please 

for(Customer_Project__c cp : trigger.new
k_bentsenk_bentsen

Try out the below. Also you can get rid of the If(Trigger.Isinsert && trigger.isAfter) and If(Trigger.Isupdate && trigger.isAfter) statements and just have one block of code.

 

    for(Customer_Project__c cp : trigger.new){
        If(cp.status__c=='Active' )
            setcp.add(cp.Id);          
    }
    if(!setcp.isEmpty()){
        List<Opportunity> oppList = [Select Id, name,Active_Cust_Project__c from Opportunity where Id IN :setcp];
        List<Id> oppIds = new List <Id>();
        for(Opportunity o; oppList){
            o.Active_Cust_Project__c = True;
            opps.add(o);
            oppIds.add(o.Id);
        }
        List<Customer_Project__c> cps = [select Id,status__c from Customer_Project__c where Opportunity__c IN :oppIds AND Id NOT IN :setcp];
        for(Customer_Project__c cs: cps){
            cs.status__c = 'Inactive';
            cpps.add(cs);
        }
        update cpps;
    }

 

 

This was selected as the best answer
Team WorksTeam Works

Thanks mate : Excellent style of coding...there were couple of mistakes as u see in red...Thanks again

for(Customer_Project__c cp : trigger.new){
        If(cp.status__c=='Active' )
            setcp.add(cp.Id);

            setcpOpp.add(cp.opportunity__c);         
    }
    if(!setcp.isEmpty()){
       // List<Opportunity> oppList = [Select Id, name,Active_Cust_Project__c from Opportunity where Id IN :setcp];

      List<Opportunity> oppList = [Select Id, name,Active_Cust_Project__c from Opportunity where Id IN :setcpOpp];
        List<Id> oppIds = new List <Id>();
        for(Opportunity o; oppList){
            o.Active_Cust_Project__c = True;
            opps.add(o);
            oppIds.add(o.Id);
        }
        List<Customer_Project__c> cps = [select Id,status__c from Customer_Project__c where Opportunity__c IN :oppIds AND Id NOT IN :setcp];
        for(Customer_Project__c cs: cps){
            cs.status__c = 'Inactive';
            cpps.add(cs);
        }

       update opps;
        update cpps;
    }