Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Jun 26, 2023

Conversation

i-just
Copy link
Contributor

@i-just i-just commented Jun 16, 2023

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:

  • the site you’re importing to has the language set to Dutch (nl)
  • you checked the “Data provided for this localized for the site the feed is for” checkbox on the feed mapping screen
  • the value in the feed should be: 1.234,56 (though not using the grouping character will work too)

Example 2:

  • the site you’re importing to has the language set to Polish (pl)
  • you checked the “Data provided for this localized for the site the feed is for” checkbox on the feed mapping screen
  • the value in the feed should be: 1 234,56 (though not using the grouping character will work too)

Example 3:

  • the site you’re importing to has the language set to English (en)
  • you checked the “Data provided for this localized for the site the feed is for” checkbox on the feed mapping screen
  • the value in the feed should be: 1,234.56 (though not using the grouping character will work too)

Example 4:

  • it doesn’t matter what language is set on the site you’re importing to
  • you didn’t check the “Data provided for this localized for the site the feed is for” checkbox on the feed mapping screen
  • the value in the feed should be: 1234.56

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

@i-just i-just requested a review from angrybrad as a code owner June 16, 2023 14:50
@angrybrad
Copy link
Member

@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
Copy link
Contributor Author

i-just commented Jun 22, 2023

@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 :)

@angrybrad angrybrad merged commit c2350ad into develop Jun 26, 2023
@angrybrad angrybrad deleted the bugfix/1315-feed-money-value-independent-of-locale branch June 26, 2023 03:11
angrybrad added a commit that referenced this pull request Jun 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants