0

I am getting error message and I believe it to be error with this trigger. I am new to coding and this was done previously so any assistance is greatly appreciated

trigger setAttendeeNumbers on PCM_Time_Record__c (after delete, after insert, after update) 
//purpose is to update a pair of cDecision custom fields with a count of valid attendees on the meeting
{
  list<PCM_Time_Record__c> triggerList;
  Map<Id, RecordType> myRTs = new Map<Id, RecordType>([SELECT Id, Name FROM RecordType WHERE SObjectType = 'PCM_Time_Record__c']);
  if(trigger.isDelete)
  {
triggerList = trigger.old;  
  }
  else
  {
triggerList = trigger.new;
  }

  for(PCM_Time_Record__c act:triggerList)
  {  
    if(myRTs.get(act.RecordTypeId).Name == 'PCM Activity')
    {    

  // this section skips the run IF: 
  //     the attendees list is empty
  //     the Account_Attendees__c amount = the length of the Account Attendees SOQL - i.e. field already calculated

  list<PCM_Activity_Attendee__c> attendees = [select id, Account_Attendees__c, Contact_Attendees__c,Account_Lookup__c, contact__c from pcm_activity_attendee__c where PCM_Activity__c = :act.id];

  AggregateResult[] existcount = [select SUM(Account_Attendees__c)att, SUM(Contact_Attendees__c) from pcm_activity_attendee__c where PCM_Activity__c = :act.id];            
  Decimal existcountint =(decimal)existcount[0].get('att'); // converts aggregateresult to figure that can be evaluated in boolean

  if ((attendees.size() > 0) & (attendees.size() <> existcountint)) 

  {

    set<id> accountIds = new set<id>();
    set<id> contactIds = new set<id>();

    for(PCM_Activity_Attendee__c att:attendees)
    {
      system.debug('att id' + att.id);
      system.debug('account id' + att.account_lookup__c);
      system.debug('contact id' + att.contact__c);
      accountIds.add(att.account_lookup__c);
      contactIds.add(att.contact__c);
    }
    for(id accountId:accountIds)
    {
// The SOQL below:
//    Looks for an account ID (based on the Activity being deleted)
//    Looks for the number of Attendees from that Account ID who are having a meeting on that day
// This is then written back in each of those Attendee records in the Account_Attendee__c field
// Why this is done, I have no idea. Well done anyway.

      list<PCM_Activity_Attendee__c> accountAttendees = [select Account_Attendees__c, Contact_Attendees__c from PCM_Activity_Attendee__c where meeting_date__c = :act.date__c and Account_Lookup__c = :accountId];
      for(PCM_Activity_Attendee__c accAtt:accountAttendees)
      {
        accAtt.Account_Attendees__c = accountAttendees.size();
      }  
      update accountAttendees;
    }
    for(id contactId:contactIds)
        {
// The SOQL below:
//    Looks for a Contact ID (based on the Activity being deleted)
//    Looks for the number of instances by that Contact ID on that particular day
// This is then written back in each of those Attendee records in the Contact_Attendee__c field
// Why this is done, I have no idea. Well done anyway.  



      list<PCM_Activity_Attendee__c> contactAttendees = [select Account_Attendees__c, Contact_Attendees__c from PCM_Activity_Attendee__c where meeting_date__c = :act.date__c and contact__c = :contactId];    
      for(PCM_Activity_Attendee__c conAtt:contactAttendees)
      {
        conAtt.Contact_Attendees__c = contactAttendees.size();
      }      
      update contactAttendees;  
    }  
  }
}


}
}

2 Answers 2

1

The reason you are getting the error is because you are using SOQL statements inside a for loop which is a very bad practice:

  for(PCM_Time_Record__c act:triggerList)
  {  
    if(myRTs.get(act.RecordTypeId).Name == 'PCM Activity')
    {   

    list<PCM_Activity_Attendee__c> attendees = [select id, Account_Attendees__c, Contact_Attendees__c,Account_Lookup__c, contact__c from pcm_activity_attendee__c where PCM_Activity__c = :act.id];

      AggregateResult[] existcount = [select SUM(Account_Attendees__c)att, SUM(Contact_Attendees__c) from pcm_activity_attendee__c where PCM_Activity__c = :act.id];            
      Decimal existcountint =(decimal)existcount[0].get('att');  


 //etc

      }
   }

In order to avoid this for example you'd have to do something like this:

Set<ID> timeIDs = new Set<Ids>

 for(PCM_Time_Record__c act:triggerList)
  {  
    if(myRTs.get(act.RecordTypeId).Name == 'PCM Activity')
    {    
          timeIDs.add(act.id);
    }
  }


  list<PCM_Activity_Attendee__c> attendees = [select id, Account_Attendees__c, Contact_Attendees__c,Account_Lookup__c, contact__c from pcm_activity_attendee__c where PCM_Activity__c = :timeIds];

for(PCM_Activity_Attendee__c act: attendees)
{
  // do what ever you want
}

So make sure you add the record ids in a set, then have only one SOQL query which uses the set of ids you constructed before. You can find more here about best apex code practices : https://developer.salesforce.com/page/Apex_Code_Best_Practices

1

You have SOQL queries inside of a FOR loop. This is NOT best practice and is likely causing your errors. You need to look into leveraging collections and use a single SOQL outside of the FOR loop.

As an example, the code below is not best practice as it has SOQL inside a for loop to get the Parent account of every contact

For(Contact c : contacts){
     Account a = [Select Id, Name, Type From Account Where Id =: c.AccountId];
}

You could do this much more efficiently by leveraging a collection outside of the FOR loop

set<Id> accIDs = new set<Id>();
for(Contact c : contacts){
     accIDs.add(c.AccountId);
}
map<Id,Account> accountMAp = new map<Id,Account>([Select Id, Name, Type From Account Where ID IN : accIDs]);

for(Contact c : contacts){
     Account a = accMap.get(c.AccountId);
}
1
  • Hi Chris - thank you for this - as I still new to coding and this wasnt written by me I am trying to work out where this needs to be updated?
    – user29991
    Mar 23, 2016 at 12:12

You must log in to answer this question.

Not the answer you're looking for? Browse other questions tagged .