[IMP] - improve code and documentation

attribute_set/models/attribute_attribute.py

@@ -125,9 +125,10 @@ def _get_attrs(self):
 
     @api.model
     def _build_attribute_field(self, attribute_egroup):
-        """Add an etree 'field' subelement (related to the current attribute 'self')
-        to attribute_egroup, with a conditional invisibility based on its
-        attribute sets."""
+        """Add field into given attribute group.
+
+        Conditional invisibility based on its attribute sets.
+        """
         self.ensure_one()
         kwargs = {"name": "%s" % self.name}
         kwargs["attrs"] = str(self._get_attrs())
@@ -183,7 +184,9 @@ def _get_native_field_context(self):
         return str(self.env[self.field_id.model]._fields[self.field_id.name].context)
 
     def _build_attribute_eview(self):
-        """Return an 'attribute_eview' including all the Attributes (in the current
+        """Generate group element for all attributes in the current recordset.
+
+        Return an 'attribute_eview' including all the Attributes (in the current
         recorset 'self') distributed in different 'attribute_egroup' for each
         Attribute's group.
         """
@@ -240,22 +243,23 @@ def onchange_attribute_type(self):
             self.widget = "many2many_tags"
 
     @api.onchange("relation_model_id")
-    def relation_model_id_change(self):
-        "Remove selected options as they would be inconsistent"
+    def _onchange_relation_model_id(self):
+        """Remove selected options as they would be inconsistent"""
         self.option_ids = [(5, 0)]
 
     @api.onchange("domain")
-    def domain_change(self):
+    def _onchange_domain(self):
         if self.domain not in ["", False]:
             try:
                 ast.literal_eval(self.domain)
             except ValueError:
                 raise ValidationError(
                     _(
-                        """ "{}" is an unvalid Domain name.\n
-                        Specify a Python expression defining a list of triplets.\
-                        For example : "[('color', '=', 'red')]" """
-                    ).format(self.domain)
+                        "`%(domain)s` is an invalid Domain name.\n"
+                        "Specify a Python expression defining a list of triplets.\n"
+                        "For example : `[('color', '=', 'red')]`",
+                    )
+                    % dict(domain=self.domain)
                 ) from ValueError
             # Remove selected options as the domain will predominate on actual options
             if self.domain != "[]":
@@ -271,7 +275,7 @@ def button_add_options(self):
         # Then open the Options Wizard which will display an 'opt_ids' m2m field related
         # to the 'relation_model_id' model
         return {
-            "context": "{'attribute_id': %s}" % (self.id),
+            "context": dict(self.env.context, attribute_id=self.id),
             "name": _("Options Wizard"),
             "view_type": "form",
             "view_mode": "form",
@@ -368,19 +372,17 @@ def create(self, vals_list):
         return super().create(vals_list)
 
     def _delete_related_option_wizard(self, option_vals):
-        """Delete the attribute's options wizards related to the attribute's options
-        deleted after the write"""
+        """Delete related attribute's options wizards."""
         self.ensure_one()
         for option_change in option_vals:
             if option_change[0] == 2:
                 self.env["attribute.option.wizard"].search(
                     [("attribute_id", "=", self.id)]
                 ).unlink()
+                break
 
     def _delete_old_fields_options(self, options):
-        """Delete attribute's field values in the objects using our attribute
-        as a field, if these values are not in the new Domain or Options list
-        """
+        """Delete outdated attribute's field values on existing records."""
         self.ensure_one()
         custom_field = self.name
         for obj in self.env[self.model].search([]):
@@ -395,7 +397,7 @@ def _delete_old_fields_options(self, options):
     def write(self, vals):
         # Prevent from changing Attribute's type
         if "attribute_type" in list(vals.keys()):
-            if self.search(
+            if self.search_count(
                 [
                     ("attribute_type", "!=", vals["attribute_type"]),
                     ("id", "in", self.ids),
@@ -413,7 +415,7 @@ def write(self, vals):
         # as the values of the existing many2many Attribute fields won't be
         # deleted if changing relation_model_id
         if "relation_model_id" in list(vals.keys()):
-            if self.search(
+            if self.search_count(
                 [
                     ("relation_model_id", "!=", vals["relation_model_id"]),
                     ("id", "in", self.ids),
@@ -428,7 +430,7 @@ def write(self, vals):
                 )
         # Prevent from changing 'Serialized'
         if "serialized" in list(vals.keys()):
-            if self.search(
+            if self.search_count(
                 [("serialized", "!=", vals["serialized"]), ("id", "in", self.ids)]
             ):
                 raise ValidationError(
@@ -443,7 +445,7 @@ def write(self, vals):
 
         for att in self:
             options = att.option_ids
-            if self.relation_model_id:
+            if att.relation_model_id:
                 options = self.env[att.relation_model_id.model]
                 if "option_ids" in list(vals.keys()):
                     # Delete related attribute.option.wizard if an attribute.option

attribute_set/models/attribute_option.py

@@ -13,13 +13,13 @@ class AttributeOption(models.Model):
     _order = "sequence"
 
     @api.model
-    def _get_model_list(self):
+    def _selection_model_list(self):
         models = self.env["ir.model"].search([])
         return [(m.model, m.name) for m in models]
 
     name = fields.Char(translate=True, required=True)
 
-    value_ref = fields.Reference(_get_model_list, "Reference")
+    value_ref = fields.Reference(selection="_selection_model_list", string="Reference")
 
     attribute_id = fields.Many2one(
         "attribute.attribute",
@@ -38,9 +38,12 @@ def _get_model_list(self):
     sequence = fields.Integer()
 
     @api.onchange("name")
-    def name_change(self):
-        """Prevent the user from adding manually an option to m2o or m2m Attributes
-        linked to another model (through 'relation_model_id')"""
+    def _onchange_name(self):
+        """Prevent improper linking of attributes.
+
+        The user could add manually an option to m2o or m2m Attributes
+        linked to another model (through 'relation_model_id').
+        """
         if self.attribute_id.relation_model_id:
             warning = {
                 "title": _("Error!"),

attribute_set/models/attribute_set_owner.py

@@ -10,12 +10,7 @@
 
 
 class AttributeSetOwnerMixin(models.AbstractModel):
-    """Override the '_inheriting' model's get_views() and replace
-    the 'attributes_placeholder' by the fields related to the '_inheriting' model's
-    Attributes.
-    Each Attribute's field will have a conditional invisibility depending on its
-    Attribute Sets.
-    """
+    """Mixin for consumers of attribute sets."""
 
     _name = "attribute.set.owner.mixin"
     _description = "Attribute set owner mixin"
@@ -52,20 +47,20 @@ def remove_native_fields(self, eview):
                 efield[0].getparent().remove(efield[0])
 
     def _insert_attribute(self, arch):
-        """Insert the model's Attributes related fields into the arch's view form
-        at the placeholder's place."""
+        """Replace attributes' placeholders with real fields in form view arch."""
         eview = etree.fromstring(arch)
         form_name = eview.get("string")
         placeholder = eview.xpath("//separator[@name='attributes_placeholder']")
 
         if len(placeholder) != 1:
             raise ValidationError(
                 _(
-                    """It is impossible to add Attributes on "{}" xml
+                    """It is impossible to add Attributes on "%(name)s" xml
                     view as there is
                     not one "<separator name="attributes_placeholder" />" in it.
-                    """
-                ).format(form_name)
+                    """,
+                    name=form_name,
+                )
             )
 
         if self._context.get("include_native_attribute"):
@@ -79,11 +74,8 @@ def _insert_attribute(self, arch):
     @api.model
     def get_views(self, views, options=None):
         result = super().get_views(views, options=options)
-        if (
-            "views" in result
-            and "form" in result["views"]
-            and "arch" in result["views"]["form"]
-        ):
+        form_arch = result.get("views", {}).get("form", {}).get("arch")
+        if form_arch:
             result["views"]["form"]["arch"] = self._insert_attribute(
                 result["views"]["form"]["arch"]
             )

attribute_set/readme/DESCRIPTION.rst

@@ -7,5 +7,5 @@ A *"custom"* Attribute can be of any type : Char, Text, Boolean, Date, Binary...
 
 In case of m2o or m2m, these attributes can be related to **custom options** created for the Attribute, or to **existing Odoo objects** from other models.
 
-Last but not least an Attribute can be **serialized** using the Odoo SA module `base_sparse_field <https://github.com/odoo/odoo/tree/12.0/addons/base_sparse_field>`_ .
+Last but not least an Attribute can be **serialized** using the Odoo SA module `base_sparse_field <https://github.com/odoo/odoo/tree/16.0/addons/base_sparse_field>`_ .
 It means that all the serialized attributes will be stored in a single "JSON serialization field" and will not create new columns in the database (and better, it will not create new SQL tables in case of Many2many Attributes),  **increasing significantly the requests speed** when dealing with thousands of Attributes.

attribute_set/tests/test_custom_attribute.py

@@ -4,20 +4,21 @@
 # Copyright 2015 Savoir-faire Linux
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl).
 
-import mock
+from unittest import mock
 
 from odoo.tests import common
 
 
 class TestAttributeSet(common.TransactionCase):
-    def setUp(self):
-        super(TestAttributeSet, self).setUp()
-        self.model_id = self.env.ref("base.model_res_partner").id
-        self.group = self.env["attribute.group"].create(
-            {"name": "My Group", "model_id": self.model_id}
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        cls.model_id = cls.env.ref("base.model_res_partner").id
+        cls.group = cls.env["attribute.group"].create(
+            {"name": "My Group", "model_id": cls.model_id}
         )
         # Do not commit
-        self.env.cr.commit = mock.Mock()
+        cls.env.cr.commit = mock.Mock()
 
     def _create_attribute(self, vals):
         vals.update(

attribute_set/utils/orm.py

@@ -69,8 +69,7 @@ def transfer_modifiers_to_node(modifiers, node):
 
 
 def setup_modifiers(node, field=None, context=None, in_tree_view=False):
-    """Processes node attributes and field descriptors to generate
-    the ``modifiers`` node attribute and set it on the provided node.
+    """Generate ``modifiers``  from node attributes and fields descriptors.
     Alters its first argument in-place.
     :param node: ``field`` node from an OpenERP view
     :type node: lxml.etree._Element
@@ -84,7 +83,7 @@ def setup_modifiers(node, field=None, context=None, in_tree_view=False):
                               displayed) with ``invisible`` and column
                               invisibility (the whole column is
                               hidden) with ``column_invisible``.
-    :returns: nothing
+    :returns: None
     """
     modifiers = {}
     if field is not None: