Skip to content

Assign a line items currency as an attribute, not an option#993

Closed
mamhoff wants to merge 1 commit intomasterfrom
unknown repository
Closed

Assign a line items currency as an attribute, not an option#993
mamhoff wants to merge 1 commit intomasterfrom
unknown repository

Conversation

@mamhoff
Copy link
Copy Markdown
Contributor

@mamhoff mamhoff commented Mar 18, 2016

When extracting pricing to service objects I came across a number of
ways of setting a line item's currency. In OrderContents, the currency,
although an attribute of the line item, is passed to the line item
through an options Hash designed for an extension point introduced around
Spree 2.2/2.3 while it could just be assigned directly like any other
attribute.

This PR refactors that bit of weird code, hopefully relieving us of the
options= method on line item entirely in the future.

When extracting pricing to service objects I came across a number of
ways of setting a line item's currency. In `OrderContents`, the currency,
although an attribute of the line item, is passed to the line item
through an options Hash designed for an extension point introduced around
Spree 2.2/2.3 while it could just be assigned directly like any other
attribute.

This PR refactors that bit of weird code, hopefully relieving us of the
`options=` method on line item entirely in the future.
@Senjai
Copy link
Copy Markdown
Contributor

Senjai commented Mar 18, 2016

👍

@mamhoff
Copy link
Copy Markdown
Contributor Author

mamhoff commented Mar 21, 2016

Also closing this in favour of #998. We don't even need this attribute, and it only creates confusion.

@mamhoff mamhoff closed this Mar 21, 2016
@mamhoff mamhoff deleted the assign-currency-as-attribute-not-option 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