If a field is imported as null (i.e. with feeds xpath parser for an xml node that is missing an element) feeds tamper will skip tampering the field. It would be nice to allow these fields to be tampered anyway.

Patch to follow.

CommentFileSizeAuthor
#1 1508584-1-feeds_tamper-allow_null_tamper.patch749 byteselliotttf

Comments

elliotttf’s picture

elliotttf commented
Status: Active » Needs review
StatusFileSize
new 1508584-1-feeds_tamper-allow_null_tamper.patch749 bytes
chrisroditis’s picture

chrisroditis commented

Works wonderfully!

twistor’s picture

twistor commented
Status: Needs review » Needs work

I'm not sure this is desirable in all cases. I know from experience that something is needed though.

There's actually two different use cases here. Tampering NULL, which would be the following patch. This is going in either way:

     foreach ($importer_instances as $element_key => $instances) {
-      if (isset($item[$element_key])) {
+      if (array_key_exists($item[$element_key])) {
         foreach ($instances as $instance) {
           $plugin = $plugins[$instance->plugin_id];
           $is_array = is_array($item[$element_key]);

Then there's setting the field to NULL if it was unset. This is also needed, but sometimes it is undesired.

elliotttf’s picture

elliotttf commented

I believe the original patch achieves both cases. The reason isset and array_key_exists were both used was to short circuit the conditional with the more performant isset if the null case wasn't needed.

I agree that there may be cases where setting the field to null might be undesirable but I haven't personally hit one of those cases yet. If someone can chime in with a use case where that'd be a bad idea I'd be happy to bounce around some other ideas.

twistor’s picture

twistor commented
Issue summary: View changes
Status: Needs work » Closed (fixed)

This was fixed a long time ago.