Merge lp://qastaging/~therp-nl/therp-backports/addons-6.1-lp754339_pricelist_on_invoice_line into lp://qastaging/therp-backports

Thanks, good on the whole. Got a couple of details:

l.8-12,33,66 To avoid conflicts with the official branch, prevent changes in whitespace.

l.20-25 For the same reason do not refactor

l.95 should say 'warning=None' but it looks like you can replace the argument by an initialisation in the method itself

l.55,94 remove price argument

l.96 So many arguments to this method, I would like to see it having a docstring explaining what they are.

l.123 can say "res[field]" instead of "eval('res.' + field)", even in the case of browse records

l.101 this is actually the default date anyway for currency rates and pricelist lookups so it does not need to be in the context

l.104-109 make translatable

Overall, naming of model pools is pretty random (_obj, obj_, _model). Can you make it a bit more consistent?

Thanks,
Stefan.

Oh, if you make l.104-109 translatable, you will need to pass the context.

Did not succeed in removing all whitespace changes. Anyway, they move the code a lot to pep8.

I did add the context to the _price_unit_get method, while dropping a lot of the other parameters.

Thanks, that is already a great improvement on the original patch.

For the remaining changes in whitespace, you can simply copy the originally spaced l.9-12 from an older version of the file and undo pep8ification of l.21-25.

l.98,128: the substitution variables are supposed to be outside of the call to _()

Shortcomings in price computation not solved by this merge are:
- The product_id_change method should have access to the invoice date. Because when retrieving
  prices, the invoice date should determine the prices in effect, instead of the current date.
- It should be possible to pass a 'target' currency to the price_get method of product_pricelist, to prevent the current double conversion taking place. Currency conversion should be done from pricelist currency to invoice currency, or no conversion when both are the same.

First of all, sorry for the delayed review.

As for l.9..13, I totally share the emotional argument for the refactoring but I want to avoid the practical risk of conflicts as much as possible. Any change in the line or in the line below will trigger a conflict. Note that I am currently looking into an automatic mechanism for updating the backports with the upstream branch, as suggested on the community mailing list. This is quite possible, but such a process necessarily stalls with every conflict it encounters. The necessary changes in the backported fixes cause conflicts enough and I am hoping not to have to intervene in this process for unnecessary conflicts too. I hope you understand.

Otherwise good.