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

[ADD] subreports #256

Merged
merged 2 commits into from
Mar 9, 2020
Merged

Conversation

sbidoul
Copy link
Member

@sbidoul sbidoul commented Feb 11, 2020

Closes #155

With this PR, you can add named "sub reports" on a report template.
When that is done, KPIs from sub reports can be used in expressions of the the parent report with the following syntax <subreport name>.<kpi name>.

The code is WIP and will be refactored, but the concept and data model should be ok.

cc/ @fclementic2c @mileo @JordiBForgeFlow

@sbidoul sbidoul added the 10.0 label Feb 11, 2020
@sbidoul sbidoul mentioned this pull request Feb 11, 2020
5 tasks
@mileo
Copy link
Member

mileo commented Feb 11, 2020

Hi, @sbidoul thanks!

I ran the tests on the runbot, it seems to be working well.

http://3408304-256-4348a7.runbot1.odoo-community.org/web#id=4&view_type=form&model=mis.report.instance&active_id=4

However, I don't know if I didn't know how to configure it, but the budget columns didn't work.

Another very interesting feature that we did in the PR of v8, was the possibility for the user, when viewing the report, to navigate to the subreport by clicking on the cell. https://github.com/OCA/mis-builder/pull/153/files#diff-3678f60340076198a6cc144a9fa4a107L37

Can we help you in any way?

Best regards

@sbidoul
Copy link
Member Author

sbidoul commented Feb 11, 2020

the budget columns didn't work.

Yes, that's expected. That should work after the refactoring.

Regarding the drilldown I'm not sure. Currently the drilldown shows move lines or budget lines. I tend to think showing other reports could be confusing because there could be many of them for different periods, or different preset analytic filters. Maybe something could be possible with the the features that generates reports instances on the fly. Need to think about it.

Can we help you in any way?

Thanks for proposing! Let me go through the refactoring first.

@sbidoul sbidoul force-pushed the 10.0-subreport-sbi branch 2 times, most recently from db23f5b to b309b43 Compare February 15, 2020 18:50
@sbidoul sbidoul added this to the Version 3.6 milestone Feb 16, 2020
@sbidoul sbidoul force-pushed the 10.0-subreport-sbi branch 3 times, most recently from e71ad42 to 38d9912 Compare February 21, 2020 17:32
@sbidoul sbidoul marked this pull request as ready for review February 21, 2020 18:13
@sbidoul
Copy link
Member Author

sbidoul commented Feb 21, 2020

@mileo in the end, budget with subreports is not that obvious. Budget by account will work though.
For budget by KPIs to work with subreports, we probably need to extend mis.budget to either create a budget that allows budgeting KPIs from all subreports. Or make the budget source a m2m so the user can provide a budget for each subreport when defining a budget column. And that probably requires finding a good design that allows combining budget by account and budget by KPI in a single report. So that definitely requires more design thinking before doing a PR.

So this specific PR is done :)

Copy link
Member

@fclementic2c fclementic2c left a comment

Choose a reason for hiding this comment

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

I tested a reports fetching KPIs from 3 others subreports : test ok
Fantastic :)

sbidoul added 2 commits March 9, 2020 11:12
not UserError.

Also, remove redundant onchange warnings.
@sbidoul sbidoul force-pushed the 10.0-subreport-sbi branch from d8a9afa to 3fa1058 Compare March 9, 2020 10:13
@sbidoul
Copy link
Member Author

sbidoul commented Mar 9, 2020

Merging into the 3.6 staging branch. People interested to test further can do so in #254

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 10.0-3.6-staging-ocabot-merge-pr-256-by-sbidoul-bump-no, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 5d7a902 into OCA:10.0-3.6-staging Mar 9, 2020
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at b0f71c4. 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.

5 participants