Skip to content

Commit

Permalink
Fix Custom Threshold Rule ViewInAppUrl does not honor space (elasti…
Browse files Browse the repository at this point in the history
…c#201793)

## Summary

Close elastic#201378
Fix elastic#201333

- [Share] Allow to pass `spaceId` to `getRedirectUrl` to build a URL
with a specific `spaceId`
- Fix Custom Threshold Rule ViewInAppUrl does not honor Space

---------

Co-authored-by: Maryam Saeidi <[email protected]>
  • Loading branch information
Dosant and maryam-saeidi authored Dec 11, 2024
1 parent 4511abe commit f1f3a4f
Show file tree
Hide file tree
Showing 19 changed files with 421 additions and 113 deletions.
47 changes: 45 additions & 2 deletions src/plugins/share/common/url_service/locators/locator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,9 @@ import { KibanaLocation } from '../../../public';
import { LocatorGetUrlParams } from '.';
import { decompressFromBase64 } from 'lz-string';

const setup = () => {
const baseUrl = 'http://localhost:5601';
const setup = (
{ baseUrl = 'http://localhost:5601' }: { baseUrl: string } = { baseUrl: 'http://localhost:5601' }
) => {
const version = '1.2.3';
const deps: LocatorDependencies = {
baseUrl,
Expand Down Expand Up @@ -88,6 +89,48 @@ describe('Locator', () => {
baz: 'b',
});
});

test('returns URL of the redirect endpoint with custom spaceid', async () => {
const { locator } = setup();
const url = await locator.getRedirectUrl(
{ foo: 'a', baz: 'b' },
{ spaceId: 'custom-space-id' }
);

expect(url).toBe(
'http://localhost:5601/s/custom-space-id/app/r?l=TEST_LOCATOR&v=1.2.3&lz=N4IgZg9hIFwghiANCARvAXrNIC%2BQ'
);
});

test('returns URL of the redirect endpoint with replaced spaceid', async () => {
const { locator } = setup({ baseUrl: 'http://localhost:5601/s/space-id' });
const url = await locator.getRedirectUrl(
{ foo: 'a', baz: 'b' },
{ spaceId: 'custom-space-id' }
);

expect(url).toBe(
'http://localhost:5601/s/custom-space-id/app/r?l=TEST_LOCATOR&v=1.2.3&lz=N4IgZg9hIFwghiANCARvAXrNIC%2BQ'
);
});

test('returns URL of the redirect endpoint without spaceid', async () => {
const { locator } = setup({ baseUrl: 'http://localhost:5601/s/space-id' });
const url = await locator.getRedirectUrl({ foo: 'a', baz: 'b' }, { spaceId: 'default' });

expect(url).toBe(
'http://localhost:5601/app/r?l=TEST_LOCATOR&v=1.2.3&lz=N4IgZg9hIFwghiANCARvAXrNIC%2BQ'
);
});

test('returns URL of the redirect endpoint with untouched spaceId', async () => {
const { locator } = setup({ baseUrl: 'http://localhost:5601/s/space-id' });
const url = await locator.getRedirectUrl({ foo: 'a', baz: 'b' });

expect(url).toBe(
'http://localhost:5601/s/space-id/app/r?l=TEST_LOCATOR&v=1.2.3&lz=N4IgZg9hIFwghiANCARvAXrNIC%2BQ'
);
});
});

describe('.navigate()', () => {
Expand Down
18 changes: 14 additions & 4 deletions src/plugins/share/common/url_service/locators/locator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,13 @@ import type {
LocatorNavigationParams,
LocatorGetUrlParams,
} from './types';
import { formatSearchParams, FormatSearchParamsOptions, RedirectOptions } from './redirect';
import {
formatSearchParams,
FormatSearchParamsOptions,
RedirectOptions,
GetRedirectUrlOptions,
addSpaceIdToPath,
} from './redirect';

export interface LocatorDependencies {
/**
Expand Down Expand Up @@ -92,20 +98,24 @@ export class Locator<P extends SerializableRecord> implements LocatorPublic<P> {
return url;
}

public getRedirectUrl(params: P, options: FormatSearchParamsOptions = {}): string {
public getRedirectUrl(params: P, options: GetRedirectUrlOptions = {}): string {
const { baseUrl = '', version = '0.0.0' } = this.deps;
const redirectOptions: RedirectOptions = {
id: this.definition.id,
version,
params,
};
const formatOptions: FormatSearchParamsOptions = {
...options,
lzCompress: options.lzCompress ?? true,
};
const search = formatSearchParams(redirectOptions, formatOptions).toString();
const path = '/app/r?' + search;

return baseUrl + '/app/r?' + search;
if (options.spaceId) {
return addSpaceIdToPath(baseUrl, options.spaceId, path);
} else {
return baseUrl + path;
}
}

public async navigate(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@
export * from './types';
export * from './format_search_params';
export * from './parse_search_params';
export * from './space_url_parser';
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import { addSpaceIdToPath } from './space_url_parser';

describe('addSpaceIdToPath', () => {
test('handles no parameters', () => {
expect(addSpaceIdToPath()).toEqual(`/`);
});

test('it adds to the basePath correctly', () => {
expect(addSpaceIdToPath('/my/base/path', 'url-context')).toEqual('/my/base/path/s/url-context');
});

test('it appends the requested path to the end of the url context', () => {
expect(addSpaceIdToPath('/base', 'context', '/final/destination')).toEqual(
'/base/s/context/final/destination'
);
});

test('it replaces existing space identifiers', () => {
expect(addSpaceIdToPath('/my/base/path/s/old-space/', 'new-space')).toEqual(
'/my/base/path/s/new-space'
);

expect(addSpaceIdToPath('/my/base/path/s/old-space-no-trailing', 'new-space')).toEqual(
'/my/base/path/s/new-space'
);
});

test('it removes existing space identifier when spaceId is default', () => {
expect(addSpaceIdToPath('/my/base/path/s/old-space', 'default')).toEqual('/my/base/path');
expect(addSpaceIdToPath('/my/base/path/s/old-space')).toEqual('/my/base/path');
});

test('it throws an error when the requested path does not start with a slash', () => {
expect(() => {
addSpaceIdToPath('', '', 'foo');
}).toThrowErrorMatchingInlineSnapshot(`"path must start with a /"`);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

export function addSpaceIdToPath(
basePath: string = '/',
spaceId: string = '',
requestedPath: string = ''
): string {
if (requestedPath && !requestedPath.startsWith('/')) {
throw new Error(`path must start with a /`);
}

if (basePath.includes('/s/')) {
// If the base path already contains a space identifier, remove it
basePath = basePath.replace(/\/s\/[^/]+/, '');
}

const normalizedBasePath = basePath.endsWith('/') ? basePath.slice(0, -1) : basePath;

if (spaceId && spaceId !== 'default') {
return `${normalizedBasePath}/s/${spaceId}${requestedPath}`;
}

return `${normalizedBasePath}${requestedPath}` || '/';
}
11 changes: 11 additions & 0 deletions src/plugins/share/common/url_service/locators/redirect/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
*/

import type { SerializableRecord } from '@kbn/utility-types';
import type { FormatSearchParamsOptions } from './format_search_params';

/**
* @public
Expand All @@ -27,3 +28,13 @@ export interface RedirectOptions<P extends SerializableRecord = unknown & Serial
/** Locator params. */
params: P;
}

export interface GetRedirectUrlOptions extends FormatSearchParamsOptions {
/**
* Optional space ID to use when generating the URL.
* If not provided:
* - on the client the current space ID will be used.
* - on the server the URL will be generated without a space ID.
*/
spaceId?: string;
}
4 changes: 2 additions & 2 deletions src/plugins/share/common/url_service/locators/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
PersistableStateService,
VersionedState,
} from '@kbn/kibana-utils-plugin/common';
import type { FormatSearchParamsOptions } from './redirect';
import type { GetRedirectUrlOptions } from './redirect';

/**
* URL locator registry.
Expand Down Expand Up @@ -88,7 +88,7 @@ export interface LocatorPublic<P extends SerializableRecord> extends Persistable
* @param params URL locator parameters.
* @param options URL serialization options.
*/
getRedirectUrl(params: P, options?: FormatSearchParamsOptions): string;
getRedirectUrl(params: P, options?: GetRedirectUrlOptions): string;

/**
* Navigate using the `core.application.navigateToApp()` method to a Kibana
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,3 +44,7 @@
level: custom
type: long
description: "Number of outgoing bytes"
- name: core.system.ticks
level: custom
type: long
description: "The amount of CPU time spent in kernel space"
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ export const generateEvent: GeneratorFunction = (config, schedule, index, timest
bytes: generateNetworkData(timestamp.toISOString()),
},
},
core: {
system: {
ticks: randomBetween(1_000_000, 1_500_100),
},
},
},
metricset: {
period: interval,
Expand Down Expand Up @@ -159,6 +164,11 @@ export const generateEvent: GeneratorFunction = (config, schedule, index, timest
bytes: generateNetworkData(timestamp.toISOString()),
},
},
core: {
system: {
ticks: randomBetween(1_000_000, 1_500_100),
},
},
},
metricset: {
period: interval,
Expand Down
Loading

0 comments on commit f1f3a4f

Please sign in to comment.