[9.0][IMP] Add res_config_settings_enterprise_remove

[woodbrettm]

Hey folks,

As per the discussion in pull request #499, I have merged all of the enterprise_remove modules into a single one and instead of marking the upgrade_boolean fields as invisible, they are now removed from the view. Overview of the process is as follows:

1. Remove any individual labels that are not directly associated with the upgrade_boolean field using specific xpath queries.
2. Bulk remove any upgrade_boolean fields and their respective label.
3. Loop through the page until no empty divs are found (note that groups are kept intact).

Because of the nature of this module, I took special care in constructing the xpath queries and functionally testing for issues. However should other modules use xpath queries that target any of those fields, this module could cause issues, so I made note of that in the readme.

Cheers,
Brett

@lasley

[pedrobaeza]

Weren't you able at the end to remove all elements with the widget?

[woodbrettm]

@pedrobaeza here's an example:

                    <group string="Shipping Connectors" name="shipping">
                        <label for="id" string="Carriers"/>
                        <div>
                            <div>
                                <field name="module_delivery_dhl" widget="upgrade_boolean"/>
                                <label for="module_delivery_dhl"/>
                            </div>
                            <div>
                                <field name="module_delivery_fedex" widget="upgrade_boolean"/>
                                <label for="module_delivery_fedex"/>
                            </div>
                            <div>
                                <field name="module_delivery_ups"  widget="upgrade_boolean"/>
                                <label for="module_delivery_ups"/>
                            </div>
                            <div>
                                <field name="module_delivery_usps" widget="upgrade_boolean"/>
                                <label for="module_delivery_usps"/>
                            </div>
                        </div>
                    </group>

Here, first the <label for="id" string="Carriers"/> is removed using a specific xpath query, then the upgrade_boolean fields with their sibling labels are removed, and then the divs are all removed, checking to see if the individual div contains any content. If I loop through the upgrade_boolean fields and their respective labels only, then a lot of clutter is left behind. However if we opt to keep labels such as the <label for="id" string="Carriers"/>, that would definitely cut down on the individual queries quite a bit.

[elicoidal]

I would add a note such as:
"This module does not remove the possibility to install the Enterprise modules: if necessary, all the code of Enterprise Edition is still available and can be manually installed via the Apps menu."

[woodbrettm]

@elicoidal sure thing :)

[elicoidal]

some details: other than that 👍
Thanks for the contribution!

[lasley]

@BMW95 - Is there not always the same number of parents to an upgrade_boolean widget? Seems like we could identify that number, select by upgrade_boolean then remove X preceding up the chain?

[pedrobaeza]

I think we can remove the parent div safely.

[woodbrettm]

@lasley @pedrobaeza Here's an idea. I didn't realize the return value from super contained a 'name' key which allows sorting by view name. What about if I took all the xpath queries from the single modules, added them to this one, and then filter by view name? That way each view has its own unique queries, and the only extra step is the initial filtering. Some upgrade fields are 2 divs deep, some 1 div, some 1 div with siblings that are not enterprise, some pages have different structures in different areas where the upgrade fields are, etc, so it does complicate things a bit unfortunately in terms of cleanup.

[lasley]

@BMW95 - Bah that sucks (the situation not necessarily your idea which I don't fully understand).

Do you have an example of one that had non-enterprise siblings?

[woodbrettm]

Here's a different approach to the problem as per the latest commit. Uses the exact same 6 queries on all pages, //div[div[field[@name...]]], remove, then //div[field[@name...]], remove, then //field[@name] and remove, as well as their respective labels (which make another 3 queries, making 6 total).. Cleanup is a lot simpler as it's working farthest out (i.e. the farthest out parent div possible) then going in when needed, as @pedrobaeza was suggesting should be done.

The only page that differed so much it needed 4 custom queries was account settings. @lasley link to that page here. Using //[div[div[field[@name...]]] in this page was dangerous on fields that were only 1 div deep (that query is for 2 divs deep), other pages it proved safe.

Should improve quite a bit over the previous logic in terms of efficiency :)

[lasley]

Why does the query need to be so specific though? //field[@name..] would be functionally the same as //[div[div[field[@name...]]] assuming that there is only one like named field, which is almost always the case.

[woodbrettm]

@lasley Ah sorry my bad, wrong explanation on my part. The general queries are instead like so for most of the pages: //div[div[field[@widget='upgrade_boolean']]] -> remove -> //div[field[@widget='upgrade_boolean']] -> remove -> //field[@widget='upgrade_boolean'] -> remove. The logic here differs from the previous set such that I'm prioritizing deleting whole sections of pages first whereas previously I was going straight for the fields themselves, and then cleaning up empty divs after.

Before those queries are run, depending on the page name, any custom queries I have defined will be run which are more specific, targeting @name instead of @widget. The only custom queries I have defined are mostly for account, because of the following xml portion of the pg:

<div>
    <div>
        <field name="module_account_reports" class="oe_inline" widget="upgrade_boolean"/>
        <label for="module_account_reports"/>
    </div>
    <div name="group_multi_currency">
        <field name="group_multi_currency" class="oe_inline"/>
        <label for="group_multi_currency"/>
        <group attrs="{'invisible': [('group_multi_currency', '&lt;&gt;', True)]}" col="2">
            <group>
                <field name="currency_exchange_journal_id"/>
            </group>
            <group>
            </group>
        </group>
    </div>
    <div>
        <field name="group_analytic_accounting" class="oe_inline"/>
        <label for="group_analytic_accounting"/>
    </div>
    <div>
        <field name="module_account_asset" class="oe_inline"/>
        <label for="module_account_asset"/>
    </div>
    <div>
        <field name="module_account_budget" class="oe_inline"/>
        <label for="module_account_budget"/>
    </div>
    <div>
        <field name="module_account_tax_cash_basis" class="oe_inline"/>
        <label for="module_account_tax_cash_basis" />
    </div>
</div>

The account settings page has about 50% of the fields 2 divs deep, and 50% 1 div deep. However in this snippet, the upgrade_boolean field is 2 divs deep, but I only want to remove its parent div, not grandparent. By default, //div[div[field[@widget='upgrade_boolean']]] is run first, which would delete that grandparent div. To fix that, I ran some custom queries that delete any upgrade_boolean sections that are 2 divs deep using @name (as there's other parts of the pg where the grandparent div does need to be deleted), and then started the remove function at //div[field[@widget='upgrade_boolean']] instead of //div[div[field[@widget='upgrade_boolean']]].

Hope that clears things up :).

[woodbrettm]

After studying further, refactored again which achieved the following:

• Reduce loss of code coverage
• Reduce code complexity by a large factor

Functionally tested, and ensured minimal risk of accidental xml tag removal should any parts of the settings pages change.

Ready for final review.

[lasley]

Much much cleaner. 👍 code+functional