-
-
Notifications
You must be signed in to change notification settings - Fork 370
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
[18.0][MIG] account_analytic_spread_by_tag: Migration to 18.0 #712
base: 18.0
Are you sure you want to change the base?
Conversation
Currently translated at 100.0% (16 of 16 strings) Translation: account-analytic-17.0/account-analytic-17.0-account_analytic_spread_by_tag Translate-URL: https://translation.odoo-community.org/projects/account-analytic-17-0/account-analytic-17-0-account_analytic_spread_by_tag/it/
Hi @peluko00 , Please rebase and remove test-requirements.txt as the dependency has been merged. Thanks. |
ae2e449
to
d8cfa29
Compare
Done! |
{ | ||
"name": default_name, | ||
"date": self.date, | ||
"account_id": aac.id, |
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 approach doesn't work correctly because it always assigns the analytic account to the project plan defined in the settings:
self.env["ir.config_parameter"].sudo().get_param("analytic.project_plan", 0)
This retrieves the plan that should be in the account_id.
You need to determine whether the plan belongs in the account_id or in one of the dynamic x_plan{id}_id fields. These x_plan fields are autogenerated based on the analytic plans you have, identified by their ID.
To fix this, you can do the following:
"account_id": aac.id, | |
aac.root_plan_id._column_name(): aac.id, |
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.
Maybe @miquelalzanillas can review the changes and then open a FIX pr for this.
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.
Thanks for the review @BernatObrador, @miquelalzanillas what do you think about it?
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.
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.
Done, thanks! Can you review again @miquelalzanillas @BernatObrador please?
d8cfa29
to
a05b7e2
Compare
a05b7e2
to
67e842e
Compare
Module migrated to version 18.0
cc https://github.com/APSL 163686
@miquelalzanillas @javierobcn @mpascuall @BernatObrador @ppyczko please review
[Issue] Migration to version 18.0 #696
Depends on:
[account_analytic_tag] [18.0][MIG] account_analytic_tag: Migration to 18.0 #709