Skip to content

Conversation

@gavrelj
Copy link

@gavrelj gavrelj commented Oct 19, 2015

This module allows using several analytic dimensions through a structure related to an object model.

See the README.rst file for informations about how to integrate analytic functionalities into a model.

Alexandre Allouche and others added 30 commits October 19, 2015 17:10
new group group_ans_manager, people of this group can create analytic structure
… analytic_structure class.

These methods should be called by models that implement analytic fields, respectively in their fields_get and fields_view_get methods.
…y, etc. in

the view.

When updating the invisible and tree_invisible modifiers, the analytic_fields_view_get method used to override the entire "modifiers" attribute. This isn't the case anymore.
@oca-clabot
Copy link

Hey @gavrelj, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/website.cla
Here is a list of the users:

  • @gavrelj (login unknown in OCA database)

Appreciation of efforts,
OCA CLAbot

@gavrelj
Copy link
Author

gavrelj commented Oct 19, 2015

This module was merged from the XCG repositories at the PyConFR 2015. It requires base_model_metaclass_mixin to function. See pull request #273

@dreispt
Copy link
Member

dreispt commented Oct 19, 2015

Very interesting. AFAICS this implements true multi-dimensions like Ms NAV and Oracle.
I wonder if this shouldn't be best hosted at the account-analytic repo?
If we want to keep both modules in the same repo, I would rather have also base_model_metaclass_mixin in the analytic repo...

Thoughts?

@pedrobaeza
Copy link
Member

I also see this for account-analytic

@dreispt
Copy link
Member

dreispt commented Oct 19, 2015

Challenge: I wonder if the base_model_metaclass_mixin is really needed? Maybe there's an alternate implementation that drops it...

@houzefa-abba
Copy link
Member

Hey,

I'm not the best person to explain, but basically: this is not (explicitly) related to accounting, and also not related to Odoo analytics. This module should work fine alongside Odoo analytics, but using both would be functionally useless.

That is why, with Alexandre Fayolle, we figured server-tools would be a better fit for analytic_structure.

@dreispt
Copy link
Member

dreispt commented Oct 20, 2015

I see, I didn't notice that.
IMO, despite not having any dependency on analytic, people looking for this type of feature will look into the account-analytic rather than here.
But let's hear other opinions.

@dreispt
Copy link
Member

dreispt commented Oct 20, 2015

I had a look at the code, and there's a lot of magic going there, to dynamically inject fields in other models and views. I'm afraid it's too much for my Python skills.

I wonder if there isn't a way to achieve the same effect in a less "invasive" way?
Even if we had to sacrifice some of the dynamic configuration possibilities, and have the "dimension awareness" be added to models by auto_install helper modules.

It might not be a masterpiece, but it would allow to significantly cut down the complexity and line count.

@houzefa-abba
Copy link
Member

Hey Daniel,

Thanks for looking closer into this! Regarding your 2 points:

  • After some discussion, it appears the best place to be putting analytic_structure and addons that make use of it (such as accounting, purchase, sales, hr & so on enhancements) would be a pack of vertical addons that could be called something like "matrix analytics" or (:D) "serious / real-world accounting"... Still to be thought about...
    Regardless, please continue reviewing this request, even if it does not eventually get into server-tools; it helps us provide correct README / linting / tests / etc.
  • About the metaclass, we initially used a static way: we would add "a1_id" to "a5_id" fields in each Odoo model. As you can guess, that quickly became a mess to maintain; and we could not have more than 5 dimensions per Odoo model...
    IMHO, the metaclass isn't that huge and alternatives would much more hacky... To be discussed.... I'll be sure @gavrelj (who has done most of the work in that regard) sees your suggestion.

@dreispt
Copy link
Member

dreispt commented Oct 21, 2015

Thanks for being open for a design discussion.
From a theoretical PoV, what do you think of this:

  • Have a Model to store the Analytic Dimension Combinations
  • Dimension aware models have a Many2one field to the Combinations table, with the delegate=True flag

The delegate flag makes all fields in the related table available to the Model as if they were it's own fields.
When you create a new record in the dimension aware model, it automatically creates a related record in the Combinations table.
If you add a new field to the Combinations table, it will be automatically available to the dimension-aware models.

It doesn't automatically add them to the views. You can keep view_get magic for that, but I'm not sure if that's really needed in most use cases.

For example , Ms NAV usually has only Dim1 and Dim2 directly available in forms. To see the full dimension combination you have a button opening a popup navigate to a popup window. A similar feature would be simple to implement in Odoo. And IMO you can customize views to add extra dimensions to be available directly in the main form.

snap 2015-10-21 at 09 29 26

The view_getmagic, if implemented, could be left optional, available in a separate module.

@faide
Copy link

faide commented Oct 21, 2015

Hi Daniel,

thanks for your review. I am happy you see this as "true" multi dimensional analytics :) This is indeed inspired by our experience with true accounting & analytics :)

Getting to the point: I think removing the metaclass adds a burden on the developper putting him in charge of the maintenance of a running system and moreover putting him in the critical path of the functionnal teams who typically designs and implements the analytic matrix without asking developper help in our deployment scenario.

I don't see the metaclass as a big hassle regarding the fact that it gives you dynamic addition of dimensions in your structure without any development.

Maintaining multi dimensionnal analytics and multi currency systems is (somewhat) complex and will require a certain level of dedication that we are ready to maintain. You can see these modules have been around for a long time and we constantly improve and bugfix on them (and port the from one version to another).

From the functionnal team perspective it would be perceived as a regression to be forced to ask a developper to add dimension columns to the analysed object. And I don't see how we could achieve the same level of dynamicity without a metaclass.

To tell you the truth I challenged the metaclass proposition a lot when it first popped up in our team. The first implementation I did (not in this repos) was without metaclasses and limited to 5 dimensions per object. When my team came to me proposing to become more dynamic with the use of metaclass I was reluctant for the same reasons as you are (level of entry for maintenance). But after challenging the team they convinced me. I never regretted my decision to allow them to use metaclasses for this purpose.

Metaclass programing is not that hard and gives a real advantage in that case.

@dreispt
Copy link
Member

dreispt commented Oct 21, 2015

@faide That's a constructive discussion. Please bear with me challenging the approach a little more.
So a design goal is to functional consultants to be able to setup n dimensions without requiring developer work.

At the model level, I argue that my Many2one delegated field solution achieves that:
The related "Dimension Combinations" table can have two designs: columns (more similar to current approach, I believe) or rows (like NAV).
Assuming the columns solution, functional consultants can add x_ columns to it through the GUI. The delegation mechanism should then make them available in all dimension aware models (never tried that through the GUI, but at worst it should work after a module upgrade or a server restart).

At the Views level, adding a field to a form view XML is not that complicated. But supposing that you still object that, we could keep the existing approach of view _get magic to inject fields into the view. As mentioned, I would implement this in a separate companion module.

Wrapping up, my point is that wisely used delegation inheritance can replace the field injection metaclass.

@faide
Copy link

faide commented Oct 22, 2015

@dreispt thanks for taking time to help us review this module. I appreciate this and the constructive approach you take on it.

I'll try to clear up some points I may not have correctly expressed.

Our design goal was:

  • allow a functional consultant to specify any number of "analytic dimensions"
  • allow a functional consultant to specify the analytic structure of any object (provided a developer already made that object "analyzable" (ATM: through our metaclass)
  • declare some business objects of the system as "analytic codes" which thus generate analytic codes in their respective analytic dimension
  • when a functional consultant designs dimensions & matrix everything should be done without coding nor "technical skill" (know models & views)
  • the analytic codes should be stored "in line". This means that is I select from the table I get the code_id. This is an important point because "analytics" is to be exploited down the road for reporting purposes and we want to ensure data coherence between the line being analyzed and its analytics.

Using delegate=True on a many2one is the new way for inherits (iirc). This means the data is not stored "in line" but in another table. If we wanted to use this mechanism for eg: account.move.line we would have a new object spawned for every line in the system linked to the account.move.line

This would make things slower when creating a lot of account.move.line and things slower and harder on the reporting side (end user of analytics is always a report designer)

For the field_get injection what kind of companion module would you see? The metaclass permits to auto inject the field_get mechanism on all analyzable object without ever implementing the field get by yourself. This represents a huge time and maintenance boost for the integrator.

@dreispt
Copy link
Member

dreispt commented Oct 22, 2015

@faide And what about inheriting from an AbstractModel, like what it's done in the core to enable messaging features in Models. That inheritance style adds the extra field directly on each model.

A non-technical person would add the needed fields to the AbstractModel, which is as simple as filling a form to create a record (probably followed by module upgrades). AFAICS, at the model level this achieves your goals.

@okuryan
Copy link

okuryan commented Feb 18, 2016

Any news about this PR? As runbot has errors - even not possible to test it. Also will there be any instructions on testing?

@hbrunn
Copy link
Member

hbrunn commented Oct 4, 2016

I only looked at the code superficially, but this looks like having a lot of potential! I'd suggest to get started by making it testable on runbot, this way, you should get functional people in the boat. Also consider adding some self-demo features on runbot like https://github.com/OCA/server-tools/blob/8.0/field_rrule/hooks.py#L10

@houzefa-abba
Copy link
Member

Thanks for the review and test suggestions - for now though, this addon does not install in travis because it depends on base_model_metaclass_mixin, which is PR #273.

@houssine78
Copy link

any news on this PR somewhere? Does it inclusion under OCA umbrella is abandoned?

@yajo
Copy link
Member

yajo commented Jan 26, 2018

AFAICS, 3 years later there's still no signed CLA and CI is still ❌, so closing. Feel free to reopen if needed.

@yajo yajo closed this Jan 26, 2018
SiesslPhillip pushed a commit to grueneerde/OCA-server-tools that referenced this pull request Nov 20, 2024
Syncing from upstream OCA/server-tools (8.0)
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.