Currency providers and OrderPart currency

[MatteoPiovanelli-Laser]

I added the interface for currency selection discussed in #99 as well as the currency in the OrderPart as per #102 .

The default ISelectedCurrencyProvider in the Nwazet.Commerce feature replicates the original behaviour of using the Site's culture for currency. Anothe ISelectedCurrencyProvider is in its own feature and uses a Site setting to select the currency to be used across the whole store.

There were changes in several controllers to inject the active ISelectedCurrencyProvider, as well as changes in views to use it in formatting prices. I also added the Currency service to give some static formatting methods.

I added a "Settings" submenu under "Commerce" for SIteSettings strictly related to the store. The one setting like that I implemented is for the currency provider that uses a Site Setting to set the currency used (it is in its own feature).

Adding the CurrencyCode to the OrderPart caused some changes that are not retrocompatible, to populate that value when creating the order. For new orders, the currency code will be populated (in the changes I made, when paying with stripe I use the Stripe currency). For pre-existing orders, that property is not populated: on displaying/editing an order, I show the prices using the current currency (as per the active provider) but I don't change the information in the order, since there is no guarantee that the current currency is the correct one.

As things are now, it is still possible to set a currency for payments (Stripe here) that is different than the one used to display prices, and there is nothing special happening when the currency is changed: the absolute value of prices and such does not change.

[MatteoPiovanelli-Laser]

@bleroy @petedavis How does this look?

[bleroy]

I also think we should have tests associated with those changes.

[MatteoPiovanelli-Laser]

Implementing the currency class and the tests (insert animated work in progress icon here)

[MatteoPiovanelli-Laser]

I think the current PR answers the changes above, except: What tests did you have in mind? I didn't implement any yet.

[MatteoPiovanelli-Laser]

Also, A separate project for the tests? Or should I place them where the code is? Maybe in a block like

#IF TESTS
...
#ENDIF

[MatteoPiovanelli-Laser]

In case of a separate test project, I don't know how to include that in the PR/the repo

[MatteoPiovanelli-Laser]

A further note, on the code: In CurrencyProviderBase.cs you'll see I am always using the CurrentCulture. The idea there is that if I set my UI to Italian, I still want to see "1.234,00 $", rather than "$1,234.00" as would be the "representation" in the en-US culture. However, in the methods of the Currency class I am changing the currency symbol and decimal digits to match the currency being used.

[MatteoPiovanelli-Laser]

Something annoying I noticed is that many currencies use the "$" symbol. Thta may lead to confusion unless noted. e.g. The Argentine Peso uses the "$" sign.

[bleroy]

Tests should go in the test project: https://github.com/bleroy/Nwazet.Commerce/tree/master/Nwazet.Commerce.Tests

[bleroy]

if I set my UI to Italian, I still want to see "1.234,00 $", rather than "$1,234.00" as would be the "representation" in the en-US culture.

I don't understand what you're saying here. The dollar sign should always be in front for a sum in dollars. Decimal points and commas are culture-based, yes.

[bleroy]

Something annoying I noticed is that many currencies use the "$" symbol. Thta may lead to confusion unless noted. e.g. The Argentine Peso uses the "$" sign.

That is the problem of the site administrator. We should just implement cultures and currencies as they are standardized and used.

[MatteoPiovanelli-Laser]

I don't understand what you're saying here. The dollar sign should always be in front for a sum in dollars. Decimal points and commas are culture-based, yes.

Actually, the position of the currency symbol is culture based as well.

[MatteoPiovanelli-Laser]

About the tests:

What did you have in mind? I was thinking about verifying that things are formatted as expected, but that feels more like I'd be testing that ToString() works as expected, rather than anything I have implemented here.

[MatteoPiovanelli-Laser]

Note a thing that is left to the site's administrator to handle:

Supposing Stripe is being used for payments, the currency used there should be set to be the same as the store's, otherwise it's probably a crime of some kind.

[bleroy]

Looks good except for one small thing in the migration. Thanks!

[bleroy]

On tests: I'd like to see verification that all non-trivial behavior is as expected, for example currency formatting, yes.

[MatteoPiovanelli-Laser]

In my latest commits:

• I changed what gets injected into the currency providers: IWorkContextAccessor rather than IOrchardServices.
• I fixed the build-errors in the test project, related to changes in the constructors of some driver.
• I fixed a bug in the ValidateAttribute method of ProductAttributesPartDriver (see #109)
• I updated the test project so it work with O1.10: upgraded the used framework and changed the nunit references to NuGet.
• I added a few formatting tests related to the Currency class.

[bleroy]

Looks good, thanks. Can you check the migration issue?

[bleroy]

Thanks!