StockRecord: rename price_excl_tax

[maiksprenger]

The docstring for price_excl_tax already explains what it truly is: the base price for tax calculations. I've had two projects where we knew the tax-inclusive price, and only calculated the price excl tax. I'd argue that a stock record is merely a record of a base price, and only the pricing policy gives it any meaning in regards to tax, so the field name should reflect that.
I realize that the field is probably used directly more often than it should (instead of through pricing strategies), so we need a good idea about being friendly as far as backwards-compatibility is concerned.

[mvantellingen]

Yeah I agree! 👍 All projects i've worked on communicate the tax-inclusive price and we need to calculate the excl price. This is required for prices like 19.99 where going from excl to incl results in 20.00 which isn't what the client wants :-)

[maiksprenger]

If that's the case, we should probably ship some sample strategies to show how to work with this as well. Note to self: when doing this, also look at potential off-by-one bugs when calculating line and basket totals.

[maiksprenger]

This issue actually contained two issues, renaming a field and deleting two. As I found more candidates for deletion, I moved that part of this issue into #1968

[sasha0]

@maikhoepfel @mvantellingen how about renaming into base_price? What about backward compatibility?

[maerteijn]

I think you should be careful with renaming this field. Having the price excluding tax and calculate the price including tax yourselves make more sense than the other way around (Especially if you have international shops).

However, I agree that the StockRecord should supply a base price and that the pricing policy gives a meaning to it, so it should not named like this.

I would say to deprecate the price_excl_tax field for the next release (1.2?) and rename it to a more sane name in the release after?

[john-parton]

@sasha0 Backwards compatibility could be tricky. Initially, I thought we could define a price_excl_tax property, but that would only handle a few cases. Basically any code that references price_excl_tax using the ORM would break.

[sasha0]

After research and observation, I came to conclusion it does make sense to rename price_excl_tax into price but as @john-parton mentioned before, there's no chance to make this change backwards compatible which drastically reduces motivation to apply this change.

I also totally agree with @maerteijn that we'd firstly deprecate this field and rename in the next major release (2.0).

[devkral]

nice to hear that, I had to disable the tax calculation because of it. The disadvantages I see is to deal with multiple countries with different taxes and in b2b pricing.
I may would propose the ability to limit stockrecords to countries.

[rollue]

The doc in the partner apps abstract_models.py says fields like price_excl_tax will be deprecated in 2.1. But it seems to stay the same.

Just a friendly reminder out of curiosity.

[solarissmoke]

We forgot about the renaming of price_excl_tax - will definitely do this before the next release.