search

craftcms / feed-me

Craft CMS plugin for importing entry data from XML, RSS or ATOM feeds—routine task or on-demand.
Other
286 stars 139 forks source link

fetchArrayValue declares $value with a default of null, which breaks array_merge() #1298

Closed jamesmacwhite closed 1 year ago

jamesmacwhite commented 1 year ago

Description

We are modifying a certain XML feed, due to it not representing certain data types in formats for importing i.e. arrays. In this case I am modifying the value to swap values that look like this: Value 1, Value 2, Value 3, to Value 1-|-Value 2-|-Value 3 so they get imported into array fields i.e. categories.

The change made here: https://github.com/craftcms/feed-me/blob/v4/src/helpers/DataHelper.php#L63 has broken our modification with EVENT_BEFORE_FETCH_FEED.

TypeError
array_merge(): Argument #1 must be of type array, null given

The change has made $value default to null, however this function uses array_merge, which $value needs to be an array whether empty or not. This was set to this until recently.

I'd have to say this is a regression, unless I am modifying the XML value wrong for the purpose.

Additional info

jamesmacwhite commented 1 year ago

Looks like it's been introduced here: https://github.com/craftcms/feed-me/commit/a59fe4e9716233b44a988c4367a359a98e673a35#diff-00b65bfbafca7d94fc79ef28c99cb363e43a28e3e7a56c1822e7b447e41055c1

As part of the empty value changes.

i-just commented 1 year ago

Hi, thanks for reporting! I submitted a PR for this.

jamesmacwhite commented 1 year ago

Thank you! I have updated our XML parsing modifications to implement proper XML array syntax to map in the feed mapping, rather than using the delimiter method, to avoid the problem currently, but thank you for your reactive PR to the issue.

angrybrad commented 1 year ago

Fixed in https://github.com/craftcms/feed-me/pull/1299 for the next v4 and v5 releases.

angrybrad commented 1 year ago

We just released Feed Me 4.6.3. and 5.1.3 with this fix.