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

Replacement of read_group by _read_group in v16 to v17 Migration is not actually expected #90

Closed
xmglord opened this issue Mar 14, 2024 · 3 comments
Labels
bug Something isn't working

Comments

@xmglord
Copy link

xmglord commented Mar 14, 2024

Upon reviewing the functionalities of both methods as per the latest Odoo documentation, it appears that this change might not be accurate due to the distinct purposes and functionalities that each method serves.

  1. Method Signatures and Purposes:
  • Model._read_group: This method is primarily designed for getting fields aggregations, specified by aggregates, and grouped by the given groupby fields where records are filtered by the domain. It supports advanced aggregations and is tailored for more complex group-by queries.

  • Model.read_group: This method focuses on retrieving a list of records in a list view, grouped by specified groupby fields. It is designed to be used for generating grouped views in the UI, supporting a simpler and more direct way of aggregating data for presentation.

  1. Functional Differences:
  • Parameters and Return Types: The Model._read_group method allows for a detailed specification of aggregates and having clauses, catering to complex SQL-like aggregations. In contrast, Model.read_group is structured around the list view's needs, providing a simpler interface for aggregating and displaying data.

  • Use Cases: The usage of _read_group is more aligned with backend data processing and complex data aggregation scenarios. On the other hand, read_group is tailored towards frontend data presentation, particularly in grouping data for display in Odoo's list views.

Given these distinctions, replacing read_group by _read_group can potentially disrupt the intended functionality of modules that rely on these methods for specific purposes. The direct replacement overlooks the nuanced differences in their applications, leading to possible issues in data presentation and aggregation in migrated modules.

@xmglord xmglord added the bug Something isn't working label Mar 14, 2024
@xmglord
Copy link
Author

xmglord commented Mar 14, 2024

@bruno-zanotti and @jjscarafia please review.

@luisg123v
Copy link

FYI this is related to #82

CC @desdelinux, @moylop260

@xmglord xmglord changed the title Replacement of read_group with _read_group in v16 to v17 Migration is not actually expected Replacement of read_group by _read_group in v16 to v17 Migration is not actually expected Mar 14, 2024
@luisg123v
Copy link

After reviewing the main motivation for this change (odoo/odoo#110737), I consider that the replace of read_group by _read_group is actually expected in almost most changes. This because read_group was initially meant to be used by the web client, but we started to use it in backend to group data. Now _read_group serves that purpose better and on a more-efficient manner.

So @xmglord I think this issue should be closed.

Regards,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants