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

[12.0][ADD] web_pivot_computed_measure #1547

Merged
merged 1 commit into from Mar 23, 2020

Conversation

Tardo
Copy link
Member

@Tardo Tardo commented Mar 12, 2020

Adds support for computed measures on the pivot view.

Go to pivot view and click on the "Measures" menu, you will see a new option called 'Computed Measure'.
These measures can be mixed and save as favorite.

measures

cc @Tecnativa TT22499

@pedrobaeza pedrobaeza added this to the 12.0 milestone Mar 12, 2020
@pedrobaeza pedrobaeza changed the title [ADD] web_pivot_computed_measure [12.0][ADD] web_pivot_computed_measure Mar 12, 2020
@pedrobaeza
Copy link
Member

@ged-odoo I think this is something that is going to interest you.

web_pivot_computed_measure/README.rst Outdated Show resolved Hide resolved
web_pivot_computed_measure/README.rst Show resolved Hide resolved
web_pivot_computed_measure/README.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@ivantodorovich ivantodorovich left a comment

Choose a reason for hiding this comment

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

Nice PR! 👍

Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Tested functionnaly with purchase module (and report)
That is a very great job ! Thanks !

It is finally fixing Odoo bug, on average values ! (see bellow with the average price value).

  • Odoo is wrong,
  • With that module, the value is OK

image

👍 Functional Review. (+ light code review).

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza pedrobaeza requested a review from yajo March 17, 2020 20:02
Copy link
Contributor

@legalsylvain legalsylvain left a comment

Choose a reason for hiding this comment

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

Hi.
I tried again your module, and in a different way, the loading of the computed measures from the context is not done properly.

Step to reproduce :

  • Add a computed measures in the context of an action like this :
    image
  • go the menu that enable the action : KO : the field is not computed
    image
  • add any filter, domain, group by, etc... : OK : the field is computed
    image

@pedrobaeza
Copy link
Member

Well, this is a bit out of scope, as the initial intention is not to load that computed measures from the context. @Tardo check a bit if it's simple to be done, but if not, we will just annotate that on the known issues.

@legalsylvain
Copy link
Contributor

Well, this is a bit out of scope, as the initial intention is not to load that computed measures from the context.

Well, that is not the use case proposed by this module, but this is exactly the intention of this module, as the context is the tools used by this module to save and restore computed measures. ;-)

  • It works from filter context
  • It half works from action context. (the column is displayed, but the computation is not done).

@pedrobaeza
Copy link
Member

As said, if easy to be added, we'll do it. If not, we have set the base, other contributors can add that support.

Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

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

It's a pleasure to read your JS code, 👏 👏

Please remove some 🗑️, and one DIV/0 question.

@Tardo Tardo force-pushed the 12.0-add-web_pivot_computed_measure branch from 9be67e9 to 48a69be Compare March 20, 2020 17:45
@Tardo
Copy link
Member Author

Tardo commented Mar 20, 2020

Thanks for comments and reviews! ❤️

@legalsylvain i can't reproduce your issue :/
pivot_action

@legalsylvain
Copy link
Contributor

@Tardo : thanks a lot for your screenshot. indeed, there is no bug in your module. my context was wrong. (forgot pivot_measures key).

image
Now ok.

@yajo
Copy link
Member

yajo commented Mar 23, 2020

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1547-by-Yajo-bump-no, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

- Integer
- Float
- Percentage (value * 100)
- Formula*: Custom operation formula
Copy link
Member

Choose a reason for hiding this comment

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

Is this always visible?

@OCA-git-bot OCA-git-bot merged commit 9f295f5 into OCA:12.0 Mar 23, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 2a4b3a6. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants