-
-
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 #4600
Conversation
/ocabot migration mail Depends on :
|
""" | ||
UPDATE mail_gateway_allowed SET email='admin@example.com' | ||
WHERE email IS NULL" | ||
""" |
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.
@remi-filament I think we should fill this with a dummy value after all, otherwise Odoo won't be able to set up the not null constraint
DEL ir.model.access: mail.access_res_users_settings_volumes_all | ||
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 |
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.
@remi-filament I don't think we need to remove this manually - that's necessary when we do something during migration that would violate it, and afterwards it's deleted anyways
to discuss: |
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.
In general, the work file is very hard to be followed, due to not proper logical blocks grouping, and the models renaming that are not taken into account for it.
May you consider to re-run analysis to get a cleaner one and do the groupings (or at least this second one).
openupgrade_scripts/scripts/mail/17.0.1.15/upgrade_analysis_work.txt
Outdated
Show resolved
Hide resolved
openupgrade_scripts/scripts/mail/17.0.1.15/upgrade_analysis_work.txt
Outdated
Show resolved
Hide resolved
openupgrade_scripts/scripts/mail/17.0.1.15/upgrade_analysis_work.txt
Outdated
Show resolved
Hide resolved
openupgrade_scripts/scripts/mail/17.0.1.15/upgrade_analysis_work.txt
Outdated
Show resolved
Hide resolved
I don't think we should rename. The goal of OU is the preservation, and this is achieved not doing anything, as the column is preserved with |
@pedrobaeza thanks for your points, I'll pick them up next Monday. Or some member of @OCA/openupgrade-maintainers before, I suspect I couldn't push to the original PRs because they come from a fork of a fork, while my copy is forked directly from here. |
Co-Authored-By: Pedro M. Baeza <pedro.baeza@tecnativa.com>
I have pushed a rebased branch with my comments applied. I see that the new analysis doesn't contain the model renaming for shortening the analysis file, but I have shorten it myself. |
@pedrobaeza thanks for taking over, but I'm not sure I agree with shortening/reordering the work file. What I do currently to verify all analysis points are covered is How do you do that? Compare manually? |
I don't need to do anything with the In your PR https://github.com/OCA/OpenUpgrade/pull/4606/files, I already checked that there's nothing touched in |
#4431 with review followup