Skip to content

[12.0][MIG] bi_view_editor - #293

Merged
OCA-git-bot merged 13 commits into
OCA:12.0from
onesteinbv:12_mig_bi_view_editor
Nov 29, 2019
Merged

OCA-git-bot merged 13 commits into
OCA:12.0from
onesteinbv:12_mig_bi_view_editor

Conversation

@astirpe

@astirpe astirpe commented Jun 6, 2019

Copy link
Copy Markdown
Member

This PR is the porting of bi_view_editor to V12 with the following improvements:

  • Store the JSON data structure in ORM
  • Improved robustness
  • Fix of fields/models translation
  • Add check for consistency of groups
  • Development of new tests
  • Add LEFT JOIN capabilities
  • Add sums and avg capabilities for tree views
  • Provide graph view for table relations

@pedrobaeza pedrobaeza added this to the 12.0 milestone Jun 6, 2019
@OCA-git-bot OCA-git-bot mentioned this pull request Jun 6, 2019
16 tasks

@MiquelRForgeFlow MiquelRForgeFlow left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

code review 👍

Comment thread bi_view_editor/models/ir_model.py Outdated
@astirpe

astirpe commented Jun 12, 2019

Copy link
Copy Markdown
Member Author

I'm adding tests but the codecov/patch is not increasing. The coverage is actually much higher, around 90%: https://codecov.io/gh/OCA/reporting-engine/tree/e99acf4ad56455afe6574307ac9712a3ed429cf6/bi_view_editor

@astirpe

astirpe commented Jun 14, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza do you know why the codecov/patch percentage does not increase?

@patel1008b

patel1008b commented Jul 2, 2019

Copy link
Copy Markdown

@astirpe @pedrobaeza @mreficent what do you think it will merge in the main branch. It's Badly required for one of project.

Thanks for this PR.

@astirpe

astirpe commented Jul 2, 2019

Copy link
Copy Markdown
Member Author

@bhaveshselarka this PR is not yet merged because of one extra review is still missing. You may want to test this PR on Runbot and report here the results. That would speed up the approval process. Thanks!

@MiquelRForgeFlow

MiquelRForgeFlow commented Jul 11, 2019

Copy link
Copy Markdown
Contributor

I have a tested two migrations from v11. Without last commit, no problem; with last commit, it breaks. Please, remove last commit. That monkey patch of the last commit is not obsolete.

@astirpe

astirpe commented Jul 11, 2019

Copy link
Copy Markdown
Member Author

Thank you @mreficent !

@patel1008b

Copy link
Copy Markdown

@astirpe @pedrobaeza @mreficent what do you think it will merge in the main branch. It's Badly required for one of project.

Thanks for this PR.

@pedrobaeza

Copy link
Copy Markdown
Member

@bhaveshselarka why don't you review the PR, try on runbot and give your feedback for speeding up the process?

@patel1008b

Copy link
Copy Markdown

Yes, @pedrobaeza I think its best idea, Let me do that stuff.

I am at currently: https://runbot.odoo-community.org/runbot/repo/github-com-oca-reporting-engine-143

Where I can find a find this PR's correct instance. Can you please help me out in that?

@pedrobaeza

Copy link
Copy Markdown
Member

On the PR itself:
Selección_008
I have launched a rebuild, as the instance has expired. Wait for the completion and you'll be able to access through the blue button. More details in https://odoo-community.org/page/review

@HviorForgeFlow HviorForgeFlow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@patel1008b

Copy link
Copy Markdown

@pedrobaeza @hveficent @astirpe @mreficent will it merge in the main branch ?

@astirpe

astirpe commented Nov 26, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza the codecov is red but the real coverage is much higher, see also my #293 (comment). There's something wrong with codecov, so for me the actual coverage is more than acceptable.

This PR has two approvals since a long time and I don't think we will get soon any other approvals. Is it okay to merge?

@pedrobaeza

Copy link
Copy Markdown
Member

Please squash a bit the commits and I'll proceed to merge. The codecov patch is base on previous commits done, so if you have done several pushes, it can get confused by that.

astirpe and others added 12 commits November 26, 2019 12:58
* Add menu items creation feature
* Added selection of fields of a tree view
* Improved usability and strings made translatable
* Avoid display duplicated nodes
* Robustness
* Updated Dutch translation
* Avoid possible sql injection in bi_view_editor
* Removed deprecated RegistryManager
Apostrophe in model name raised ValueError. Added needed migration script.
Add LEFT JOIN capabilities

Add sums and avg capabilities for tree views

Robustness and code review

Provide ER diagram view for table relations
@astirpe

astirpe commented Nov 26, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza done!

@astirpe

astirpe commented Nov 29, 2019

Copy link
Copy Markdown
Member Author

@pedrobaeza would be great if we can get this PR merged. Thank you!

@pedrobaeza

Copy link
Copy Markdown
Member

/ocabot merge

@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

OCA-git-bot added a commit that referenced this pull request Nov 29, 2019
Signed-off-by pedrobaeza
@OCA-git-bot
OCA-git-bot merged commit 53f89ad into OCA:12.0 Nov 29, 2019
@OCA-git-bot

Copy link
Copy Markdown
Contributor

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

@astirpe
astirpe deleted the 12_mig_bi_view_editor branch November 30, 2019 10:50
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.

10 participants