Apex Trigger: Use a trigger to update a custom field with data from another object

Question

I am attempting to write a trigger and I relatively new to both Salesforce and Object oriented programming and I am struggling, so any help would be appreciated!

Task Description:

I have two custom objects, Employee, and Position Associate. Position Associate is an object that creates a mapping and record between an employee and a respective position that they hold. I am attempting to write a trigger, that runs any time a Position Associate is created or updated, that takes three fields, Training Start, Training End, and Home Study Start, and populates it with data from the employee object, from the employee that is mapped to the Position Associate record in question. This must be a trigger as opposed to a making the field a lookup value. So while that may be simpler, it is not an option in this scenario

Current Behavior: While I am not getting any errors, after a record is updated, the fields still remain blank.

Code:

trigger MapDatesPA on Position_Employee__c(after insert, after update) {
    // Create trigger that runs when a new record is inserted or when a record is updated that maps the employee fields
    // of Home Study Start Date, Training Start and End Dates to Position Associate Object
    // Prevent Iterative Loop
    if (checkRecursive.runOnce()) {
        Map<Id, Employee__c> pm = new Map<Id, _Employee__c>();
        Set<Id> empIds = new Set<Id>();
        for (Position_Employee__c posEmp : Trigger.new) {
            if (posEmp.Employee__c != null) {
                empIds.add(posEmp.Employee__c);
            }
        }

        for (Employee__c emp : [
            SELECT Id, HOME_STUDY_START_DATE__c, Training_Start_Date__c, Training_End_Date__c
            FROM Employee__c
            WHERE Id IN :empIds
        ]) {
            if (pm.containsKey(emp.Id)) {
                pm.put(emp.id, emp);
            }
        }

        for (Position_Employee__c posemp : Trigger.New) {
            if (pm.containsKey(posemp.Id)) {
                posemp.Training_Start_Date__c = pm.get(posemp.Id).Training_Start_Date__c;
                posemp.Training_End_Date__c = pm.get(posemp.Id).Training_End_Date__c;
                posemp.Home_Study_Start_Date__c = pm.get(posemp.Id).HOME_STUDY_START_DATE__c;
            }
        }
    }
}

Answer 1

That checkRecursive.runOnce() that you have worries me. I get the feeling that it's just checking a static boolean variable. If that's the case, and you happen to insert or update more than 200 records at a time, your trigger would only perform its work on the first 200 records.

Even though you say this must be a trigger, what you have right now really could be accomplished with some very simple formula fields (assuming that Employee__c on your Position_Employee__c object is either a lookup relationship field or a master-detail relationship field). If there's any room for you to push back on the "use a trigger" requirement you've been given, this is something I'd fight for. Using 3 formula fields is going to be faster to develop, and doesn't require any tests to be written.

If you still insist on doing this with a trigger, some things to note:

• If you're changing a field value in a trigger, you generally need to perform DML to "save" that new value. If you don't, then that change never makes it to the database
• The exception to this is when you're in a before trigger context (before insert or before update) and are modifying a value on a record contained in either trigger.new or trigger.newMap. Modifying a value on a record in either of those will cause the new value to be saved without the need for DML
• It's appropriate to do work in a "before" trigger when you're making changes to the same SObject that the trigger is defined on (this is true in your case, your trigger is on Position_Employee__c and you're trying to make changes to Position_Employee__c records)

Beyond that, you have two main issues in your current logic.

The first issue is your second loop

for (Employee__c emp : [
            SELECT Id, HOME_STUDY_START_DATE__c, Training_Start_Date__c, Training_End_Date__c
            FROM Employee__c
            WHERE Id IN :empIds
]) {
    if (pm.containsKey(emp.Id)) {
        pm.put(emp.id, emp);
    }
}

As Mark Pond pointed out, that if() statement is wrong. You're saying "if this map already contains this key, then put this record into the map". If you put a system.debug(pm.size()); right after that for loop, it'll be guaranteed to show you a "0" (meaning the map is empty).

You could fix that, but the entire for loop is unnecessary. Salesforce gives us a way to put the result of a query (which is a List<SObject>) into a Map<Id, SObject> in a single line.

pm.putAll([SELECT Id, <other fields here> FROM Employee__c WHERE Id IN :empIds]);

The second issue is your third loop

for (Position_Employee__c posemp : Trigger.New) {
    if (pm.containsKey(posemp.Id)) {
        posemp.Training_Start_Date__c = pm.get(posemp.Id).Training_Start_Date__c;
        posemp.Training_End_Date__c = pm.get(posemp.Id).Training_End_Date__c;
        posemp.Home_Study_Start_Date__c = pm.get(posemp.Id).HOME_STUDY_START_DATE__c;
    }
}

Different SObjects in Salesforce have different Ids. The first 3 characters of an Id indicate what SObject it is. '001' indicates it's an Account, '500' indicates it's a Case, and so on.

The point here is that your pm map uses the Id of Employee__c records as the key, and this map will never contain any keys for Position_Employee__c records.

Your Position_Employee__c records do contain an Id for your Employee__c records, just not in the Id field. Instead, that's in the Employee__c field (which you used to build the empIds set which allowed you to query for Employees).

Everywhere that you have posemp.Id in this third loop, you should instead be using posemp.Employee__c.

You could also save yourself some typing by fetching the Employee__c record from the map once, storing the result in a variable, and then using that variable to get at the field values.

example

for(Account acct :myAccounts){
    if(accountOwnersMap.containsKey(acct.OwnerId)){
        User owner = accountOwnersMap.get(acct.OwnerId);

        // This is shorter to type
        acct.ownerEmail__c = owner.Email;

        // compared to this, when you have at least 2 or 3 fields to access.
        // It depends on how long your object/variable names are.
        // Less typing = less chance to make a mistake (so less typing is generally
        //   considered "good style")
        //acct.ownerEmail__c = accountOwnersMap.get(acct.OwnerId);
    }
}

Answer 2

One issue with the logic exists here:

if (pm.containsKey(emp.Id)) {
    pm.put(emp.id, emp);
}

This map never contains the key because you have not put it in the map yet. Your logic should likely be to put it in the map if it does NOT contain the key.

if (!pm.containsKey(emp.Id)) {
    pm.put(emp.id, emp);
}