+ Start a Discussion
ckellieckellie 

Is this Trigger Bulkified?

I am working on a trigger that pulls data from the related opportunity to the to the custom object related record (MPS__c). I have yet to pull information from a related record, I have only pushed information to a record, and am wondering whether the trigger is bulkified.

 

I am primarily concerned about the following:

 

 

    trigger.new[0].Forecast_Category__c = o[0].ForecastCategoryName;
    trigger.new[0].Combined_Probability__c = o[0].Combined_Probability__c;
    trigger.new[0].Close_Date__c = o[0].CloseDate;

 

Here is the full trigger:

 

 

trigger MPSUpdate on MPS__c (Before Insert, before update) {

    Set<Id> mIds = new Set<Id>();
    Set<Id> oIds = new Set<Id>();
    
    for(MPS__c mps : trigger.new){
            System.debug('**** 0 mps id : '+ mps.id);
        mIds.add(mps.id);
            System.debug('**** 1 mps id : '+ mps.id);
            System.debug('**** 2 o id : '+ mps.Opportunityid__c);
        oIds.add(mps.OpportunityID__c);
            System.debug('**** 3 o id : '+ mps.Opportunityid__c);   
    }
    
   List<Opportunity> o = [select id, MPS_BV__c, MPS_Comment__c, 
                MPS_Scheduled_Ship_Date__c, MPS_Status__c, CloseDate,
                ForecastCategoryName, Combined_Probability__c
                from Opportunity where id in :oIds];

   List<MPS__c> p = [select id, MPS_BV__c, MPS_Comment__c, MPS_Scheduled_Ship_Date__c, MPS_Status__c,
                Close_Date__c, Combined_Probability__c, Forecast_Category__c
                from MPS__c where id in :mIds];

    trigger.new[0].Forecast_Category__c = o[0].ForecastCategoryName;
    trigger.new[0].Combined_Probability__c = o[0].Combined_Probability__c;
    trigger.new[0].Close_Date__c = o[0].CloseDate;
    
    for(MPS__c s : p){
    
            o[0].MPS_Status__c = s.MPS_Status__c;
            o[0].MPS_BV__c = s.MPS_BV__c;
            o[0].MPS_Comment__c = s.MPS_Comment__c;
            o[0].MPS_Scheduled_Ship_Date__c = s.MPS_Scheduled_Ship_Date__c;

       update o;
    }
}

 

This trigger does work, just am wondering whether this is bulkified?

 

Thanks,

ckellie

 

 

Best Answer chosen by Admin (Salesforce Developers) 
MarkWaddleMarkWaddle

It should be

 

 

mapMpsToOpportunity.values()

 

 

Sets are great, but I find that with most non-trivial use cases I have to use Maps as well.

 

Check out http://www.salesforce.com/us/developer/docs/apexcode/Content/langCon_apex_collections_maps.htm#kanchor80 and http://www.salesforce.com/us/developer/docs/apexcode/Content/langCon_apex_collections_maps_from_SObjects.htm#kanchor82 for reference.

 

If you have any questions about the code I'd be happy to answer.

 

Mark

All Answers

MarkWaddleMarkWaddle

HI there!

 

I have a couple of questions about your object model and your requirements that will help me advise on the trigger.

 

  1. I see the MPS__c has a lookup to Opportunity. This implies that multiple MPS__c can be associated with the same Opportunity. Is this the case?
  2. If MPS__c does have a lookup to Opportunity, that would mean that there is a "related list" for MPS records on the Opportunity page layout (or at least you could add it to the layout if you wanted). Does this not meet your needs to show the values in the list?
  3. If only one MPS__c is associated with each Opportunity, have you considered putting the MPS__c fields directly on the Opportunity, negating the need for the MPS__c?

Regards,

Mark

ckellieckellie

Hi Mark,

 

Answer to your questions:

 

1.  I see the MPS__c has a lookup to Opportunity. This implies that multiple MPS__c can be associated with the same Opportunity. Is this the case?

 

 

Yes, multiple MPS__c can be associated with the Opportunity.

 

 

2. If MPS__c does have a lookup to Opportunity, that would mean that there is a "related list" for MPS records on the Opportunity page layout (or at least you could add it to the layout if you wanted). Does this not meet your needs to show the values in the list?

 

 

Yes the MPS has a lookup to the Opportunity. Originally the MPS fields do reside on the opportunity page, but with the growing need for automation, increased interaction on the record, and the need for simplicity we are needing to move these workflows to a custom object. There are too many fields involved to be a related list, and we currently have a whole series of related lists on the Opportunity is not preferrable.

 

 

If only one MPS__c is associated with each Opportunity, have you considered putting the MPS__c fields directly on the Opportunity, negating the need for the MPS__c?

 

 

The amount of interaction and collaboration around these fields are becoming too complex to remain on the Oppotunity. This aspect of the Opportunity has evolved to necessitate the need for a custom object.

 

I wish we could keep the information on the Opportunity, but that is not possible.

 

Thank you for your help.

MarkWaddleMarkWaddle

OK. When you have an Opportunity that has more than one MPS associated, which MPS values do you want to put in the MPS-related fields on that Opportunity? The latest one?

 

The fields I am referring to:

 

o[0].MPS_Status__c = s.MPS_Status__c;
o[0].MPS_BV__c = s.MPS_BV__c;
o[0].MPS_Comment__c = s.MPS_Comment__c;
o[0].MPS_Scheduled_Ship_Date__c = s.MPS_Scheduled_Ship_Date__c;
ckellieckellie

Yes, I want to update the latest MPS record.

 

Thank you

MarkWaddleMarkWaddle

I think this might be what you want. It will set the Opportunity fields based on the last MPS in the trigger.new.

 

 

trigger MPSUpdate on MPS__c (Before Insert, before update) {
    Map<Id, Id> mapMpsToOpportunity = new Map<Id, Id>();
     
    for(MPS__c mps : trigger.new){
        // Populate map of MPS.Id to Opportunity.Id
        if (mps.OpportunityID__c != null) {
            mapMpsToOpportunity.put(mps.Id, mps.OpportunityID__c);
        }
    }
    
    if (!mapMpsToOpportunity.isEmpty()) {
        // Query all relevant opportunities to a map with key of Opportunity.Id
        Map<Id, Opportunity> opps = new Map<Id, Opportunity>([select id, MPS_BV__c, MPS_Comment__c, 
                         MPS_Scheduled_Ship_Date__c, MPS_Status__c, CloseDate,
                         ForecastCategoryName, Combined_Probability__c
                         from Opportunity where id in :mapMpsToOpportunity.values()]);

        List<Opportunity> oppsToUpdate = new List<Opportunity>();
        Id oppId;
        Opportunity o;
        for (MPS__c s : trigger.new) {
            if (mapMpsToOpportunity.containsKey(s.Id)) {
                // Get the opportunity ID from the map
                oppId = mapMpsToOpportunity.get(s.Id);
                // Get the opportunity based on the opportunity ID
                o = opps.get(oppId);
                // Set fields on MPS based on Opportunity
                s.Forecast_Category__c = o.ForecastCategoryName;
                s.Combined_Probability__c = o.Combined_Probability__c;
                s.Close_Date__c = o.CloseDate;
                // Set fields on Opportunity based on MPS
                o.MPS_Status__c = s.MPS_Status__c;
                o.MPS_BV__c = s.MPS_BV__c;
                o.MPS_Comment__c = s.MPS_Comment__c;
                o.MPS_Scheduled_Ship_Date__c = s.MPS_Scheduled_Ship_Date__c;
                oppsToUpdate.add(o);
            }
        }
        
        // Update all opportunities in one call
        update oppsToUpdate;
    }
}

 

 

ckellieckellie

I believe this is what I am looking for, but I am getting the following error when I paste the trigger into SFDC:

 

 

Compile Error: Method does not exist or incorrect signature: mps.Id.values() at line 16 column 56

 I have not seen this type of call before. Can you help with the correction?

 

Thanks

 

MarkWaddleMarkWaddle

It should be

 

 

mapMpsToOpportunity.values()

 

 

Sets are great, but I find that with most non-trivial use cases I have to use Maps as well.

 

Check out http://www.salesforce.com/us/developer/docs/apexcode/Content/langCon_apex_collections_maps.htm#kanchor80 and http://www.salesforce.com/us/developer/docs/apexcode/Content/langCon_apex_collections_maps_from_SObjects.htm#kanchor82 for reference.

 

If you have any questions about the code I'd be happy to answer.

 

Mark

This was selected as the best answer
ckellieckellie

Mark,

 

Thank you for your help and for the links to the explanations about Maps. The links are helping me understand maps better.

 

Thank you

ckellie