Feeds "update existing" - how to avoid wiping out all existing data?

I dont think the "set a field value" tamper resolves this and it could be worth a module update -- Thinking back to #3, maybe this is different for non-CSV sources, but for a CSV source couldn't we compare the CSV columns to the mapped columns and not update the content value if the column was omitted?
If the column is present, but blank, I think that is different and feeds should update the content with empty values as it does now.

Maybe #3049832 that #7 mentioned could mitigate it in some instances, but I dont think feeds tamper's "set a field value" can set the field value to be whatever is currently stored in the content (just some arbitrary value you provide in the tamper ui). It also seems like a hack to something that should just work that way out of the box.

Hm, ✨ Update target more granularly Active and this issue look now the same, with the only difference that it is reported here that an unmapped source was cleared. I think we should close this or the other one as a duplicate.

Maybe it makes sense to close that other one as a duplicate.

In there I commented the following:

If I remember correctly, there are the following challenges in play:

1. Feeds cannot always make the distinction between empty vs non-existent. This was stated in #1107522-147: Framework for expected behavior when importing empty/blank values + text field fix → .
   We should test whether or not this is still true for the current version of Feeds or if this was the case for the D7 version of Feeds only.
2. Mapping multiple times to the same target (a way to import multiple values for the same field).
   If the first mapped source is non-existent, will the target still be emptied when the second mapped source does exist?
3. Expectations of the current behavior.
   Right now it is ensured that all mapped targets will get overwritten. So I think we should at least keep supporting that workflow, in order to not break existing import workflows that rely on that behavior.

Other things to take in consideration: I'm about to open an issue of moving part of the implementation of clearTarget() to the target plugin. A patch has already been written.

So if we're going to support this workflow, we should at least make it configurable. For the D7 version of Feeds, there was the module Feeds empty → , that allowed to configure the behavior per target.
I'm not sure if we should still make this configurable per target or if it should be a one on-off switch.

ericgsmith → changed the visibility of the branch 3162496-feeds-update-existing to hidden.

I have taken a look at this today (coming from ✨ Update target more granularly Active )

Feeds cannot always make the distinction between empty vs non-existent. This was stated in #1107522-147: Framework for expected behavior when importing empty/blank values + text field fix.
We should test whether or not this is still true for the current version of Feeds or if this was the case for the D7 version of Feeds only.

I assumed my test for this would fail - as all I could see to check on the item was if it was NULL. I suspected that there would be a difference between NULL and empty columns. But for the CSV at least it is seems to parse ,, to an empty string - so we can for CSV at least tell the difference between an empty item and a missing item.

Mapping multiple times to the same target (a way to import multiple values for the same field).
If the first mapped source is non-existent, will the target still be emptied when the second mapped source does exist?

I have not tested this - but I believe yes, if source 1 => field_1 is missing from the source then the target field_1 is not cleared. If source 2 => field_1 has a value then target is cleared.

Expectations of the current behavior.
Right now it is ensured that all mapped targets will get overwritten. So I think we should at least keep supporting that workflow, in order to not break existing import workflows that rely on that behavior.

Agreed - in my work in progress branch I have added this to the advanced fieldset if update existing is checked.

I also have an additional question around targets comprised of multiple sources.

Any feeling for how these should be handled?

An example I added to the test was mapping to both body:value and body:summary from a CSV field - I'd love to be able to update just the summary and keep the original value but its pretty unpredictable what the output would be here - partially missing / updating fields would likely mean we need further changes in the target itself.

Ok - I have enough there for an early review.

I have only tested and focused on this using CSV as a source - I imagine it could be more complicated with other sources depending on how they provide their data - specifically if they are provide NULL for empty values rather than missing values.

This MR introduces a new option under the Advanced tab:

The wording is not particularly friendly - but I wanted to distinguish between a field mapping having all source values provided and some source values provided.

E.g - if one of body:text or body:summary is missing - is it clear that we expect the body field to be skipped, or that we only want the provided source updated (e.g. only update summary, leave text in place).

Additional to the tests I have given this a go on a site with many fields mapped in a feed - and it works as expected. A few cool things this unlocks alongside skipping fields when they are completely missing:

• updating alt text via feeds without having to have the file id in the source
• updating body summary values without having to have the body value in the feed

I think if the general approach can be reviewed - the follow ups would be testing with other parsers and then improving the UX for the field labels and descriptions.

Found an issue with the partially incomplete source values with file / image fields. The target_id set when attempting to merge with current values later caused issues if the reference_by was not the fid.

I changed the approach slightly to get the target plugin to return the current target values for this situation - although its starting to get a but messy.

Have done some more testing with CSVs and its working pretty well.

I have one remaining issue and that is with fields with that have tampers applied to them.

When feeds tamper runs for a missing source it passes NULL to the tamper plugins.

If it either receives NULL back from the tamper plugins on L126 or if we have the SkipOnEmpty plugin causing NULL to be set on L131 here then the item suddenly have a NULL source value an sets this to the field.

This is more complicated to solve, as I have things like the default value plugin to provide values for things that are always missing, but also have had to use SkipOnEmpty as lot due to working around issues summarised in ✨ Improve handling of empty data Needs review

I'm still keen for a review on this initial implementation so leaving as needs review, but will need to also think about how this affects feeds tamper.

I have given this another round of improvements - most conversation points are on the MR.

I appreciate you were part way through a review @Megachriz but I hope what I have done has simplified it so there is less to review.

Summary of recent changes

• Reduce UI complexity by offering only one new option "Only process mappings found in the source"
• Test for the FileTarget to show that the issue reported in comment #18 is resolved.
• Opened ✨ Avoid setting a value to an item when source is missing Postponed in feeds tamper for comment #19

I have also updated the issue summary with what is proposed.

Overall, the MR looks good to me.

Postponing on ✨ Add has() method to ItemInterface Active based on https://git.drupalcode.org/project/feeds/-/merge_requests/169/diffs#note... .

Adding related.

Thanks @stefanvaduva for the review and picking that UI bug up - I have pushed a fix for this.

@max-ar thanks for the comment. Interesting that the missing json field ends up as an empty array - that warrants further investigation and would be helpful when evaluating ✨ Add has() method to ItemInterface Active