[Lens] Correctly handle references and migrations for filters in saved objects

[flash1293]

References

Lens has local code to handle references in filters

kibana/x-pack/plugins/lens/public/persistence/filter_references.ts

Line 14 in 4681a80

references: SavedObjectReference[];

However as the filters part of the saved object is owned by app services, its references should be handled by the provided helpers of the data plugin exported here:

kibana/src/plugins/data/common/query/persistable_state.ts

Line 14 in 204efae

export const extract = (filters: Filter[]) => {

As the injection scheme changes (indexRefName in the Lens code vs index in the data code), a migration has to be added as well to make sure existing Lens visualizations continue to work.

There is one relevant difference between the Lens code and the data code - the former removes the value property because it used to be non-persistable at some point:

kibana/x-pack/plugins/lens/public/persistence/filter_references.ts

Line 37 in 4681a80

delete newFilter.meta.value;

As part of this issues it has to be checked whether this is still necessary - if that's the case the logic has to be moved to the data helper.

Migrations

Filters also can be migrated (

kibana/src/plugins/data/common/query/persistable_state.ts

Line 62 in 204efae

export const getAllMigrations = () => {

) - these migrations should be run as part of the regular migration system.
To do this, it's necessary to mix the filter migrations into the regular Lens SO migrations (and the embeddable migrations used by dashboard):

kibana/x-pack/plugins/lens/server/migrations/saved_object_migrations.ts

Line 441 in 4681a80

'7.7.0': removeInvalidAccessors,

kibana/x-pack/plugins/lens/server/embeddable/lens_embeddable_factory.ts

Line 30 in 4681a80

// This migration is run in 7.13.1 for `by value` panels because the 7.13 release window was missed.

An example for how the migration objects can be mixed can be found in the dashboard plugin which has the same problem for embedded by-value panels:

kibana/src/plugins/dashboard/server/saved_objects/dashboard_migrations.ts

Line 254 in 4681a80

return mergeMigrationFunctionMaps(dashboardMigrations, embeddableMigrations);

[elasticmachine]

Pinging @elastic/kibana-vis-editors (Team:VisEditors)

[drewdaemon]

the former removes the value property because it used to be non-persistable at some point

@flash1293 Who/what would have imposed this constraint?

[drewdaemon]

As the injection scheme changes (indexRefName in the Lens code vs index in the data code), a migration has to be added as well to make sure existing Lens visualizations continue to work.

Not sure I understand what purpose this migration would serve since that indexRefName property isn't actually stored in the Lens state or the saved object. Just derived in the selector.

In other words, wouldn't we just need to update the application code to expect index instead of indexRefName when we start using the data plugin's extract function?

Edit: I was mistaken. I see now that it does get saved.