-
-
Notifications
You must be signed in to change notification settings - Fork 695
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[11.0][MIG]stock_inventory_revaluation #881
[11.0][MIG]stock_inventory_revaluation #881
Conversation
89d5ef1
to
f4d61cf
Compare
I don't understand how this PR passes the tests. When I run the tests locally, they all fail. Doesn't the runbot perform tests? For example:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested for an AVCO structure.
for revaluation in self: | ||
for reval_quant in revaluation.reval_quant_ids: | ||
reval_quant.quant_id.sudo().write( | ||
{'cost': reval_quant.old_cost}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This only works for realtime inventory valuation? If inventory valuation is average shouldn't the cost price be reset?
|
||
if revaluation.revaluation_type == 'inventory_value': | ||
if float_compare(revaluation.new_value, 0.0, | ||
precision_rounding=account_prec) < 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new_value can never be negative?
digits=dp.get_precision('Account'), | ||
readonly=True) | ||
|
||
new_value = fields.Float('Credit/Debit amount', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Onchange revaluation_type this should be reset to 0 to avoid incorrect warnings
compute="_compute_calc_current_cost", | ||
readonly=True) | ||
|
||
new_cost = fields.Float('New cost', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Onchange revaluation_type this should be reset to 0 to avoid incorrect warnings
'increase_account_id': increase_account_id, | ||
'decrease_account_id': decrease_account_id | ||
}) | ||
reval.button_post() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valuation can't be posted if increase_account_id or decrease_account_id is not set. The warning raised from the valuation is a bit confusing. Maybe improve the warning?
This module needs major refactoring, even for standard and average cost. For example, look at this comment in the stock_account module: Look at this PR (#894) for 12.0. The module should be working with |
…ventory revaluation, instead of directly posting the journal entries.
…proper traceability
… quants with real method
857f83c
to
cf8fe3e
Compare
cf8fe3e
to
21df7f7
Compare
Sorry not working on this anymore. just feel free to supersede in case you need this module. IMHO the version proposed by @matt454357 is the one needed in v11 too. |
Standard migration