Bug fix:Adds a migration for transforming TSVB vis split_filters #49000

TinaHeiligers · 2019-10-22T21:57:22Z

Summary

Fixes #48710
When migrating to Kibana 7.3, split_filters from the 'Group by' series option were not migrated to query:language objects. After an upgrade to 7.3, visualizations that had split_filters in previous versions don't interpret the filter as a string.
For example, if we have a TSVB visualization in Kibana 6.8 with the following configuration:

After an upgrade to 7.3, the filters appear to have been lost:

However, the filter strings haven't been deleted, they are not interpretable by the code because they are not query:language objects.
Inspecting the visState data using the utilities in the Management -> Saved Objects list, we see that the filter strings are still there:

Patches are not being backported to 7.3 anymore, so a fix to the original migration can't be applied.
A new migration has been added and runs for version 7.5. The migration transforms any remaining split_filters filters that are still strings. The migration will effectively restore the original filters if they have not been deleted.

Unfortunately, the only way to 'fix' the missing filters in a version between 7.3 and 7.5 is to manually convert the filter string to an object in the visState panel shown when inspecting the saved object as follows:

Go to the Management page and select Saved Objects.
Find the visualization with 'missing' filters
Click on the 'inspect' icon. This will show a series of panels with json in them.
Find the panel that shows the visState
Look for the split_filters array and convert each filter:<string> into an object containing the original filter string as the value for the query key and use lucene as the value for the language key.
Save the object.
You should see that the visualization's filters have been restored.

The other way to restore the filters after an upgrade to 7.3 is to inspect the saved object and manually reapply the filter as seen in the json under visState -> params -> series -> split_filters -> filter in the visualization itself.

Checklist

Use ~~strikethroughs~~ to remove checklist items you don't feel are applicable to this PR.

~~- [ ] This was checked for cross-browser compatibility, including a check against IE11~~
~~- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support~~
~~- [ ] Documentation was added for features that require explanation or tutorials~~

Unit or functional tests were updated or added to match the most common scenarios
~~- [ ] This was checked for keyboard-only and screenreader accessibility~~

For maintainers

~~- [ ] This was checked for breaking API changes and was labeled appropriately~~
No breaking changes in this PR
~~- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately~~

…lters from strings to query:language objects to 7.5

…he saved object's migrationVersion in failing functional test

elasticmachine · 2019-10-22T23:16:40Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: dc4ed92

elasticmachine · 2019-10-23T00:23:20Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: d09ed80

elasticmachine · 2019-10-23T14:14:49Z

Pinging @elastic/kibana-app (Team:KibanaApp)

TinaHeiligers · 2019-10-23T21:52:59Z

@timroes
@rayafratkina said I should backport this to 7.4 proactively, even though there isn't a patch planned for 7.4.
I need advice on PR labelling for 7.4 (should I create a label for 7.4.2?). I'll also have to change the migration number here to 7.4.2(?) but the migration won't run if we only get the patch into 7.5 and the migration number is 7.4. How should I handle this?

timroes · 2019-10-24T11:19:53Z

@TinaHeiligers I think I would wait with the 7.4.x labeling here until we know if there will be another version (nevertheless ideally already backport it to that branch). Given the migration version numbers: They don't really need to correlate to Kibana versions (it's nice if they do, but it's not a technical must). So since there are no other migrations in 7.5.0 for visualizations it basically doesn't matter if you use 7.5.0 or 7.4.2 as a version for the migration. The migration system will just check if there are any migrations with a higher version number available than the current one and execute them all. So even if you update to 7.5.0 and the migration version would be 7.4.2 it would immediately run on any saved object that has a lower migration version (e.g. 7.3.1) saved in it's saved object. The only time the versioning might cause problems (or is tricky) is when we're trying to backport a migration "before" another migration, which is not the case here.

timroes · 2019-10-24T11:20:23Z

@elastic/kibana-qa Since that is a critical backport, could you please have a look and test this?

timroes

A couple of minor suggestions left (the only mandatory one would be the missing toHaveProperty not being called. Otherwise looks good to me. Please wait for @elastic/kibana-qa approval before merging.

src/legacy/core_plugins/kibana/migrations/migrations.js

src/legacy/core_plugins/kibana/migrations/migrations.test.js

elasticmachine · 2019-10-24T12:44:48Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: d09ed80

…split_filters_from_7_3

TinaHeiligers · 2019-10-24T16:23:21Z

@TinaHeiligers I think I would wait with the 7.4.x labeling here until we know if there will be another version (nevertheless ideally already backport it to that branch). Given the migration version numbers: They don't really need to correlate to Kibana versions (it's nice if they do, but it's not a technical must). So since there are no other migrations in 7.5.0 for visualizations it basically doesn't matter if you use 7.5.0 or 7.4.2 as a version for the migration. The migration system will just check if there are any migrations with a higher version number available than the current one and execute them all. So even if you update to 7.5.0 and the migration version would be 7.4.2 it would immediately run on any saved object that has a lower migration version (e.g. 7.3.1) saved in it's saved object. The only time the versioning might cause problems (or is tricky) is when we're trying to backport a migration "before" another migration, which is not the case here.

Backporting: there isn't a 7.4.x branch, so I'll backport to 7.4.

Migration numbers, I've changed it to 7.4.2 here so that we don't end up with another migration that might need to happen before this one.

… test expectations to more explicit ones

elasticmachine · 2019-10-24T17:51:03Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: 30c39de

TinaHeiligers · 2019-10-24T23:19:41Z

@elasticmachine merge upstream

elasticmachine · 2019-10-25T00:39:47Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 23ab008

TinaHeiligers · 2019-10-25T14:34:50Z

@bhavyarm Could you please test this PR so that I can merge it and do the backport?

Steps to reproduce the missing filters situation:

Load up a sample data set in Kibana 6.8.
Edit a TSVB visualization, changing the aggregation to a Filters aggregation (in the 'Group by' option).
Add a few filters and give each a label.
Observe that the visualization changes and the filters are applied.
Save the visualization and export it.
Change to Kibana 7.3 and load the same sample data you used in steps 1 to 5.
Import the visualization from the Management App, Saved Objects page.
Open the imported visualization.
Observe that the filters aggregation filters appear to be missing but the labels are there.

With this PR, the filters will be present. To test that the fix works:

Change to an instance of Kibana running this bug fix.
Load the data and import the visualization with missing filters from above.
Observe that the filters are restored.

Please let me know if you have any questions.

LeeDr · 2019-10-25T15:19:23Z

@TinaHeiligers we need to find a Kibana App dev to do the test you requested Bhavya to do. The test you've outlined (which is great, thanks for describing the steps so well) is going to take a fair amount of time and Bhavya and other QA are busy testing PRs that are already in 7.5.0.

The QA team will still have to test it after it's merged and in the next BC.

elasticmachine · 2019-10-25T15:47:23Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 23ab008

TinaHeiligers · 2019-10-25T15:54:07Z

@TinaHeiligers we need to find a Kibana App dev to do the test you requested Bhavya to do. The test you've outlined (which is great, thanks for describing the steps so well) is going to take a fair amount of time and Bhavya and other QA are busy testing PRs that are already in 7.5.0.

The QA team will still have to test it after it's merged and in the next BC.

@timroes Should I merge?

elasticmachine · 2019-10-25T16:37:16Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 23ab008

elasticmachine · 2019-10-25T17:11:43Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 23ab008

TinaHeiligers · 2019-10-25T21:26:19Z

If possible, also backport to 7.3.x, in which case we may need to change the migration number.

TinaHeiligers · 2019-10-28T14:50:57Z

@elasticmachine merge upstream

elasticmachine · 2019-10-28T16:03:42Z

💚 Build Succeeded

continuous-integration/kibana-ci/pull-request
Commit: 924a26f

…stic#49000) * Bug fix:Adds a migration for transforming TSVB visualization split_filters from strings to query:language objects to 7.5 * Changes hard coded migration version to get the version number from the saved object's migrationVersion in failing functional test * Changes migration number from '7.5.0' to '7.4.2', fixes typo, changes test expectations to more explicit ones

) (#49496) * Bug fix:Adds a migration for transforming TSVB visualization split_filters from strings to query:language objects to 7.5 * Changes hard coded migration version to get the version number from the saved object's migrationVersion in failing functional test * Changes migration number from '7.5.0' to '7.4.2', fixes typo, changes test expectations to more explicit ones

) (#49492) * Bug fix:Adds a migration for transforming TSVB visualization split_filters from strings to query:language objects to 7.5 * Changes hard coded migration version to get the version number from the saved object's migrationVersion in failing functional test * Changes migration number from '7.5.0' to '7.4.2', fixes typo, changes test expectations to more explicit ones

) (#49493) * Bug fix:Adds a migration for transforming TSVB visualization split_filters from strings to query:language objects to 7.5 * Changes hard coded migration version to get the version number from the saved object's migrationVersion in failing functional test * Changes migration number from '7.5.0' to '7.4.2', fixes typo, changes test expectations to more explicit ones

LeeDr · 2019-10-29T15:35:23Z

I verified this fix on Cloud.

Starting with 7.2.1, Installed eCommerce sample data, modified a TSVB visualization to have a Group by filters and saved it.
Upgraded the Cloud instance to 7.3.2, re-opened the saved TSVB visualization and saw the problem (the filter was gone)
Upgraded the Cloud instance to 7.4.2 BC1, re-opened the saved TSVB visualization and saw the problem was corrected. The filter was back and functioning.

TinaHeiligers · 2019-10-29T22:28:28Z

@LeeDr Thank you for following up! I'm glad it works.

Bug fix:Adds a migration for transforming TSVB visualization split_fi…

dc4ed92

…lters from strings to query:language objects to 7.5

TinaHeiligers added Feature:TSVB TSVB (Time Series Visual Builder) v8.0.0 v7.5.0 labels Oct 22, 2019

TinaHeiligers requested review from timroes and gospodarsky October 22, 2019 21:57

TinaHeiligers added the release_note:skip Skip the PR/issue when compiling release notes label Oct 22, 2019

TinaHeiligers mentioned this pull request Oct 22, 2019

Kibana loses filters in TSVB after upgrade from to 7.3 #48710

Closed

Changes hard coded migration version to get the version number from t…

d09ed80

…he saved object's migrationVersion in failing functional test

TinaHeiligers added the Team:Visualizations Visualization editors, elastic-charts and infrastructure label Oct 23, 2019

timroes approved these changes Oct 24, 2019

View reviewed changes

Merge branch 'master' of github.com:elastic/kibana into TSVB_migrate_…

05bb493

…split_filters_from_7_3

Changes migration number from '7.5.0' to '7.4.2', fixes typo, changes…

30c39de

… test expectations to more explicit ones

TinaHeiligers requested review from bhavyarm and removed request for bhavyarm October 24, 2019 16:40

Merge branch 'master' into TSVB_migrate_split_filters_from_7_3

23ab008

TinaHeiligers requested a review from a team October 24, 2019 23:21

Merge branch 'master' into TSVB_migrate_split_filters_from_7_3

924a26f

TinaHeiligers added the v7.4.2 label Oct 28, 2019

TinaHeiligers merged commit c2672ae into elastic:master Oct 28, 2019

timroes added the v7.6.0 label Oct 28, 2019

This was referenced Oct 28, 2019

[7.x] Bug fix:Adds a migration for transforming TSVB vis split_filters (#49000) #49492

Merged

[7.5] Bug fix:Adds a migration for transforming TSVB vis split_filters (#49000) #49493

Merged

timroes mentioned this pull request Oct 28, 2019

[7.4] Bug fix:Adds a migration for transforming TSVB vis split_filters (#49000) #49496

Merged

TinaHeiligers deleted the TSVB_migrate_split_filters_from_7_3 branch October 28, 2019 18:25

rayafratkina added release_note:fix and removed release_note:skip Skip the PR/issue when compiling release notes labels Oct 30, 2019

tylersmalley mentioned this pull request Dec 10, 2019

Moving saved object management to an ES plugin #49764

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug fix:Adds a migration for transforming TSVB vis split_filters #49000

Bug fix:Adds a migration for transforming TSVB vis split_filters #49000

TinaHeiligers commented Oct 22, 2019 •

edited

Loading

elasticmachine commented Oct 22, 2019

elasticmachine commented Oct 23, 2019

elasticmachine commented Oct 23, 2019

TinaHeiligers commented Oct 23, 2019

timroes commented Oct 24, 2019

timroes commented Oct 24, 2019

timroes left a comment

elasticmachine commented Oct 24, 2019

TinaHeiligers commented Oct 24, 2019 •

edited

Loading

elasticmachine commented Oct 24, 2019

TinaHeiligers commented Oct 24, 2019

elasticmachine commented Oct 25, 2019

TinaHeiligers commented Oct 25, 2019

LeeDr commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

TinaHeiligers commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

TinaHeiligers commented Oct 25, 2019

TinaHeiligers commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

LeeDr commented Oct 29, 2019

TinaHeiligers commented Oct 29, 2019

Bug fix:Adds a migration for transforming TSVB vis split_filters #49000

Bug fix:Adds a migration for transforming TSVB vis split_filters #49000

Conversation

TinaHeiligers commented Oct 22, 2019 • edited Loading

Summary

Checklist

For maintainers

elasticmachine commented Oct 22, 2019

💔 Build Failed

elasticmachine commented Oct 23, 2019

💚 Build Succeeded

elasticmachine commented Oct 23, 2019

TinaHeiligers commented Oct 23, 2019

timroes commented Oct 24, 2019

timroes commented Oct 24, 2019

timroes left a comment

Choose a reason for hiding this comment

elasticmachine commented Oct 24, 2019

💔 Build Failed

TinaHeiligers commented Oct 24, 2019 • edited Loading

elasticmachine commented Oct 24, 2019

💔 Build Failed

TinaHeiligers commented Oct 24, 2019

elasticmachine commented Oct 25, 2019

💚 Build Succeeded

TinaHeiligers commented Oct 25, 2019

LeeDr commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

💚 Build Succeeded

TinaHeiligers commented Oct 25, 2019

elasticmachine commented Oct 25, 2019

💚 Build Succeeded

elasticmachine commented Oct 25, 2019

💚 Build Succeeded

TinaHeiligers commented Oct 25, 2019

TinaHeiligers commented Oct 28, 2019

elasticmachine commented Oct 28, 2019

💚 Build Succeeded

LeeDr commented Oct 29, 2019

TinaHeiligers commented Oct 29, 2019

TinaHeiligers commented Oct 22, 2019 •

edited

Loading

TinaHeiligers commented Oct 24, 2019 •

edited

Loading