craftcms / feed-me

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

money value in the feed should be float-like #1331

Closed i-just closed 1 year ago

i-just commented 1 year ago

Description

Adds money field template with “Data provided for this localized for the site the feed is for” checkbox. If checked, the value provided in the feed will be parsed as localised for the site you’re importing to. If left unchecked, a float-like notation will be used to parse.

Dutch number formatting uses a full stop as a grouping character and a comma as a decimal character. Polish number formatting uses a space as a grouping character and a comma as a decimal character. English number formatting uses a comma as a grouping character and a full stop as a decimal character.

You have a Money field with EUR currency. You want to import a value of one thousand two hundred thirty-four euro and fifty-six cents into that field.

Example 1:

Example 2:

Example 3:

Example 4:

If, like in the original issue, you want to use the same data to import the Commerce price and value for a Money field, you should use the float-like notation and leave the “Data provided for this localized for the site the feed is for” unchecked.

Related issues

1315

angrybrad commented 1 year ago

@i-just

Some random thoughts:

This seems like it will work if you have a money field you’re importing to in a multi-site environment with different currency formatting needs per site.

Does it do anything (for example) for a single Money field in a Matrix field in a single site environment, but the feed has different currency formats?

A contrived example for the feed:

[
  {
    "title": "Product 1",
    "sku": "product-1",
    "price": "1 234,56",
    "locale": "pl"
  },
  {
    "title": "Product 2",
    "sku": "product-2",
    "price": "1,234.56",
    "locale": "en"
  }
]

Matrix block 1 has a locale text field of pl and a price Money field of 1 234,56. Matrix block 2 has a locale text field of en and a price Money field of 1,234.56.

For that matter, the same issue would apply to Date/Time fields, too, because they can be formatted differently per locale. Do they already handle this in Feed Me? If not, we should probably add similar support?

i-just commented 1 year ago

@angrybrad

This seems like it will work if you have a money field you’re importing to in a multi-site environment with different currency formatting needs per site.

Yes, this whole logic is needed to convert whatever format is provided in the feed to the integer that’s stored in the database because the Money field values are actually stored as integers. For example, a value of one thousand two hundred thirty-four euro and fifty-six is stored as 123456. It’s then displayed in the control panel based on the user’s formatting locale. If I have my formatting locale set to EN, it will show as 1,234.56; if I change my formatting locale to PL, it will show as 1 234,56; if I change my formatting locale to NL, it will show as 1.234,56.

Does it do anything (for example) for a single Money field in a Matrix field in a single site environment, but the feed has different currency formats?

Short answer is: no, it doesn’t do anything different/special.

Long answer: when importing into a Money field, all that matters is that we’re able to parse the value from the feed to the correct integer. If you check the “Data provided for this localized for the site the feed is for”, then we parse the value according to the site’s locale (otherwise, we expect the number to be float-like). Currency and user’s formatting locale (in control panel) don’t matter.

Regarding your example, the locale won’t matter. There’s (currently) no ability to map the locale when importing into a Money field. Just like there’s no way to map the currency - that’s defined on the field itself and is independent of the numerical value.

For that matter, the same issue would apply to Date/Time fields, too, because they can be formatted differently per locale. Do they already handle this in Feed Me? If not, we should probably add similar support?

With the Date/Time fields, there’s a dropdown on the mapping screen where you can choose the format of the value in your feed. I can import a date with UK formatting (dd/mm/yyyy) into a site that’s set to US locale (mm/dd/yyyy), and it will work as expected (if you choose “auto” then Carbon tries to parse it and all bets are off).

I think some improvements could be made to the Number field, though. At the moment, the outcome can be dependent on who runs the import. For example, if I set my language and locale to NL and the feed is set to import to the EN site, if I run the import while logged in - it will attempt to parse the number based on NL rules. If I run the import while logged out via the direct feed url, it will use the EN rules. I can apply the same logic as in this PR to the Number field, but I’d first like to confirm if this is the way to go. What do you think?

Please LMK if I didn’t fully clarify things, if you want to talk through any of this in more detail or if you think we should handle it differently :)