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

Fix uninitialized currency providers and settings #38

Merged
merged 1 commit into from
Sep 13, 2019

Conversation

bleroy
Copy link
Member

@bleroy bleroy commented Sep 11, 2019

Also disambiguates usage of ISO symbol/code, and adds a bunch of tests for the money service.

Fixes #35

@bleroy bleroy added the bug Something isn't working label Sep 11, 2019
@bleroy bleroy requested a review from jeffolmstead September 11, 2019 22:59
@bleroy bleroy self-assigned this Sep 11, 2019
@jeffolmstead
Copy link

The clarity added in naming is fantastic and makes perfect sense. I am going to try and pull it local and run it on a fresh instance to test. With these changes, do you even still need the "CommerceSettingsConfiguration.cs" file? I don't think a "Default Currency" can ever be null or empty with the changes you have made... If it is still needed, it should be cleaned up as it still uses "Symbol" instead of "IsoCode" for setting the "DefaultCurrency".

@jeffolmstead
Copy link

I have tested on a fresh install and works as expected. If you can comment on the "CommerceSettingsConfiguration.cs" file (whether it needs kept or modified) then I would be ready to merge this (or you can of course).

@bleroy bleroy force-pushed the fix-default-currency-master branch from ef49131 to 4d0184e Compare September 13, 2019 19:08
@bleroy
Copy link
Member Author

bleroy commented Sep 13, 2019

Removed the CommerceSettingsConfiguration file. Good catch.

@bleroy bleroy merged commit 892fded into master Sep 13, 2019
@bleroy bleroy deleted the fix-default-currency-master branch September 13, 2019 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Price part crashes if default currency has never been set.
2 participants