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

[FIX] pivot: apply format on measure only a basic formula #5299

Closed
wants to merge 1 commit into from

Conversation

LucasLefevre
Copy link
Collaborator

@LucasLefevre LucasLefevre commented Dec 3, 2024

Description:

Let's say I have a computed measure "weighted delay:sum", on which I applied an integer format.

I later use it in a formula such as:

=PIVOT.VALUE(1,"weighted delay:sum","#user_id",A3)/PIVOT.VALUE(1,"__count:sum","#user_id",A3)

This is no longer an integer. If I change the format on this formula ☝️, the format is applied on the entire measure, which is not what I want.

With this commit, we apply the format on the measure (rather than on the cell)
if and only if the value comes from a spilled PIVOT formula.

Task: 4377411

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 Dec 3, 2024

Pull request status dashboard

Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

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

👋

Comment on lines 100 to 102
if (!cell?.isFormula) {
return true;
}
Copy link
Contributor

Choose a reason for hiding this comment

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

"If the cell is not a formula, the cell is a basic pivot value formula" ?

@@ -43,7 +46,7 @@ export class FormatPlugin extends UIPlugin {
for (let row = zone.top; row <= zone.bottom; row++) {
const position = { sheetId, col, row };
const pivotCell = this.getters.getPivotCellFromPosition(position);
if (pivotCell.type === "VALUE") {
if (this.isBasicPivotValueFormula(position, pivotCell)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

TBH I would just disable the functionnality on non spreaded pivot formula. It doens't make much sense for splitted formulas to me and sounds very much like black magic that people will not really understand.

I don't expect changing the format on a single =PIVOT.VALUE cell that I put alone on a random sheet to affect the format of the whole pivot on all the other sheet. The point of having splitted pivot is that I can do things on it independantly from the pivot no ?

@LucasLefevre LucasLefevre force-pushed the 18.0-measure-format-basic-lul branch 3 times, most recently from 989a17a to d7b4868 Compare December 12, 2024 08:16
Let's say I have a computed measure "weighted_delay:sum", on which I applied
an integer format.

I later use it in a formula such as:

=PIVOT.VALUE(1,"weighted_delay:sum","#user_id",A3)/PIVOT.VALUE(1,"__count:sum","#user_id",A3)

This is no longer an integer. If I change the format on this formula ☝️, the
format is applied on the entire measure, which is not what I want.

With this commit, we apply the format on the measure (rather than on the cell)
if and only if the value comes from a spilled PIVOT formula.

Task: 4377411
@LucasLefevre LucasLefevre force-pushed the 18.0-measure-format-basic-lul branch from d7b4868 to 68eb4c6 Compare December 12, 2024 08:17
@rrahir
Copy link
Collaborator

rrahir commented Dec 12, 2024

@robodoo r+

@robodoo robodoo closed this in 8a18d36 Dec 12, 2024
@fw-bot fw-bot deleted the 18.0-measure-format-basic-lul branch December 26, 2024 08:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants