[Discover] Migrate context AppState / GlobalState to use new app state helpers

[kertal]

Summary

Remove legacy AppState & GlobalState from Discover's Context application, replace with our shiny new state syncing utils #56479

Testing

Check if changed settings in the URL are propagated to UI and vice versa, and whether reloading of data takes place.

Checklist

Delete any items that are not applicable to this PR.

• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for cross-browser compatibility, including a check against IE11

[kertal]

Retest

[kertal]

@elasticmachine merge upstream

[elasticmachine]

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

[kertal]

Jenkins, test this ... last test ran into a timeout

[kertal]

@elasticmachine merge upstream

[Dosant]

ACK. will review and test later today

[flash1293]

Not sure what this is caused by, but setting a range filter pill ("is between") with just the end defined will result in two pills in this PR and the history doesn't work correctly afterwards:

On master this works fine for me.

Same with pinning a filter, then hitting the back button (won't unpin it)

[kertal]

Not sure what this is caused by, but setting a range filter pill ("is between") with just the end defined will result in two pills in this PR and the history doesn't work correctly afterwards:

On master this works fine for me.

Same with pinning a filter, then hitting the back button (won't unpin it)

@flash1293 excellent catch!
first issue seems an issue with having a NaN as value in the filters
i've got an idea what's the cause of the second issue, maybe 2 updates of the state after each other, first global, then local, but didn't test so far ... currently on it

[kertal]

FYI @flash1293 @Dosant I managed to fix the problem @flash1293 reported, but I need to further investigate the app's behavior. Seems a missing reloadOnSearch: false in the legacy $routeProvider's route, triggered a reload of the ContextAppRouteController every time the URL changed. Adding it breaks other functionality. So there are some further adaptions necessary before the review can be finished.

[Dosant]

I wonder if it makes sense in scope of this to remove this.filters from src/legacy/core_plugins/kibana/public/discover/np_ready/angular/context.js and allow filterManager to handle everything, just like @lizozom did in discover #56160.
It should remove plenty of code

[kertal]

@flash1293 @Dosant @sulemanof Context was built around reloadOnSearch: true, each change of the url triggered a reload of the controller. That was the cause for troubles. A change to reloadOnSearch: false would lead to a bigger refactoring.

What I did is improve the code, remove state subscriptions (since: change of state => change of url => controller reload => reinitialize state), and add more tests. It's nice to write unit tests concerning state <-> url syncing. Hope this is fine now.

[kibanamachine]

💚 Build Succeeded

• continuous-integration/kibana-ci/pull-request
• Commit: 3bbffe5

History

• 💚 Build #26807 succeeded 5a324d4
• 💚 Build #26766 succeeded 58fa7e4
• 💔 Build #26752 failed d5d796c
• 💚 Build #26175 succeeded 96ddfc7
• 💔 Build #26119 failed c33c7d9

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

[Dosant]

Code LGTM. Nice work!

I noticed an issue, similar to one I had in dashboard - #56284
Looks like 'state:storeInSessionStorage' doesn’t take effect for without full page reload.

It is likely caused by kbnStateStorage is created once when angular is initialised and then when you get back to Context App, it is not recreated with new 'state:storeInSessionStorage' value.

[flash1293]

Tested and works fine for me. Great work 🎉

[kertal]

@Dosant when I change state:storeInSessionStorage to true, navigate back to content, the initial URL is still unhashed, but all follow up URL modifications are hashed, so I didn't need to reload. Is this the behavior you've noticed? thx!