Context: SfO is an object in Salesforce. DrO is the equivalent object in Drupal. A workflow rule exists that triggers on any change to SfO and pushes an outbound message to drupalsite/sf_notifications/endpoint with all fields in SfO selected.
Given the above, if I log into Salesforce and set SfO::datefield = '2011-09-19', rules will trigger and a moment later DrO->datefield == '2011-09-19T00:00:00+00:00', pretty much just as you'd expect.
If I then set SfO::datefield = null (or empty string, which seem to be treated equivalently), the rules trigger, and a moment later DrO->datefield == '2011-09-19T00:00:00+00:00' still, which seems like a bad thing.
There are two blocks of code that seem to be conspiring together to keep this from happening. The first is in sf_contrib.module (196-199):
function _sf_node_import_cck_date(&$node, $drupal_fieldname, $drupal_field_definition, $sf_data, $sf_fieldname, $sf_field_definition) {
if (empty($sf_data->{$sf_fieldname})) {
return;
}
...
This looks like it'd be straightforward enough to get around by replacing the "return;" line with "$date = null;" and putting most of the rest of the function in an else clause. The red flag for me here is that the commit comment for this code block was "prevent fatal errors when importing with incomplete data", which seems rather ominous. Since I don't know anything about the context for those changes, I'd be interested to hear from aaronbauman about whether this would be a good or a terrible idea.
The second block is much more subtle. In sf_node.module (845-851):
if (isset($previous_value[0]['date_type'])) {
$new_value = $node->$drupal_fieldname;
$new_value = $new_value[0]['value']; // This looks wrong but I need to convert an object element which is an array to a string
if (strncmp($previous_value[0]['value'], $new_value, strlen($new_value)) != 0) {
$changed_fields++;
}
}
This block checks to see that the dates are the same. The odd methodology is due, I believe, to the fact that the date values passed by Salesforce are never longer (higher resolution) than the values stored by Drupal, but often (or always?) lower resolution.
The problem here is that for zero length strings or null values on the Salesforce side, the comparison boils down to strncmp($previous, $new, 0), which, due to the way strncmp() works, will always return zero, and therefore we get a false negative.
Without a change to the sf_contrib.module though, this bug is effectively moot, since the old value never gets overwritten with a null to begin with.
Thoughts?
Comments
Comment #1
mdunn commentedI should've pointed out in the original post that this is a different issue than http://drupal.org/node/1096712 because fieldsToNull never comes into play with the sf_notifications entry point. I suspect this may work much differently in 7.x, but I haven't yet reviewed the relevant code.
Comment #2
kostajh commented