+ Start a Discussion
SFAdmin5SFAdmin5 

Is my trigger bulkified?

Hi-

 

So I've got a trigger that works as intended (took me a week to write this thing, with help from the boards here, as I'm not a developer).

 

The trigger below works and the test class passes it at 95%.  The issue is that even though it's got high enough test coverage to go to prod, and even though I've used Demand Tools to insert thousands of records into a test environment where this code is live to try to hit governor limits, and I'm not hitting any limits or having any issues, I don't know for sure whether or not this trigger would be considered by a real developer as "bulkified".

 

Can someone let me know if this is a bulkified trigger, and if not, perhaps adjust the code or make some suggestions as to how I can change it to make is "bulkified".

 

Thanks a lot.

 

Trigger:

 

trigger CustomerUsage on Customer_Usage__c (before insert, before update, after insert, after update, after delete) {
   
    // ----------------------------------------------------------------
    // Partner First Gross Add & Most Recent Gross Add Processing
    // -----------------------------------------------------------

    if ((Trigger.isInsert || Trigger.isUpdate) && Trigger.isAfter) {    
     
      set<Id> AccountIds = new set<Id>();
        if(trigger.isInsert || trigger.isUpdate){    
          for(Customer_Usage__c p : trigger.new){      
              AccountIds.add(p.Account__c);
          }
        }
 
     map<Id,DateTime> AccountMap = new map <Id,Datetime>(); 
       for(Customer_Usage__c q : [select Id,First_Positive_Invoice_Date__c,Account__c
           from Customer_Usage__c 
           where Account__c IN :AccountIds  
           order by First_Positive_Invoice_Date__c asc limit 1]){       
           AccountMap.put(      
             (ID)q.get('Account__c'),
             (Datetime)q.get('First_Positive_Invoice_Date__c')
           );
       }
 
      List<String> tempLst=new List<String>();
      Map<String, String> accountid_partner_map=new Map<String, String>();
        for(Account a : [Select Id, Referring_Partner__c
            from Account
            where Id IN :AccountIds]){
    
            tempLst.add(a.Referring_Partner__c);
            accountid_partner_map.put(a.Referring_Partner__c, a.id);
        }

       List <Account> AccountsToUpdate = new List <Account>();
         for(Account a : [Select Id,Referring_Partner__c,Partner_First_Gross_Add__c,Partner_Most_Recent_Gross_Add__c
             from Account 
             where Id IN :tempLst]){
             Datetime FPID = AccountMap.get(accountid_partner_map.get(a.Id));
    
           if(a.Partner_First_Gross_Add__c==null && a.Partner_Most_Recent_Gross_Add__c ==null){            
              a.Partner_First_Gross_Add__c = FPID ;  
              a.Partner_Most_Recent_Gross_Add__c = FPID;           
           }else {           
           if((a.Partner_Most_Recent_Gross_Add__c == a.Partner_First_Gross_Add__c && a.Partner_Most_Recent_Gross_Add__c < FPID) ||   a.Partner_Most_Recent_Gross_Add__c < FPID )
              a.Partner_Most_Recent_Gross_Add__c = FPID ;
           }
              AccountsToUpdate.add(a);
              update AccountsToUpdate; 
         } 
    }
}

 

 

 

Test Class:

 

@isTest
private class Test_PFGA {

  static testMethod void Test_PFGA() {
  

        Account a = new Account ();
        a.Name = 'TEST_PartnerAccount';
        a.Partner_First_Gross_Add__c = null;
        a.Partner_Most_Recent_Gross_Add__c = null;


        insert a;
    
        System.assertEquals(
        null, 
        a.Partner_First_Gross_Add__c);
        
        System.assertEquals(
        null, 
        a.Partner_Most_Recent_Gross_Add__c );
    
    
        Account a2 = new Account ();
        a2.Name = 'TEST_SalesAccount';
        a2.Referring_Partner__c = a.Id;

        insert a2;

        Customer_Usage__c cu = new Customer_Usage__c();
        cu.Account__c = a2.Id;
        cu.First_Positive_Invoice_Date__c =  datetime.newInstance(2008, 12, 1);

        insert cu;

        System.assertEquals(
        cu.First_Positive_Invoice_Date__c,
        a.Partner_First_Gross_Add__c);
        
        System.assertEquals(
        a.Partner_First_Gross_Add__c, 
        a.Partner_Most_Recent_Gross_Add__c);
    
        Customer_Usage__c cu2 = new Customer_Usage__c();
        cu2.Account__c = a2.Id;
        cu2.First_Positive_Invoice_Date__c =  datetime.newInstance(2010, 12, 1);

        insert cu2;    
        
        System.assertEquals(
        cu2.First_Positive_Invoice_Date__c,
        a.Partner_First_Gross_Add__c);
    
    
  }
  

  
  
  

}

 

sfdcfoxsfdcfox

I'm concerned about the "LIMIT 1" you've got in there. I don't think you can bulk-update with this trigger. Looks like you want an aggregate query here:

 

for(aggregateresult summary:[select account__c Id, min(first_positive_invoice_date__c) invDate from customer_usage__c where account__c in :accountids group by account__c]) {
  accountmap.put((id)summary.get('Id'), (datetime)summary.get('invDate'));
}

Other than that, while your code is not "optimized" (I'm sure you could reduce some of this code, maybe up to 50% of it is not strictly necessary), it will run within governor limits and as expected, so it's good enough to be qualified as "bulkified", given the change I've suggested above.

 

Edit: Had a typo on line 2.

SFAdmin5SFAdmin5

Awesome thanks a lot for the help.

 

I think I made the revision as you suggest successfully.  Basically I used that aggregate query you provided in my first map collection, replacing my soql/limit 1 in the map with your query, and the trigger compiled just fine.  I ran a quick test and the trigger still works as intended.

 

I'll work on optimizing this.  If you think this thing isn't optimized now, you should have seen the thing 2 days ago: )

 

Thanks again.

 

Here's my revised code in case anyone wants to use it

 

    // ------------------------------------------------------------------------------
    // Partner First Gross Add & Most Recent Gross Add Processing
    // ------------------------------------------------------------------------------

    if ((Trigger.isInsert || Trigger.isUpdate) && Trigger.isAfter) {    
     
      set<Id> AccountIds = new set<Id>();
        if(trigger.isInsert || trigger.isUpdate){    
          for(Customer_Usage__c p : trigger.new){      
              AccountIds.add(p.Account__c);
          }
        }
 
       map<Id,DateTime> AccountMap = new map <Id,Datetime>();   
           for(aggregateresult summary:[select account__c Id, min(first_positive_invoice_date__c) invDate 
                                        from customer_usage__c 
                                        where account__c in :AccountIds group by Account__c]) {
           accountmap.put((id)summary.get('Id'), (datetime)summary.get('invDate'));
           }
       
 
      List<String> tempLst=new List<String>();
      Map<String, String> accountid_partner_map=new Map<String, String>();
        for(Account a : [Select Id, Referring_Partner__c
            from Account
            where Id IN :AccountIds]){
    
            tempLst.add(a.Referring_Partner__c);
            accountid_partner_map.put(a.Referring_Partner__c, a.id);
        }

       List <Account> AccountsToUpdate = new List <Account>();
         for(Account a : [Select Id,Referring_Partner__c,Partner_First_Gross_Add__c,Partner_Most_Recent_Gross_Add__c
             from Account 
             where Id IN :tempLst]){
             Datetime FPID = AccountMap.get(accountid_partner_map.get(a.Id));
    
           if(a.Partner_First_Gross_Add__c==null && a.Partner_Most_Recent_Gross_Add__c ==null){            
              a.Partner_First_Gross_Add__c = FPID ;  
              a.Partner_Most_Recent_Gross_Add__c = FPID;           
           }else {           
           if((a.Partner_Most_Recent_Gross_Add__c == a.Partner_First_Gross_Add__c && a.Partner_Most_Recent_Gross_Add__c < FPID) ||   a.Partner_Most_Recent_Gross_Add__c < FPID )
              a.Partner_Most_Recent_Gross_Add__c = FPID ;
           }
              AccountsToUpdate.add(a);
              update AccountsToUpdate; 
         } 
    }