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

Role fix #9

Merged
merged 3 commits into from
Apr 6, 2017
Merged

Role fix #9

merged 3 commits into from
Apr 6, 2017

Conversation

Senjai
Copy link
Contributor

@Senjai Senjai commented Apr 6, 2017

Volume prices by default are not associated with any role. Users however
in old systems are associated with the "User" role by default.

This means that when a user is logged in, they will never get prices
that don't require a role.

Instead, if a user is logged in, they should see prices that are non
role specific, and prices that are.

Also bumps the gem version. Will need a push to rubygems

@Senjai
Copy link
Contributor Author

Senjai commented Apr 6, 2017

Volume prices by default are not associated with any role. Users however
in old systems are associated with the "User" role by default.

This means that when a user is logged in, they will never get prices
that don't require a role.

Instead, if a user is logged in, they should see prices that are non
role specific, and prices that are.
Because you can get prices from roles and non roles (if you use one
system, you probably don't want to use the other), after ordering by
position order by amount. That way the user gets the cheapest price for
their role/non-role combination
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

I think there's room to clean this up a lot, including the silly config option for the default role that might not be user. But this is a clean enough fix to move us forward, and it adds tests aiding in a future refactor.

@mamhoff mamhoff merged commit b0cb125 into solidusio-contrib:master Apr 6, 2017
@mamhoff
Copy link
Contributor

mamhoff commented Apr 6, 2017

Thank you!

This pull request was closed.
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