Skip to content

Always set a line item's currency to order currency#1007

Merged
gmacdougall merged 1 commit intomasterfrom
unknown repository
Mar 31, 2016
Merged

Always set a line item's currency to order currency#1007
gmacdougall merged 1 commit intomasterfrom
unknown repository

Conversation

@mamhoff
Copy link
Copy Markdown
Contributor

@mamhoff mamhoff commented Mar 24, 2016

Before, a line item could under certain circumstances be assigned
the store's default currency, rather than its order's currency.

With this commit, we're making sure Solidus does not do that, as
well as deprecating setting the currency manually to something
other than the order's currency.

@gmacdougall
Copy link
Copy Markdown
Member

Looks good to me. Thanks for working on this. 👍

@adammathys
Copy link
Copy Markdown
Member

👍 Looks good to me.

Comment thread core/app/models/spree/line_item.rb Outdated
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe not everyone intended to change the line item currency and something else change it. Without knowing that, the current warning could be misleading.

Why not just: "The line items currency is different from it's order currency. This behavior is not supported anymore and will be deleted soon."?

@tvdeyen
Copy link
Copy Markdown
Member

tvdeyen commented Mar 30, 2016

👍 otherwise

Before, a line item could under certain circumstances be assigned
the store's default currency, rather than its order's currency.

With this commit, we're making sure Solidus does not do that, as
well as deprecating setting the currency manually to something
other than the order's currency.
@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Mar 30, 2016

Thanks @tvdeyen - I took your suggested wording for the deprecation message.

@tvdeyen
Copy link
Copy Markdown
Member

tvdeyen commented Mar 30, 2016

👍

@mamhoff mamhoff mentioned this pull request Mar 31, 2016
23 tasks
@gmacdougall
Copy link
Copy Markdown
Member

👍 again!

@gmacdougall gmacdougall merged commit 9c50cf5 into solidusio:master Mar 31, 2016
@mamhoff mamhoff deleted the always-use-order-currency-on-line-item branch May 24, 2016 19:46
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.

4 participants