Skip to content

Remove Line Item currency attribute#998

Closed
mamhoff wants to merge 1 commit intomasterfrom
unknown repository
Closed

Remove Line Item currency attribute#998
mamhoff wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@mamhoff
Copy link
Copy Markdown
Contributor

@mamhoff mamhoff commented Mar 20, 2016

Somehow, in the past, it was possible to have orders with line items
in multiple currencies. The code to remedy this (Order#homogenize_line_item_currencies)
disappeared in #635, and now we have a bunch of code in different places
that allows setting it, raises errors if it's not there, and so on.

This commit delegates the line items currency to the order.
Whenever people try setting a line item's currency, they get a
pretty deprecation warning telling them not to do so.

The error handling functionality that seems to be used in the order
importer is still present (ie: If we know an imported line item has
a currency different from the order, we'll error out).

This commit replaces all that behaviour with a simple delegate that
always obtains a line item's currency from it's order.

Adds a changelog entry.

I also refactored OrderContents#add_to_line_item, taking code from
MountainRoseHerbs/spree@461f975.

That method should now be much easier to understand.

Somehow, in the past, it was possible to have orders with line items
in multiple currencies. The code to remedy this (Order#homogenize_line_item_currencies)
disappeared in #635, and now we have a bunch of code in different places
that allows setting it, raises errors if it's not there, and so on.

This commit delegates the line items currency to the order.
Whenever people try *setting* a line item's currency, they get a
pretty deprecation warning telling them not to do so.

The error handling functionality that seems to be used in the order
importer is still present (ie: If we know an imported line item has
a currency different from the order, we'll error out).

This commit replaces all that behaviour with a simple delegate that
always obtains a line item's currency from it's order.

Adds a changelog entry.

I also refactored `OrderContents#add_to_line_item`, taking code from
https://github.com/MountainRoseHerbs/spree/commit/461f97510c29d50c08c6d8b75805b66f12de12af

That method should now be much easier to understand.
@gmacdougall
Copy link
Copy Markdown
Member

I do think that this makes sense. However, I am concerned about potentially destroying data that people were relying upon if they had customized their store. I would appreciate some more opinions on this one.

@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Mar 24, 2016

Closing in favour of #1007, which does not remove the column but makes handling line item currencies saner.

@mamhoff mamhoff closed this Mar 24, 2016
@mamhoff mamhoff deleted the delegate-line-item-currency-to-order 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.

2 participants