0

I am trying to create a Trigger which updates a lookup field on the source record. The idea is that the related list will only ever hold 1 record. Still, I have limited to 1 anyway. There are no errors, however the field isn't being updated. Why?

trigger SurveyTrigger on Survey__c (after insert,after Update) {

    Map<Id, Survey__c> relatedDataMap = new Map<Id, Survey__c>([
        SELECT Id, Course_Enrollment__c
        FROM Survey__c 
        WHERE Id IN :trigger.new]);

    Survey__c relatedData;

    if(Trigger.isAfter) 
    {
        if (Trigger.isInsert || Trigger.isUpdate)
        {
            for(Survey__c sur : trigger.new){
                relatedData = relatedDataMap.get(sur.Id);

                Course_Enrollment__c thisCE = [SELECT id FROM Course_Enrollment__c WHERE Completion_Survey__c = :relatedData.Id LIMIT 1];

                if (relatedData.Course_Enrollment__c == NULL) {
                     relatedData.Course_Enrollment__c = thisCE.id;
                }
            }
        }
    }
}
1
  • Please stop adding greetings, thanks, etc to your posts, they will be removed. Also, the format here is Question and Answer. Please try to formulate your post so you ask something. Saying "Any help is appreciated" is not a question.
    – Adrian Larson
    Sep 27 '17 at 14:24
2

Your current trigger is, to put it bluntly, a bit of a mess. Some of these points have been covered by others, but I feel it worth repeating.

  • If you're updating values of the records being triggered, you want to use a before trigger.

It's not impossible to update records being triggered in an after trigger, but you run the risk of infinite recursion. The automatic updating (without DML) behavior of triggers is only present in before triggers, and only for the records being triggered. For anything else, you need to use DML.

  • If you try to update a record in a trigger context variable in an after event, you'll get the error

    System.FinalException: Record is read-only:

This is Salesforce nudging you into using the appropriate trigger events. Even if you could update field values of records in trigger context variables in an after trigger, they wouldn't automatically update without DML (again, that behavior is exclusive to before triggers)

  • Query inside of a loop

This is one of the things that are a universally bad idea. Queries in loops may not cause you issues now, but they set you up for failure later. If you ever use the Apex DataLoader to insert/update more than 100 Survey__c records, you'll run into a System.LimitException (which is uncatchable). When your org grows, and more objects start interacting with one another, queries in loops will limit the amount of records you can handle.

  • The query you have outside of your loop serves no purpose

Trigger.new, when it is populated, will contain values for all of the fields on that object. If you only need to use one dot/period/full-stop to access a field (like sur.Id or relatedData.Course_Enrollment__c), you don't need to query for it.

A better implementation of this trigger would be as follows:

trigger SurveyTrigger on Survey__c (before insert, before Update) {

    // I assume that Course_Enrollment__c has a lookup to Survey__c via the field
    // Completion_Survey__c.
    // Given that, we can use a parent-child subquery (a.k.a. a LEFT OUTER JOIN)
    //   to grab one (and only one) child record per parent.
    Map<Id, Survey__c> relatedDataMap = new Map<Id, Survey__c>([
        SELECT Id,
            // This is our subquery.
            // We can only have 1 level of subquery, so if Course_Enrollment__c
            //   itself had a child object, you would need a separate query to
            //   grab that "grandchild" data.
            // The FROM clause in the subquery uses the _child relationship name_ that you set up
            //   when you created the lookup field.
            // Unless you specifically altered the child relationship name, it should
            //   be the plural of your object name, with __r (instead of __c)
            // Using the child relationship name is sufficient for filtering
            //   the child records related to that parent.
            // There is no need for 'WHERE Completion_Survey__c IN :Trigger.new'
            (SELECT Id FROM Course_Enrollments__r LIMIT 1)
        FROM Survey__c 
        WHERE Id IN :trigger.new]);

    Survey__c relatedData;

    if(Trigger.isBefore) 
    {
        if (Trigger.isInsert || Trigger.isUpdate)
        {
            // Instead of iterating over trigger.new, we can iterate over
            //   relatedData.values().
            // This saves us the trouble of doing that inside the loop
            for(Survey__c sur : relatedData.values()){
                // Accessing the results of a sub-query ends up being very similar
                //   to accessing fields on parent records.
                // The big difference is that Course_Enrollments__r (yep, still
                //   using the child relationship name) is a List<Course_Enrollment__c>
                //   rather than a single instance of the child record.

                // The list of related child records will never be null.
                // It is still a good idea to make sure that the list isn't empty though
                if(!relatedData.Course_Enrollments__r.isEmpty()) {
                     // Here, we set ourselves up to take advantage of the 'before'
                     //   trigger behavior that allows us to avoid using DML.
                     // Any changes to the record stored in trigger.new or trigger.newMap
                     //   are updated without the need for DML.
                     // The other thing happening here is that, because Course_Enrollments__r
                     //   is a List (and we know that it contains exactly 1
                     //   record at this point), we can use square bracket notation
                     //   to access the record(s) inside of it
                     trigger.newMap.get(relatedData.Id).Course_Enrollment__c = relatedData.Course_Enrollments__r[0].Id;
                }
            }
        }
    }
}

That's better, but there is still one big problem. Records in Trigger.new during before insert triggers will not have Ids. before triggers act before the database operation taking place. Records do not have Ids until they are inserted, therefore records in trigger.new don't have Ids in before insert triggers (this is also why trigger.newMap is not available in before insert triggers).

The soonest trigger event in the chain that will have record Ids available is after insert. If you are in a before insert trigger, and try to query for records related to a record being inserted (or for the record being inserted, for that matter), your query will return no results. This makes sense though; how could other records be related to something that doesn't exist in the database yet?

I hear you asking, "...but if we're using after insert again, doesn't that put us back to square one?". The answer to that is "not quite, and we can work around it".

Before I cover the workaround, you should ask yourself if you really need this trigger to be run on insertion of a new record. If you have other code that runs after inserting a new Survey__c record that inserts new Course_Enrollment__c records, then you have a case for keeping the after insert event. Otherwise, if Course_Enrollment__c records are created through user interaction, or automatically at a later time (scheduled/batch apex), then you'd be better served by doing this through an after insert trigger on Course_Enrollment__c.

If you do have reason to keep the after insert event on Survey__c's trigger, the following could be used.

trigger SurveyTrigger on Survey__c (after insert, before Update) {
    // I've left out my notes from the previous iteration of this trigger.
    // I highly encourage you to go back and read them if you haven't already.

    // Since this block takes advantage of the 'before' trigger, and updates
    //   trigger.new to avoid needing DML, we can't run this code after insert
    if(Trigger.isBefore && Trigger.isUpdate){
        Map<Id, Survey__c> relatedDataMap = new Map<Id, Survey__c>([
            SELECT Id,
                (SELECT Id FROM Course_Enrollments__r LIMIT 1)
            FROM Survey__c 
            WHERE Id IN :trigger.new]);

        for(Survey__c relatedData: relatedDataMap.values()){
            if(!relatedData.Course_Enrollments__r.isEmpty()) {
                 trigger.newMap.get(relatedData.Id).Course_Enrollment__c = relatedData.Course_Enrollments__r[0].Id;
            }
        }
    }else if(Trigger.isAfter && Trigger.isInsert){
        // So, how are we to update records being inserted?
        // We could duplicate the code from the above block, and modify it to
        //   store records that need updating in a list, and then DML update.
        // However, since we're relegated to using DML here anyway, we don't need to
        //   copy/paste and modify the logic from the before update trigger.
        // ***Calling update on our inserted records will cause the before update
        //      trigger to run, meaning our existing code will handle the situation
        //      for us***
        // Furthermore, we don't need to worry about infinite recursion because
        //   (as is), there is no way to get back to this line of code from updating
        //   Survey__c records.
        update trigger.new;

    }
}

A similar approach can be taken if you determine that keeping the after insert event in Survey__c's trigger doesn't make sense. When you do get Course_Enrollment__c records, in a Course_Enrollment__c trigger (after insert would be appropriate here), simply gather the Survey__c ids, make mostly empty instances of Survey__c, and update those and let your existing logic take care of the rest

// This is only a fragment of a trigger
List<Survey__c> surveysToUpdate = new List<Survey__c>();

for(Course_Enrollment__c ce :Trigger.new){
    // We can set most SObject fields, even some that would otherwise be read-only,
    //   by using the SObject constructor.
    // Here, we use this to set the Id for a Survey__c record that we want to update.
    // This could be accomplished with a query instead...but we have scant few queries, and
    //   this is an easy way to save one (for later use elsewhere, should we need it)
    surveysToUpdate.add(new Survey__c(Id = ce.Completion_Survey__c));
}

update surveysToUpdate;

+edit:

Ok, so you have a Course_Enrollment__c trigger that creates/inserts Survey__c records to go along with it.

That means that we've kinda been approaching this problem from the wrong end.

I'll pull a quote from the documentation on Apex Triggers

There are two types of triggers:

  • Before triggers are used to update or validate record values before they’re saved to the database.
  • After triggers are used to access field values that are set by the system (such as a record's Id or LastModifiedDate field), and to affect changes in other records, such as logging into an audit table or firing asynchronous events with a queue. The records that fire the after trigger are read-only.

Populating Course_Enrollment__c relationship field on Survey__c can be done even before inserting the Survey__c records, it's populating Completion_Survey__c on Course_Enrollment__c that's the issue. Lucky for us, the general structure of my previous example will work with just a few tweaks.

// Only change to this line is the trigger name, and the object the trigger is for.
// In fact, most of the adjustments will be simply swapping object names around.
// I also swap the order of the insert/update blocks to keep the visual flow
//   of execution from top to bottom. 
// It really doesn't matter which you do first, but I find that not having to 
// jump up and down reading code makes it easier to understand.
trigger CourseEnrollmentTrigger on Course_Enrollment__c (after insert, before Update) {

    if(Trigger.isAfter && Trigger.isInsert){
        // Now that we've worked out what creates what, it makes sense to have code
        //   in the after insert portion of this trigger.
        // Since we need the Id value of the Course_Enrollment__c records, and
        //   because we're affecting changes on an object that isn't Course_Enrollment__c,
        //   the after insert trigger event is the appropriate one to use.

        // Creating Survey__c records should look something like this.
        List<Survey__c> surveysToInsert = new List<Survey__c>();

        for(Course_Enrollment__c ce :Trigger.new){
            surveysToInsert.add(new Survey__c(Course_Enrollment__c = ce.Id));
        }

        // When we perform DML, 'control' of program execution is passed to the 
        //   trigger of the object being DML'd (if one exists).
        // No code past this next (uncommented) line is executed until the 
        //   trigger order of execution ( https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_triggers_order_of_execution.htm ) 
        // finishes.
        insert surveysToInsert;

        // When we get to this point (assuming you haven't gotten an exception), the 
        //   insert has finished, and everything that was done in Survey__c's trigger
        //   is available to query.
        // Thus, when we update our working records, and find ourselves in the
        //   update portion of the trigger, our query will return results
        update trigger.new;

    } else if(Trigger.isBefore && Trigger.isUpdate){
        // Again, we need to do this in an update trigger because the first logical
        //   part of this trigger (inserting Survey__c records) requires us to
        //   have Ids for the Course_Enrollment__c records.
        // That necessitates the use of 'after insert', which means we can no
        // longer modify the records in Trigger.new.
        // We could execute this code as part of the 'after insert' trigger, but
        //   we would need to dml update our Course_Enrollment__c records anyway...
        //   so there isn't really any benefit to avoiding a 'before update'
        //   portion of this trigger.

        // We're working with Course_Enrollment__c as our 'base' object now, so...
        // The map's value data type changes
        Map<Id, Course_Enrollment__c> relatedDataMap = new Map<Id, Course_Enrollment__c>([
            SELECT Id,
                // ...as does the relationship name that we use
                //   (we're now querying the Survey__c records related to the
                //   Course_Enrollment__c records as opposed to the other way around)
                (SELECT Id FROM Completion_Surveys__r LIMIT 1)
            FROM Course_Enrollment__c
            WHERE Id IN :trigger.new]);

        // Swap the data type of the loop variable
        for(Course_Enrollment__c relatedData: relatedDataMap.values()){
            // More swapping of the relationship name
            if(!relatedData.Completion_Surveys__r.isEmpty()) {
                 trigger.newMap.get(relatedData.Id).Completion_Survey__c = relatedData.Completion_Surveys__r[0].Id;
            }
        }
    }
}

My concern at the end of my answer (before making this edit) about whether or not it was appropriate to keep the after insert portion of Survey__c's trigger was based on gut feeling and experience.

I was feeling that trying to use the trigger on Survey__c to do work when that information was available on Course_Enrollment__c records was trying to force a square peg through a round hole.

I don't have the same concern with my most recent example.

Now, this isn't the only way to approach your problem (you could have two triggers, this one on Course_Enrollment__c, and another on `Survey__c and split up some of the work), but this approach lends itself nicely to being (at a later time) restructured to get closer to trigger 'best practices'.

The current 'best practice' for triggers is to keep virtually all logic/code outside of the trigger itself, choosing to implement logic as discrete Apex classes instead. Something like...

public class CreateAndRelateSurvey{
    // You would need to replace usage of trigger context variables.
    public static void createSurveys(List<Course_Enrollment__c> inputList){
        // all of the 'after insert' code would go here
    }

    public static void relateSurveys(List<Course_Enrollment__c> inputList, Map<Id, Course_Enrollment__c> inputMap){
        // all of the 'before update' code would go here
    }
}

// These would be two separate files (one class, one trigger)
// I'm just keeping them in a single code block here because I'm lazy
trigger CourseEnrollment on Course_Enrollment__c (after insert, before update){
    // As a result, the trigger is much shorter
    if(Trigger.isAfter && Trigger.isInsert){
        CreateAndRelateSurvey.createSurveys(Trigger.new);
    } else if(Trigger.isBefore && Trigger.isUpdate){
        CreateAndRelateSurvey.relateSurveys(Trigger.new, Trigger.newMap);
    }
}

Again, don't feel the need to go out and move to this structure right away. Doing so does have tangible benefits (more modular, easier to test, fits better with the new development model coming with SalesforceDX), but just knowing that there's another way to structure your triggers is really all I'm aiming for here. You switch to a structure like this when you feel you're ready to.

1
1

If i am not wrong this will be working for you.

trigger SurveyTrigger on Survey__c (before  insert,before Update) {
    Map<id,id> Completion_Survey=new Map<id,id>();

    for(Completion_Survey__c CS: [SELECT id,Completion_Survey__c FROM Course_Enrollment__c where Completion_Survey__c in : Trigger.newmap.Keyset()])
        Completion_Survey.put(CS.Completion_Survey__c, CS.id);

    for(Survey__c sur : trigger.new)
        if (sur.Course_Enrollment__c == null) 
            sur.Course_Enrollment__c = Completion_Survey.get(sur.id);                
}

beforeupdate

- Can change fields using trigger.new

afterupdate

- Not allowed. A run-time error is thrown, as trigger.new is already saved.

Avoid using Soql inside For loop

There is a governor limit that enforces a maximum number of SOQL queries. When queries are placed inside a for loop, a query is executed on each iteration and governor limit is easily reached. Instead, move the SOQL query outside of the for loop and retrieve all the necessary data in a single query.

0
-3



Here, I wrote some piece of code for you, please try it, Hope it will help you.

trigger SurveyTrigger on Survey__c (after insert, after Update) {

    Map<Id, Survey__c> relatedDataMap = new Map<Id, Survey__c>();
    for(Survey__c sur : trigger.new){
        if(sur.Course_Enrollment__c == NULL){
            relatedDataMap.put(sur.Id, sur);
        }
    }
    Map<Id, Course_Enrollment__c> SurveyVSCourseEnroll = new Map<Id, Course_Enrollment__c>();
    for(Course_Enrollment__c CE : [SELECT id,Completion_Survey__c FROM Course_Enrollment__c WHERE Completion_Survey__c = :relatedDataMap.keySet()]){
        if(CE.Completion_Survey__c != null){
            SurveyVSCourseEnroll.put(CE.Completion_Survey__c, CE);
        }
    }
    if(Trigger.isAfter) 
    {
        if (Trigger.isInsert || Trigger.isUpdate)
        {
            List<Survey__c> srList = new List<Survey__c>();
            for(Survey__c sur : trigger.new){
                if(relatedDataMap.containsKey(sur.Id)){
                    Survey__c relatedData = relatedDataMap.get(sur.Id);
                    if(SurveyVSCourseEnroll.containsKey(relatedData.Id)){
                        Survey__c sr = new Survey__c();
                        sr.Id = relatedData.Id;
                        sr.Course_Enrollment__c = SurveyVSCourseEnroll.get(relatedData.Id).Id;
                        srList.add(sr);
                    }
                }
            }
            if(srList.size()>0){
                update srList;
            }
        }
    }
}

Regards,
Ajay

1
  • 3
    -1 from me. Code dumps without explanation are not good answers. They encourage the OP to simply copy/paste without trying to figure out why it works, or why their original code didn't work. On top of that, if your code is wrong (doesn't work as OP expects, doesn't compile, etc...), it tends to set of a cascade of comments along the lines of "try this <more code in comments>", and "No, that didn't work either". Providing no explanation also means that people with similar issues visiting this question/answer in the future are not likely to be able to apply this answer to their own situation.
    – Derek F
    Sep 26 '17 at 15:04

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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