diff --git a/upgrade_analysis/models/upgrade_analysis.py b/upgrade_analysis/models/upgrade_analysis.py index 23b42785911..a8b747b5b64 100644 --- a/upgrade_analysis/models/upgrade_analysis.py +++ b/upgrade_analysis/models/upgrade_analysis.py @@ -314,9 +314,27 @@ def _get_node_value(element): return element.text return etree.tostring(element) + def _get_available_modules(self, module_name): + """Return the set of module names a given module may reference in its + data files: the module itself plus its (transitive) dependencies. + """ + module = self.env["ir.module.module"].search([("name", "=", module_name)]) + if not module: + return set() + upstream = module.upstream_dependencies( + exclude_states=("uninstalled", "uninstallable", "to remove") + ) + return {module_name} | set(upstream.mapped("name")) + def _get_xml_diff( - self, remote_update, remote_noupdate, local_update, local_noupdate + self, + remote_update, + remote_noupdate, + local_update, + local_noupdate, + local_module, ): + available_modules = self._get_available_modules(local_module) odoo = etree.Element("odoo") for xml_id in sorted(local_noupdate.keys()): local_record = local_noupdate[xml_id] @@ -355,12 +373,21 @@ def _get_xml_diff( # Does the field still exist? if record_remote_dict[key].tag == "field": field_name = remote_record.xpath(key)[0].attrib.get("name") + model_name = local_record.attrib["model"] if ( - local_record.attrib["model"] not in self.env - or field_name - not in self.env[local_record.attrib["model"]]._fields.keys() + model_name not in self.env + or field_name not in self.env[model_name]._fields ): continue + # When the record was moved to another module, its + # previous definition may set a field that is declared + # in a module the current one does not depend on (e.g. + # the field lives in a module that depends on this one). + # That field is still managed by its own module, so it + # must not be reset from here. + field = self.env[model_name]._fields[field_name] + if field._module and field._module not in available_modules: + continue # Overwrite an existing value with an empty one. attribs = deepcopy(record_remote_dict[key]).attrib for attr in ["eval", "ref"]: @@ -389,7 +416,23 @@ def _get_xml_diff( eval_constant = ast.unparse(ast.Constant(field.falsy_value)) if eval_constant != "''": attribs["eval"] = eval_constant - element.append(etree.Element(record_remote_dict[key].tag, attribs)) + new_element = etree.Element( + record_remote_dict[key].tag, attribs + ) + # Only emit the reset when the value a fresh install + # would give the field actually differs from what the + # previous version set. If they are equal, the + # (noupdate) record already holds the right value and + # needs no change. + if self._get_node_value(new_element) == self._get_node_value( + record_remote_dict[key] + ): + continue + element.append(new_element) + else: + element.append( + etree.Element(record_remote_dict[key].tag, attribs) + ) else: oldrepr = self._get_node_value(record_remote_dict[key]) newrepr = self._get_node_value(record_local_dict[key]) @@ -575,7 +618,11 @@ def generate_noupdate_changes( local_files = local_record_obj.get_xml_records(local_module) local_update, local_noupdate = self._parse_files(local_files, local_module) diff = self._get_xml_diff( - remote_update, remote_noupdate, local_update, local_noupdate + remote_update, + remote_noupdate, + local_update, + local_noupdate, + local_module, ) if diff: module = self.env["ir.module.module"].search( diff --git a/upgrade_analysis/tests/test_module.py b/upgrade_analysis/tests/test_module.py index f7cfd34cea3..4ba99e6b20e 100644 --- a/upgrade_analysis/tests/test_module.py +++ b/upgrade_analysis/tests/test_module.py @@ -150,6 +150,7 @@ def test_xml_comparison(self): """ ), }, + "upgrade_analysis", ) self.assertIn('', diff) self.assertIn('', diff) @@ -157,7 +158,19 @@ def test_xml_comparison(self): def test_analyze(self): """ Test a full analysis run. - For the time being, only xmlid related functionality is tested + For the time being, only xmlid related functionality is tested. + + The record is detected as renamed from ``other_module`` to + ``upgrade_analysis`` and the generated ``noupdate_changes.xml`` + exercises the discarded-field handling: + + - ``name``: value changed, so it is written; + - ``port``: dropped, and the value a fresh install gives (its default) + differs from the previous one, so a reset is written; + - ``username``: dropped, but the default equals the previous value, so + nothing is written; + - ``database``: dropped, but its field is (patched to be) declared in a + module the current one does not depend on, so it is left untouched. """ analysis = self.env["upgrade.analysis"].create( { @@ -235,7 +248,9 @@ def get_xml_records(self, module): Noupdate xmlid from other_module - some_server + + + some_database """ @@ -253,7 +268,6 @@ def local_get_xml_records(module): Noupdate xmlid from upgrade_analysis - some_server """ @@ -264,6 +278,10 @@ def local_get_xml_records(module): def write_file(module_name, version, content, filename="upgrade_analysis.txt"): written_files[f"{module_name}-{version}-{filename}"] = content + # Pretend the "database" field is declared in a module that depends on + # (and is therefore not reachable from) upgrade_analysis. + database_field = self.env["upgrade.comparison.config"]._fields["database"] + with ( patch.object(analysis.config_id.__class__, "get_connection"), patch.object( @@ -273,6 +291,7 @@ def write_file(module_name, version, content, filename="upgrade_analysis.txt"): patch.object( self.env["upgrade.record"].__class__, "get_xml_records" ) as patched_get_xml_records, + patch.object(database_field, "_module", "some_dependent_module"), ): patched_get_remote_model.side_effect = lambda *args: RemoteUpgradeRecord() patched_get_xml_records.side_effect = local_get_xml_records @@ -283,6 +302,7 @@ def write_file(module_name, version, content, filename="upgrade_analysis.txt"): Noupdate xmlid from upgrade_analysis + """