-
-
Notifications
You must be signed in to change notification settings - Fork 699
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] mail: migration to 17.0 #4431
Conversation
805a748
to
cd8ea66
Compare
/ocabot migration mail |
cd8ea66
to
9ed931f
Compare
I need to send suggestion to this PR. I need remove some views. What is the best way to send it?. I need to test previously in my own branch. This is the reason because I send PR to this branch |
Which view you want to remove and why? |
No, views are never needed to be removed manually in OU. |
The mail.mail_channel_view_kanban view have an element with attrs attribute that is forbbiden in 17.0. What is the correct way to solve this without remove it manually . . .if exists? |
Check my comment at #4431 (comment) If that doesn't happen, then the patch is not correctly done (or should be expanded) at |
|
||
domain = icp.get_param("mail.catchall.domain") | ||
if domain: | ||
alias_domain_id = openupgrade.logged_query( |
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 is not how logged_query
works, as it returns cr.rowcount
- though it will work accidentally here because the table is empty and we insert one record, so the created id is 1 and the count of inserted rows is 1 too.
So please fetch the result anyways to be sure this won't blow up when the table is nonempty for whatever reason.
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.
Sorry my bad. but this pr is no longer my burden to bear , at least until next year. You can continue my work if you want. Thanks
if company.alias_domain_id: | ||
openupgrade.logged_query( | ||
env.cr, | ||
f""" |
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.
why don't you just join with mail_alias_domain
here unconditionally, given mail_alias_domain
is filled above?
SET alias_incoming_local = True | ||
""", | ||
) | ||
openupgrade.logged_query( | ||
env.cr, | ||
""" | ||
UPDATE mail_alias | ||
SET alias_status = 'not_tested' |
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.
why not one query?
Further, I think we should assume that we get a correctly configured database and we migrate correct configurations correctly, so the alias status should be valid
in my opinion.
I will resume the work this week. |
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 @duong77476-viindoo for the time you spent on that one
I have completed my review if that can help on your work of takeover of this PR @pedrobaeza
mail / mail.gateway.allowed / email (char) : now required | ||
# DONE pre-migration: fill dummy value if Null found |
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 cannot find where this is filled in pre-migration ? I suppose we should leave it empty to be filled manually by an admin after migration.
"discuss.channel", | ||
"discuss_channel", |
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.
Should be
"discuss.channel", | |
"discuss_channel", | |
"mail.tracking.value", | |
"mail_tracking_value", |
|
||
mail / mail.template / report_template (many2one) : DEL relation: ir.actions.report | ||
mail / mail.template / report_template_ids (many2many): NEW relation: ir.actions.report | ||
# DONE pre-migraton: m2o to m2m |
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 pre-migraton: m2o to m2m | |
# DONE post-migraton: m2o to m2m |
Also, in pre-migration, you should probably copy the column so that it is not dropped during upgrade (as stated here : https://oca.github.io/openupgradelib/API.html#openupgradelib.openupgrade.m2o_to_x2m)
|
||
mail / mail.tracking.value / new_value_monetary (float) : DEL | ||
mail / mail.tracking.value / old_value_monetary (float) : DEL | ||
# TODO pre-migration: fill value into old/new_value_float |
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.
# TODO pre-migration: fill value into old/new_value_float | |
# DONE: pre-migration: fill value into old/new_value_float |
mail / res.users.settings / _order : module is now 'base' ('mail') | ||
mail / res.users.settings / display_name (char) : module is now 'base' ('mail') | ||
mail / res.users.settings / user_id (many2one) : module is now 'base' ('mail') | ||
# NOTHING TODO |
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.
# NOTHING TODO | |
# NOTHING TO DO: done in base migration scripts |
DEL ir.model.constraint: mail.constraint_bus_presence_partner_or_guest_exists | ||
DEL ir.model.constraint: mail.constraint_mail_activity_check_res_id_is_set | ||
DEL ir.model.constraint: mail.constraint_mail_alias_alias_unique | ||
DEL ir.model.constraint: mail.constraint_mail_blacklist_unique_email | ||
DEL ir.model.constraint: mail.constraint_mail_channel_channel_type_not_null | ||
DEL ir.model.constraint: mail.constraint_mail_channel_group_public_id_check | ||
DEL ir.model.constraint: mail.constraint_mail_channel_member_partner_or_guest_exists | ||
DEL ir.model.constraint: mail.constraint_mail_channel_rtc_session_channel_member_unique | ||
DEL ir.model.constraint: mail.constraint_mail_channel_uuid_unique | ||
DEL ir.model.constraint: mail.constraint_mail_followers_mail_followers_res_partner_res_model_id_uniq | ||
DEL ir.model.constraint: mail.constraint_mail_message_reaction_partner_or_guest_exists | ||
DEL ir.model.constraint: mail.constraint_mail_notification_notification_partner_required | ||
DEL ir.model.constraint: mail.constraint_res_users_notification_type | ||
DEL ir.model.constraint: mail.constraint_res_users_settings_unique_user_id [renamed to base module] | ||
DEL ir.model.constraint: mail.constraint_res_users_settings_volumes_partner_or_guest_exists |
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 am not sure where this list of constraints come from (I do not have the same list in upgrade_analysis.txt) and most of the ones listed here still exist, or have corresponding ones on renamed models.
From my point of view you should have :
NEW ir.model.constraint: mail.constraint_discuss_channel_channel_type_not_null
DEL ir.model.constraint: mail.constraint_mail_channel_channel_type_not_null
NEW ir.model.constraint: mail.constraint_discuss_channel_group_public_id_check
DEL ir.model.constraint: mail.constraint_mail_channel_group_public_id_check
NEW ir.model.constraint: mail.constraint_discuss_channel_member_partner_or_guest_exists
DEL ir.model.constraint: mail.constraint_mail_channel_member_partner_or_guest_exists
NEW ir.model.constraint: mail.constraint_discuss_channel_rtc_session_channel_member_unique
DEL ir.model.constraint: mail.constraint_mail_channel_rtc_session_channel_member_unique
NEW ir.model.constraint: mail.constraint_discuss_channel_uuid_unique
DEL ir.model.constraint: mail.constraint_mail_channel_uuid_unique
# NOTHING TO DO: handled by model rename from pre-commit
NEW ir.model.constraint: mail.constraint_discuss_gif_favorite_user_gif_favorite
NEW ir.model.constraint: mail.constraint_mail_alias_domain_bounce_email_uniques
NEW ir.model.constraint: mail.constraint_mail_alias_domain_catchall_email_uniques
NEW ir.model.constraint: mail.constraint_mail_partner_device_endpoint_unique
# NOTHING TO DO: will be created by ORM
DEL ir.model.constraint: mail.constraint_mail_alias_alias_unique
# TODO: this one needs to be safely removed in post-migration using openupgrade.delete_sql_constraint_safely
DEL ir.model.constraint: mail.constraint_res_users_settings_unique_user_id [renamed to base module]
# NOTHING TO DO: done in base module
DEL ir.rule: mail.ir_rule_mail_channel_member_group_user (noupdate) | ||
DEL ir.rule: mail.mail_channel_admin (noupdate) | ||
DEL ir.rule: mail.mail_channel_rule (noupdate) | ||
# NOTHING TO DO: removed in post-migration |
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.
# NOTHING TO DO: removed in post-migration | |
# DONE: post-migration: safe removal |
|
||
DEL mail.channel: mail.channel_all_employees (noupdate) | ||
DEL mail.channel.member: mail.channel_member_general_channel_for_admin (noupdate) | ||
# NOTHING TO DO: removed in post-migration |
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.
# NOTHING TO DO: removed in post-migration | |
# DONE: post-migration: safe removal |
_tables_renames = [ | ||
("mail_channel", "discuss_channel"), | ||
("mail_channel_member", "discuss_channel_member"), | ||
("mail_channel_rtc_session", "discuss_channel_rtc_session"), | ||
] |
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.
You should also rename mail_channel_res_groups_rel table to discuss_channel_res_groups_rel
And column mail_channel_id to discuss_channel_id in that table.
Hello, as usual, odoo-module-diff may help you write or check the scripts. Here are 14 key commits for the the mail migration to v17: https://github.com/akretion/odoo-module-diff-analysis/tree/main/17.0/mail |
superseded by #4600 @remi-filament thank for your very thorough review, I took nearly all of your comments and add comments on the new PR where I deviated |
No description provided.