-
-
Notifications
You must be signed in to change notification settings - Fork 785
[17.0][OU-ADD] analytic: migration to 17.0 #4458
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
[17.0][OU-ADD] analytic: migration to 17.0 #4458
Conversation
|
/ocabot migration analytic |
85b2268 to
c2a763b
Compare
|
Hi @duong77476-viindoo , First of all, thank you for the work you've done on this pull request. While testing in my environment, I encountered a small issue related to property creation in the The problem seemed to be that some properties already existed, causing an error when trying to recreate them. I suggest a solution to check if the property exists before creating it. The fix I applied is as follows: if vals_list:
for vals in vals_list:
property = env['ir.property'].search([('fields_id', '=', vals['fields_id']),
('company_id', '=', vals['company_id']),
('res_id', '=', vals['res_id'])])
if property:
property.write(vals)
else:
env["ir.property"].create(vals) This prevents recreating existing properties and only applies changes when necessary. I'm not sure of the best way to submit this modification, so I wanted to ask if I should create another pull request with these changes or if you'd prefer to integrate them directly into this one. Thanks again for your work and help! |
|
@acpMicrocom Thanks, i will apply your fix soon, i don't have time to test though because we stop migration campaign for now |
| } | ||
| ) | ||
| if vals_list: | ||
| env["ir.property"].create(vals_list) |
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.
I suggest to use ir.property#_set_multi here, this also takes care of @acpMicrocom's problem
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.
Yeah indeed hehe
|
@duong77476-viindoo I don't manage to push commits onto your branch, so I'll start superseding your PRs with new ones |
No description provided.