function readOnly(count){ }
Starting November 20, the site will be set to read-only. On December 4, 2023,
forum discussions will move to the Trailblazer Community.
+ Start a Discussion
lescclarkcpdlescclarkcpd 

A tough trigger to bulkify

Hi

 

I am getting the hang of these but need to bulkify this trigger.  Essentially it takes a course_application and checks to see if the course applicant is a campaign member.  There are a couple of many to many relationships so suspect that I will need to use a map with a list?  If a campaign member exists it updates it, if not create a new one.  Managed to do this by iterating through the course apps and building a list then checking the size of the list.  However, It also doesn't take into account any new course_applications created during the fioring of the trigger.  Any help appreciated

 

les

 

 List<CampaignMember> cmbadd = new List<CampaignMember>();
       List<CampaignMember> cmbupdate = new List<CampaignMember>();
        
        for(Course_Application__c c:Trigger.new)      	
        {
        List<CampaignMember> cammem =[select id
             from CampaignMember
             where CampaignMember.contactid =:c.Course_Applicant_del__c 
             AND CampaignMember.CampaignId=:c.PullCampaignID__c];
    
             if(cammem.size() == 0)
             	{          
		     		CampaignMember cmb = new CampaignMember(contactId=c.Course_Applicant_del__c, CampaignId=c.PullCampaignID__c, status=c.status__c);
		            cmbadd.add(cmb);
		            
		     	}    
             	Else
             	{
             		 CampaignMember cmbup = cammem[0];
		             cmbup.status=c.status__c;
		             cmbupdate.add(cmbup);
		             
             	} 
             	
        } 
                
    	    Database.insert(cmbadd);
    	    Database.update(cmbupdate);

 

GlynAGlynA

Les,

 

Give this a try:

 

    Map<Id,Map<Id,CampaignMember>> map_ContactID_Members = new Map<Id,Map<Id,CampaignMember>>();

    for ( Course_Application__c application : Trigger.new )
    {
        map_ContactID_Members.put( application.Course_Applicant_del__c, new Map<Id,CampaignMember>() );
    }

    for ( CampaignMember member :
        [   SELECT  Id, ContactId, CampaignId, Status
            FROM    CampaignMember
            WHERE   ContactId IN :map_CampaignID_Members.keySet()
        ]
        )
    {
        map_ContactID_Members.get( member.ContactId ).put( member.CampaignId, member );
    }

    List<CampaignMember> list_CampaignMembersToUpsert = new List<CampaignMember>();

    for ( Course_Application__c application : Trigger.new )
    {
        Map<Id,CampaignMember> map_CampaignMembers = map_ContactID_Members.get( application.Course_Applicant_del__c );

        if ( map_CampaignMembers.containsKey( application.PullCampaignID__c ) )
        {
            CampaignMember member = map_CampaignMembers.get( application.PullCampaignID__c );

            member.Status = application.Status__c;

            list_CampaignMembersToUpsert.add( member );
        }
        else
        {
            list_CampaignMembersToUpsert.add
            (   new CampaignMember
                (   ContactId   = application.Course_Applicant_del__c,
                    CampaignId  = application.PullCampaignID__c,
                    Status      = application.Status__c
                )
            );
        }
    }

    upsert list_CampaignMembersToUpsert;

 

 

 

If this helps, please mark it as a solution, and give kudos (click on the star) if you think I deserve them. Thanks!

 

-Glyn Anderson
Certified Salesforce Developer | Certified Salesforce Administrator

lescclarkcpdlescclarkcpd

thanks, there was one typo but fixed it and yes it works.

 

However, when I deploy to production I get too many SOQL queries, this is the full code which I'm pretty sure is now fine.

 

   {     
       Map<Id,Map<Id,CampaignMember>> map_ContactID_Members = new Map<Id,Map<Id,CampaignMember>>();

    for ( Course_Application__c application : Trigger.new )
    {
        map_ContactID_Members.put( application.Course_Applicant_del__c, new Map<Id,CampaignMember>() );
    }

    for ( CampaignMember member :
        [   SELECT  Id, ContactId, CampaignId, Status
            FROM    CampaignMember
            WHERE   ContactId IN :map_ContactID_Members.keySet()
        ]
        )
    {
        map_ContactID_Members.get( member.ContactId ).put( member.CampaignId, member );
    }

    List<CampaignMember> list_CampaignMembersToUpsert = new List<CampaignMember>();

    for ( Course_Application__c application : Trigger.new )
    {
        Map<Id,CampaignMember> map_CampaignMembers = map_ContactID_Members.get( application.Course_Applicant_del__c );

        if ( map_CampaignMembers.containsKey( application.PullCampaignID__c ) )
        {
            CampaignMember member = map_CampaignMembers.get( application.PullCampaignID__c );

            member.Status = application.Status__c;

            list_CampaignMembersToUpsert.add( member );
        }
        else
        {
            list_CampaignMembersToUpsert.add
            (   new CampaignMember
                (   ContactId   = application.Course_Applicant_del__c,
                    CampaignId  = application.PullCampaignID__c,
                    Status      = application.Status__c
                )
            );
        }
    }

    upsert list_CampaignMembersToUpsert;
    	   
    	   
    	  // for(Course_Application__c d:Trigger.new)      	
       // {CalcNumberOfCourseApps.CalcApps(d.Course_del__c);
       // }
    	
    	
	Map<id, Integer> coursesToassigntsMap = new Map<id, Integer>();
	  set<Id> courseIds = new Set<Id>();
	   for(Course_Application__c a: trigger.new){
		   courseIds.add(a.course_del__c);	   
   		}
   	    list<course__c> CList = [Select id,Confirmed_applications__c from Course__C where id in :CourseIds];
	   
	   
	  for (AggregateResult agr :[select count(id) idCount,course_del__c from Course_Application__c where course_del__r.Id in :CourseIds and status__c='Ordered'group by course_del__c])
	   						     
		{
          coursesToassigntsMap.put((ID)agr.get('course_del__c'), integer.valueof(agr.get('idCount')));
         
		}
	   
	  List<Course__C> UpdateList = new List<Course__C>();
		for(course__c ids: CList)
		{
		
		ids.Confirmed_applications__c = coursesToassigntsMap.get(ids.id);
		UpdateList.add(ids);	
	}	

	if(UpdateList.size()>0)
	Update UpdateList;	

 but I also have another triggger that fires on the same object, could this cause the problem?  If so here's the code.

 

  //Limit the size of list by using Sets which do not contain duplicate elements
  set<Id> MyOrderIds = new set<Id>();

  //When adding new payments or updating existing payments
  if(trigger.isInsert || trigger.isUpdate){
    for(Course_application__c p : trigger.new){
      MyOrderIds.add(p.MyOrder__c);
    }
  }

  //When deleting payments
  if(trigger.isDelete){
    for(Course_application__c p : trigger.old){
      MyOrderIds.add(p.MyOrder__c);
    }
  }

  //Map will contain one Opportunity Id to one sum value
  map<Id,Double> MyOrderMap = new map <Id,Double>();

  //Produce a sum of Payments__c and add them to the map
  //use group by to have a single Opportunity Id with a single sum value
  for(AggregateResult q : [select MyOrder__c,sum(cost__c)
    from Course_application__c where myOrder__c IN :myOrderIds AND Status__c<>'Cancelled' AND Status__c<>'Error' and Status__c<>'Transferred out to' group by myOrder__c])
    
    {
      myOrderMap.put((Id)q.get('myOrder__c'),(Double)q.get('expr0'));
  }
  
  List<myOrder__c> myOrdersToUpdate = new List<myOrder__c>();

  //Run the for loop on Opportunity using the non-duplicate set of Opportunities Ids
  //Get the sum value from the map and create a list of Opportunities to update
  for(myOrder__c o : [Select Id, Pre_Discount_Total__c, Discount_Amount__c, to_be_charged_total__c,Total_Collected__c, payment_status__c from myOrder__c where Id IN :MyOrderIds]){
    Double PaymentSum = myOrderMap.get(o.Id);
    if(PaymentSum==NULL){PaymentSum=0;}
    o.Pre_Discount_Total__c = PaymentSum;
    if(o.Total_Collected__c-PaymentSum==0){o.payment_status__c = 'Settled';}
    if(o.Total_Collected__c-PaymentSum - o.Discount_Amount__c<0){o.payment_status__c = 'Balance Outstanding';}
    if(o.Total_Collected__c-PaymentSum>0){o.payment_status__c = 'In Credit';} 
    myOrdersToUpdate.add(o);
  }
 
  update myOrdersToUpdate;

 

GlynAGlynA

Les,

 

I'm not seeing anything obvious in the two triggers that would cause the "Too Many SOQL Queries" error; but keep in mind that these triggers also insert or update CampaignMembers, Course__c and myOrder__c objects, so those triggers will run, too.  And some trigger might run twice if there are any Workflow field updates happening.  You'll have to look at the triggers on those other objects to figure out if they might be causing the problem.

 

-Glyn

GlynAGlynA

Les,

 

You didn't ask for this, but I've made a couple of changes to your second trigger.  These changes will make it easier to get code coverage with your test methods.

 

The first change is to have one for loop that iterates either trigger.new or trigger.old based on whether it's a delete trigger.  With a single loop, the code is covered by an insert or update, without having to do a separate delete test.  (You may want to anyway, to assert functionality).

 

The second change is to assign the Payment_Status__c field using the ternary operator '?:' (also called a conditional evaluation operator).  This reduces three if statements into a single statement that is covered no matter which condition is true.  You don't have to write three different tests to cover the code.  (Again, you may want to, to test functionality - but you won't have to just to cover the code.)

 

I've also reformatted bits of the code.  That's just to help me read the code and because I'm **bleep**-retentive that way.  I don't mean to foist my particular coding style on you.  I hope you find this useful.

 

    //Limit the size of list by using Sets which do not contain duplicate elements
    set<Id> MyOrderIds = new set<Id>();

    //When adding new payments or updating existing payments
    for ( Course_application__c p : (trigger.isDelete ? trigger.old : trigger.new) )
    {
        MyOrderIds.add( p.MyOrder__c );
    }

    //Map will contain one Opportunity Id to one sum value
    map<Id,Double> MyOrderMap = new map <Id,Double>();

    //Produce a sum of Payments__c and add them to the map
    //use group by to have a single Opportunity Id with a single sum value
    for ( AggregateResult q :
        [   SELECT  MyOrder__c, SUM(cost__c) sum
            FROM    Course_application__c
            WHERE   (   myOrder__c IN :myOrderIds
                    AND Status__c<>'Cancelled'
                    AND Status__c<>'Error'
                    AND Status__c<>'Transferred out to'
                    )
            GROUP BY myOrder__c
        ]
        )
    {
        myOrderMap.put( (Id)q.get('myOrder__c'), (Double)q.get('sum') );
    }

    List<myOrder__c> myOrdersToUpdate = new List<myOrder__c>();

    //Run the for loop on Opportunity using the non-duplicate set of Opportunities Ids
    //Get the sum value from the map and create a list of Opportunities to update
    for ( myOrder__c o :
        [   SELECT  Id, Pre_Discount_Total__c, Discount_Amount__c,
                    to_be_charged_total__c,Total_Collected__c, payment_status__c
            FROM    myOrder__c
            WHERE   Id IN :MyOrderIds
        ]
        )
    {
        Double PaymentSum = myOrderMap.get(o.Id);
        if(PaymentSum==NULL){PaymentSum=0;}
        o.Pre_Discount_Total__c = PaymentSum;
        o.payment_status__c = 
o.Total_Collected__c == PaymentSum ? 'Settled' : o.Total_Collected__c < (PaymentSum + o.Discount_Amount__c) ? 'Balance Outstanding' : o.Total_Collected__c > PaymentSum ? 'In Credit' : null;
myOrdersToUpdate.add(o); } update myOrdersToUpdate;

 

-Glyn

lescclarkcpdlescclarkcpd

Glyn

 

Thanks for your help on this one, greatly appreciated.

 

I've looked at this code again and it seems to me that there is a query in a loop in the later part of the code?  

 

for ( myOrder__c o :
        [   SELECT  Id, Pre_Discount_Total__c, Discount_Amount__c,
                    to_be_charged_total__c,Total_Collected__c, payment_status__c
            FROM    myOrder__c
            WHERE   Id IN :MyOrderIds
        ]
        )

 

I also have a similar trigger that I think is doing similar, see attached.  Do these need bulkifying further?

 

trigger UpdateTotalCharged on Income_Card_Payment__c (after delete, after insert, after update) {
	
	 //Limit the size of list by using Sets which do not contain duplicate elements
  set<Id> MyOrderIds = new set<Id>();
  set<Id> CompletedOrderIDs = new set<ID>();
   
   RecordType OrderRecordType =[Select Name, Id From RecordType where sObjectType='MyOrder__c' and name='Completed Order' and isActive=true];
   string OrderRecordTypeid = OrderRecordType.id;
   
   RecordType CAPRecordType =[Select Name, Id From RecordType where sObjectType='Course_application__c' and name='Completed Course Application' and isActive=true];
   string CAPRecordTypeid = CapRecordType.id;
   
  
  //When adding new payments or updating existing payments
  if(trigger.isInsert || trigger.isUpdate){
    for(Income_Card_Payment__c p : trigger.new){
      if (p.Payment_Status__c=='Authorised'){MyOrderIds.add(p.MyOrder__c);} 
    }
  }

  //When deleting payments
  if(trigger.isDelete){
    for(Income_Card_Payment__c p : trigger.old){
      MyOrderIds.add(p.MyOrder__c);
    }
  }

  //Map will contain one Opportunity Id to one sum value
  map<Id,Double> MyOrderMap = new map <Id,Double>();

  //Produce a sum of Payments__c and add them to the map
  //use group by to have a single Opportunity Id with a single sum value
  for(AggregateResult q : [select MyOrder__c,sum(Total_Amount_Taken__c)
    from Income_Card_Payment__c where myOrder__c IN :myOrderIds group by myOrder__c])
    
    {
      myOrderMap.put((Id)q.get('myOrder__c'),(Double)q.get('expr0'));
  }
  List<course_application__c> myappstoupdate = new List<course_application__c>();
  List<myOrder__c> myOrdersToUpdate = new List<myOrder__c>();
  for(myOrder__c o : [Select Id, RecordTypeId, Discount_Amount__c, to_be_Charged_total__c,payment_status__c,order_completion_status__c from myOrder__c where Id IN :MyOrderIds]){
    Double PaymentSum = myOrderMap.get(o.Id);
    if(o.to_be_Charged_total__c-PaymentSum==0){o.order_completion_status__c = 'Complete'; o.RecordTypeID=OrderRecordTypeid;CompletedOrderIDs.add(o.id);
    }
    myOrdersToUpdate.add(o);
  } 
  
  for(course_application__c a:[select id, recordtypeid from course_application__c where myorder__c IN :CompletedOrderIDs]){
    	a.RecordTypeId=CAPRecordTypeid;
    	a.status__c='Ordered';
    	myappstoupdate.add(a);
    }
  update myappstoupdate;
  update myOrdersToUpdate;


}

 

GlynAGlynA

Les,

 

When you use a SOQL query as the iterable object in a for loop, the query isn't "in" the for loop - the query is only performed once and the loop iterates over the result.  You only need to worry about queries that are in the body of the for loop, where they will be performed once for every iteration of the loop.

 

As for the other trigger, I gave it a quick look and don't see any obvious problems vis-a-vis bulk operation.

 

-Glyn

lescclarkcpdlescclarkcpd

ok, so if I write a test script that, for example, creates objects in a sequence that fires & re-fires triggers, could the error be that the test script has exceeded to SOQL limit rather than the code itself exceeding the limit?

GlynAGlynA

Yes, it's easy to write a test method that does too much, particularly if you're firing the trigger many times.  A better design is to split the test up as much as possible, testing one case in each test method.  It helps to write a small utility routine that all your test methods call to set up test records common to all tests.

 

Also, be sure to use startTest/stopTest around the actual test in your test method.  Everything that runs between startTest and stopTest has its own limits, so you can tell whether the code you're testing is exceeding the limits (separate from what your test code is doing).

 

For example:

public static testMethod void myTest()
{
    //  do all your test preparation here
    myObject__c obj = new myObject__c( Some_Field__c = 'x' );

    //  start the test
    Test.startTest();

    insert obj;    //  this is the test

    //  stop the test
    Test.stopTest();

    //  check the results
    myObject__c newObj = [SELECT Id, Some_Field__c FROM myObject__c WHERE Id = :obj.id];
    System.assertEquals( 'y', newObj.Some_Field__c );
}

Note that you can only use startTest/stopTest once per test method.

 

-Glyn