[Dashboard] Remove URL Generator

[ThomThomson]

Summary

Part of #121823
Part of #103846

Removes the deprecated URL Generator from the dashboard API

[ThomThomson]

Closing for now, blocked on #122300

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[ThomThomson]

@elasticmachine merge upstream

[qn895]

Testing the ML changes and I think there must have been a regression or format change with the new Dashboard locator, where the query is not being reflected the generated url. Potentially out of the scope of this PR, but good to address in 8.1.

For the same payload:

{
    "dashboardId": "edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b",
    "timeRange": {
        "from": "$earliest$",
        "to": "$latest$",
        "mode": "absolute"
    },
    "filters": [],
    "query": {
        "language": "kuery",
        "query": "clientip:\"$clientip$\""
    },
    "useHash": false
}

With the url generator (using main branch):

/abc/app/dashboards#/view/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_a=(filters:!(),query:(language:kuery,query:'clientip:%22$clientip$%22'))&_g=(filters:!(),time:(from:'$earliest$',mode:absolute,to:'$latest$'))

With the new locator (notice there's no 'query' here):

/abc/app/dashboards#/view/edf84fe0-e1a0-11e7-b6d5-4dc382ef7f5b?_g=(filters:!(),time:(from:%27$earliest$%27,mode:absolute,to:%27$latest$%27))

[ThomThomson]

Yes you are correct, there has been a format change! The Locator is capable of sending state via the router state rather than sending everything through the URL. This is causing this issue, which is being fixed.

When you click through the link, the query comes through correctly, right? It may not work so far when you open in new tab, but we will work on that when we fix the issue above.

[ThomThomson]

@elasticmachine merge upstream

[stephmilovic]

SIEM changes LGTM!

[crob611]

Looks good to me. One small little nit but nothing major

[crob611]

Can you further destructure locator out of here, since I think that's the only part of dashboard that it uses. Then the hook can only have a dependency on the locator?

[ThomThomson]

Unfortunately we can't destructure locator out of dashboard, because the dashboard service is optional.

[qn895]

When you click through the link, the query comes through correctly, right? It may not work so far when you open in new tab, but we will work on that when we fix the issue above.

Indeed that seems to be the case but this behavior is a blocker for us since it doesn't make sense specifically in our UI (for this use case) to open the link in the same tab. More importantly, we rely on this service in order to generate custom url formats for Anomaly detection alerts, so ideally the url generated should be correct. I'll give this PR a test #124770 but our preference is that the fix should be merged before this PR goes in.

[ThomThomson]

@qn895, I actually removed all the ML changes in this PR, because the change was already merged in here #124285

As soon as #124770 is merged we can take advantage of that change to make the entire URL available in ML.

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 93ac24c
• Storybooks Preview
• Webpack Bundle Analyzer

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
dashboard	260	259	-1

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id	before	after	diff
dashboard	142	136	-6

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
dashboard	293.1KB	293.1KB	-21.0B
securitySolution	4.7MB	4.7MB	-48.0B
total			-69.0B

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
dashboard	66.1KB	64.5KB	-1.7KB

Unknown metric groups

API count

id	before	after	diff
dashboard	155	138	-17

References to deprecated APIs

id	before	after	diff
dashboard	100	84	-16
securitySolution	49	48	-1
total			-17

Unreferenced deprecated APIs

id	before	after	diff
dashboard	2	0	-2

History

• 💛 Build #22214 was flaky 9b701bf
• 💚 Build #21993 succeeded e9064a6
• 💚 Build #21970 succeeded e9f2ca6
• 💚 Build #21715 succeeded 456eed9
• 💔 Build #21703 failed 26bedc2
• 💔 Build #21659 failed ce4b979

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

[kibanamachine]

Friendly reminder: Looks like this PR hasn’t been backported yet.
To create backports run node scripts/backport --pr 121832 or prevent reminders by adding the backport:skip label.

[bhavyarm]

@ThomThomson how do I test this PR please? Thanks!

[ThomThomson]

Hey @bhavyarm, there aren't really any user-facing tests you can run against this. It's mostly a behind-the-scenes change.

If you wanted to verify that everything still works, you could could test the sharing functionality of the dashboard, snapshot and saved object share, as well as dashboard to dashboard drilldowns.