[IMP] base_location: Switch to computed writable fields

* Adapt code to this style
* Adapt tests and use Form for mimicking UI behavior

base_location/models/res_partner.py

@@ -9,41 +9,96 @@
 class ResPartner(models.Model):
     _inherit = "res.partner"
 
-    zip_id = fields.Many2one("res.city.zip", "ZIP Location", index=True)
-    city_id = fields.Many2one(index=True)  # add index for performance
+    allowed_zip_ids = fields.Many2many(
+        comodel_name="res.city.zip", compute="_compute_allowed_zip_ids"
+    )
+    zip_id = fields.Many2one(
+        comodel_name="res.city.zip",
+        string="ZIP Location",
+        index=True,
+        compute="_compute_zip_id",
+        readonly=False,
+        store=True,
+        domain="[('id', 'in', allowed_zip_ids)]",
+    )
+    city_id = fields.Many2one(
+        index=True,  # add index for performance
+        compute="_compute_city_id",
+        readonly=False,
+        store=True,
+    )
+    city = fields.Char(compute="_compute_city", readonly=False, store=True)
+    zip = fields.Char(compute="_compute_zip", readonly=False, store=True)
+    country_id = fields.Many2one(
+        compute="_compute_country_id", readonly=False, store=True
+    )
+    state_id = fields.Many2one(compute="_compute_state_id", readonly=False, store=True)
 
-    @api.onchange("city_id")
-    def _onchange_city_id(self):
-        if not self.zip_id:
-            super()._onchange_city_id()
-        if self.zip_id and self.city_id != self.zip_id.city_id:
-            self.update({"zip_id": False, "zip": False, "city": False})
-        if self.city_id and self.country_enforce_cities:
-            return {"domain": {"zip_id": [("city_id", "=", self.city_id.id)]}}
-        return {"domain": {"zip_id": []}}
+    @api.depends("city_id")
+    def _compute_allowed_zip_ids(self):
+        for record in self:
+            if record.city_id:
+                domain = [("city_id", "=", record.city_id.id)]
+            else:
+                domain = []
+            record.allowed_zip_ids = self.env["res.city.zip"].search(domain)
 
-    @api.onchange("country_id")
-    def _onchange_country_id(self):
-        res = super()._onchange_country_id()
-        if self.zip_id and self.zip_id.city_id.country_id != self.country_id:
-            self.zip_id = False
-        return res
+    @api.depends("state_id", "country_id")
+    def _compute_zip_id(self):
+        """Empty the zip auto-completion field if data mismatch when on UI."""
+        for record in self.filtered("zip_id"):
+            for field in ["state_id", "country_id"]:
+                if (
+                    record[field]
+                    and record[field] != record._origin[field]
+                    and record[field] != record.zip_id.city_id[field]
+                ):
+                    record.zip_id = False
 
-    @api.onchange("zip_id")
-    def _onchange_zip_id(self):
-        if self.zip_id:
-            vals = {
-                "city_id": self.zip_id.city_id,
-                "zip": self.zip_id.name,
-                "city": self.zip_id.city_id.name,
-            }
-            if self.zip_id.city_id.country_id:
-                vals.update({"country_id": self.zip_id.city_id.country_id})
-            if self.zip_id.city_id.state_id:
-                vals.update({"state_id": self.zip_id.city_id.state_id})
-            self.update(vals)
-        elif not self.country_enforce_cities:
-            self.city_id = False
+    @api.depends("zip_id")
+    def _compute_city_id(self):
+        if hasattr(super(), "_compute_city_id"):
+            super()._compute_city_id()  # pragma: no cover
+        for record in self:
+            if record.zip_id:
+                record.city_id = record.zip_id.city_id
+            elif not record.country_enforce_cities:
+                record.city_id = False
+
+    @api.depends("zip_id")
+    def _compute_city(self):
+        if hasattr(super(), "_compute_city"):
+            super()._compute_city()  # pragma: no cover
+        for record in self:
+            if record.zip_id:
+                record.city = record.zip_id.city_id.name
+
+    @api.depends("zip_id")
+    def _compute_zip(self):
+        if hasattr(super(), "_compute_zip"):
+            super()._compute_zip()  # pragma: no cover
+        for record in self:
+            if record.zip_id:
+                record.zip = record.zip_id.name
+
+    @api.depends("zip_id", "state_id")
+    def _compute_country_id(self):
+        if hasattr(super(), "_compute_country_id"):
+            super()._compute_country_id()  # pragma: no cover
+        for record in self:
+            if record.zip_id.city_id.country_id:
+                record.country_id = record.zip_id.city_id.country_id
+            elif record.state_id:
+                record.country_id = record.state_id.country_id
+
+    @api.depends("zip_id")
+    def _compute_state_id(self):
+        if hasattr(super(), "_compute_state_id"):
+            super()._compute_state_id()  # pragma: no cover
+        for record in self:
+            state = record.zip_id.city_id.state_id
+            if state and record.state_id != state:
+                record.state_id = record.zip_id.city_id.state_id
 
     @api.constrains("zip_id", "country_id", "city_id", "state_id")
     def _check_zip(self):
@@ -70,12 +125,3 @@ def _check_zip(self):
                     _("The city of partner %s differs from that in " "location %s")
                     % (rec.name, rec.zip_id.name)
                 )
-
-    @api.onchange("state_id")
-    def _onchange_state_id(self):
-        vals = {}
-        if self.state_id.country_id:
-            vals.update({"country_id": self.state_id.country_id})
-        if self.zip_id and self.state_id != self.zip_id.city_id.state_id:
-            vals.update({"zip_id": False, "zip": False, "city": False})
-        self.update(vals)

base_location/tests/test_base_location.py

@@ -4,7 +4,7 @@
 import psycopg2
 
 from odoo.exceptions import ValidationError
-from odoo.tests import common, tagged
+from odoo.tests import Form, common, tagged
 from odoo.tools.misc import mute_logger
 
 
@@ -23,9 +23,9 @@ def setUpClass(cls):
         )
         cls.env.ref("base.es").write({"enforce_cities": True})
         cls.company = cls.env.ref("base.main_company")
-
+        cls.country_es = cls.env.ref("base.es")
         cls.state_bcn = state_obj.create(
-            {"name": "Barcelona", "code": "08", "country_id": cls.env.ref("base.es").id}
+            {"name": "Barcelona", "code": "08", "country_id": cls.country_es.id}
         )
         cls.state_madrid = state_obj.create(
             {"name": "Madrid", "code": "28", "country_id": cls.env.ref("base.es").id}
@@ -56,9 +56,8 @@ def setUpClass(cls):
 
     def test_onchange_partner_city_completion(self):
         """Test that partner data is filled accodingly"""
-        partner1 = self.partner_obj.new({"name": "Camptocamp"})
+        partner1 = Form(self.env["res.partner"])
         partner1.zip_id = self.barcelona
-        partner1._onchange_zip_id()
         self.assertEqual(partner1.zip, self.barcelona.name)
         self.assertEqual(partner1.city, self.barcelona.city_id.name)
         self.assertEqual(partner1.state_id, self.barcelona.city_id.state_id)
@@ -110,9 +109,19 @@ def test_onchange_company_city_id_completion(self):
         self.assertEqual(company.city_id, self.barcelona.city_id)
 
     def test_constrains_partner_01(self):
-        """Test partner 1 constraints"""
+        """Test zip_id constraints"""
+        with self.assertRaises(ValidationError):
+            self.partner_obj.create(
+                {"name": "P1", "zip_id": self.barcelona.id, "state_id": False}
+            )
+        with self.assertRaises(ValidationError):
+            self.partner_obj.create(
+                {"name": "P1", "zip_id": self.barcelona.id, "country_id": False}
+            )
         with self.assertRaises(ValidationError):
-            self.partner_obj.create({"name": "P1", "zip_id": self.barcelona.id})
+            self.partner_obj.create(
+                {"name": "P1", "zip_id": self.barcelona.id, "city_id": False}
+            )
 
     def test_writing_company(self):
         self.company.zip_id = self.barcelona
@@ -171,22 +180,23 @@ def test_partner_onchange_country(self):
 
     def test_partner_onchange_city(self):
         """Test partner onchange city_id"""
-        partner = self.partner_obj.new({"name": "TEST", "zip_id": self.lausanne.id})
+        partner = Form(self.env["res.partner"])
+        partner.zip_id = self.lausanne
         self.city_bcn.country_id.enforce_cities = False
         partner.city_id = self.city_bcn
-        partner._onchange_city_id()
         self.assertFalse(partner.zip_id)
-        partner.city_id = False
-        res = partner._onchange_city_id()
-        self.assertFalse(res["domain"]["zip_id"])
+        partner.city_id = self.env["res.city"]
+        self.assertEqual(
+            len(partner.allowed_zip_ids), self.env["res.city.zip"].search_count([])
+        )
 
     def test_partner_onchange_state(self):
         """Test partner onchange state_id"""
-        partner = self.partner_obj.new({"name": "TEST", "zip_id": self.lausanne.id})
+        partner = Form(self.env["res.partner"])
+        partner.zip_id = self.lausanne
         partner.state_id = self.state_bcn
-        partner._onchange_state_id()
         self.assertFalse(partner.zip_id)
-        self.assertEqual(partner.country_id, partner.state_id.country_id)
+        self.assertEqual(partner.country_id, self.country_es)
 
     def test_company_onchange_state(self):
         """Test company onchange state_id"""

base_location/views/res_partner_view.xml

@@ -6,6 +6,7 @@
         <field name="inherit_id" ref="base.view_partner_form" />
         <field name="arch" type="xml">
             <field name="city" position="before">
+                <field name="allowed_zip_ids" invisible="1" />
                 <field
                     name="zip_id"
                     options="{'create_name_field': 'city', 'no_open': True, 'no_create': True}"
@@ -18,6 +19,7 @@
                 expr="//field[@name='child_ids']/form//field[@name='city']"
                 position="before"
             >
+                <field name="allowed_zip_ids" invisible="1" />
                 <field
                     name="zip_id"
                     options="{'create_name_field': 'city', 'no_open': True, 'no_create': True}"