You need to sign in to do that
Don't have an account?
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); } }
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:
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.
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