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.

Sign up for GitHub

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

Jump to bottom

Master pivot spreadsheet pro rar #4025

Closed
wants to merge 5 commits into from
Closed

Master pivot spreadsheet pro rar #4025

wants to merge 5 commits into from

Conversation

pro-odoo
Copy link
Collaborator

@pro-odoo pro-odoo commented Apr 9, 2024

Description:

description of this task, what is implemented and why it is implemented that way.

Task: : TASK_ID

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Apr 9, 2024

This PR targets the un-managed branch odoo/o-spreadsheet:master-pivot-pro, it needs to be retargeted before it can be merged.

@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 5 times, most recently from d7ddf84 to 496a4ff Compare April 10, 2024 13:00
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 4 times, most recently from 96f8c40 to ce7f6cc Compare April 11, 2024 12:53
@rrahir rrahir force-pushed the master-pivot-spreadsheet-pro-rar branch from ce7f6cc to 96f8c40 Compare April 11, 2024 12:54
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch from 0fbcc09 to ebed644 Compare April 11, 2024 13:32
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch from ba74163 to ece61d6 Compare April 15, 2024 12:11
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 3 times, most recently from ac0f715 to 57929ae Compare April 18, 2024 06:37
@pro-odoo pro-odoo force-pushed the master-pivot-pro branch 5 times, most recently from d2f4d98 to 575f647 Compare April 18, 2024 10:33
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 3 times, most recently from 543266b to a973988 Compare April 18, 2024 12:17
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 2 times, most recently from 0af1b10 to 248d623 Compare April 22, 2024 06:01
VincentSchippefilt
VincentSchippefilt reviewed May 7, 2024
View reviewed changes
src/helpers/pivot/pivot_runtime_definition.ts Outdated Show resolved Hide resolved
src/helpers/pivot/pivot_runtime_definition.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated
const orderedKeys = orderDataEntriesKeys(groups, row);
const spTableRows: SPTableRow[] = [];
for (const value of orderedKeys) {
const _fields = fields.concat(rowName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

this could be moved outside of the loop (probably push rowName into fields)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

probably push rowName into fields

No, fields should not be updated. (Hate recursive functions)

src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
VincentSchippefilt
VincentSchippefilt reviewed May 7, 2024
View reviewed changes
Copy link
Collaborator

@VincentSchippefilt VincentSchippefilt left a comment

Choose a reason for hiding this comment

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

and done! thanks for this fantastic work. We are almost there :)

src/helpers/pivot/spreadsheet_pivot/data_entry_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/date_spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts Outdated Show resolved Hide resolved
src/helpers/pivot/spreadsheet_pivot/spreadsheet_pivot.ts Show resolved Hide resolved
src/plugins/core/pivot.ts
private getPivotCore(pivotId: UID): Pivot {
const pivot = this.pivots[pivotId];
if (!pivot) {
throw new Error(`Pivot with id ${pivotId} not found`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

we should we do with this error? review the revision log and fix it ?

Copy link
Collaborator Author

@pro-odoo pro-odoo May 8, 2024 •
edited

Choose a reason for hiding this comment

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

It should not happen, if it happens, it's a bug and we will have to find the cause to fix it, that could be revision log, but not only

@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch 8 times, most recently from d97fa2b to 0cc9d97 Compare May 14, 2024 05:21
LucasLefevre
LucasLefevre reviewed May 14, 2024
View reviewed changes
src/types/pivot.ts Show resolved Hide resolved
tests/test_helpers/pivot_helpers.ts Outdated Show resolved Hide resolved
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch from 232c115 to fea92ab Compare May 14, 2024 11:02
pro-odoo and others added 3 commits May 15, 2024 07:24
@pro-odoo @rrahir
[REF] functions: create helper for COUNTUNIQUE
e30d1cf 
This commit is a step to introduce the spreadsheet pivot. The function
COUNTUNIQUE is used in the pivot table to count the unique values in a
range. The helper function is created to avoid code duplication.

Task: 3748717

Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
@pro-odoo @rrahir
[REF] functions: create helper for AND and OR
d997c16 
This commit is a step to introduce the spreadsheet pivot. The functions
AND and OR are used in the pivot to compute the values when the measure
is a boolean.

Task: 3748717

Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
@pro-odoo @rrahir
[IMP] side_panel: can compute title based on props
01d352e 
With this commit, the function to compute the title of the side panel
can now takes props as argument. This is useful to compute the title
based on the props of the component.

Task: 3748717

Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch from fea92ab to 67d276d Compare May 15, 2024 07:19
pro-odoo and others added 2 commits May 15, 2024 10:17
@pro-odoo @rrahir
[ADD] pivot: introduce spreadsheet pivot table
1ffcbf8 
This commit introduces the spreadsheet pivot table. It is a new type of
pivot table that is based on values from the spreadsheet itself.

The pivot table is created by selecting a range of cells in the spreadsheet.
The first row of the range is used as the available fields, and the rest
of the range is used as the data.

The type of the data is automatically detected based on the content of
the cells in the column. It could be `date` if all the cells in the column
are dates, `boolean` if all the cells are boolean, `number` if all the
cells are numbers, or `char` otherwise.

Each field can be used as a dimension (row or column) or as a measure
(value), based on its type. The user can drag and drop the fields to
the corresponding area in the pivot table side panel, select the order,
select the granularity if the field is a date. Each measure can be
aggregated using different functions (sum, average, count, etc.).

For now, the pivot table is updated in real-time when the user changes
the range of cells in the spreadsheet. This could be heavy for large
spreadsheets, but a flag will be implemented to delay the update in a
future task (3897841).

This commit is a first version of the pivot table. Features are missing
compared to the pivot table implemented in Odoo, for example the
possibility to use `PIVOT.HEADER` and `PIVOT.VALUE` functions (and all
the features that come with it, autocompletion of formulas, positional
arguments, etc.). This will be implemented in a future task (3897857).

Task: 3748717

Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
@pro-odoo
[FIX] pivot: deactivate support for exploded pivot
4e7eff0 
For now, the spreadsheet pivot does not fully support PIVOT.VALUE and
PIVOT.HEADER functions, so we prefer to disable it. It will be reactivate
later.

List of features that are not supported:
- Normalization of arguments, especially with the dates
- Positional arguments (with #)

Task: 3748717
@pro-odoo pro-odoo force-pushed the master-pivot-spreadsheet-pro-rar branch from 67d276d to 4e7eff0 Compare May 15, 2024 08:17
LucasLefevre
LucasLefevre approved these changes May 15, 2024
View reviewed changes
Copy link
Collaborator

@LucasLefevre LucasLefevre left a comment

Choose a reason for hiding this comment

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

robodoo r+ rebase-ff ça a l'air bien :)

@robodoo
Copy link
Collaborator

robodoo commented May 15, 2024

Merge method set to rebase and fast-forward.

robodoo pushed a commit that referenced this pull request May 15, 2024
@pro-odoo @rrahir
[REF] functions: create helper for COUNTUNIQUE
51336c1 
This commit is a step to introduce the spreadsheet pivot. The function
COUNTUNIQUE is used in the pivot table to count the unique values in a
range. The helper function is created to avoid code duplication.

Task: 3748717
Part-of: #4025
Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
robodoo pushed a commit that referenced this pull request May 15, 2024
@pro-odoo @rrahir
[REF] functions: create helper for AND and OR
9bd97d9 
This commit is a step to introduce the spreadsheet pivot. The functions
AND and OR are used in the pivot to compute the values when the measure
is a boolean.

Task: 3748717
Part-of: #4025
Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
robodoo pushed a commit that referenced this pull request May 15, 2024
@pro-odoo @rrahir
[IMP] side_panel: can compute title based on props
fecb914 
With this commit, the function to compute the title of the side panel
can now takes props as argument. This is useful to compute the title
based on the props of the component.

Task: 3748717
Part-of: #4025
Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
robodoo pushed a commit that referenced this pull request May 15, 2024
@pro-odoo @rrahir
[ADD] pivot: introduce spreadsheet pivot table
6e447f0 
This commit introduces the spreadsheet pivot table. It is a new type of
pivot table that is based on values from the spreadsheet itself.

The pivot table is created by selecting a range of cells in the spreadsheet.
The first row of the range is used as the available fields, and the rest
of the range is used as the data.

The type of the data is automatically detected based on the content of
the cells in the column. It could be `date` if all the cells in the column
are dates, `boolean` if all the cells are boolean, `number` if all the
cells are numbers, or `char` otherwise.

Each field can be used as a dimension (row or column) or as a measure
(value), based on its type. The user can drag and drop the fields to
the corresponding area in the pivot table side panel, select the order,
select the granularity if the field is a date. Each measure can be
aggregated using different functions (sum, average, count, etc.).

For now, the pivot table is updated in real-time when the user changes
the range of cells in the spreadsheet. This could be heavy for large
spreadsheets, but a flag will be implemented to delay the update in a
future task (3897841).

This commit is a first version of the pivot table. Features are missing
compared to the pivot table implemented in Odoo, for example the
possibility to use `PIVOT.HEADER` and `PIVOT.VALUE` functions (and all
the features that come with it, autocompletion of formulas, positional
arguments, etc.). This will be implemented in a future task (3897857).

Task: 3748717
Part-of: #4025
Co-authored-by: rrahir <rar@odoo.com>
Co-authored-by: Pierre Rousseau <pro@odoo.com>
robodoo pushed a commit that referenced this pull request May 15, 2024
@pro-odoo
[FIX] pivot: deactivate support for exploded pivot
2800874 
For now, the spreadsheet pivot does not fully support PIVOT.VALUE and
PIVOT.HEADER functions, so we prefer to disable it. It will be reactivate
later.

List of features that are not supported:
- Normalization of arguments, especially with the dates
- Positional arguments (with #)

closes #4025

Task: 3748717
Signed-off-by: Lucas Lefèvre (lul) <lul@odoo.com>
@robodoo robodoo closed this May 15, 2024
@robodoo robodoo added the 17.3 label May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VincentSchippefilt VincentSchippefilt VincentSchippefilt left review comments

@hokolomopo hokolomopo hokolomopo left review comments

@anhe-odoo anhe-odoo anhe-odoo left review comments

@LucasLefevre LucasLefevre LucasLefevre approved these changes

Assignees
No one assigned
Labels
17.3
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@pro-odoo @robodoo @VincentSchippefilt @LucasLefevre @hokolomopo @anhe-odoo