[19.0][IMP] upgrade_analysis: generate changes for renamed noupdate records

upgrade_analysis/compare.py

@@ -540,7 +540,7 @@ def match(match_fields, match_type="direct"):
         if entry["noupdate_switched"]:
             content += " (noupdate switched)"
         reprs[module_map(entry["module"])].append(content)
-    return reprs
+    return reprs, moved_records, renamed_records, modified_records
 
 
 def compare_model_sets(old_records, new_records):

upgrade_analysis/models/upgrade_analysis.py

@@ -164,7 +164,9 @@ def analyze(self):
             {field: record[field] for field in flds}
             for record in RemoteRecord.read(remote_xml_record_ids, flds)
         ]
-        res_xml = compare.compare_xml_sets(remote_xml_records, local_xml_records)
+        res_xml, moved_xml_records, renamed_xml_records, modified_xml_records = (
+            compare.compare_xml_sets(remote_xml_records, local_xml_records)
+        )
 
         # Retrieve model representations and compare
         flds = [
@@ -270,7 +272,9 @@ def analyze(self):
             )
         noupdate_modules = []
         try:
-            noupdate_modules = self.generate_noupdate_changes()
+            noupdate_modules = self.generate_noupdate_changes(
+                moved_xml_records, renamed_xml_records, modified_xml_records
+            )
         except Exception as e:
             _logger.exception(f"Error generating noupdate changes: {e}")
             general_log += "ERROR: error when generating noupdate changes: {e}\n"
@@ -486,7 +490,9 @@ def _parse_files(self, xml_files, module_name):
 
         return records_update, records_noupdate
 
-    def generate_noupdate_changes(self):
+    def generate_noupdate_changes(
+        self, moved_xml_records, renamed_xml_records, modified_xml_records
+    ):
         """Communicate with the remote server to fetch all xml data records
         per module, and generate a diff in XML format that can be imported
         from the module's migration script using openupgrade.load_data()
@@ -498,6 +504,14 @@ def generate_noupdate_changes(self):
         local_modules = local_record_obj.list_modules()
         all_remote_modules = remote_record_obj.list_modules()
         changed_modules = []
+        # {new_module: {name: previous_module}}
+        renamed_xmlids = {}
+        for renamed_xml_record in renamed_xml_records:
+            if renamed_xml_record.get("old") or not renamed_xml_record.get("new"):
+                continue
+            renamed_xmlids.setdefault(renamed_xml_record["module"], {}).update(
+                {renamed_xml_record["suffix"]: renamed_xml_record["renamed"]}
+            )
         for local_module in local_modules:
             remote_files = []
             remote_modules = []
@@ -515,6 +529,35 @@ def generate_noupdate_changes(self):
                     )
                     remote_update.update(add_remote_update)
                     remote_noupdate.update(add_remote_noupdate)
+                if any(
+                    renamed_from_module == remote_module
+                    for renamed_from_module in renamed_xmlids.get(
+                        local_module, {}
+                    ).values()
+                ):
+                    # if xmlids have been renamed (moved) to the current module, query
+                    # their definition from the module that contained it previously
+                    remote_modules.append(remote_module)
+                    renamed_from_module_files = remote_record_obj.get_xml_records(
+                        remote_module
+                    )
+                    renamed_from_module_update, renamed_from_module_noupdate = (
+                        self._parse_files(renamed_from_module_files, remote_module)
+                    )
+                    remote_update.update(
+                        {
+                            name: xml
+                            for name, xml in renamed_from_module_update.items()
+                            if renamed_xmlids[local_module].get(name) == remote_module
+                        }
+                    )
+                    remote_noupdate.update(
+                        {
+                            name: xml
+                            for name, xml in renamed_from_module_noupdate.items()
+                            if renamed_xmlids[local_module].get(name) == remote_module
+                        }
+                    )
             if not remote_modules:
                 continue
             local_files = local_record_obj.get_xml_records(local_module)

upgrade_analysis/tests/test_module.py

@@ -1,4 +1,5 @@
 from copy import deepcopy
+from unittest.mock import patch
 
 from lxml import etree
 
@@ -152,3 +153,142 @@ def test_xml_comparison(self):
         )
         self.assertIn('<field name="module_ids" eval="None"/>', diff)
         self.assertIn('<field name="display_name"/>', diff)
+
+    def test_analyze(self):
+        """
+        Test a full analysis run.
+        For the time being, only xmlid related functionality is tested
+        """
+        analysis = self.env["upgrade.analysis"].create(
+            {
+                "config_id": self.env["upgrade.comparison.config"]
+                .create(
+                    {
+                        "database": self.env.cr.dbname,
+                    }
+                )
+                .id,
+            }
+        )
+        upgrade_analysis_version = (
+            self.env["ir.module.module"]
+            .search([("name", "=", "upgrade_analysis")])
+            .latest_version
+        )
+
+        self.env["upgrade.record"].create(
+            {
+                "name": "upgrade_analysis.test_noupdate_xmlid",
+                "mode": "create",
+                "type": "xmlid",
+                "module": "upgrade_analysis",
+                "model": "upgrade.comparison.config",
+                "noupdate": True,
+            }
+        )
+
+        class RemoteUpgradeRecord:
+            _records = {
+                1: {
+                    "name": "other_module.test_noupdate_xmlid",
+                    "mode": "create",
+                    "type": "xmlid",
+                    "module": "other_module",
+                    "prefix": "other_module",
+                    "model": "upgrade.comparison.config",
+                    "noupdate": True,
+                    "suffix": "test_noupdate_xmlid",
+                },
+            }
+
+            def search(self, domain):
+                if domain == [("type", "=", "xmlid")]:
+                    return [
+                        _id
+                        for _id, vals in self._records.items()
+                        if vals["type"] == "xmlid"
+                    ]
+                return []
+
+            def read(self, ids, fields):  # pylint: disable=method-required-super
+                return [
+                    {field: record.get(field) for field in fields}
+                    for _id, record in self._records.items()
+                    if _id in ids
+                ]
+
+            def field_dump(self):
+                return []
+
+            def list_modules(self):
+                return set(record["module"] for record in self._records.values())
+
+            def get_xml_records(self, module):
+                if module == "other_module":
+                    return [
+                        """
+                        <odoo noupdate="1">
+                            <record
+                                id="test_noupdate_xmlid"
+                                model="upgrade.comparison.config"
+                            >
+                                <field
+                                    name="name"
+                                >Noupdate xmlid from other_module</field>
+                                <field name="server">some_server</field>
+                            </record>
+                        </odoo>
+                        """
+                    ]
+
+        def local_get_xml_records(module):
+            if module == "upgrade_analysis":
+                return [
+                    """
+                    <odoo noupdate="1">
+                        <record
+                            id="test_noupdate_xmlid"
+                            model="upgrade.comparison.config"
+                        >
+                            <field
+                                name="name"
+                            >Noupdate xmlid from upgrade_analysis</field>
+                            <field name="server">some_server</field>
+                        </record>
+                    </odoo>
+                    """
+                ]
+
+        written_files = {}
+
+        def write_file(module_name, version, content, filename="upgrade_analysis.txt"):
+            written_files[f"{module_name}-{version}-{filename}"] = content
+
+        with (
+            patch.object(analysis.config_id.__class__, "get_connection"),
+            patch.object(
+                analysis.__class__, "_get_remote_model"
+            ) as patched_get_remote_model,
+            patch.object(analysis.__class__, "_write_file") as patched_write_file,
+            patch.object(
+                self.env["upgrade.record"].__class__, "get_xml_records"
+            ) as patched_get_xml_records,
+        ):
+            patched_get_remote_model.side_effect = lambda *args: RemoteUpgradeRecord()
+            patched_get_xml_records.side_effect = local_get_xml_records
+            patched_write_file.side_effect = write_file
+            analysis.analyze()
+
+        expected_noupdate_content = """<?xml version='1.0' encoding='utf-8'?>
+<odoo>
+  <record id="test_noupdate_xmlid" model="upgrade.comparison.config">
+    <field name="name">Noupdate xmlid from upgrade_analysis</field>
+  </record>
+</odoo>
+"""
+        self.assertEqual(
+            written_files[
+                f"upgrade_analysis-{upgrade_analysis_version}-noupdate_changes.xml"
+            ],
+            expected_noupdate_content,
+        )