[Dashboards] Register dashboard locator on server start

[nickpeihl]

Summary

Registers the dashboard locator definition with the share plugin on the Dashboard server start contract.

This moves Dashboard locator code from public to common so that we can expose the Dashboard Locator definition on the server. The getLocation method exposed by the locator does not appear to be used by any server code. So the getDashboardFilterFields function provided to the DashboardLocatorDefinition will throw a not supported error on the server. This should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error and thus would need to augment this function.

For posterity, here's an example where, in the future, we might need to provide a scoped content management client to the DashboardLocatorParams that can be used to load filters from a saved dashboard in the getDashboardFilterFields function.

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts

export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => {

router.handleLegacyErrors(async (ctx, req, res) => {
  const savedObjects = (await ctx.core).savedObjects.client;

  const cmClient = contentManagement.contentClient
        .getForRequest({ request: req, requestHandlerContext: ctx })

  const shortUrls = url.shortUrls.get({ savedObjects });
  const { locatorId, params, slug } = req.body;
  const locator = url.locators.get(locatorId);
  // TODO find a way to pass cmClient to the locator. Maybe in params?
  
  try {
  const shortUrl = await shortUrls.create({
    locator,
    params,
    slug,
  });

  return res.ok({
    headers: {
      'content-type': 'application/json',
    },
    body: shortUrl.data,
  });
} catch (error) {

[nickpeihl]

If we want to get the filters from a saved dashboard in the same way we do on the client-side then we'll somehow need to include the RequestHandlerContext so we can scope the content management client to the current user.

Similar to the useHashedUrls example above, we need to use the ctx argument to create a content management client scoped to the user to pass to the locator.

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts

export const registerCreateRoute = (router: IRouter, url: ServerUrlService, contentManagement: ContentManagementServerSetup) => {

...

router.handleLegacyErrors(async (ctx, req, res) => {
  const savedObjects = (await ctx.core).savedObjects.client;

  const cmClient = contentManagement.contentClient
        .getForRequest({ request: req, requestHandlerContext: ctx })

  const shortUrls = url.shortUrls.get({ savedObjects });
  const { locatorId, params, slug } = req.body;
  const locator = url.locators.get(locatorId);
  // TODO find a way to pass cmClient to the locator. Maybe in params?
  
  try {
  const shortUrl = await shortUrls.create({
    locator,
    params,
    slug,
  });

  return res.ok({
    headers: {
      'content-type': 'application/json',
    },
    body: shortUrl.data,
  });
} catch (error) {

[nickpeihl]

AFAICT, the getLocation method from the LocatorPublic interface is never actually called by the server. So, it currently does not matter on the Dashboard server contract what we set for useHashedUrls or the return value of getDashboardFilterFields because, currently, they are never be used by Kibana server. It seems that the getLocation is only currently ever used by the client.

Also, the URLService on the Share server plugin only exposes certain locator methods but they are not supported on the server. So I suspect we can safely assume the getLocation is not directly exposed on the server side URL service and it will likely not get called from the server. Maybe someone from @elastic/appex-sharedux can keep me honest here?

So I think we have two options for registering the Dashboard locator on the server:

1. Have getDashboardFilterFields always return an empty array. Currently, this function is never called. But any future implementations where getLocation is called on the server would not retrieve any saved filters on a dashboard. So it would not break, but might return unexpected results.
2. Have getDashboardFilterFields throw an error as not supported on the server. Again, this should currently never throw since getLocation is not called by the server. But any future implementations where getLocation is called on the server would throw an error. cc @ThomThomson

[ThomThomson]

Good find! That will definitely make this simpler. I'd suggest making it throw an error on the serverside - which will make sure we revisit this if we ever need to call getLocation on the server.

[nickpeihl]

If we want to support hashed urls on the server the same way we do on the client-side, we'll need the RequestHandlerContext) so we can scope the server-side UISettingsServiceStart.asScopedToClient to retrieve the 'state:storeInSessionStorage' setting as we do in the client.

For example: The short URL create route passes the ctx: RequestHandlerContext. We use this to scope the savedObjects client to the user making the request. To retrieve the 'state:storeInSessionStorage' setting we need to do something like this:

// in src/platform/plugins/shared/share/server/url_service/http/short_urls/register_create_route.ts
router.handleLegacyErrors(async (ctx, req, res) => {
  const savedObjects = (await ctx.core).savedObjects.client;

  const uiSettingsClient = core.uiSettings.asScopedToClient(soClient);
  const useHashedUrl = uiSettingsClient.get('state:storeInSessionStorage');

  const shortUrls = url.shortUrls.get({ savedObjects });
  const { locatorId, params, slug } = req.body;
  const locator = url.locators.get(locatorId);
  // TODO find a way to pass useHashedUrl to the locator. Maybe in params?
  
  try {
  const shortUrl = await shortUrls.create({
    locator,
    params,
    slug,
  });

  return res.ok({
    headers: {
      'content-type': 'application/json',
    },
    body: shortUrl.data,
  });
} catch (error) {

FWIW, the server-side Discover locator does not support hashed URLs.

[ThomThomson]

I'd say it's safe to not support hashing on the serverside locator for Dashboards.

[elasticmachine]

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

[nickpeihl]

These top level exports were removed to avoid bloating the bundle size. Aside from types, the imports from common in dashboard/public have been changed to deep imports.

[rmyz]

x-pack/solutions/observability/plugins/inventory/public/hooks/use_detail_view_redirect.ts changes LGTM

[jgowdyelastic]

ML changes LGTM

[ThomThomson]

Changes LGTM! Mostly import changes, code only review.

[ThomThomson]

Thanks for throwing an error here!

[Ikuni17]

Removed Operations as we no longer have code owner changes in this PR.

[kibanamachine]

Starting backport for target branches: 8.19

https://github.com/elastic/kibana/actions/runs/14711519816

[elasticmachine]

💚 Build Succeeded

• Buildkite Build
• Commit: 5d20701
• Storybooks Preview

Metrics [docs]

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	92	82	-10

Async chunks

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

id	before	after	diff
dashboard	548.0KB	548.1KB	+22.0B

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id	before	after	diff
dashboard	11	10	-1

Page load bundle

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

id	before	after	diff
dashboard	17.7KB	17.8KB	+132.0B

Unknown metric groups

API count

id	before	after	diff
dashboard	96	88	-8

History

• 💛 Build #295580 was flaky 9d11723
• 💚 Build #294579 succeeded 10aa7dc
• 💔 Build #294448 failed dc1cf22
• 💛 Build #294400 was flaky be4a587
• 💛 Build #294332 was flaky 25f3446
• 💔 Build #294317 failed 48b0df0

[kibanamachine]

💔 All backports failed

Status	Branch	Result
❌	8.19	Backport failed because of merge conflicts

Manual backport

To create the backport manually run:

node scripts/backport --pr 218495

Questions ?

Please refer to the Backport tool documentation

[nickpeihl]

💚 All backports created successfully

Status	Branch	Result
✅	8.19

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation