First: I know this question has been posted many times so I apologize for making a new one, but I read over a lot of the old ones and they almost always seem to be that the SOQL query is inside of a loop. However, unless I'm totally blind, my query is not within a loop and I can't figure out what else I can do to optimize it further to avoid the governor limit.
The Ultimate Question: Why is this trigger being triggered in this context, and how can I optimize it so that I don't hit the Query limit?
Scenario:
We have a custom object called Account Administration.
When Account Administration is edited we send an outbound message to a receiver that updates an external SQL database with the new data from Account Admin. However, some of the important data from Account Admin is actually in formula fields that pull the data from the Account object that exists as its parent. Since formula changes don't trigger an edit/save, the outbound messages weren't going out when one of those important pieces changed on the account.
To combat this, we created a very basic trigger (Trigger.UpdateAccountAdmin) that gets a list of accounts where relevant fields have changed, and then does a SOQL query to get the list of Account Admin records that match the acctIds with relevant changes. We then just do a quick edit/save on each of those account admin records by way of setting a value to equal itself. This creates a save without modifying any actual record data, which triggers the outbound message as expected.
This trigger is live in Production and doing its job.
However, I am receiving the following Apex Test Failure when validating another ChangeSet against production:
Class Name: AccountUpdateHandler_TEST
Method Name: updateUsedTrainingTimeUsingEventMapsTest
Error Message: System.LimitException: Too many SOQL queries: 101.
Stack Trace: Trigger.UpdateAccountAdmin: line 34, column 1
Stack Trace points to line 34 in Trigger.UpdateAccountAdmin:
//get Account Admins For Update
acctAdminToUpdate = [SELECT Id, Enable_Online_Registration__c FROM Account_Administration__c WHERE Account_Name__c IN :acctIds];
Full UpdateAccountAdmin Trigger:
Trigger UpdateAccountAdmin on Account (before insert, after update)
{
boolean haveRelevantFieldsChanged(Account newAccount, Account oldAccount){
System.debug('!! newAccount: '+newAccount);
System.debug('!! oldAccount: '+oldAccount);
if(newAccount.Email__c != oldAccount.Email__c
|| newAccount.Billing_Status__c != oldAccount.Billing_Status__c
|| newAccount.Customer_Number__c != oldAccount.Customer_Number__c
|| newAccount.Delinquency_Date__c != oldAccount.Delinquency_Date__c
|| newAccount.Parent.Customer_Number__c != oldAccount.Parent.Customer_Number__c
|| newAccount.Parent.LEADS_Legacy_ID__c != oldAccount.Parent.LEADS_Legacy_ID__c
|| newAccount.Password__c != oldAccount.Password__c
|| newAccount.Support_Level__c != oldAccount.Support_Level__c){
return true;
}
else{
return false;
}
}
if (Trigger.isAfter && Trigger.isUpdate) {
// Lists
List<Id> acctIds = new List<Id>();
List<Account_Administration__c> acctAdminToUpdate = new List<Account_Administration__c>();
//Get Account IDs
for(Account a :Trigger.new)
{
if(haveRelevantFieldsChanged(a, Trigger.oldMap.get(a.Id)))
acctIds.add(a.Id); // Add IDs to List
}
//get Account Admins For Update
acctAdminToUpdate = [SELECT Id, Enable_Online_Registration__c FROM Account_Administration__c WHERE Account_Name__c IN :acctIds];
//iterate through Account Admins to update
for(Account_Administration__c aa :acctAdminToUpdate)
{
aa.Enable_Online_Registration__c = aa.Enable_Online_Registration__c;
}//trigger update on save
//update Account Admin
update acctAdminToUpdate;
}
}
The Problem:
The Class (AccountUpdateHandler_TEST) and Method Name (updateUsedTrainingTimeUsingEventMapsTest) mentioned in the Test Failure don't even modify any of the relevant fields that are checked by the boolean in the Trigger that we use to determine if changes are relevant before adding them to the acctIds list, so I don't understand why it's even triggering in the first place.
AccountUpdateHandler_TEST.updateUsedTrainingTimeUsingEventMapsTest:
@isTest
private class AccountUpdateHandler_TEST {
//tests method: updateUsedTrainingTimeUsingEventMaps(Map<Id,Event> oldEventMap, Map<Id,Event> newEventMap)
@isTest static void updateUsedTrainingTimeUsingEventMapsTest() {
// Create Custom Settings for MSI TriggerControl
MSITriggerControlHelper.create();
// Implement test code
Product2 product = new Product2(Name = 'Non-Account WhatId');
insert product;
Account account = new Account(Name = 'Test Account');
account.Total_Training_Minutes__c = 0;
insert account;
Event event = new Event();
event.DurationInMinutes = 60;
event.ActivityDateTime = Datetime.now();
insert event;
// If the event was not associated with an account, don't bother looking at this event
update event;
// If the account associated with this event could not be retrieved, don't bother looking at this event
// WhatId is set to a product, so no related event was returned in query
event.WhatId = product.Id;
update event;
// Move on to the next event if:
// (a) the event does not contain training or
// (b) the event length was not specified
event.WhatId = account.Id;
update event;
// If the account has no training minutes left
//move on to the next event
event.Activity__c = 'Onsite Training';
update event;
// If the event is 'newly' completed (i.e., the status is moving from 'Open' to 'Completed', we add the account id and the training minutes
// used to a map and move on to the next event
// Check if the amount of minutes we are about to claim as 'used' will exceed the total number of training minutes remaining on the account
event.Event_length_in_Minutes__c = '60';
update event;
account.Total_Training_Minutes__c = 30;
update account;
update event;
//usedMinutes >= account.Total_Training_Minutes__c
event.Status__c = 'Open';
event.Event_length_in_Minutes__c = '240';
update event;
event.Status__c = 'Completed';
update event;
//usedMinutes < account.Total_Training_Minutes__c
account.Total_Training_Minutes__c = 600;
update account;
event.Status__c = 'Open';
event.Event_length_in_Minutes__c = '60';
update event;
event.Status__c = 'Completed';
update event;
// Another scenario is if the status of the event was previously completed but the event minutes has changed.
// If the status has stayed as 'completed' and there was no change in the event length, move on to the next event
event.Status__c = 'Completed';
update event;
Account account2 = new Account(Name = 'Test Account');
account2.Total_Training_Minutes__c = 600;
insert account2;
event.WhatId = account2.Id;
update event;
//account.Used_Training_Minutes__c == null
event.Event_length_in_Minutes__c = '240';
update event;
//account.Used_Training_Minutes__c >= previousEventLength
account2.Used_Training_Minutes__c = 600;
update account2;
event.Event_length_in_Minutes__c = '60';
update event;
//account.Used_Training_Minutes__c < previousEventLength
account2.Used_Training_Minutes__c = 10;
update account2;
event.Event_length_in_Minutes__c = '240';
update event;
//// Check if the amount of minutes we are about to claim as 'used' will exceed the total number of training minutes remaining on the account
//usedMinutes >= account.Total_Training_Minutes__c
account2.Used_Training_Minutes__c = 600;
account2.Total_Training_Minutes__c = 50;
update account2;
event.Event_length_in_Minutes__c = '60';
update event;
}
The Ultimate Question (Repeated from Top): Why is this trigger being triggered in this context, and how can I optimize it so that I don't hit the Query limit?
Best Answer
Your trigger looks fine to me. Just one quick tip. You don't actually need to do anything to the collection of
Account_Administration__c
records before calling DML update. You will cause your trigger to fire (assuming you have a before/after update trigger onAccount_Administration__c
) simply by virtue of callingupdate <collection>
.You could go so far as to just issue an update on the result set of a query.
Now, on to your failing test.
From the look of it, you're doing a ton of setup. I didn't count exactly how many dml updates were issued, but it's not a small number.
Each of those updates causes triggers to run, which probably contribute to the number of SOQL queries being run before your test proper even begins.
I have two suggestions:
Event
updatesTest.startTest()
andTest.stopTest()
The second suggestion in particular will help you out. It is used to separate test setup from the actual test that you're trying to run. It also separates the governor limits, giving you a fresh set for use in your test proper.
+edit:
On further inspection, your unit test is also incomplete. There aren't any assertions being made, so you're not really testing anything. You're getting the code coverage, yes, but covered != tested.
I could still look over this test some more; but, after my second reading, I get the feeling that instead of being a single test, this should be broken up into multiple tests (each testing only one of the event updates, querying the account after the test, and making assertions against the result state of your account).