Opportunity close date is not being set to today when closed while close date is in the future

Question

We are implementing an override of the close date functionality. By default, opportunities that are closed and have a close date set to the future get their close date set to today. We want this to hold true for all opportunities except for opportunities of a specific record type. So I wrote a trigger to persist the original close date for these opportunities.

However I am struggling to get this functionality to work in a test. My first assertion throws an error that the close date is not being changed.

Assertion Failed: Expected: 2019-07-01 00:00:00, Actual: 2019-07-06 00:00:00

I have verified that when I first close the opportunity the not persisting debug is printed. I've also verified there are no other triggers/workflows affecting this process.

Why is the close date not changing as it should?

Trigger:

public static void persistOriginalCloseDate(Map<Id, Opportunity> opps, Map<Id, Opportunity> oldOpps) {
    Id maintenanceRecordTypeId = [SELECT Id FROM RecordType WHERE SObjectType = 'Opportunity' AND DeveloperName = 'Maintenance_Renewal'].Id;
    for (Opportunity opp : opps.values()) {
        Opportunity oldOpp = oldOpps.get(opp.Id);
        if (opp.RecordTypeId == maintenanceRecordTypeId && opp.IsWon == true && oldOpp.IsWon == false && opp.CloseDate != oldOpp.CloseDate) {
            opp.CloseDate = oldOpp.CloseDate;
            System.debug('persisting');
        } else {
            System.debug('not persisting');
        }
    }
}

Test:

@IsTest
static void persistOriginalCloseDateTest() {
    Account a = [SELECT Id FROM Account WHERE Name = :ACCOUNT_NAME];

    Opportunity opp = new Opportunity();
    opp.Name = 'Test Opportunity';
    opp.CloseDate = Date.today().addDays(5);
    opp.StageName = 'Active';
    opp.AccountId = a.Id;
    insert opp;

    Test.startTest();
        opp.StageName = 'Won';
        update opp;

        opp = [SELECT Id, CloseDate, StageName, RecordTypeId FROM Opportunity WHERE Id = :opp.Id];
        System.assertEquals(Date.today(), opp.CloseDate);

        opp.RecordTypeId = [SELECT Id FROM RecordType WHERE SObjectType = 'Opportunity' AND DeveloperName = 'Maintenance_Renewal'].Id;
        opp.StageName = 'Active';
        opp.CloseDate = Date.today().addDays(1);
        update opp;

        opp.StageName = 'Won';
        update opp;

        opp = [SELECT Id, CloseDate FROM Opportunity WHERE Id = :opp.Id];
        System.assertEquals(Date.today().addDays(1), opp.CloseDate);
    Test.stopTest();
}

Answer 1

You didn't explicity say so in your post, but since your trigger method isn't doing any DML, I'm assuming that you're calling the method from a before update trigger. And this is the problem. Although it's not documented where in the Order of Execution a system field update like this takes place, a quick test shows that it hasn't happened in the before update context:

trigger CloseDateTest on Opportunity (before update, after update) {
    if (Trigger.isBefore) {
        for(Opportunity o : Trigger.New) {
            System.debug('Old Close Date: ' + Trigger.oldMap.get(o.Id).CloseDate);
            System.debug('New Close Date Before: ' + o.CloseDate);
            System.debug('Changed Before? ' + !(Trigger.oldMap.get(o.Id).CloseDate == o.CloseDate));
        }
    }

    if (Trigger.isAfter) {
        for(Opportunity o : Trigger.New) {
            System.debug('New Close Date After: ' + o.CloseDate);
            System.debug('Changed After? ' + !(Trigger.oldMap.get(o.Id).CloseDate == o.CloseDate));
        }
    }
}

Which generates this debug output for an Opp with a CloseDate of 2019-09-30 that is updated to Closed/Won:

14:05:50:038 USER_DEBUG [4]|DEBUG|Old Close Date: 2019-09-30 00:00:00
14:05:50:038 USER_DEBUG [5]|DEBUG|New Close Date Before: 2019-09-30 00:00:00
14:05:50:038 USER_DEBUG [6]|DEBUG|Changed Before? false
14:05:50:686 USER_DEBUG [13]|DEBUG|New Close Date After: 2019-07-01 00:00:00
14:05:50:687 USER_DEBUG [14]|DEBUG|Changed After? true

So, you'll need to call you trigger method from the after update context, and add DML to commit the reversion to the database:

public static void persistOriginalCloseDate(Map<Id, Opportunity> opps, Map<Id, Opportunity> oldOpps) {
    Id maintenanceRecordTypeId = [SELECT Id FROM RecordType WHERE SObjectType = 'Opportunity' AND DeveloperName = 'Maintenance_Renewal'].Id;
    List<Opportunity> oppsForUpdate = new List<Opportunity>();
    for (Opportunity opp : opps.values()) {
        Opportunity oldOpp = oldOpps.get(opp.Id);
        if (opp.RecordTypeId == maintenanceRecordTypeId && opp.IsWon == true && oldOpp.IsWon == false && opp.CloseDate != oldOpp.CloseDate) {
            opp.CloseDate = oldOpp.CloseDate;
            oppsForUpdate.add(opp);
            System.debug('persisting');
        } else {
            System.debug('not persisting');
        }
    }
    if (!oppsForUpdate.isEmpty()) {
        update oppsForUpdate;
}

A couple other notes: Your test doesn't set the RecordTypeId of the test Opportunity at first, so it will get the default RT of the test's running user, which is a bad idea, especially when testing functionality that branches on Record Type. You should also break your tests out into separate test methods. Create the Account and Opp in a @testSetup method at the top of the class, and then have separate methods for each of your scenarios: allowing system CloseDate change, and reverting it for the one RT. Your Test.startTest(); Test.stopTest(); pairs should enclose only the DML that saves after updating to Closed/Won, and your asserts should be after.