feed-me icon indicating copy to clipboard operation
feed-me copied to clipboard

Passing an empty string to an existing value doesn't overwrite that value

Open jan-dh opened this issue 4 years ago • 16 comments

Description

When the data in your API gets updated so a field has a new value of an empty string but it previously had a proper value, Feed-me simple ignores it and there is no way to update the entry to reflect the new data. I know values that are set to null are being ignored by Feed-me, but an empty string is a valid value and shouldn't be ignored.

Think this is where this behaviour is implemented:

https://github.com/craftcms/feed-me/blob/d6817eb0d5cb738cd9c8180b874b09f17eafa9d4/src/services/Process.php#L358-L367

The code suggests it only checks on null but empty strings are parsed somewhere I believe to be set to a value of null if they are empty.

Maybe there are arguments to be made that sometimes you would like this behaviour, but it would be very handy to be able to set this as a settings option and not a default you have to hack your way around. At the moment the only way around this we found is by doing something like this:

   Event::on(
            Entry::class,
            Element::EVENT_BEFORE_SAVE,

Then looping over all the fields and do the checking ourselves, based on a keyword EMPTY, setting that value manually.

Steps to reproduce

  1. Set up a feed and map a data value to a field
  2. Update the feed with that same data value now set as an empty string

Additional info

  • Craft version: 3.5.17.1
  • PHP version: 7.4
  • Plugins & versions: Feed-me 4.3.4

jan-dh avatar Feb 03 '21 07:02 jan-dh

We are experiencing this same issue. An empty string in the import data results in no changes to the field's existing data.

darinlarimore avatar Aug 17 '21 16:08 darinlarimore

Same issue here. Subsequent importing of a JSON feed where a value for one field is changed from a defined value to null or "" (blank) results in the previous value remaining and not being updated with the new null or "" (blank) value.

grokcodile avatar Nov 03 '21 18:11 grokcodile

Same here.

nickdunn avatar Nov 17 '21 11:11 nickdunn

An easier solution would be to use the afterParseField event which gets called after a field's data has been parsed. There you can just overwrite the parsed value. You can check the original values in $e->feedData. You might even add some more logic to differentiate between really empty and just an empty string. But the safer version would be to use a keyword like @jan-dh suggests, e.g.:

use craft\feedme\events\FieldEvent;
use craft\feedme\services\Fields;
use yii\base\Event;

Event::on(Fields::class, Fields::EVENT_AFTER_PARSE_FIELD, function(FieldEvent $e) {
  if ($e->parsedValue === "EMPTY") {
    $e->parsedValue = "";
  }
});

dgsiegel avatar Nov 24 '21 13:11 dgsiegel

I got the same issue. I don't see the reason why empty strings would be treated as "empty" (as in "ignore me") in the first place. dgsiegel's solution wouldn't solve the problem in my case, since - if I don't get it wrong - every empty value would be overwritten by an empty string. And I don't wanna go through the data in the event since that might get complicated.

We are using a forked version of the plugin anyway and our solution is modifying the DataHelper-class. I'm providing a pull request soon.

floatingbits avatar Dec 08 '21 08:12 floatingbits

I elaborated on this, and added a settings field so this is opt-in functionality!

Screen Shot 2022-01-19 at 4 37 45 PM

darinlarimore avatar Jan 19 '22 21:01 darinlarimore

When will this feature be available as an upgrade from the Craft CMS plugin store?

grokcodile avatar Feb 21 '22 17:02 grokcodile

I've been waiting for months for this feature and my clients are getting more and more impatient... this is an issue that really needs fiksing. Is there a workaround in the meantime?

mdoorschodt avatar May 09 '22 10:05 mdoorschodt

I had to take a pretty crappy approach of changing the feed I was importing to output "None" instead of nulls or blank strings, and check for this value in my template to treat as a blank.

nickdunn avatar May 09 '22 10:05 nickdunn

can someone please fix this issue? we have a lot of problems due to this not being properly handled

mdoorschodt avatar Jun 10 '22 08:06 mdoorschodt

According to what I heard on one of the talks of dotAll (I believe 🤔) I think Brandon said they were looking into not investing a lot more time into Feed-me in the future, but looking into a new approach etc. for content migration in Craft 4. Haven't seen an update on this since last year though. Basically. I wouldn't count on this to ever get fixed.

jan-dh avatar Jun 13 '22 13:06 jan-dh

@mdoorschodt Did you see my pull request? Maybe you can integrate that on your site, e.g. by forking the repo and using my code?

floatingbits avatar Jun 13 '22 14:06 floatingbits

@mdoorschodt Did you see my pull request? Maybe you can integrate that on your site, e.g. by forking the repo and using my code?

yes i did. not a big fan of forking, as updating/upgrading will be an issue later on. I just don't get it that merging a bugfix is such a problem

mdoorschodt avatar Jun 15 '22 14:06 mdoorschodt

Also related #723

@angrybrad @olivierbon could we get this looked at? Would close at least 3 issues and 2 pull requests.

pixelmachine avatar Sep 16 '22 17:09 pixelmachine

And #854 #680

So would close 5+ issues.

pixelmachine avatar Sep 16 '22 17:09 pixelmachine

Yeah, I was thinking about a non breaking space… just as crappy

Mark Doorschodt - Projectleider Tel: ‭+31 (0)651839574‬

Zicht online & Fabrique Weena-Zuid 108 / 3012 NC Rotterdam / NL www.zicht.nlhttp://www.zicht.nl en www.fabrique.nlhttp://www.fabrique.nl

Op 9 mei 2022, om 12:44 heeft Nick Dunn @.@.>> het volgende geschreven:

I had to take a pretty crappy approach of changing the feed I was importing to output "None" instead of nulls or blank strings, and check for this value in my template to treat as a blank.

— Reply to this email directly, view it on GitHubhttps://github.com/craftcms/feed-me/issues/797#issuecomment-1120940190, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABJZZMJ7STOMWUSOXI2O7ELVJDUBJANCNFSM4XAL2AMA. You are receiving this because you commented.Message ID: @.***>

mdoorschodt avatar Oct 11 '22 08:10 mdoorschodt

+1 for this feature

siffring avatar Dec 07 '22 18:12 siffring

@brianjhanson not sure if this is on your radar but taking a look at this issue would be great!

pixelmachine avatar Jan 21 '23 00:01 pixelmachine

Resolved in https://github.com/craftcms/feed-me/pull/1228

angrybrad avatar Feb 10 '23 00:02 angrybrad

Feed Me 4.6.0 (Craft 3) and 5.1.0 (Craft 4) have been released with the new “Set Empty Values” setting, which resolves this (via #1228) 🎉

brandonkelly avatar Mar 17 '23 21:03 brandonkelly

Happy St Pattie's day everyone!

darinlarimore avatar Mar 18 '23 02:03 darinlarimore

@brandonkelly unfortunately this change only partially fixes this issue for us. It seems that the PR makes no difference between an empty value and a non-existent value. Ie, let's say you want to import this XML via Feedme:

<?xml version="1.0" encoding="UTF-8"?>
<root>
   <object>
      <ID>12345</ID>
      <TITLE>Some title</TITLE>
      <FOO>foobar</FOO>
    </object>
    […]
</root>

Now let's say FOO is an optional field and can be empty. If the entry exists like in the example above, both removing FOO from the XML as well as an empty FOO will clear out the original value, whereas removing it only in the latter example would be correct:

<?xml version="1.0" encoding="UTF-8"?>
<root>
   <object>
      <ID>12345</ID>
      <TITLE>Some title</TITLE>
    </object>
    […]
</root>
<?xml version="1.0" encoding="UTF-8"?>
<root>
   <object>
      <ID>12345</ID>
      <TITLE>Some title</TITLE>
      <FOO></FOO>
    </object>
    […]
</root>

dgsiegel avatar Mar 20 '23 13:03 dgsiegel