Line item price modifier adjustments

[mamhoff]

When looking deeply at the line item, I found out that the price of a line item need not be the price of its variant. A number of things can come inbetween: Currencies, Price modifiers from the variant, or hard-setting a line item's price. In a large way, that's also how the test suite is structured: The line item factory makes line items worth 10, while the associated variant is 19.99.

That's very flexible, but a little unfortunate for the upcoming MOSS refactoring. For MOSS, prices have to change depending on the order's tax address. Previously, we helped ourselves by using the variant's price as the single source of truth. However, the variant's price is not necessarily the right price for your already price-adjusted line item.

What do you think about the following idea: We introduce price modifier adjustments. They can be added to a line item the same way a promotion or tax adjustment is added, and their sum is then added to the line item's price. I'll try to illustrate this in pseudo-ruby:

class Adjustment
  scope :price_modifier, -> { where(price_modifier: true) }
end

class LineItem 
  def price
    super + price_modifier_total
  end
end

VAT rates will have to make price modifications, so they can be the adjustment's source. I think the model would also quite nicely fit the existing, variant-based price modifiers: The source is the variant, and the tax stuff just wouldn't touch those adjustments.

I hope I've explained myself well here. The nice thing about it would be that we could adjust and re-adjust line item prices in a transparent, reproducible way. The not-so-nice-thing is that it would introduce yet another complex moving piece, but if we have to do it, let's at least do it in a way that mirrors other complex moving pieces.

What do you think?

[cbrunsdon]

This has been discussed in the past, but I do not think we should be doing taxes with adjustments.

Adjustments have been a "kitchen sink catch-all" for modifying prices and amounts, and they almost always add more complexity to the code than they save in the data model.

I would prefer we be able to model out all taxation than try lumping them into a more generic "adjustable" class.

Honestly this one is probably worth jumping on a hangout for with anyone that has an opinion. (after black friday though, when more of our lives are sane)

[mamhoff]

I'm in the process of writing requirements in RSpec for all of this, so that's a step. The problem with MOSS is that it requires the price of the line item to change before we apply taxes. So it's a two-step thingy: First get the price right for the area you're sending to, then calculate promos and then the taxes that actually apply.

For this to work though, I need some place where I can obtain the original price of the line item. It can't be the variant (even though that would make most sense). It could also be yet another column on the line item (initial_price?).

[cbrunsdon]

Putting on the variant is almost definitely wrong, and would have a bunch of historical data problems (as it would actually have to be on the Spree::Price and not the variant). We're also hoping to make a large number of changes to allow for more dynamic pricing which wouldn't make sense on the variant.

Putting the initial_price on line items makes much more sense to me and something I definitely support.

[mamhoff]

For the time being, I'll go with an initial_price column, but if a framework for modifying prices shows up that can be used for this, I'd be more than happy to incorporate it. Thank @cbrunsdon for the feedback!