Summary: the filefield_feeds_set_target() function only adds files to a node's file or image field, but it does not remove existing files first. Thus, if you are using Feeds to update nodes, the old files remain and new ones are added.

The expected behavior is that after an update, the only files that should end up on the node are the ones in the Feeds source (ie: the files in the CSV column, in the case of a CSV importer).

See also:
#2070061: Content taxonomy mapper only adds terms
#2073671: Content mapper can only add values

CommentFileSizeAuthor
#1 feeds_filefield_update-2073745-1.patch854 bytesm.stenta
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

m.stenta’s picture

Status: Active » Needs review
FileSize
854 bytes

Patch attached clears out the node's file field before adding items to it. It also deletes the files themselves and the record for them in the {files} database table.

twistor’s picture

Status: Needs review » Needs work

There seems to be some other bug going on. I closed the other two issues as duplicate. Feeds handles resetting the field value before the mapper ever executes. See ProcessorBase::map(). The job of the mapper is only to add fields.

Forcibly deleting the files is also not an option, since they could be used elsewhere.

I'm not sure what your specific problem is. Are you using the latest version? There was a bug in reference to this a long time ago.

m.stenta’s picture

I'm using 6.x-1.0-beta13 (the latest 6.x release).

I don't see a ProcessorBase class anywhere in the 6.x-1.x or 7.x-2.x branch. Are you talking about the 8.x-3.x branch?

I don't think the other two issues are duplicates. They are similar, but the code to fix them is different.

As for deleting files, my assumption was that it would be ok to delete them because they are FileFields, so they should only be used by the FileField, right? Maybe I'm overlooking something. Since this is D6, though, there is no {file_managed} table to keep track of file use.

twistor’s picture

Ahh, you're right, see plugins/FeedsProcessor.inc::map()

The thing is that we've spent a lot of time doing the exact opposite of what you're trying to do in each of these patches. The reason that a mapper should re-use the field on the node, rather than start with an empty array is that we want to support mapping more than one source to the same target.

m.stenta’s picture

I had a feeling that was the reason. And that makes sense. But it seems to be a big sacrifice to achieve that.

So does it work the same in D7 and D8 versions, too? Fields that are updated by an Importer cannot remove existing values?

Would it make sense to make a configuration option in the Processor class that allows you to specific which approach to take? a) do not touch existing data (for cases where you want to map multiple sources to one target), or b) wipe existing data (when you want to start fresh each time)

twistor’s picture

That's what I'm not understanding. The problem you're describing is a bug that I've never seen. In plugins/FeedsProcessor.inc::map() Feeds automatically resets each field before it gets to the target callbacks. So, there shouldn't be any sacrifice.

twistor’s picture

Issue summary: View changes
Status: Needs work » Closed (outdated)