From 9f11a84d718eeb2a717087b77009e8444fedcfb1 Mon Sep 17 00:00:00 2001 From: Adrian Torres Date: Wed, 1 Sep 2021 13:12:48 +0000 Subject: [PATCH] [IMP] Order groups by selection declaration order in read_group Before this commit, calling read_group on a model and grouping by a selection field would return the groups in alphabetical order, meaning that if your selection field's options were declared as: ('z', 'Z'), ('a', 'A') read_group would return first group 'a' and then group 'z', instead of groups 'z' and then 'a' which some might expect. With this commit, it is now possible to set a fields.Selection group_expand attribute to `True` when declaring it, which will use a default group_expand implementation specific to Selection fields, this means that when grouping by a Selection field with `group_expand=True` it will always return the groups in the definition order of the selection options. We achieve this by leveraging the group_expand field attribute which was designed for changing the groups returned by read_group. Since this attribute was thought only to be implemented on a Model-by-Model basis and here we need to use it as a generic function for all Selection fields (explicit group_expand declarations have higher precedence), the generic method has been implemented inside fields.Selection and takes an extra records parameter which holds a reference to the recordset/model on which read_group was called. This extra parameter only applies to field implementations of group_expand and in this case is what allows this specific feature to work with dynamic Selection fields (function as options). A side-effect of this implementation is that a read_group call that groups by a Selection field with `group_expand=True` that uses the default group_expand will always return all possible groups, even empty ones. Task-ID 2635052 closes odoo/odoo#75856 Signed-off-by: Raphael Collet (rco) --- odoo/addons/test_read_group/models.py | 8 ++ .../tests/test_group_expand.py | 134 ++++++++++++++++++ odoo/fields.py | 7 + odoo/models.py | 18 ++- 4 files changed, 160 insertions(+), 7 deletions(-) diff --git a/odoo/addons/test_read_group/models.py b/odoo/addons/test_read_group/models.py index 84c5f775129..be49a2b6381 100644 --- a/odoo/addons/test_read_group/models.py +++ b/odoo/addons/test_read_group/models.py @@ -31,11 +31,19 @@ class Aggregate(models.Model): partner_id = fields.Many2one('res.partner') +# we use a selection that is in reverse lexical order, in order to check the +# possible reordering made by read_group on selection fields +SELECTION = [('c', "C"), ('b', "B"), ('a', "A")] + + class GroupOnSelection(models.Model): _name = 'test_read_group.on_selection' _description = 'Group Test Read On Selection' state = fields.Selection([('a', "A"), ('b', "B")], group_expand='_expand_states') + static_expand = fields.Selection(SELECTION, group_expand=True) + dynamic_expand = fields.Selection(lambda self: SELECTION, group_expand=True) + no_expand = fields.Selection(SELECTION) value = fields.Integer() def _expand_states(self, states, domain, order): diff --git a/odoo/addons/test_read_group/tests/test_group_expand.py b/odoo/addons/test_read_group/tests/test_group_expand.py index 69217981abb..9a9111dd430 100644 --- a/odoo/addons/test_read_group/tests/test_group_expand.py +++ b/odoo/addons/test_read_group/tests/test_group_expand.py @@ -87,3 +87,137 @@ class TestGroupOnSelection(common.TransactionCase): '__domain': [('state', '=', False)], }, ]) + + +@common.tagged("test_read_group_selection") +class TestSelectionReadGroup(common.TransactionCase): + + def setUp(self): + super().setUp() + self.Model = self.env['test_read_group.on_selection'] + + def test_static_group_expand(self): + # this test verifies that the following happens when grouping by a Selection field with + # group_expand=True: + # - the order of the returned groups is the same as the order in which the + # options are declared in the field definition. + # - the groups returned include the empty groups, i.e. all groups, even those + # that have no records assigned to them, this is a (wanted) side-effect of the + # implementation. + # - the false group, i.e. records without the Selection field set, is last. + self.Model.create([ + {"value": 1, "static_expand": "a"}, + {"value": 2, "static_expand": "c"}, + {"value": 3}, + ]) + + groups = self.Model.read_group( + [], + fields=["static_expand", "value"], + groupby=["static_expand"], + ) + self.assertEqual(groups, [ + { + 'static_expand': 'c', + 'static_expand_count': 1, + 'value': 2, + '__domain': [('static_expand', '=', 'c')], + }, + { + 'static_expand': 'b', + 'static_expand_count': 0, + 'value': 0, + '__domain': [('static_expand', '=', 'b')], + }, + { + 'static_expand': 'a', + 'static_expand_count': 1, + 'value': 1, + '__domain': [('static_expand', '=', 'a')], + }, + { + 'static_expand': False, + 'static_expand_count': 1, + 'value': 3, + '__domain': [('static_expand', '=', False)], + }, + ]) + + def test_dynamic_group_expand(self): + # this test tests the same as the above test but with a Selection field whose + # options are dynamic, this means that the result of read_group when grouping by this + # field can change from one call to another. + self.Model.create([ + {"value": 1, "dynamic_expand": "a"}, + {"value": 2, "dynamic_expand": "c"}, + {"value": 3}, + ]) + + groups = self.Model.read_group( + [], + fields=["dynamic_expand", "value"], + groupby=["dynamic_expand"], + ) + + self.assertEqual(groups, [ + { + 'dynamic_expand': 'c', + 'dynamic_expand_count': 1, + 'value': 2, + '__domain': [('dynamic_expand', '=', 'c')], + }, + { + 'dynamic_expand': 'b', + 'dynamic_expand_count': 0, + 'value': 0, + '__domain': [('dynamic_expand', '=', 'b')], + }, + { + 'dynamic_expand': 'a', + 'dynamic_expand_count': 1, + 'value': 1, + '__domain': [('dynamic_expand', '=', 'a')], + }, + { + 'dynamic_expand': False, + 'dynamic_expand_count': 1, + 'value': 3, + '__domain': [('dynamic_expand', '=', False)], + }, + ]) + + def test_no_group_expand(self): + # if group_expand is not defined on a Selection field, it should return only the necessary + # groups and in alphabetical order (PostgreSQL ordering) + self.Model.create([ + {"value": 1, "no_expand": "a"}, + {"value": 2, "no_expand": "c"}, + {"value": 3}, + ]) + + groups = self.Model.read_group( + [], + fields=["no_expand", "value"], + groupby=["no_expand"], + ) + + self.assertEqual(groups, [ + { + 'no_expand': 'a', + 'no_expand_count': 1, + 'value': 1, + '__domain': [('no_expand', '=', 'a')], + }, + { + 'no_expand': 'c', + 'no_expand_count': 1, + 'value': 2, + '__domain': [('no_expand', '=', 'c')], + }, + { + 'no_expand': False, + 'no_expand_count': 1, + 'value': 3, + '__domain': [('no_expand', '=', False)], + }, + ]) diff --git a/odoo/fields.py b/odoo/fields.py index 27accd7ad9e..f916a741b19 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -2370,6 +2370,9 @@ class Selection(Field): attrs = super()._get_attrs(model_class, name) # arguments 'selection' and 'selection_add' are processed below attrs.pop('selection_add', None) + # Selection fields have an optional default implementation of a group_expand function + if attrs.get('group_expand') is True: + attrs['group_expand'] = self._default_group_expand return attrs def _setup_attrs(self, model_class, name): @@ -2487,6 +2490,10 @@ class Selection(Field): else: return selection + def _default_group_expand(self, records, groups, domain, order): + # return a group per selection option, in definition order + return self.get_values(records.env) + def get_values(self, env): """Return a list of the possible values.""" selection = self.selection diff --git a/odoo/models.py b/odoo/models.py index 6e16adb906d..dda5837c306 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -1988,11 +1988,15 @@ class BaseModel(metaclass=MetaModel): if not field.group_expand: return read_group_result - # field.group_expand is the name of a method that returns the groups - # that we want to display for this field, in the form of a recordset or - # a list of values (depending on the type of the field). This is useful - # to implement kanban views for instance, where some columns should be - # displayed even if they don't contain any record. + # field.group_expand is a callable or the name of a method, that returns + # the groups that we want to display for this field, in the form of a + # recordset or a list of values (depending on the type of the field). + # This is useful to implement kanban views for instance, where some + # columns should be displayed even if they don't contain any record. + group_expand = field.group_expand + if isinstance(group_expand, str): + group_expand = getattr(type(self), group_expand) + assert callable(group_expand) # determine all groups that should be returned values = [line[groupby] for line in read_group_result if line[groupby]] @@ -2003,14 +2007,14 @@ class BaseModel(metaclass=MetaModel): order = groups._order if read_group_order == groupby + ' desc': order = tools.reverse_order(order) - groups = getattr(self, field.group_expand)(groups, domain, order) + groups = group_expand(self, groups, domain, order) groups = groups.sudo() values = lazy_name_get(groups) value2key = lambda value: value and value[0] else: # groups is a list of values - values = getattr(self, field.group_expand)(values, domain, None) + values = group_expand(self, values, domain, None) if read_group_order == groupby + ' desc': values.reverse() value2key = lambda value: value