Problem/Motivation

We have a feature which allows a Salesforce user to request a pull on specific objects, without waiting for Cron. This works great! It just takes the mapping ID and the Salesforce object ID, uses them to generate a Pull task, and processes it. I may submit it as a submodule at some point.

Recently, we started to get this error when using the feature on new contact records:

> Deprecated: strtotime(): Passing null to parameter #1 ($datetime) of type string is deprecated

This is a PHP 8.1.26 deprecation. (or something after 8.1.20, not sure exactly). And it happens because we use a custom Pull Trigger Date field on Contacts. On new Contacts, this can be blank. That's kind of dumb -- it shouldn't be possible. But it does raise a case where you are pulling a record from SF and the "pull trigger date" is blank. I'd argue that this is a reasonable corner case.

And it isn't a problem! Except that the logic in \Drupal\salesforce_pull\Plugin\QueueWorker\PullBase::updateEntity happens to use the configured "pull date" to determine the updated date for the SF record when comparing it to the Drupal record. That... sort of makes sense, but does it? Why not just use the actual updated date from SF, which is guaranteed to be available and quite accurate for the purpose of our function?

CommentFileSizeAuthor
#7 3420549-7.patch1.69 KBgcb
#4 3420549-4.patch1.63 KBgcb
#3 3420549-3.patch687 bytesgcb
#2 3420549-2.patch660 bytesgcb

Comments

gcb created an issue. See original summary.

gcb’s picture

gcb
he/him
Location Portland, OR
commented
StatusFileSize
new 3420549-2.patch660 bytes
gcb’s picture

gcb
he/him
Location Portland, OR
commented
StatusFileSize
new 3420549-3.patch687 bytes

Re-rolled against 5.0.4

gcb’s picture

gcb
he/him
Location Portland, OR
commented
StatusFileSize
new 3420549-4.patch1.63 KB

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

aaronbauman’s picture

aaronbauman
he/him
commented
Status: Active » Needs review

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
gcb’s picture

gcb
he/him
Location Portland, OR
commented

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?

gcb’s picture

gcb
he/him
Location Portland, OR
commented
StatusFileSize
new 3420549-7.patch1.69 KB

Here's an updated patch with the suggested change.

  • aaronbauman committed adede34d on 5.1.x 
    Issue #3420549 by gcb: Wrong date field used in Pull Queue Worker
aaronbauman’s picture

aaronbauman
he/him
commented
Status: Needs review » Fixed

OK, committed. We'll see how it goes.

  • aaronbauman committed 571b6b20 on 5.0.x 
    Issue #3420549 by gcb: Wrong date field used in Pull Queue Worker

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.