[ADD] crm_enterprise_remove

[woodbrettm]

This module removes any settings marked as enterprise from the crm res_config view.

Cheers,
Brett

@lasley

[lasley]

👍

[elicoidal]

👍

[rvalyi]

👍

[pedrobaeza]

At least this one hasn't been merged. Please remove the SVG file squashing the commits.

[woodbrettm]

svg has been removed.

[dreispt]

I wonder: what is the rationale for this?
Any issue on keeping those config options? They're not visible to regular users.

[lasley]

@dreispt These options are visible to any user with the ability to edit configuration options. This is advertisement in my eyes and Enterprise is not an option for my customers, therefore the options must be removed in order to avoid me having to field annoying questions that have already been answered.

[gurneyalex]

@BMW95 the 9.0 travis build was recently fixed, can you rebase your PR?

[dreispt]

@lasley OK I see the reasoning.
Implementation wise I have a suggestion:

• have a central module, such as base_enterprise_remove;
• on all the xxx_enpterprise_remove module add a dependency to the central module and add the 'auto_install': True flag.

With this, you just need to install the central module to enable the enterprise remove feature.
Odoo will automatically install the specific removal modules according to the Apps actually installed in the instance.

[lasley]

Excellent idea @dreispt - I love it! I have never actually used the auto install feature; does it just trigger when all of the dependencies have been installed or something?

[dreispt]

@lasley auto_install makes it a "glue" module: it gets installed when all it's dependencies are installed. Something like "if base_enterprise_remove is installed and crm is installed, then install crm_enterprise_remove".

[lasley]

@dreispt Oh that's simply sexy - I'm for it. A lot of these modules have already been merged, so we'll need to submit some patches as well. Can you please set the remaining as a WIP?

@BMW95 please create a story for yourself in next sprint to complete this update & let me know if you have any questions.

[dreispt]

@lasley We can do one thing at a time: let them be merged as they are, and then create a PR with this improvement.

[hbrunn]

I know I'm late to the party, but I'm a bit concerned about the proliferation of modules here. And with all of them, the ratio between overhead code and code that actually does something is beyond good and evil.
Can't we use a technique similar to https://github.com/OCA/server-tools/blob/8.0/field_rrule/hooks.py#L8 to do all of this in just one module? The post_load hook won't work here because it's not database specific. But what should be possible is that on every registry reload (_register_hook), you detect which modules are installed, and load the appropriate xml file.

[dreispt]

I see your point about module proliferation, but I'm not sure the added complexity by that solution is worth it.
An alternative would be to have a name scheme for these glue modules to stay next to each other on the module list.
Another is to move them into their own repo.

This last point reminds me how OCA/server-tools quite a big bag right know, and would surely benefit with the spin-off of a couple new repos.
Out of the top of my mind I can think on a few topics:

• OCA/server-brand (Odoo debranding, no phone home,...)
• OCA/server-auth
• OCA/server-orm (fields, search extensions, etc.)

[woodbrettm]

@hbrunn That seems like a good idea. Does the _register_hook run automatically when other modules are installed/uninstalled, or would the removal module have to be updated in order for the _register_hook to run? Also how would we load the extra xml files? Would we just open the manifest file with python and add the new views to the data array, or would we have a master xml file, and just append or remove code to that master file? Not really sure.

[lasley]

I am personally with @dreispt on needing to split this repo up. I was somewhat hinting at this with the issue I created before we submitted all these PRs (#493).

We have things spanning from LDAP auth, to debranding, to importing ODBC, to Gravatar in this repo. IMO, this feels very much like a dumping ground for modules that don't otherwise have a home.

[hbrunn]

@BMW95 _register_hook is called every time the registry is reloaded, which (also) happens when a module is installed/uninstalled. For the loading, you'd let Odoo do the job by calling https://github.com/OCA/OCB/blob/9.0/openerp/tools/misc.py#L134 and https://github.com/OCA/OCB/blob/9.0/openerp/tools/convert.py#L837

@dreispt @lasley yes, this repo was and still is used as the if-it-doesnt-fit-anywhere-else choice, resulting in the current situation. If you have the time to spare, follow the last section of https://odoo-community.org/page/List to set up a new project for more specific repos

[hbrunn]

@BMW95 a helper module I have in mind for a while, but never wrote might come in handy here: base_conditional_import which does a few unholy monkey patches to achieve the following things:

Only depend on a module if it's installed (maybe by a key depends_if_available in the module manifest)

Allow for stanzas like

<if module_installed="modulename1,modulename2">
    <!-- this is only executed if above condition applies /-->
</if>

In a naive implementation, the latter could be enough, but then we run into nasty situations if the module load order doesn't match, and we can only force this to be correct by manipulating the dependencies table.

[dreispt]

@hbrunn Yes, it's not trivial and there can be a few subtleties.

[gurneyalex]

regarding the state of the project, I just started a thread on the tools at odoo-communty.org mailing list.

[oca-clabot]

Hey @BMW95, 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:

• Brett Wood (no github login found)

Appreciation of efforts,
OCA CLAbot

[lasley]

He is covered under LasLabs ECLA.

@BMW95 please resubmit that commit under your corporate email.

[dreispt]

going back to the topic here, 👍

[lasley]

👍 (we have the off-topics planned)

[pedrobaeza]

But what about @hbrunn's proposal of having only one module called base_enterprise_remove that dynamically removes theses options. I think it can be done even in the fields_view_get method. As all setting screens are inherited from res.config, we can overwrite the method at res.config level and check if the opened transient model is one of the possible with enterprise items (this can be done in a base module, as we check through text), and if so, we only need to remove the fields by code in the passed XML architecture.

[woodbrettm]

Hmmm. I think so too, especially since having 10 modules that basically all do the same thing is quite clunky. Also if the Odoo xpath function is updated to conditionally return all the items that xpath returns instead of the first-only (maybe by checking a boolean attribute 'return_multiple' on the xpath tag?), then 10 branches will need to be made with 10 pr's just to update the modules that will basically all run the exact same xpath query in the end, just applied to different views. I just realized on the flip-side, if we add conditional xml existence features either as an addon or to the core, would that decrease the maintainability of Odoo as the years progress? I usually take some extra thought when constructing xpath queries. This is to ensure they return the same item regardless of any external changes made to the xml from other module installations. (I'm not 100% sure yet of how all that works, but I do notice that the base view is separated from all inherited views. Perhaps my concern is unfounded since that separation is how Odoo deals with the issue. If that is the case, then the following concerns may be unfounded as well). Adding conditional existence to sections of views could increase that complexity quite a bit. In addition to any current checks, I would also need to check if there are any conditionals in the inherited views, and then also check if there's any conditionals set in the base view. I think that would be okay in a closed environment, but with Odoo being open source and spreading across so many industries and modules, I'm wondering if conditional xml existence could cause some problems.

[elicoidal]

I agree to the idea of one-module-remove-all option:

• easier to maintain and migrate
• easier for the users to install

[lasley]

@pedrobaeza interesting idea, it seems we do have ourselves a nice central point for the arch generation in res_config_settings.fields_view_get.

Now that I'm looking at the way this is created, I am wondering if there isn't also already a method determining if a module is Enterprise at this level. The question of how the "Enterprise Only" icon is added to the DOM is the key to this I believe.

[pedrobaeza]

Yes, there is! There's a widget that is used for all these fields. It's called 'upgrade_boolean'.

We can use this xml attribute to find and remove all the fields with it.

[hbrunn]

widget="upgrade_boolean" sounds suspicious: https://github.com/OCA/OCB/blob/9.0/addons/website/views/res_config.xml#L95
and https://github.com/OCA/OCB/blob/9.0/addons/web/static/src/js/views/form_widgets.js#L1676 confirms this

[lasley]

And there we have it, thanks guys!

Looking at this JS, things seem to have gotten significantly easier in terms of theory. This all boils into the defined template & get_enterprise_label method in the JS & the related QWeb template for the field. IMO we just found the maintainable & upgradeable nuclear weapon for these fields!

[pedrobaeza]

I still think it's better to remove the field definition at fields_view_get.

[lasley]

True, that would be an actual removal vs. the JS/Qweb approach is really just a hide. We'll implement that direction then.

This ticket will be going on backlog for our next sprint, so we can look at completion in ~1 month. This raises two questions:

1. What shall we do with the already merged modules in the interim?
2. First question will likely answer the second, but we should close the existing PRs in wait for the real module?

[hbrunn]

my .02 EUR:

1. remove them in the same PR where you offer the 'real' module
2. close them referring to this discussion

[lasley]

Hah never thought about how exchange rate works with the fact that my thoughts are only worth .02 USD 😛

Good plan though, @BMW95 please close current PRs referencing this one - to be superseded by solution that avoids module proliferation. Please also create us an internal ticket for next sprint, because I forgot to do that.

[dreispt]

The single module approach is more concise, but AFAICS it's based on a smart hack, and you know what they say about smart code. I would really like to see a proof of concept for it.