Do not specify include or cols in product/views

[kbrock]

Overview

Lets stop defining redundant cols and include in report.yaml files and defer to the definitive associations from the models associations and virtual_attribute :uses.

Defining View columns

We have 2 ways to define the visible columns in report.yaml:

A:

# Columns to fetch from the main table
cols:
- name
- fingerprint

# Included tables (joined, has_one, has_many) and columns
include:
  ext_management_system:
    columns:
    - name

# Order of columns (from all tables)
col_order:
- name
- fingerprint
- ext_management_system.name

B:

# Order of columns (from all tables)
col_order:
- name
- fingerprint
- ext_management_system.name

The fields have the following definitions:

• col_order is the list of columns from the primary table and the join tables in display order.
• cols is the list of columns from the primary table.
• include is the list of joins necessary to display the columns and virtual attributes.

When cols and/or include are left blank, the value is populated from col_order and the virtual attribute's uses: values. (/via ManageIQ/manageiq#11338)

Note: The include is still needed when non-displayed columns are used on a view.

Issue

The proper values for include change over time.

A ruby virtual attribute needs a number of fields brought back to calculate an attribute, typically from another table. A sql virtual attribute, on the other hand, is self contained. So when an attribute is converted from ruby to sql, the join tables change.

The uses value is defined in the model, and this is kept up to date.
The include: value is defined in another repo (manageiq-ui-classic vs manageiq) and it is not kept up to date. Customer's also have these defined in their custom reports.

So the typical scenario is we improve a virtual attribute, but the include value is not updated so we end up with reports bringing back too much data.

Solution

Don't define include: when the value is redundant.

It is redundant when the value describes how to make virtual attributes work.
It is not redundant when it describes other non-displayed columns that are needed like type

Not sure if we want to create a big PR that fixes these values, or just state our intention and then update the values when making PRs against reports.

Why has this not been done yet?

There was the concern that when creating a new report, it would be easier if you could copy paste from an existing report and all the fields would be in the definition.

There was also the concern that the report builder would want to add these fields even though it was not necessary. (Not sure if this ui still exists nor if it even looks at the yaml files)

Instead, I have been syncing these values when I remember or when a customer reports a screen is slow. I have found issues with dozens of report yaml files.

Today, when I was updating a few report.yaml files, it felt like it was time to just drop these redundant columns when they are unneeded. I estimate they are unneeded 95% of the time.

Aside

In a perfect world, I would like to drop include and replace it with a simpler field like hidden_cols: [], which would look just like col_order: and describe columns that are not displayed. This change will probably never happen.

I would like to change the report data builder, which uses the include column. This component was build when there were only ruby virtual_attributes and requires objects when they should not be necessary. This forces us to build many more objects than necessary and join to tables that are not necessary.