Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] Order groups by selection declaration order in read_group #75856

Closed

Conversation

Elkasitu
Copy link
Contributor

@Elkasitu Elkasitu commented Sep 1, 2021

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

@robodoo
Copy link
Contributor

robodoo commented Sep 1, 2021

@C3POdoo C3POdoo added the RD research & development, internal work label Sep 1, 2021
@Elkasitu Elkasitu force-pushed the master-order-read_group-selection-adt branch from cf94bc8 to a1bbaad Compare September 1, 2021 14:24
odoo/models.py Outdated Show resolved Hide resolved
odoo/addons/base/models/ir_model.py Outdated Show resolved Hide resolved
odoo/addons/test_read_group/models.py Outdated Show resolved Hide resolved
odoo/addons/test_read_group/tests/test_group_expand.py Outdated Show resolved Hide resolved
odoo/fields.py Outdated Show resolved Hide resolved
odoo/fields.py Outdated Show resolved Hide resolved
@Elkasitu Elkasitu force-pushed the master-order-read_group-selection-adt branch 2 times, most recently from cb3e638 to 4ccf874 Compare September 2, 2021 09:57
@C3POdoo C3POdoo requested a review from a team September 2, 2021 10:00
@Elkasitu Elkasitu force-pushed the master-order-read_group-selection-adt branch from 4ccf874 to 45496cb Compare September 2, 2021 14:37
@Elkasitu Elkasitu removed the request for review from a team September 2, 2021 14:44
@Elkasitu Elkasitu force-pushed the master-order-read_group-selection-adt branch from 45496cb to ea0cf7f Compare September 3, 2021 08:34
@rco-odoo rco-odoo force-pushed the master-order-read_group-selection-adt branch from ea0cf7f to 0420953 Compare September 6, 2021 07:36
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
@rco-odoo rco-odoo force-pushed the master-order-read_group-selection-adt branch from 0420953 to 637ef29 Compare September 6, 2021 07:40
Copy link
Member

@rco-odoo rco-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

robodoo pushed a commit that referenced this pull request Sep 6, 2021
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 #75856

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
@robodoo robodoo added the 14.5 label Sep 6, 2021
@robodoo robodoo closed this Sep 6, 2021
@robodoo robodoo temporarily deployed to merge September 6, 2021 09:32 Inactive
Elkasitu pushed a commit to odoo-dev/odoo that referenced this pull request Sep 15, 2021
PR odoo#75856 introduced a default implementation of group_expand for
Selection fields that set the value of group_expand to True.

However the PR lacked the necessary bits that allow the same behavior
for fields created on the fly instead of through code.

This commit allows the propagation of the group_expand attribute for
fields of type Selection, this commit also exposes the checkbox in the
UI to enable/disable the setting.
robodoo pushed a commit that referenced this pull request Sep 17, 2021
PR #75856 introduced a default implementation of group_expand for
Selection fields that set the value of group_expand to True.

However the PR lacked the necessary bits that allow the same behavior
for fields created on the fly instead of through code.

This commit allows the propagation of the group_expand attribute for
fields of type Selection, this commit also exposes the checkbox in the
UI to enable/disable the setting.

closes #76592

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Sep 17, 2021
PR odoo#75856 introduced a default implementation of group_expand for
Selection fields that set the value of group_expand to True.

However the PR lacked the necessary bits that allow the same behavior
for fields created on the fly instead of through code.

This commit allows the propagation of the group_expand attribute for
fields of type Selection, this commit also exposes the checkbox in the
UI to enable/disable the setting.

X-original-commit: 1cacc3e
robodoo pushed a commit that referenced this pull request Sep 18, 2021
PR #75856 introduced a default implementation of group_expand for
Selection fields that set the value of group_expand to True.

However the PR lacked the necessary bits that allow the same behavior
for fields created on the fly instead of through code.

This commit allows the propagation of the group_expand attribute for
fields of type Selection, this commit also exposes the checkbox in the
UI to enable/disable the setting.

closes #76775

X-original-commit: 1cacc3e
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
robodoo pushed a commit that referenced this pull request Sep 18, 2021
PR #75856 introduced a default implementation of group_expand for
Selection fields that set the value of group_expand to True.

However the PR lacked the necessary bits that allow the same behavior
for fields created on the fly instead of through code.

This commit allows the propagation of the group_expand attribute for
fields of type Selection, this commit also exposes the checkbox in the
UI to enable/disable the setting.

closes #76775

X-original-commit: 1cacc3e
Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
@fw-bot fw-bot deleted the master-order-read_group-selection-adt branch September 20, 2021 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
14.5 RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants