From a01c6d478eaa95e8afc1ad0f8bff83b8776cccdb Mon Sep 17 00:00:00 2001 From: Stefan Rijnhart Date: Tue, 11 Oct 2022 21:27:36 +0200 Subject: [PATCH] [FIX] upgrade_analysis: misleading representation of read/write computed fields Odoo 14 introduced the widescale usage of computed fields with readonly=False. In that case, the compute method functions as a default that can also be used to compute a value some time *after* the initial creation of the record. In the OpenUpgrade analysis files, these fields would be misrepresented as computed fields rather than fields with a default function. This change fixes that. --- upgrade_analysis/compare.py | 23 ++--- upgrade_analysis/models/upgrade_record.py | 1 - upgrade_analysis/upgrade_log.py | 95 +++++++++++-------- .../wizards/upgrade_install_wizard.py | 10 +- 4 files changed, 72 insertions(+), 57 deletions(-) diff --git a/upgrade_analysis/compare.py b/upgrade_analysis/compare.py index 0a6f9ebb9..e43150af6 100644 --- a/upgrade_analysis/compare.py +++ b/upgrade_analysis/compare.py @@ -134,8 +134,6 @@ def report_generic(new, old, attrs, reprs): if attr == "required": if old[attr] != new["required"] and new["required"]: text = "now required" - if new["req_default"]: - text += ", req_default: %s" % new["req_default"] fieldprint(old, new, "", text, reprs) elif attr == "stored": if old[attr] != new[attr]: @@ -284,15 +282,19 @@ def compare_sets(old_records, new_records): ], ) - printkeys = [ + # Info that is displayed for deleted fields + printkeys_old = [ "relation", "required", "selection_keys", - "req_default", "_inherits", "mode", "attachment", ] + # Info that is displayed for new fields + printkeys_new = printkeys_old + [ + "hasdefault", + ] for column in old_records: if column["field"] == "_order": continue @@ -304,7 +306,7 @@ def compare_sets(old_records, new_records): extra_message = ", ".join( [ k + ": " + str(column[k]) if k != str(column[k]) else k - for k in printkeys + for k in printkeys_old if column[k] ] ) @@ -312,11 +314,6 @@ def compare_sets(old_records, new_records): extra_message = " " + extra_message fieldprint(column, "", "", "DEL" + extra_message, reprs) - printkeys.extend( - [ - "hasdefault", - ] - ) for column in new_records: if column["field"] == "_order": continue @@ -325,13 +322,13 @@ def compare_sets(old_records, new_records): continue if column["mode"] == "create": column["mode"] = "" - printkeys_plus = printkeys.copy() + printkeys = printkeys_new.copy() if column["isfunction"] or column["isrelated"]: - printkeys_plus.extend(["isfunction", "isrelated", "stored"]) + printkeys.extend(["isfunction", "isrelated", "stored"]) extra_message = ", ".join( [ k + ": " + str(column[k]) if k != str(column[k]) else k - for k in printkeys_plus + for k in printkeys if column[k] ] ) diff --git a/upgrade_analysis/models/upgrade_record.py b/upgrade_analysis/models/upgrade_record.py index b8886fc61..f62083036 100644 --- a/upgrade_analysis/models/upgrade_record.py +++ b/upgrade_analysis/models/upgrade_record.py @@ -104,7 +104,6 @@ class UpgradeRecord(models.Model): "required", "stored", "selection_keys", - "req_default", "hasdefault", "table", "_inherits", diff --git a/upgrade_analysis/upgrade_log.py b/upgrade_analysis/upgrade_log.py index bfab4e55b..429c33f19 100644 --- a/upgrade_analysis/upgrade_log.py +++ b/upgrade_analysis/upgrade_log.py @@ -74,33 +74,56 @@ def compare_registries(cr, module, registry, local_registry): old_field[key] = value -def isfunction(model, k): +def hasdefault(field): + """Return a representation of the field's default method. + + The default method is only meaningful if the field is a regular read/write + field with a `default` method or a `compute` method. + + Note that Odoo fields accept a literal value as a `default` attribute + this value is wrapped in a lambda expression in odoo/fields.py: + https://github.com/odoo/odoo/blob/7eeba9d/odoo/fields.py#L484-L487 + """ if ( - model._fields[k].compute - and not model._fields[k].related - and not model._fields[k].company_dependent + not field.readonly # It's not a proper computed field + and not field.inverse # It's not a field that delegates their data + and not isrelated(field) # It's not an (unstored) related field. + ): + if field.default: + return "default" + if field.compute: + return "compute" + return "" + + +def isfunction(field): + if ( + field.compute + and (field.readonly or field.inverse) + and not field.related + and not field.company_dependent ): return "function" return "" -def isproperty(model, k): - if model._fields[k].company_dependent: +def isproperty(field): + if field.company_dependent: return "property" return "" -def isrelated(model, k): - if model._fields[k].related: +def isrelated(field): + if field.related: return "related" return "" -def _get_relation(v): - if v.type in ("many2many", "many2one", "one2many"): - return v.comodel_name - elif v.type == "many2one_reference": - return v.model_field +def _get_relation(field): + if field.type in ("many2many", "many2one", "one2many"): + return field.comodel_name + elif field.type == "many2one_reference": + return field.model_field else: return "" @@ -125,41 +148,31 @@ def log_model(model, local_registry): if model._inherits: model_registry["_inherits"] = {"_inherits": str(model._inherits)} model_registry["_order"] = {"_order": model._order} - for k, v in model._fields.items(): + for fieldname, field in model._fields.items(): properties = { - "type": typemap.get(v.type, v.type), - "isfunction": isfunction(model, k), - "isproperty": isproperty(model, k), - "isrelated": isrelated(model, k), - "relation": _get_relation(v), - "table": v.relation if v.type == "many2many" else "", - "required": v.required and "required" or "", - "stored": v.store and "stored" or "", + "type": typemap.get(field.type, field.type), + "isfunction": isfunction(field), + "isproperty": isproperty(field), + "isrelated": isrelated(field), + "relation": _get_relation(field), + "table": field.relation if field.type == "many2many" else "", + "required": field.required and "required" or "", + "stored": field.store and "stored" or "", "selection_keys": "", - "req_default": "", - "hasdefault": model._fields[k].default and "hasdefault" or "", + "hasdefault": hasdefault(field), } - if v.type == "selection": - if isinstance(v.selection, (tuple, list)): - properties["selection_keys"] = str(sorted(x[0] for x in v.selection)) + if field.type == "selection": + if isinstance(field.selection, (tuple, list)): + properties["selection_keys"] = str( + sorted(x[0] for x in field.selection) + ) else: properties["selection_keys"] = "function" - elif v.type == "binary": - properties["attachment"] = str(getattr(v, "attachment", False)) - default = model._fields[k].default - if v.required and default: - if ( - callable(default) - or isinstance(default, str) - and getattr(model._fields[k], default, False) - and callable(getattr(model._fields[k], default)) - ): - properties["req_default"] = "function" - else: - properties["req_default"] = str(default) + elif field.type == "binary": + properties["attachment"] = str(getattr(field, "attachment", False)) for key, value in properties.items(): if value: - model_registry.setdefault(k, {})[key] = value + model_registry.setdefault(fieldname, {})[key] = value def log_xml_id(cr, module, xml_id): diff --git a/upgrade_analysis/wizards/upgrade_install_wizard.py b/upgrade_analysis/wizards/upgrade_install_wizard.py index da28996fa..295f4b23b 100644 --- a/upgrade_analysis/wizards/upgrade_install_wizard.py +++ b/upgrade_analysis/wizards/upgrade_install_wizard.py @@ -42,9 +42,15 @@ class UpgradeInstallWizard(models.TransientModel): modules = self.env["ir.module.module"].search(domain) for start_pattern in BLACKLIST_MODULES_STARTS_WITH: - modules = modules.filtered(lambda x: not x.name.startswith(start_pattern)) + modules = modules.filtered( + lambda x, start_pattern=start_pattern: not x.name.startswith( + start_pattern + ) + ) for end_pattern in BLACKLIST_MODULES_ENDS_WITH: - modules = modules.filtered(lambda x: not x.name.endswith(end_pattern)) + modules = modules.filtered( + lambda x, end_pattern=end_pattern: not x.name.endswith(end_pattern) + ) return [("id", "in", modules.ids)] @api.depends("module_ids")