Take the 2-minute tour ×
Salesforce Stack Exchange is a question and answer site for Salesforce administrators, implementation experts, developers and anybody in-between. It's 100% free, no registration required.

I would like to copy the field Outlet_ID__c from my Opportunity object to my Service Orders custom object. I have added the code below to an existing trigger I have which updates the status of an order.

The code I have inserted do not work however, can someone point me in the right direction on how to fix this? I think its most likely due to how I am referencing it. I have ensured both fields exist and have been referenced with the correct API names.

trigger LogServiceOrderHistory on Opportunity (after Update) {        
    Service_Order__c[] ServOrdList = [select Id,Opportunity__c,Opp_Status__c from Service_Order__c ];           
    if(ServOrdList.size() != 0 )
    {
        for(Service_Order__c ServOrd :ServOrdList)
        {            
            if(ServOrd.Opportunity__c == trigger.new[0].Id)
            {                    
                // Update Service History
                Status_History__c StatHistory = new Status_History__c();

                // changes to get the custom setting working
                Stages__c s = [SELECT Id, Name, Value__c FROM Stages__c WHERE Name = :trigger.new[0].StageName];                
                // ServOrd.Opp_Status__c = StatHistory.name = Stages__c.getInstance(trigger.new[0].StageName).Value__c;
                ServOrd.Opp_Status__c = StatHistory.name = s.Value__c; 
                ServOrd.Outlet_ID__c = trigger.new[0].Outlet_ID__c; // THIS DOESNT WORK :(

                Update ServOrd;            
                StatHistory.Service_Order__c =ServOrd.Id;               
                insert StatHistory ;                

            } 
        }         
    }
}
share|improve this question

1 Answer 1

up vote 4 down vote accepted

A couple of things:

  1. You're retrieving all of your Service_Order__c records in the SELECT query
  2. You're only comparing your Service_Order__c against the first Opportunity in the Trigger
  3. You're performing DML operations and an SOQL query within a for loop. This is bad. You need to bulkify your trigger. For more information, see: Best Practice: Bulkify Your Code

It looks like Service_Order__c is a child of Opportunity. Do you want the Outlet_ID__c value to be copied down to all Service_Order__c records related to a given Opportunity?

You could use something like the following:

trigger LogServiceOrderHistory on Opportunity (after Update) {

    //  Get the IDs of the Opportunities
    Set<ID> oppIDs = Trigger.newMap.keySet();

    //  Get all Service Orders associated with the Opportunities in this Trigger
    Service_Order__c[] ServOrdList = [
        SELECT Id, Outlet_ID__c, Opportunity__c, Opp_Status__c, Opportunity__r.Outlet_ID__c
        FROM Service_Order__c
        WHERE Opportunity__c IN :oppIDs ] ;           

    //  Loop through them to copy Outlet ID from parent (Opportunity) to the Service Order
    for(Service_Order__c so : ServOrdList)
    {
        so.Outlet_ID__c = so.Opportunity__r.Outlet_ID__c ;
    }

    //  Updates the Service Orders
    update ServOrdList ;
}

NOTE: This code hasn't been tested.

I haven't included other items in this example (such as the service history) as you haven't asked about it and it's best to keep things simple to start with. If you want other fields to be updated accordingly, add them to the SELECT query, and include them within the for loop.

You may also want to handle any errors the update DML operation may throw.

Also, what test classes do you currently have that are testing this? It's the best way to work out if the code is doing what you want.

share|improve this answer
3  
It's good to filter only the required records (by checking for relevant field changes) from the Trigger.new.keySet() instead querying for all the trigger context records. –  highfive Jul 14 at 10:25
    
That's a good point @highfive. I get the feeling Imran is trying to do a few things, so maybe we'll leave that be for the time being. –  Nick Cook Jul 14 at 10:31
    
The Service Order is a child of Opportunities, there is one field Outlet_ID which I need to copy to SO's if edited on Opportunities. I have not yet made a test class for the trigger (but that is next on my to-do list). –  Imran Jul 14 at 10:37
    
I'm getting compile error here Compile Error: Method does not exist or incorrect signature: [LIST<Opportunity>].keySet() at line 4. Do I need to get the ID's from Opps manually? –  Imran Jul 14 at 10:38
1  
Sorry, it should have been Trigger.newMap.keySet(). I've updated the answer accordingly. –  Nick Cook Jul 14 at 10:42

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.