Skip to content

[10.0][MIG] bi_view_editor: Port BI View Editor to V10 - #106

Merged
JordiBForgeFlow merged 15 commits into
OCA:10.0from
onesteinbv:10_porting_bi_view_editor
Jan 24, 2018
Merged

JordiBForgeFlow merged 15 commits into
OCA:10.0from
onesteinbv:10_porting_bi_view_editor

Conversation

@astirpe

@astirpe astirpe commented Mar 6, 2017

Copy link
Copy Markdown
Member

Based on the porting made for V9 #59

@pankk pankk 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.

Tested on runbot by creating a BI view. Ran into a javascript error (string of undefined), but clicked it away too fast and wasn't able to reproduce this error. For reference: I got this error when going to the pivot view from the pie view.

@pedrobaeza

Copy link
Copy Markdown
Member

Can you maybe squash a bit the commits?

@pedrobaeza

Copy link
Copy Markdown
Member

In a quick test, I see that tables are always joined with INNER JOIN. Can this be configurable?

@astirpe

astirpe commented Jan 17, 2018

Copy link
Copy Markdown
Member Author

@pedrobaeza yes, is possible. The initial idea behind this module is to keep the usability very simple, so adding a checkbox or a select to choose the type of join shouldn't be a problem. The potentials of this module are a lot, so we can add many other features, but then the actual user interface starts to show its limits. It would be nice to implement a more advanced UI, unfortunately I never had enough time for it.
Anyway I would prefer to use this PR only for the pure porting and, after the merge, develop the improvements in a separate PR.

@pedrobaeza

Copy link
Copy Markdown
Member

Please add it to the roadmap then, but I would use LEFT JOIN as default instead of INNER JOIN, as it's the usual way of thinking when building a view: see all starting table records, and let empty those that doesn't have data for the linked tables.

@astirpe

astirpe commented Jan 18, 2018

Copy link
Copy Markdown
Member Author

Rebased and squashed a bit

@LoisRForgeFlow LoisRForgeFlow 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.

Functional review LGTM 👍

@astirpe astirpe changed the title [10.0][MIG] Port BI View Editor [10.0][MIG] bi_view_editor: Port BI View Editor to V10 Jan 22, 2018
@LoisRForgeFlow

Copy link
Copy Markdown
Contributor

@astirpe Maybe I'm lucky... 😄 have you ever migrated using OpenUpgrade a database with this module? I'm facing some issues at the very beginning loading registry cause of this module.

@astirpe

astirpe commented Jan 23, 2018

Copy link
Copy Markdown
Member Author

@lreficent are you talking about V8? Anyway I never migrated databases with this module. Uninstalling and installing again doesn't help?

@JordiBForgeFlow

Copy link
Copy Markdown
Member

my advice is to uninstall in the original version and re-install in the new version.

@LoisRForgeFlow

Copy link
Copy Markdown
Contributor

@astirpe @jbeficent yes, maybe that's the easiest. Thanks!

@yung-wang yung-wang left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested on runbot. I created a simple view consisting of the partner and country tabel. Please merge,

@JordiBForgeFlow
JordiBForgeFlow merged commit 3bbece8 into OCA:10.0 Jan 24, 2018
@astirpe
astirpe deleted the 10_porting_bi_view_editor branch January 24, 2018 18:51
@pedrobaeza pedrobaeza mentioned this pull request Jan 24, 2018
10 tasks
@astirpe
astirpe restored the 10_porting_bi_view_editor branch January 27, 2018 07:49
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.

6 participants