fix: detect-conflict-at-sync-doesnt-work

[jeffb-sfdc]

What does this PR do?

Fixes a bug where the "Detect Conflicts At Sync" setting is ignored

What issues does this PR fix or reference?

#3522, @W-9873036@
https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE000005Ps0vYAC/view

Functionality Before

With the "Detect Conflicts At Sync" setting turned on, running "Deploy Source in Manifest to Org" would sometimes not prompt the user, and would always deploy.

Functionality After

When running the "Deploy Source in Manifest to Org" command, the user is now prompted.

Steps to Reproduce

1. Authorize an org which has Apex classes in it.

2. Edit one of the classes and set it's apiVersion to 52.0

3. Switch to VS Code and make sure the same class has its apiVersion set to 52.0 in its metadata file.

4. Click on the package.xml file, and run "SFDX: Deploy Source in Manifest to Org"
   -> you should get an alert/prompt

5. select the“override conflicts and deploy” button

6. Once finished, go back to the same class's metadata file and change the apiVersion to 51.0

7. Click on the package.xml file, and run "SFDX: Deploy Source in Manifest to Org"
   -> you should get an alert/prompt

8. select the“override conflicts and deploy” button

9. Go back to the org, refresh the Apex Classes page, and verify that the apiVersion is updated to 51.0

[jeffb-sfdc]

In my investigation, I found that in TimestampConflictChecker.check(), there is:

        const diffs = detector.createDiffs(result);
        channelService.showCommandWithTimestamp(
          `${nls.localize('channel_end')} ${nls.localize(
            'conflict_detect_execution_name'
          )}\n`
        );
        return await this.handleConflicts(inputs.data, username, diffs);

When createDiffs() is called, determineConflicts() is called, and the conditional here on line 54 was always false, and diffComponents() on line 55 was never being called.

Since the diffs array was never being hydrated, later on when when handleConflicts() is called (and diffs is passed in), the user would never be prompted.

It seems to me, that determineConflicts() shouldn't be using a cache to determine whether the sources should be pushed. My reasoning for this is, say we do the following:

1. Edit the metadata of an Apex class, and set its apiVersion to say, 51.0
2. Run "deploy source in manifest to org" (and select override)
3. Edit the metadata of the same Apex class, and set its apiVersion to say, 50.0
4. Run "deploy source in manifest to org" (and select override)

At this point, no matter what, when I go back to the org and view the class, it should have the correct apiVersion. If using a caching mechanism to determine whether to push or not, the second time the file is pushed, the logic would be, "this file is cached, so don't push it", which leads to this bug.

[jag-j]

This is a great find @jeffb-sfdc. The timestamp based diff detection is a recent change introduced this summer and I think this is an edge case that we missed. You can read more about the design here - https://salesforce.quip.com/yNriA1RWTGUz#UHbACAJxowc. My preference would be instead of removing lastModifiedInCache based checks completely, is there a way to update it correctly when the first deploy happens? It could complicate the existing design, but worth giving it a try.

[jeffb-sfdc]

Thanks for the help and insight/background on this the other day @jag-sfdc . I added the condition check at line 54 back in, and removed setPropertiesForFilesDeploy(), and also updated the unit tests.

[jag-j]

This looks right to me. Can you please assign @AnanyaJha as the QA for this once you merge? She worked on the initial design and let's make sure we didn't break anything by doing this.

[randi274]

@jeffb-sfdc was this test working with the old code? I'm wondering if there are any failing tests we could add with the old code so that we could remove the commented test above, but still capture the use case that you've fixed. Maybe something like 'should not report differences if the files match BUT timestamps are different'?