Wrong date field used in Pull Queue Worker

Re-rolled against 5.0.4

We also need to ensure that LastModifiedDate is always included in our queries...

Have you run with this patch?

Historically, we compare the SF updated date to Drupal updated date to prevent race conditions between the two systems.
ie. same record updated in both systems between cron runs: we use this date comparison to decide precedence.

seems like this change probably won't have ill effect for the default scenario.
but I am wary that there's some even further edge case where we're relying on this behavior e.g. to prevent some kind of recursion.

how about if, rather than always use LastModified for comparison in PullBase, we fall back on it only if getPullTriggerDate() returns a non-date?

something like this maybe?

       $pull_trigger_date =
-        $sf_object->field($mapping->getPullTriggerDate());
+        $sf_object->field($mapping->getPullTriggerDate()) ?? $sf_object->field('LastModifiedDate');
       $sf_record_updated = $pull_trigger_date ? strtotime($pull_trigger_date) : 0;
 
       $mapped_object

We are using it in production as-is, yes. I don't mind the fallback suggestion, although I'm having trouble imagining the edge case you might see.

Case for your solution: "Pull Trigger Date" is what we consider for the purposes of syncing to be the authoritative date of new information appearing.

Case for just using the true modified date: "Pull Trigger" doesn't actually reflect when it was updated, it just reflects when we want to get information from Salesforce, thus the name.

I think I agree with your solution. For our purposes, and the data-integrity purposes, we care about whatever is used as the pull trigger date under normal circumstances, so why not now?

Here's an updated patch with the suggested change.