-
Notifications
You must be signed in to change notification settings - Fork 2
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
[16.0][3825][IMP] stock_picking_accounting_date, stock_valuation_layer_accounting… #86
Conversation
This module adds an accounting date in both stock pickings and stock moves. | ||
The accounting date from the picking is propagated to its corresponding stock move. | ||
If a picking doesn't specify an accounting date, the stock move's accounting date | ||
will be set to the 'Effective Date'. This value is then passed to the SVL's journal entry\ |
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.
nitpick: what is the backslash at the end of the line about?
rec.accounting_date = fields.Datetime.context_timestamp( | ||
self, rec.create_date | ||
) | ||
rec.accounting_date = rec.stock_move_id.accounting_date |
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.
Shouldn't we still fall back on to rec.create_date
, in case there is a SVL record with no stock_move_id
and with account_move_id
which is in draft state (i.e. user has temporarily changed the state to draft). Otherwise there will be a server errror I think.
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.
Please add stock_picking_accounting_date
as a dependency of stock_valuation_layer_accounting_date.
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.
Code review. 👍
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.
LGTM
@@ -9,7 +9,9 @@ | |||
<field name="show_accounting_date" invisible="1" /> | |||
<field | |||
name="accounting_date" | |||
attrs="{'invisible': [('show_accounting_date', '=', False)]}" | |||
attrs="{'invisible': [('show_accounting_date', '=', False)], | |||
'readonly': [('show_accounting_date', '=', True), |
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.
@rinaldifirdaus Any reason to have this ('show_accounting_date', '=', True)
?
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.
If we don't define ('show_accounting_date', '=', True), the field with string consumable accounting date will also become readonly as far as i test.
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.
so the purpose is to make it readonly only when storable product on the transfers. the one with string "Accounting Date".
We don't need migration script.
Procedure to update :