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

New preference to control available currencies #2461

Merged
merged 1 commit into from
Dec 19, 2017

Conversation

softr8
Copy link
Contributor

@softr8 softr8 commented Dec 18, 2017

This change allows stores to control what currencies will be available, it could improve usability a bit in some dropdowns by limiting the number of options.

@softr8 softr8 force-pushed the available-currencies branch from 01b8bc7 to 6a9178e Compare December 18, 2017 17:26
Copy link
Contributor

@jhawthorn jhawthorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Great!

#
# @!attribute [r] available_currencies
# @returns [Array] An array of available currencies from Money::Currency.all
preference :available_currencies, :array, default: ::Money::Currency.all
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only concern is that this should maybe be a proc. We load preferences quite early and customizations might be made to ::Money. (this is really quite an edge case, so I don't know if it's necessary, but can't hurt)

@jhawthorn
Copy link
Contributor

This seems to be failing because money isn't loaded yet when app_configuration is

@softr8 softr8 force-pushed the available-currencies branch from 6a9178e to d8fbf86 Compare December 18, 2017 21:22
@softr8 softr8 force-pushed the available-currencies branch from d8fbf86 to 0caf2b8 Compare December 18, 2017 21:37
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@jhawthorn jhawthorn merged commit e2f9c6a into solidusio:master Dec 19, 2017
@jhawthorn
Copy link
Contributor

Thank you!

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.

3 participants