[All] Improve naming of elements for safe xpath expressions

[Yenthe666]

Impacted versions: 9.0, 10.0, 11.0 master.

Steps to reproduce: Create a new module in which you do an xpath to an existing view that has an element without any name. Let us take an example from the marketing_campaign module. In marketing_campaign_views.xml there is the following code:

    <record id="view_marketing_campaign_search" model="ir.ui.view">
        <field name="name">marketing.campaign.search</field>
        <field name="model">marketing.campaign</field>
        <field name="arch" type="xml">
            <search string="Campaigns">
                <field name="name" string="Campaign"/>
                <filter string="Draft" name="draft" domain="[('state','=','draft')]"/>
                <filter string="Running" domain="[('state','=','running')]"/>
                <separator/>
                <filter string="Test Mode" name="test" domain="[('mode','=','test')]"/>
                <filter string="Manual Mode" domain="[('mode','=','manual')]"/>
                <field name="object_id" string="Resource"/>
                <group expand="0" string="Group By">
                    <filter string="Resource" name="Object" context="{'group_by':'object_id'}"/>
                    <filter string="Mode" name="Mode" context="{'group_by':'mode'}"/>
                    <filter string="Status" name="Status" context="{'group_by':'state'}"/>
                </group>
            </search>
        </field>
    </record>

If I inherit this view and want to replace / modify the filter for "Manual Mode" I cannot xpath by string since this is removed (thank god it was). If I want to replace or remove this line:

<filter string="Manual Mode" domain="[('mode','=','manual')]"/>

My only resort is to either do an xpath on the whole view and remove a whole block or by using an index. For example:

<xpath expr="//filter[5]" position="replace">
  <!-- Your code -->
</xpath>

Both of these options are not ideal. Using the index might break if a filter is removed somewhere along or if another custom developed module adds a new filter. In this case I would either get an error if it is removed or I would be doing the wrong things on the wrong filter. In order to be able to xpath safely every element should have a name on which you can xpath. This is safe and cannot produce errors.
This behaviour can be found through the whole Odoo code and goes further than only filters.
We should create a solution in which we can xpath on every element without using an index or removing a big part of the view with an xpath to re-add it again in the xpath.

This is an example Python script to find all filters in Odoo that do not have a name to xpath on (for showing how big the issue is):

import os

amount_missing_filter_names = 0

for path, subdirs, files in os.walk('/odoo10ent'):
    for name in files:
        if '.xml' in name:
            pathvar = os.path.join(path, name)
            with open(pathvar) as f:
                for i, line in enumerate(f):
                    if '<filter' in line and not 'name' in line:
                        amount_missing_filter_names += 1
                        print('File ' + str(pathvar) + ' line ' + str(i) + ' misses <filter name="" ')

print('Number of missing filter names: ' + str(amount_missing_filter_names))

When I run this script there are 567 filter elements in Odoo that do not have a name defined to safely xpath on. This shows how many elements there are through-out Odoo without a name.
Edit: as pointed out by @mart-e this number is probably a bit too high, since some filters use multiple lines and my script will not catch this (https://github.com/odoo/odoo/blob/10.0/addons/crm/views/crm_lead_views.xml#L321). This is just a PoC to point out the amount is quite high though.

Expected behavior:
Every element in Odoo should be accessible with an xpath option that is not breaking if something changes.
This behaviour was already discussed with @mart-e and @odony and this issue will allow us to debate about a good solution.
One suggestion is to have default generated names for elements if a name is not specified. Something along the lines of a default string="My field" but then for elements in views. Another option is to simply add the name="" on every element, but this was not something @odony was in favour of.

[Yenthe666]

Hi @mart-e and @odony could I ask you guys for feedback and to start our brainstorm? It would be neat to have this fixed before V11.
Edit: @JKE-be welcome to join the discussion too.

[mart-e]

Hello,

The idea, if easily doable without big overhead could be to have a name with "_filter_num".
It is still dependant of the view order but at least it is a bit more precise.
edit: (messed up the assignees, sorry)

[MMstp]

Hi,
another problem if there is no name specified is, that the filters cannot be used in contexts.
Example: Filter inside module account, file account_view (record "view_account_move_line_filter")

I was not able to use: context={'search_default_account':1}

[pedrobaeza]

@mart-e any plans on implementing this?

[intero-chz]

Are there any compatibility problems by just adding the name attributes throughout the codebase of odoo search views? Would only be nice to get a naming guideline for filters, like "filter_<plausible_name>" and for group-by-only filters maybe filter_group_by_<field_list>".

[mart-e]

@intero-chz @pedrobaeza lets see if it is doable to generate automatically a name based on the filter definition. If something that is properly documented and implementing it is clean and minimal (to check), it could be nice to have.

[Yenthe666]

@mart-e did you ever get around to this one?

[mart-e]

No, I said, if somebody makes a pull request to fix it, it should be about having an autogenerated name. But we do not plan to implement it ourself at the moment.
Sorry it is was not clear.

[Yenthe666]

Okay, I misunderstood! Not sure if I can find the time to do something like this, let us leave it open as is for now.

[ghost]

Ran into this issue? Inheriting product.product_template_search_view I wished to add a filter to the "Group By" field, however like your example above group is not named. If this is a related issue, it makes inheriting flawed.
Update: Do not need to xpath for first group. But if I wanted to update an existing group here, would it not work without a name= attribute? Thanks.