[16.0][FIX] upgrade_analysis: blacklist + no reinstall + UI tweaks #2739

pedrobaeza · 2023-10-22T16:17:56Z

Several patches for problems found when performing a OU analysis:

[FIX] Add to blacklist deprecated modules

Odoo has deprecated such modules with a pre-hook raising an error, so they are not installable anymore.

This way, we avoid them to be included in the Install Modules Wizard.
[IMP] Don't reinstall installed modules

If a module is already installed, there's no need of reinstalling it.
[FIX] Proper UI in forms

The colspan property of some UI elements were not correctly adjusted to the v16 sytem.

This commits fixes it.

@Tecnativa

OCA-git-bot · 2023-10-22T16:18:00Z

Hi @StefanRijnhart, @legalsylvain,
some modules you are maintaining are being modified, check this out!

legalsylvain · 2023-10-22T17:26:51Z

hi @pedrobaeza. Thanks for working on this module.

[FIX] Add to blacklist deprecated modules

👍

[FIX] Proper UI in forms

👍

[IMP] Don't reinstall installed modules. If a module is already installed, there's no need of reinstalling it.

I don't use this module very often, so I don't have a clear point of view. But I remember that the analysis was performed when modules already installed were reinstalled. (but I don't know why).

I'm not available for the next two weeks. I can look at this topic during OCA days in a little more detail.

@StefanRijnhart perhaps you have an opinion on the subject?

pedrobaeza · 2023-10-22T17:30:03Z

That part is not altered. The reinstallation takes place the same:

server-tools/upgrade_analysis/wizards/upgrade_generate_record_wizard.py

Line 41 in 3a53051

self.env["ir.module.module"].search([("state", "=", "installed")]).write(

This is only for avoiding to reinstall on the first wizard.

StefanRijnhart · 2023-12-24T11:36:47Z

From https://github.com/OCA/server-tools/blob/15.0/upgrade_analysis/upgrade_log.py#L192-L197 I gather that not reinstalling all modules at this point might give different results if the code was updated in the mean time and new data records were added. This is a common situation in the timeframe surrounding a new Odoo release, so it's not completely safe. Maybe this can be made into an option defaulting to 'reinstall already installed modules'?

pedrobaeza · 2023-12-27T09:59:07Z

Are you sure that statement is still applying? I did the analysis for 16.0, and didn't notice any problem. Anyway, it was just an improvement, so it's not the main fix at all, and I can remove it.

legalsylvain

[FIX] Add to blacklist deprecated modules

LGTM, but I wonder this list should be better in the apriori.py file of the openupgrade project. (to have all the merged / renamed / deprecated changes in the same place).

[IMP] Don't reinstall installed modules

Not sure to understand. See inline.

[FIX] Proper UI in forms

👍

legalsylvain · 2023-12-27T13:31:39Z

upgrade_analysis/blacklist.py

@@ -1,4 +1,9 @@
-BLACKLIST_MODULES = []
+BLACKLIST_MODULES = [
+    "payment_alipay",


Hi. Naive question. How do you know this module is deprecated. it is installable in V16. the only way to find the information is to read the description of the manifest.

https://github.com/odoo/odoo/blob/16.0/addons/payment_alipay/__manifest__.py#L8

Yes, @legalsylvain, I found it by the hard way, when errors arose, as @hbrunn found out in #3086

legalsylvain · 2023-12-27T16:39:12Z

upgrade_analysis/wizards/upgrade_install_wizard.py

@@ -34,7 +34,7 @@ class UpgradeInstallWizard(models.TransientModel):
    def _module_ids_domain(self, extra_domain=None):
        domain = [
            "&",
-            ("state", "not in", ["uninstallable", "unknown"]),
+            ("state", "not in", ["installed", "uninstallable", "unknown"]),


I'm not sure to understand. how do you run the analysis of the base module, that is allways installed.

This is for reinstalling things, so no need to reinstall what is already installed.

StefanRijnhart · 2023-12-28T08:39:00Z

Are you sure that statement is still applying? I did the analysis for 16.0, and didn't notice any problem. Anyway, it was just an improvement, so it's not the main fix at all, and I can remove it.

This is a valid question. If it is safe to perform the analysis without a reinstall of installed modules (even if new xml records were added to these installed modules in the mean time), it should also be safe to skip this step entirely in the analysis process. We can then generate an analysis by only installing each module once on a fresh database.

This is also easily testable: does a one step analysis and a two step analysis both yield equivalent results?

hbrunn

I've scripted this for myself as creating all databases from scratch for all analysis runs, so I can't really say something about the reinstall question.

pedrobaeza · 2024-10-21T07:49:18Z

@StefanRijnhart

This is also easily testable: does a one step analysis and a two step analysis both yield equivalent results?

Yes, I ran it and got the same results, that were reflected in OCA/OpenUpgrade#4180.

upgrade_analysis/tests/test_module.py

Odoo has deprecated such modules with a pre-hook raising an error, so they are not installable anymore. This way, we avoid them to be included in the Install Modules Wizard.

If a module is already installed, there's no need of reinstalling it.

The colspan property of some UI elements were not correctly adjusted to the v16 sytem. This commits fixes it.

StefanRijnhart · 2024-10-22T06:56:54Z

/ocabot merge minor

OCA-git-bot · 2024-10-22T06:57:01Z

This PR looks fantastic, let's merge it!
Prepared branch 16.0-ocabot-merge-pr-2739-by-StefanRijnhart-bump-minor, awaiting test results.

Signed-off-by StefanRijnhart

StefanRijnhart · 2024-10-22T06:58:10Z

Thanks! The commit to not reinstall modules would have to be ported to17.0

OCA-git-bot · 2024-10-22T07:04:00Z

@StefanRijnhart your merge command was aborted due to failed check(s), which you can inspect on this commit of 16.0-ocabot-merge-pr-2739-by-StefanRijnhart-bump-minor.

After fixing the problem, you can re-issue a merge command. Please refrain from merging manually as it will most probably make the target branch red.

pedrobaeza · 2024-10-22T07:07:43Z

This is #3066 hitting

I'm going to deactivate such harmful test.

pedrobaeza added this to the 16.0 milestone Oct 22, 2023

pedrobaeza requested a review from legalsylvain October 22, 2023 16:17

pedrobaeza force-pushed the 16.0-fix-upgrade_analysis-ui_deprecated_modules branch 2 times, most recently from 60bf9de to 6d14f56 Compare October 22, 2023 16:42

pedrobaeza force-pushed the 16.0-fix-upgrade_analysis-ui_deprecated_modules branch from 6d14f56 to 899d597 Compare October 22, 2023 18:04

legalsylvain reviewed Dec 27, 2023

View reviewed changes

StefanRijnhart mentioned this pull request Feb 2, 2024

[17.0][MIG] upgrade_analysis #2826

Merged

pedrobaeza mentioned this pull request Oct 21, 2024

[16.0][ADD] upgrade_analysis: add blacklist from v17 #3086

Closed

hbrunn approved these changes Oct 21, 2024

View reviewed changes

StefanRijnhart reviewed Oct 21, 2024

View reviewed changes

upgrade_analysis/tests/test_module.py Outdated Show resolved Hide resolved

pedrobaeza added 3 commits October 22, 2024 08:46

[FIX] upgrade_analysis: Add to blacklist deprecated modules

709a50c

Odoo has deprecated such modules with a pre-hook raising an error, so they are not installable anymore. This way, we avoid them to be included in the Install Modules Wizard.

[IMP] upgrade_analysis: Don't reinstall installed modules

b400590

If a module is already installed, there's no need of reinstalling it.

[FIX] upgrade_analysis: Proper UI in forms

673bbfa

The colspan property of some UI elements were not correctly adjusted to the v16 sytem. This commits fixes it.

pedrobaeza force-pushed the 16.0-fix-upgrade_analysis-ui_deprecated_modules branch from 899d597 to 673bbfa Compare October 22, 2024 06:47

StefanRijnhart approved these changes Oct 22, 2024

View reviewed changes

OCA-git-bot added the approved label Oct 22, 2024

OCA-git-bot added a commit that referenced this pull request Oct 22, 2024

Merge PR #2739 into 16.0

48aaa9e

Signed-off-by StefanRijnhart

OCA-git-bot added the bot is merging ⏳ label Oct 22, 2024

OCA-git-bot removed the bot is merging ⏳ label Oct 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][FIX] upgrade_analysis: blacklist + no reinstall + UI tweaks #2739

[16.0][FIX] upgrade_analysis: blacklist + no reinstall + UI tweaks #2739

pedrobaeza commented Oct 22, 2023

OCA-git-bot commented Oct 22, 2023

legalsylvain commented Oct 22, 2023 •

edited

Loading

pedrobaeza commented Oct 22, 2023 •

edited

Loading

StefanRijnhart commented Dec 24, 2023

pedrobaeza commented Dec 27, 2023

legalsylvain left a comment

legalsylvain Dec 27, 2023

pedrobaeza Oct 21, 2024

legalsylvain Dec 27, 2023

pedrobaeza Oct 21, 2024

StefanRijnhart commented Dec 28, 2023

hbrunn left a comment

pedrobaeza commented Oct 21, 2024

StefanRijnhart commented Oct 22, 2024

OCA-git-bot commented Oct 22, 2024

StefanRijnhart commented Oct 22, 2024

OCA-git-bot commented Oct 22, 2024

pedrobaeza commented Oct 22, 2024

[16.0][FIX] upgrade_analysis: blacklist + no reinstall + UI tweaks #2739

Are you sure you want to change the base?

[16.0][FIX] upgrade_analysis: blacklist + no reinstall + UI tweaks #2739

Conversation

pedrobaeza commented Oct 22, 2023

OCA-git-bot commented Oct 22, 2023

legalsylvain commented Oct 22, 2023 • edited Loading

pedrobaeza commented Oct 22, 2023 • edited Loading

StefanRijnhart commented Dec 24, 2023

pedrobaeza commented Dec 27, 2023

legalsylvain left a comment

Choose a reason for hiding this comment

legalsylvain Dec 27, 2023

Choose a reason for hiding this comment

pedrobaeza Oct 21, 2024

Choose a reason for hiding this comment

legalsylvain Dec 27, 2023

Choose a reason for hiding this comment

pedrobaeza Oct 21, 2024

Choose a reason for hiding this comment

StefanRijnhart commented Dec 28, 2023

hbrunn left a comment

Choose a reason for hiding this comment

pedrobaeza commented Oct 21, 2024

StefanRijnhart commented Oct 22, 2024

OCA-git-bot commented Oct 22, 2024

StefanRijnhart commented Oct 22, 2024

OCA-git-bot commented Oct 22, 2024

pedrobaeza commented Oct 22, 2024

legalsylvain commented Oct 22, 2023 •

edited

Loading

pedrobaeza commented Oct 22, 2023 •

edited

Loading