[MIG] stock_account_operating_unit: Migration to 17.0 #725
Conversation
Added Test Cases. Improved code for test cases Migrated Valuation method of quants. Migrated Valuation method _account_entry_move of quants. Completed test cases and modified valuation method
Currently translated at 100.0% (3 of 3 strings) Translation: operating-unit-13.0/operating-unit-13.0-stock_account_operating_unit Translate-URL: https://translation.odoo-community.org/projects/operating-unit-13-0/operating-unit-13-0-stock_account_operating_unit/es/
FIX test-requirements
…it is not longer required
BT-dherreros
force-pushed
the
17.0-mig-stock-account-operating-unit
branch
from
e6fc13f to
fb9abce
Compare
| @@ -0,0 +1,2 @@ | |||
| # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl.html). | |||
BT-dherreros
force-pushed
the
17.0-mig-stock-account-operating-unit
branch
from
fb9abce to
3938389
Compare
|
This PR has the |
I had another functional look at this code and it seems to be creating 2 journal entries, one that seems to be correct and comes form Odoo's core and the other one coming from this migrated module with an incorrect ref field. Could you confirm this? maybe this migration is not needed anymore |
There was a problem hiding this comment.
Yes. I think it is a past improvement done in a previous version:
Create stock moves between internal locations within the same company, but
different OU’s. The journal entries are created and they are self-balanced
within the OU when the journal entries are posted
This may be creating double JE when the products are actually moved out of the company. In my opinion, it is best to remove the action_done method, or at least, check if the products are move between internal locations of different OUs for doing the change.
I've some issues with moving products between different OU. That is why I have forbidden this action in my instances and I force to move first out of the company from OU1 and then move into the company using OU2. So, I am ok with removing the overwrite in the action_done method of stock.move
BT-dherreros
force-pushed
the
17.0-mig-stock-account-operating-unit
branch
from
3938389 to
238681a
Compare
BT-dherreros
force-pushed
the
17.0-mig-stock-account-operating-unit
branch
from
238681a to
fbe0c16
Compare
|
@AaronHForgeFlow I have removed the action_done inherited method. I also adapted the unit tests that were failing. Do you think my changes to the tests make sense? To me it does of course, as the balances must change with different amount of moves. |
There was a problem hiding this comment.
Thank you for attending the comments @BT-dherreros I still have some doubts in regards inter OU transfers. Can you please give your opinion?
| # GL account ‘Inventory’ has balance 0 on OU main_operating_unit | ||
| expected_balance = 0.0 | ||
| # GL account ‘Inventory’ has balance 1 on OU main_operating_unit | ||
| expected_balance = 1.0 | ||
| self._check_account_balance( | ||
| self.account_inventory.id, | ||
| operating_unit=self.ou1, | ||
| expected_balance=expected_balance, | ||
| ) | ||
| # GL account ‘Inventory’ has balance 2 on OU b2c |
There was a problem hiding this comment.
In my opinion the comment is correct, it should be 1 (the previous reception) + 1 (this internal transfer).
So the removal of the action_done method probably made this to have balance 1 in b2c instead of 2. Perhaps we should not remove the method but check we are not moving from in/out the company to avoid the double posting. So the override of the action_done method would be only for this inter-OU transfers between internal locations. What do you think?
| operating_unit=self.ou1, | ||
| expected_balance=expected_balance, | ||
| ) | ||
| # GL account ‘Goods Received Not Invoiced’ has balance 0 on OU b2c |
No description provided.