[4710][IMP] stock_picking_accounting_date: Get currency rate based on accounting date #148
Conversation
| and self.accounting_date | ||
| and not self.origin_returned_move_id | ||
| and self.purchase_line_id | ||
| and self.product_id.id == self.purchase_line_id.product_id.id |
There was a problem hiding this comment.
Why do we need this condition?
| def _get_price_unit(self): | ||
| """Set date for convert price unit multi currency.""" | ||
| self.ensure_one() | ||
| price_unit = super()._get_price_unit() | ||
| if ( | ||
| hasattr(self, "purchase_line_id") | ||
| and self.accounting_date | ||
| and not self.origin_returned_move_id | ||
| and self.purchase_line_id | ||
| and self.product_id.id == self.purchase_line_id.product_id.id | ||
| ): | ||
| line = self.purchase_line_id | ||
| order = line.order_id | ||
| price_unit = line.price_unit | ||
| if order.currency_id != order.company_id.currency_id: | ||
| price_unit = order.currency_id._convert( | ||
| price_unit, | ||
| order.company_id.currency_id, | ||
| order.company_id, | ||
| self.accounting_date, | ||
| round=False, | ||
| ) | ||
| return price_unit |
There was a problem hiding this comment.
As we just talked, let's do something like this, and extend _convert() of the currency to use the date from the context if available.
| def _get_price_unit(self): | |
| """Set date for convert price unit multi currency.""" | |
| self.ensure_one() | |
| price_unit = super()._get_price_unit() | |
| if ( | |
| hasattr(self, "purchase_line_id") | |
| and self.accounting_date | |
| and not self.origin_returned_move_id | |
| and self.purchase_line_id | |
| and self.product_id.id == self.purchase_line_id.product_id.id | |
| ): | |
| line = self.purchase_line_id | |
| order = line.order_id | |
| price_unit = line.price_unit | |
| if order.currency_id != order.company_id.currency_id: | |
| price_unit = order.currency_id._convert( | |
| price_unit, | |
| order.company_id.currency_id, | |
| order.company_id, | |
| self.accounting_date, | |
| round=False, | |
| ) | |
| return price_unit | |
| def _get_price_unit(self): | |
| self.ensure_one() | |
| if self.accounting_date: | |
| self = self.with_context(accounting_date=self.accounting_date) | |
| return super()._get_price_unit() |
There was a problem hiding this comment.
I believe we don't need to add if self.accounting_date: because accounting_date will always have value.
I didn't add the tests case at the moment because I have no idea how to test conversion without |
There was a problem hiding this comment.
Code review.
@AungKoKoLin1997 Please do the same in the OCA and for THC and FAL.
@kanda999 Please test and make sure it works, and take care of the rest in the coming week.
| @@ -46,3 +46,9 @@ def _prepare_account_move_vals( | |||
| if self.accounting_date: | |||
| am_vals.update({"date": self.accounting_date}) | |||
| return am_vals | |||
|
|
|||
| def _get_price_unit(self): | |||
| """Set date for convert price unit multi currency.""" | |||
There was a problem hiding this comment.
| """Set date for convert price unit multi currency.""" | |
| """Passes the accounting_date to be used in currency conversion for receipts in foreign currency purchases.""" |
|
|
||
| def _get_price_unit(self): | ||
| """Passes the accounting_date to be used in currency conversion for receipts | ||
| in foreign currency purchases.""" |
There was a problem hiding this comment.
| in foreign currency purchases.""" | |
| in foreign currency purchases. | |
| """ |
https://peps.python.org/pep-0257/
Unless the entire docstring fits on a line, place the closing quotes on a line by themselves. This way, Emacs’ fill-paragraph command can be used on it.
|
@yostashiro I just noticed we don't have the adjustment that we did in thc-oca for axls-oca repo. |
Thanks for noticing that. Let's do those in the same PRs with different commits. |
AungKoKoLin1997
force-pushed
the
16.0-imp-stock_picking_accounting_date
branch
from
f7a10d8 to
061687f
Compare
|
@yostashiro I overlooked this part. axls-oca/stock_picking_accounting_date/models/stock_move.py Lines 42 to 45 in 4085c38 We don't need to add anything. |
4710