From 28305237a318212715491c4655f2b26e0f3d77bc Mon Sep 17 00:00:00 2001 From: Ryan Keairns Date: Tue, 10 Nov 2020 10:50:54 -0600 Subject: [PATCH 01/57] Remove kibana-core-ui-designers (#82962) The people in `kibana-core-ui-designers` have been rolled into the `kibana-design` team. All `.scss` changes will now ping `kibana-design` since the `kibana-core-ui-designers` entries were overrides (occurring later in this file). --- .github/CODEOWNERS | 7 ------- 1 file changed, 7 deletions(-) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 6da2d5d602186a..78e17de4139bd5 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -369,13 +369,6 @@ x-pack/plugins/telemetry_collection_xpack/schema/xpack_plugins.json @elastic/kib **/*.scss @elastic/kibana-design #CC# /packages/kbn-ui-framework/ @elastic/kibana-design -# Core UI design -/src/plugins/dashboard/**/*.scss @elastic/kibana-core-ui-designers -/src/plugins/embeddable/**/*.scss @elastic/kibana-core-ui-designers -/x-pack/plugins/canvas/**/*.scss @elastic/kibana-core-ui-designers -/x-pack/plugins/spaces/**/*.scss @elastic/kibana-core-ui-designers -/x-pack/plugins/security/**/*.scss @elastic/kibana-core-ui-designers - # Observability design /x-pack/plugins/apm/**/*.scss @elastic/observability-design /x-pack/plugins/infra/**/*.scss @elastic/observability-design From a63c390ae063db7ab9b4f8f1e1284b7b382d8b3b Mon Sep 17 00:00:00 2001 From: Aleh Zasypkin Date: Tue, 10 Nov 2020 18:12:47 +0100 Subject: [PATCH 02/57] Remove redundant call to `_authenticate` API after access token is created. (#82980) --- .github/CODEOWNERS | 1 - .../server/authentication/providers/base.ts | 19 +++- .../authentication/providers/kerberos.test.ts | 70 +++---------- .../authentication/providers/kerberos.ts | 61 +++++------- .../authentication/providers/oidc.test.ts | 20 +--- .../server/authentication/providers/oidc.ts | 91 +++++++---------- .../authentication/providers/pki.test.ts | 88 ++++------------- .../server/authentication/providers/pki.ts | 40 +++----- .../authentication/providers/saml.test.ts | 33 +++---- .../server/authentication/providers/saml.ts | 91 +++++++---------- .../authentication/providers/token.test.ts | 99 ++----------------- .../server/authentication/providers/token.ts | 63 +++++------- .../server/authentication/tokens.test.ts | 8 +- .../security/server/authentication/tokens.ts | 13 ++- .../security/server/elasticsearch/index.ts | 3 + x-pack/scripts/functional_tests.js | 2 +- .../tests/pki/pki_auth.ts | 2 +- .../tests/token/header.ts} | 6 +- .../tests/token/index.ts} | 6 +- .../tests/token/login.ts} | 5 +- .../tests/token/logout.ts} | 5 +- .../tests/token/session.ts} | 19 ++-- .../token.config.ts} | 8 +- 23 files changed, 258 insertions(+), 495 deletions(-) rename x-pack/test/{token_api_integration/auth/header.js => security_api_integration/tests/token/header.ts} (91%) rename x-pack/test/{token_api_integration/auth/index.js => security_api_integration/tests/token/index.ts} (72%) rename x-pack/test/{token_api_integration/auth/login.js => security_api_integration/tests/token/login.ts} (94%) rename x-pack/test/{token_api_integration/auth/logout.js => security_api_integration/tests/token/logout.ts} (91%) rename x-pack/test/{token_api_integration/auth/session.js => security_api_integration/tests/token/session.ts} (91%) rename x-pack/test/{token_api_integration/config.js => security_api_integration/token.config.ts} (81%) diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 78e17de4139bd5..150e6431ec9687 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -257,7 +257,6 @@ /x-pack/test/security_api_integration/ @elastic/kibana-security /x-pack/test/security_functional/ @elastic/kibana-security /x-pack/test/spaces_api_integration/ @elastic/kibana-security -/x-pack/test/token_api_integration/ @elastic/kibana-security #CC# /src/legacy/ui/public/capabilities @elastic/kibana-security #CC# /x-pack/legacy/plugins/encrypted_saved_objects/ @elastic/kibana-security #CC# /x-pack/plugins/security_solution/ @elastic/kibana-security diff --git a/x-pack/plugins/security/server/authentication/providers/base.ts b/x-pack/plugins/security/server/authentication/providers/base.ts index 030b2a6e968afb..a5a68f2a493157 100644 --- a/x-pack/plugins/security/server/authentication/providers/base.ts +++ b/x-pack/plugins/security/server/authentication/providers/base.ts @@ -13,7 +13,8 @@ import { ILegacyClusterClient, Headers, } from '../../../../../../src/core/server'; -import { AuthenticatedUser } from '../../../common/model'; +import type { AuthenticatedUser } from '../../../common/model'; +import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { Tokens } from '../tokens'; @@ -109,10 +110,20 @@ export abstract class BaseAuthenticationProvider { * @param [authHeaders] Optional `Headers` dictionary to send with the request. */ protected async getUser(request: KibanaRequest, authHeaders: Headers = {}) { - return deepFreeze({ - ...(await this.options.client + return this.authenticationInfoToAuthenticatedUser( + await this.options.client .asScoped({ headers: { ...request.headers, ...authHeaders } }) - .callAsCurrentUser('shield.authenticate')), + .callAsCurrentUser('shield.authenticate') + ); + } + + /** + * Converts Elasticsearch Authentication result to a Kibana authenticated user. + * @param authenticationInfo Result returned from the `_authenticate` operation. + */ + protected authenticationInfoToAuthenticatedUser(authenticationInfo: AuthenticationInfo) { + return deepFreeze({ + ...authenticationInfo, authentication_provider: { type: this.type, name: this.options.name }, } as AuthenticatedUser); } diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts index af26d1e60414a3..eb4ac8f4dcbed1 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.test.ts @@ -118,12 +118,10 @@ describe('KerberosAuthenticationProvider', () => { headers: { authorization: 'negotiate spnego' }, }); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'some-token', refresh_token: 'some-refresh-token', + authentication: user, }); await expect(operation(request)).resolves.toEqual( @@ -136,10 +134,7 @@ describe('KerberosAuthenticationProvider', () => { ) ); - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer some-token' }, - }); - + expect(mockOptions.client.asScoped).not.toHaveBeenCalled(); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, }); @@ -153,13 +148,11 @@ describe('KerberosAuthenticationProvider', () => { headers: { authorization: 'negotiate spnego' }, }); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'some-token', refresh_token: 'some-refresh-token', kerberos_authentication_response_token: 'response-token', + authentication: user, }); await expect(operation(request)).resolves.toEqual( @@ -173,10 +166,7 @@ describe('KerberosAuthenticationProvider', () => { ) ); - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer some-token' }, - }); - + expect(mockOptions.client.asScoped).not.toHaveBeenCalled(); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, }); @@ -250,33 +240,6 @@ describe('KerberosAuthenticationProvider', () => { expect(request.headers.authorization).toBe('negotiate spnego'); }); - - it('fails if could not retrieve user using the new access token.', async () => { - const request = httpServerMock.createKibanaRequest({ - headers: { authorization: 'negotiate spnego' }, - }); - - const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ - access_token: 'some-token', - refresh_token: 'some-refresh-token', - }); - - await expect(operation(request)).resolves.toEqual(AuthenticationResult.failed(failureReason)); - - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer some-token' }, - }); - - expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { - body: { grant_type: '_kerberos', kerberos_ticket: 'spnego' }, - }); - - expect(request.headers.authorization).toBe('negotiate spnego'); - }); } describe('`login` method', () => { @@ -381,32 +344,21 @@ describe('KerberosAuthenticationProvider', () => { expect(request.headers).not.toHaveProperty('authorization'); }); - it('succeeds with valid session even if requiring a token refresh', async () => { + it('succeeds with a valid session even if requiring a token refresh', async () => { const user = mockAuthenticatedUser(); const request = httpServerMock.createKibanaRequest(); const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - mockOptions.client.asScoped.mockImplementation((scopeableRequest) => { - if (scopeableRequest?.headers.authorization === `Bearer ${tokenPair.accessToken}`) { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ); - return mockScopedClusterClient; - } - - if (scopeableRequest?.headers.authorization === 'Bearer newfoo') { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - return mockScopedClusterClient; - } - - throw new Error('Unexpected call'); - }); + const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); + mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) + ); + mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.tokens.refresh.mockResolvedValue({ accessToken: 'newfoo', refreshToken: 'newbar', + authenticationInfo: user, }); await expect(provider.authenticate(request, tokenPair)).resolves.toEqual( diff --git a/x-pack/plugins/security/server/authentication/providers/kerberos.ts b/x-pack/plugins/security/server/authentication/providers/kerberos.ts index fa578b9dca45f9..2e15893d0845f2 100644 --- a/x-pack/plugins/security/server/authentication/providers/kerberos.ts +++ b/x-pack/plugins/security/server/authentication/providers/kerberos.ts @@ -10,11 +10,12 @@ import { LegacyElasticsearchErrorHelpers, KibanaRequest, } from '../../../../../../src/core/server'; +import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { HTTPAuthorizationHeader } from '../http_authentication'; import { canRedirectRequest } from '../can_redirect_request'; -import { Tokens, TokenPair } from '../tokens'; +import { Tokens, TokenPair, RefreshTokenResult } from '../tokens'; import { BaseAuthenticationProvider } from './base'; /** @@ -149,6 +150,7 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { access_token: string; refresh_token: string; kerberos_authentication_response_token?: string; + authentication: AuthenticationInfo; }; try { tokens = await this.options.client.callAsInternalUser('shield.getAccessToken', { @@ -203,23 +205,16 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { }; } - try { - // Then attempt to query for the user details using the new token - const authHeaders = { - authorization: new HTTPAuthorizationHeader('Bearer', tokens.access_token).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('User has been authenticated with new access token'); - return AuthenticationResult.succeeded(user, { - authHeaders, + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(tokens.authentication), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', tokens.access_token).toString(), + }, authResponseHeaders, state: { accessToken: tokens.access_token, refreshToken: tokens.refresh_token }, - }); - } catch (err) { - this.logger.debug(`Failed to authenticate request via access token: ${err.message}`); - return AuthenticationResult.failed(err); - } + } + ); } /** @@ -260,9 +255,9 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { private async authenticateViaRefreshToken(request: KibanaRequest, state: ProviderState) { this.logger.debug('Trying to refresh access token.'); - let refreshedTokenPair: TokenPair | null; + let refreshTokenResult: RefreshTokenResult | null; try { - refreshedTokenPair = await this.options.tokens.refresh(state.refreshToken); + refreshTokenResult = await this.options.tokens.refresh(state.refreshToken); } catch (err) { return AuthenticationResult.failed(err); } @@ -270,28 +265,22 @@ export class KerberosAuthenticationProvider extends BaseAuthenticationProvider { // If refresh token is no longer valid, let's try to renegotiate new tokens using SPNEGO. We // allow this because expired underlying token is an implementation detail and Kibana user // facing session is still valid. - if (refreshedTokenPair === null) { + if (refreshTokenResult === null) { this.logger.debug('Both access and refresh tokens are expired. Re-authenticating...'); return this.authenticateViaSPNEGO(request, state); } - try { - const authHeaders = { - authorization: new HTTPAuthorizationHeader( - 'Bearer', - refreshedTokenPair.accessToken - ).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { authHeaders, state: refreshedTokenPair }); - } catch (err) { - this.logger.debug( - `Failed to authenticate user using newly refreshed access token: ${err.message}` - ); - return AuthenticationResult.failed(err); - } + this.logger.debug('Request has been authenticated via refreshed token.'); + const { accessToken, refreshToken, authenticationInfo } = refreshTokenResult; + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(authenticationInfo), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + }, + state: { accessToken, refreshToken }, + } + ); } /** diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts index dfea7e508b3333..126306c885e53e 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.test.ts @@ -175,6 +175,7 @@ describe('OIDCAuthenticationProvider', () => { const { request, attempt, expectedRedirectURI } = getMocks(); mockOptions.client.callAsInternalUser.mockResolvedValue({ + authentication: mockUser, access_token: 'some-token', refresh_token: 'some-refresh-token', }); @@ -440,25 +441,14 @@ describe('OIDCAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); const tokenPair = { accessToken: 'expired-token', refreshToken: 'valid-refresh-token' }; - mockOptions.client.asScoped.mockImplementation((scopeableRequest) => { - if (scopeableRequest?.headers.authorization === `Bearer ${tokenPair.accessToken}`) { - const mockScopedClusterClientToFail = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClientToFail.callAsCurrentUser.mockRejectedValue( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ); - return mockScopedClusterClientToFail; - } - - if (scopeableRequest?.headers.authorization === 'Bearer new-access-token') { - return mockScopedClusterClient; - } - - throw new Error('Unexpected call'); - }); + mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) + ); mockOptions.tokens.refresh.mockResolvedValue({ accessToken: 'new-access-token', refreshToken: 'new-refresh-token', + authenticationInfo: mockUser, }); await expect( diff --git a/x-pack/plugins/security/server/authentication/providers/oidc.ts b/x-pack/plugins/security/server/authentication/providers/oidc.ts index eede9f334a5a16..250641d1cf1744 100644 --- a/x-pack/plugins/security/server/authentication/providers/oidc.ts +++ b/x-pack/plugins/security/server/authentication/providers/oidc.ts @@ -7,12 +7,12 @@ import Boom from '@hapi/boom'; import type from 'type-detect'; import { KibanaRequest } from '../../../../../../src/core/server'; -import { AuthenticatedUser } from '../../../common/model'; +import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { canRedirectRequest } from '../can_redirect_request'; import { DeauthenticationResult } from '../deauthentication_result'; import { HTTPAuthorizationHeader } from '../http_authentication'; -import { Tokens, TokenPair } from '../tokens'; +import { Tokens, TokenPair, RefreshTokenResult } from '../tokens'; import { AuthenticationProviderOptions, BaseAuthenticationProvider, @@ -243,46 +243,31 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { } // We have all the necessary parameters, so attempt to complete the OpenID Connect Authentication - let accessToken; - let refreshToken; + let result: { access_token: string; refresh_token: string; authentication: AuthenticationInfo }; try { // This operation should be performed on behalf of the user with a privilege that normal // user usually doesn't have `cluster:admin/xpack/security/oidc/authenticate`. - const authenticateResponse = await this.options.client.callAsInternalUser( - 'shield.oidcAuthenticate', - { - body: { - state: stateOIDCState, - nonce: stateNonce, - redirect_uri: authenticationResponseURI, - realm: this.realm, - }, - } - ); - - accessToken = authenticateResponse.access_token; - refreshToken = authenticateResponse.refresh_token; - } catch (err) { - this.logger.debug(`Failed to authenticate request via OpenID Connect: ${err.message}`); - return AuthenticationResult.failed(err); - } - - // Now we need to retrieve full user information. - let user: Readonly; - try { - user = await this.getUser(request, { - authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + result = await this.options.client.callAsInternalUser('shield.oidcAuthenticate', { + body: { + state: stateOIDCState, + nonce: stateNonce, + redirect_uri: authenticationResponseURI, + realm: this.realm, + }, }); } catch (err) { - this.logger.debug(`Failed to retrieve user using access token: ${err.message}`); + this.logger.debug(`Failed to authenticate request via OpenID Connect: ${err.message}`); return AuthenticationResult.failed(err); } this.logger.debug('Login has been performed with OpenID Connect response.'); - return AuthenticationResult.redirectTo(stateRedirectURL, { - state: { accessToken, refreshToken, realm: this.realm }, - user, + state: { + accessToken: result.access_token, + refreshToken: result.refresh_token, + realm: this.realm, + }, + user: this.authenticationInfoToAuthenticatedUser(result.authentication), }); } @@ -356,20 +341,17 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { * @param request Request instance. * @param state State value previously stored by the provider. */ - private async authenticateViaRefreshToken( - request: KibanaRequest, - { refreshToken }: ProviderState - ) { + private async authenticateViaRefreshToken(request: KibanaRequest, state: ProviderState) { this.logger.debug('Trying to refresh elasticsearch access token.'); - if (!refreshToken) { + if (!state.refreshToken) { this.logger.debug('Refresh token is not found in state.'); return AuthenticationResult.notHandled(); } - let refreshedTokenPair: TokenPair | null; + let refreshTokenResult: RefreshTokenResult | null; try { - refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + refreshTokenResult = await this.options.tokens.refresh(state.refreshToken); } catch (err) { return AuthenticationResult.failed(err); } @@ -380,7 +362,7 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { // message. There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it // seems logical to do the same on Kibana side and `401` would force user to logout and do full SLO if it's // supported. - if (refreshedTokenPair === null) { + if (refreshTokenResult === null) { if (canStartNewSession(request)) { this.logger.debug( 'Both elasticsearch access and refresh tokens are expired. Re-initiating OpenID Connect authentication.' @@ -393,24 +375,17 @@ export class OIDCAuthenticationProvider extends BaseAuthenticationProvider { ); } - try { - const authHeaders = { - authorization: new HTTPAuthorizationHeader( - 'Bearer', - refreshedTokenPair.accessToken - ).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { - authHeaders, - state: { ...refreshedTokenPair, realm: this.realm }, - }); - } catch (err) { - this.logger.debug(`Failed to refresh elasticsearch access token: ${err.message}`); - return AuthenticationResult.failed(err); - } + this.logger.debug('Request has been authenticated via refreshed token.'); + const { accessToken, refreshToken, authenticationInfo } = refreshTokenResult; + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(authenticationInfo), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + }, + state: { accessToken, refreshToken, realm: this.realm }, + } + ); } /** diff --git a/x-pack/plugins/security/server/authentication/providers/pki.test.ts b/x-pack/plugins/security/server/authentication/providers/pki.test.ts index 94308ab5f24038..aa85b8a43af4d7 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.test.ts @@ -120,10 +120,10 @@ describe('PKIAuthenticationProvider', () => { }), }); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' }); + mockOptions.client.callAsInternalUser.mockResolvedValue({ + authentication: user, + access_token: 'access-token', + }); await expect(operation(request)).resolves.toEqual( AuthenticationResult.succeeded( @@ -144,10 +144,7 @@ describe('PKIAuthenticationProvider', () => { ], }, }); - - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer access-token' }, - }); + expect(mockOptions.client.asScoped).not.toHaveBeenCalled(); expect(request.headers).not.toHaveProperty('authorization'); }); @@ -162,10 +159,10 @@ describe('PKIAuthenticationProvider', () => { }), }); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' }); + mockOptions.client.callAsInternalUser.mockResolvedValue({ + authentication: user, + access_token: 'access-token', + }); await expect(operation(request)).resolves.toEqual( AuthenticationResult.succeeded( @@ -181,10 +178,7 @@ describe('PKIAuthenticationProvider', () => { expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', { body: { x509_certificate_chain: ['fingerprint:2A:7A:C2:DD:base64'] }, }); - - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer access-token' }, - }); + expect(mockOptions.client.asScoped).not.toHaveBeenCalled(); expect(request.headers).not.toHaveProperty('authorization'); }); @@ -209,35 +203,6 @@ describe('PKIAuthenticationProvider', () => { expect(request.headers).not.toHaveProperty('authorization'); }); - - it('fails if could not retrieve user using the new access token.', async () => { - const request = httpServerMock.createKibanaRequest({ - headers: {}, - socket: getMockSocket({ - authorized: true, - peerCertificate: getMockPeerCertificate('2A:7A:C2:DD'), - }), - }); - - const failureReason = LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(failureReason); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' }); - - await expect(operation(request)).resolves.toEqual(AuthenticationResult.failed(failureReason)); - - expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); - expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.delegatePKI', { - body: { x509_certificate_chain: ['fingerprint:2A:7A:C2:DD:base64'] }, - }); - - expectAuthenticateCall(mockOptions.client, { - headers: { authorization: 'Bearer access-token' }, - }); - - expect(request.headers).not.toHaveProperty('authorization'); - }); } describe('`login` method', () => { @@ -365,10 +330,10 @@ describe('PKIAuthenticationProvider', () => { }); const state = { accessToken: 'existing-token', peerCertificateFingerprint256: '3A:9A:C5:DD' }; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' }); + mockOptions.client.callAsInternalUser.mockResolvedValue({ + authentication: user, + access_token: 'access-token', + }); await expect(provider.authenticate(request, state)).resolves.toEqual( AuthenticationResult.succeeded( @@ -402,25 +367,14 @@ describe('PKIAuthenticationProvider', () => { const user = mockAuthenticatedUser({ authentication_provider: { type: 'pki', name: 'pki' } }); const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser - // In response to call with an expired token. - .mockRejectedValueOnce( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ) - // In response to a call with a new token. - .mockResolvedValueOnce(user) // In response to call with an expired token. - .mockRejectedValueOnce( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ) - // In response to a call with a new token. - .mockResolvedValueOnce(user) // In response to call with an expired token. - .mockRejectedValueOnce( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ) - // In response to a call with a new token. - .mockResolvedValueOnce(user); + mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) + ); mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'access-token' }); + mockOptions.client.callAsInternalUser.mockResolvedValue({ + authentication: user, + access_token: 'access-token', + }); const nonAjaxRequest = httpServerMock.createKibanaRequest({ socket: getMockSocket({ diff --git a/x-pack/plugins/security/server/authentication/providers/pki.ts b/x-pack/plugins/security/server/authentication/providers/pki.ts index 3629a0ac34f02c..974a838127e1d6 100644 --- a/x-pack/plugins/security/server/authentication/providers/pki.ts +++ b/x-pack/plugins/security/server/authentication/providers/pki.ts @@ -7,6 +7,7 @@ import Boom from '@hapi/boom'; import { DetailedPeerCertificate } from 'tls'; import { KibanaRequest } from '../../../../../../src/core/server'; +import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { HTTPAuthorizationHeader } from '../http_authentication'; @@ -218,13 +219,11 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider { // We should collect entire certificate chain as an ordered array of certificates encoded as base64 strings. const certificateChain = this.getCertificateChain(peerCertificate); - let accessToken: string; + let result: { access_token: string; authentication: AuthenticationInfo }; try { - accessToken = ( - await this.options.client.callAsInternalUser('shield.delegatePKI', { - body: { x509_certificate_chain: certificateChain }, - }) - ).access_token; + result = await this.options.client.callAsInternalUser('shield.delegatePKI', { + body: { x509_certificate_chain: certificateChain }, + }); } catch (err) { this.logger.debug( `Failed to exchange peer certificate chain to an access token: ${err.message}` @@ -233,27 +232,18 @@ export class PKIAuthenticationProvider extends BaseAuthenticationProvider { } this.logger.debug('Successfully retrieved access token in exchange to peer certificate chain.'); - - try { - // Then attempt to query for the user details using the new token - const authHeaders = { - authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('User has been authenticated with new access token'); - return AuthenticationResult.succeeded(user, { - authHeaders, + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(result.authentication), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', result.access_token).toString(), + }, state: { - accessToken, - // NodeJS typings don't include `fingerprint256` yet. - peerCertificateFingerprint256: (peerCertificate as any).fingerprint256, + accessToken: result.access_token, + peerCertificateFingerprint256: peerCertificate.fingerprint256, }, - }); - } catch (err) { - this.logger.debug(`Failed to authenticate request via access token: ${err.message}`); - return AuthenticationResult.failed(err); - } + } + ); } /** diff --git a/x-pack/plugins/security/server/authentication/providers/saml.test.ts b/x-pack/plugins/security/server/authentication/providers/saml.test.ts index a1f2e99c133577..03c0b7404da398 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.test.ts @@ -64,6 +64,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'some-token', refresh_token: 'some-refresh-token', + authentication: mockUser, }); await expect( @@ -99,6 +100,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'some-token', refresh_token: 'some-refresh-token', + authentication: mockUser, }); provider = new SAMLAuthenticationProvider(mockOptions, { @@ -180,6 +182,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'user-initiated-login-token', refresh_token: 'user-initiated-login-refresh-token', + authentication: mockUser, }); await expect( @@ -211,6 +214,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'user-initiated-login-token', refresh_token: 'user-initiated-login-refresh-token', + authentication: mockUser, }); provider = new SAMLAuthenticationProvider(mockOptions, { @@ -250,6 +254,7 @@ describe('SAMLAuthenticationProvider', () => { mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: 'idp-initiated-login-token', refresh_token: 'idp-initiated-login-refresh-token', + authentication: mockUser, }); await expect( @@ -306,6 +311,7 @@ describe('SAMLAuthenticationProvider', () => { username: 'user', access_token: 'valid-token', refresh_token: 'valid-refresh-token', + authentication: mockUser, }); provider = new SAMLAuthenticationProvider(mockOptions, { @@ -459,6 +465,7 @@ describe('SAMLAuthenticationProvider', () => { username: 'user', access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token', + authentication: mockUser, }); const failureReason = new Error('Failed to invalidate token!'); @@ -519,6 +526,7 @@ describe('SAMLAuthenticationProvider', () => { username: 'user', access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token', + authentication: mockUser, }); mockOptions.tokens.invalidate.mockResolvedValue(undefined); @@ -566,15 +574,11 @@ describe('SAMLAuthenticationProvider', () => { // The first call is made using tokens from existing session. mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(() => response); - // The second call is made using new tokens. - mockScopedClusterClient.callAsCurrentUser.mockImplementationOnce(() => - Promise.resolve(mockUser) - ); - mockOptions.client.callAsInternalUser.mockResolvedValue({ username: 'user', access_token: 'new-valid-token', refresh_token: 'new-valid-refresh-token', + authentication: mockUser, }); mockOptions.tokens.invalidate.mockResolvedValue(undefined); @@ -849,25 +853,14 @@ describe('SAMLAuthenticationProvider', () => { realm: 'test-realm', }; - mockOptions.client.asScoped.mockImplementation((scopeableRequest) => { - if (scopeableRequest?.headers.authorization === `Bearer ${state.accessToken}`) { - const mockScopedClusterClientToFail = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClientToFail.callAsCurrentUser.mockRejectedValue( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ); - return mockScopedClusterClientToFail; - } - - if (scopeableRequest?.headers.authorization === 'Bearer new-access-token') { - return mockScopedClusterClient; - } - - throw new Error('Unexpected call'); - }); + mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) + ); mockOptions.tokens.refresh.mockResolvedValue({ accessToken: 'new-access-token', refreshToken: 'new-refresh-token', + authenticationInfo: mockUser, }); await expect(provider.authenticate(request, state)).resolves.toEqual( diff --git a/x-pack/plugins/security/server/authentication/providers/saml.ts b/x-pack/plugins/security/server/authentication/providers/saml.ts index 54619c851470ad..8f31968e5f639f 100644 --- a/x-pack/plugins/security/server/authentication/providers/saml.ts +++ b/x-pack/plugins/security/server/authentication/providers/saml.ts @@ -6,13 +6,13 @@ import Boom from '@hapi/boom'; import { KibanaRequest } from '../../../../../../src/core/server'; -import { AuthenticatedUser } from '../../../common/model'; import { isInternalURL } from '../../../common/is_internal_url'; +import type { AuthenticationInfo } from '../../elasticsearch'; import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { canRedirectRequest } from '../can_redirect_request'; import { HTTPAuthorizationHeader } from '../http_authentication'; -import { Tokens, TokenPair } from '../tokens'; +import { Tokens, TokenPair, RefreshTokenResult } from '../tokens'; import { AuthenticationProviderOptions, BaseAuthenticationProvider } from './base'; /** @@ -338,24 +338,17 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { : 'Login has been initiated by Identity Provider.' ); - let accessToken; - let refreshToken; + let result: { access_token: string; refresh_token: string; authentication: AuthenticationInfo }; try { // This operation should be performed on behalf of the user with a privilege that normal // user usually doesn't have `cluster:admin/xpack/security/saml/authenticate`. - const authenticateResponse = await this.options.client.callAsInternalUser( - 'shield.samlAuthenticate', - { - body: { - ids: !isIdPInitiatedLogin ? [stateRequestId] : [], - content: samlResponse, - realm: this.realm, - }, - } - ); - - accessToken = authenticateResponse.access_token; - refreshToken = authenticateResponse.refresh_token; + result = await this.options.client.callAsInternalUser('shield.samlAuthenticate', { + body: { + ids: !isIdPInitiatedLogin ? [stateRequestId] : [], + content: samlResponse, + realm: this.realm, + }, + }); } catch (err) { this.logger.debug(`Failed to log in with SAML response: ${err.message}`); @@ -367,17 +360,6 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { : AuthenticationResult.failed(err); } - // Now we need to retrieve full user information. - let user: Readonly; - try { - user = await this.getUser(request, { - authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), - }); - } catch (err) { - this.logger.debug(`Failed to retrieve user using access token: ${err.message}`); - return AuthenticationResult.failed(err); - } - // IdP can pass `RelayState` with the deep link in Kibana during IdP initiated login and // depending on the configuration we may need to redirect user to this URL. let redirectURLFromRelayState; @@ -401,7 +383,14 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { this.logger.debug('Login has been performed with SAML response.'); return AuthenticationResult.redirectTo( redirectURLFromRelayState || stateRedirectURL || `${this.options.basePath.get(request)}/`, - { state: { accessToken, refreshToken, realm: this.realm }, user } + { + state: { + accessToken: result.access_token, + refreshToken: result.refresh_token, + realm: this.realm, + }, + user: this.authenticationInfoToAuthenticatedUser(result.authentication), + } ); } @@ -494,20 +483,17 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { * @param request Request instance. * @param state State value previously stored by the provider. */ - private async authenticateViaRefreshToken( - request: KibanaRequest, - { refreshToken }: ProviderState - ) { + private async authenticateViaRefreshToken(request: KibanaRequest, state: ProviderState) { this.logger.debug('Trying to refresh access token.'); - if (!refreshToken) { + if (!state.refreshToken) { this.logger.debug('Refresh token is not found in state.'); return AuthenticationResult.notHandled(); } - let refreshedTokenPair: TokenPair | null; + let refreshTokenResult: RefreshTokenResult | null; try { - refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + refreshTokenResult = await this.options.tokens.refresh(state.refreshToken); } catch (err) { return AuthenticationResult.failed(err); } @@ -517,7 +503,7 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { // handshake. Obviously we can't do that for AJAX requests, so we just reply with `400` and clear error message. // There are two reasons for `400` and not `401`: Elasticsearch search responds with `400` so it seems logical // to do the same on Kibana side and `401` would force user to logout and do full SLO if it's supported. - if (refreshedTokenPair === null) { + if (refreshTokenResult === null) { if (canStartNewSession(request)) { this.logger.debug( 'Both access and refresh tokens are expired. Capturing redirect URL and re-initiating SAML handshake.' @@ -530,26 +516,17 @@ export class SAMLAuthenticationProvider extends BaseAuthenticationProvider { ); } - try { - const authHeaders = { - authorization: new HTTPAuthorizationHeader( - 'Bearer', - refreshedTokenPair.accessToken - ).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { - authHeaders, - state: { realm: this.realm, ...refreshedTokenPair }, - }); - } catch (err) { - this.logger.debug( - `Failed to authenticate user using newly refreshed access token: ${err.message}` - ); - return AuthenticationResult.failed(err); - } + this.logger.debug('Request has been authenticated via refreshed token.'); + const { accessToken, refreshToken, authenticationInfo } = refreshTokenResult; + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(authenticationInfo), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + }, + state: { accessToken, refreshToken, realm: this.realm }, + } + ); } /** diff --git a/x-pack/plugins/security/server/authentication/providers/token.test.ts b/x-pack/plugins/security/server/authentication/providers/token.test.ts index 4501004ab69c17..e09400e9bb44aa 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.test.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.test.ts @@ -49,13 +49,10 @@ describe('TokenAuthenticationProvider', () => { const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; const authorization = `Bearer ${tokenPair.accessToken}`; - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - mockOptions.client.callAsInternalUser.mockResolvedValue({ access_token: tokenPair.accessToken, refresh_token: tokenPair.refreshToken, + authentication: user, }); await expect(provider.login(request, credentials)).resolves.toEqual( @@ -65,8 +62,7 @@ describe('TokenAuthenticationProvider', () => { ) ); - expectAuthenticateCall(mockOptions.client, { headers: { authorization } }); - + expect(mockOptions.client.asScoped).not.toHaveBeenCalled(); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { body: { grant_type: 'password', ...credentials }, @@ -93,36 +89,6 @@ describe('TokenAuthenticationProvider', () => { expect(request.headers).not.toHaveProperty('authorization'); }); - - it('fails if user cannot be retrieved during login attempt', async () => { - const request = httpServerMock.createKibanaRequest({ headers: {} }); - const credentials = { username: 'user', password: 'password' }; - const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - const authorization = `Bearer ${tokenPair.accessToken}`; - - mockOptions.client.callAsInternalUser.mockResolvedValue({ - access_token: tokenPair.accessToken, - refresh_token: tokenPair.refreshToken, - }); - - const authenticationError = new Error('Some error'); - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(authenticationError); - mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); - - await expect(provider.login(request, credentials)).resolves.toEqual( - AuthenticationResult.failed(authenticationError) - ); - - expectAuthenticateCall(mockOptions.client, { headers: { authorization } }); - - expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledTimes(1); - expect(mockOptions.client.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { - body: { grant_type: 'password', ...credentials }, - }); - - expect(request.headers).not.toHaveProperty('authorization'); - }); }); describe('`authenticate` method', () => { @@ -211,27 +177,16 @@ describe('TokenAuthenticationProvider', () => { const request = httpServerMock.createKibanaRequest(); const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - mockOptions.client.asScoped.mockImplementation((scopeableRequest) => { - if (scopeableRequest?.headers.authorization === `Bearer ${tokenPair.accessToken}`) { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ); - return mockScopedClusterClient; - } - - if (scopeableRequest?.headers.authorization === 'Bearer newfoo') { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockResolvedValue(user); - return mockScopedClusterClient; - } - - throw new Error('Unexpected call'); - }); + const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); + mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( + LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) + ); + mockOptions.client.asScoped.mockReturnValue(mockScopedClusterClient); mockOptions.tokens.refresh.mockResolvedValue({ accessToken: 'newfoo', refreshToken: 'newbar', + authenticationInfo: user, }); await expect(provider.authenticate(request, tokenPair)).resolves.toEqual( @@ -381,44 +336,6 @@ describe('TokenAuthenticationProvider', () => { expect(request.headers).not.toHaveProperty('authorization'); }); - - it('fails if new access token is rejected after successful refresh', async () => { - const request = httpServerMock.createKibanaRequest(); - const tokenPair = { accessToken: 'foo', refreshToken: 'bar' }; - - const authenticationError = new errors.AuthenticationException('Some error'); - mockOptions.client.asScoped.mockImplementation((scopeableRequest) => { - if (scopeableRequest?.headers.authorization === `Bearer ${tokenPair.accessToken}`) { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue( - LegacyElasticsearchErrorHelpers.decorateNotAuthorizedError(new Error()) - ); - return mockScopedClusterClient; - } - - if (scopeableRequest?.headers.authorization === 'Bearer newfoo') { - const mockScopedClusterClient = elasticsearchServiceMock.createLegacyScopedClusterClient(); - mockScopedClusterClient.callAsCurrentUser.mockRejectedValue(authenticationError); - return mockScopedClusterClient; - } - - throw new Error('Unexpected call'); - }); - - mockOptions.tokens.refresh.mockResolvedValue({ - accessToken: 'newfoo', - refreshToken: 'newbar', - }); - - await expect(provider.authenticate(request, tokenPair)).resolves.toEqual( - AuthenticationResult.failed(authenticationError) - ); - - expect(mockOptions.tokens.refresh).toHaveBeenCalledTimes(1); - expect(mockOptions.tokens.refresh).toHaveBeenCalledWith(tokenPair.refreshToken); - - expect(request.headers).not.toHaveProperty('authorization'); - }); }); describe('`logout` method', () => { diff --git a/x-pack/plugins/security/server/authentication/providers/token.ts b/x-pack/plugins/security/server/authentication/providers/token.ts index f919c20c152255..2032db4b0a8f29 100644 --- a/x-pack/plugins/security/server/authentication/providers/token.ts +++ b/x-pack/plugins/security/server/authentication/providers/token.ts @@ -10,7 +10,7 @@ import { AuthenticationResult } from '../authentication_result'; import { DeauthenticationResult } from '../deauthentication_result'; import { canRedirectRequest } from '../can_redirect_request'; import { HTTPAuthorizationHeader } from '../http_authentication'; -import { Tokens, TokenPair } from '../tokens'; +import { Tokens, TokenPair, RefreshTokenResult } from '../tokens'; import { BaseAuthenticationProvider } from './base'; /** @@ -63,23 +63,21 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { const { access_token: accessToken, refresh_token: refreshToken, + authentication: authenticationInfo, } = await this.options.client.callAsInternalUser('shield.getAccessToken', { body: { grant_type: 'password', username, password }, }); this.logger.debug('Get token API request to Elasticsearch successful'); - - // Then attempt to query for the user details using the new token - const authHeaders = { - authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('Login has been successfully performed.'); - return AuthenticationResult.succeeded(user, { - authHeaders, - state: { accessToken, refreshToken }, - }); + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(authenticationInfo), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + }, + state: { accessToken, refreshToken }, + } + ); } catch (err) { this.logger.debug(`Failed to perform a login: ${err.message}`); return AuthenticationResult.failed(err); @@ -191,22 +189,19 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { * @param request Request instance. * @param state State value previously stored by the provider. */ - private async authenticateViaRefreshToken( - request: KibanaRequest, - { refreshToken }: ProviderState - ) { + private async authenticateViaRefreshToken(request: KibanaRequest, state: ProviderState) { this.logger.debug('Trying to refresh access token.'); - let refreshedTokenPair: TokenPair | null; + let refreshTokenResult: RefreshTokenResult | null; try { - refreshedTokenPair = await this.options.tokens.refresh(refreshToken); + refreshTokenResult = await this.options.tokens.refresh(state.refreshToken); } catch (err) { return AuthenticationResult.failed(err); } // If refresh token is no longer valid, then we should clear session and redirect user to the // login page to re-authenticate, or fail if redirect isn't possible. - if (refreshedTokenPair === null) { + if (refreshTokenResult === null) { if (canStartNewSession(request)) { this.logger.debug('Clearing session since both access and refresh tokens are expired.'); @@ -219,23 +214,17 @@ export class TokenAuthenticationProvider extends BaseAuthenticationProvider { ); } - try { - const authHeaders = { - authorization: new HTTPAuthorizationHeader( - 'Bearer', - refreshedTokenPair.accessToken - ).toString(), - }; - const user = await this.getUser(request, authHeaders); - - this.logger.debug('Request has been authenticated via refreshed token.'); - return AuthenticationResult.succeeded(user, { authHeaders, state: refreshedTokenPair }); - } catch (err) { - this.logger.debug( - `Failed to authenticate user using newly refreshed access token: ${err.message}` - ); - return AuthenticationResult.failed(err); - } + this.logger.debug('Request has been authenticated via refreshed token.'); + const { accessToken, refreshToken, authenticationInfo } = refreshTokenResult; + return AuthenticationResult.succeeded( + this.authenticationInfoToAuthenticatedUser(authenticationInfo), + { + authHeaders: { + authorization: new HTTPAuthorizationHeader('Bearer', accessToken).toString(), + }, + state: { accessToken, refreshToken }, + } + ); } /** diff --git a/x-pack/plugins/security/server/authentication/tokens.test.ts b/x-pack/plugins/security/server/authentication/tokens.test.ts index e8cf37330aff26..18fdcf8608d29d 100644 --- a/x-pack/plugins/security/server/authentication/tokens.test.ts +++ b/x-pack/plugins/security/server/authentication/tokens.test.ts @@ -7,6 +7,7 @@ import { errors } from 'elasticsearch'; import { elasticsearchServiceMock, loggingSystemMock } from '../../../../../src/core/server/mocks'; +import { mockAuthenticatedUser } from '../../common/model/authenticated_user.mock'; import { ILegacyClusterClient, @@ -78,13 +79,18 @@ describe('Tokens', () => { }); it('returns token pair if refresh API call succeeds', async () => { + const authenticationInfo = mockAuthenticatedUser(); const tokenPair = { accessToken: 'access-token', refreshToken: 'refresh-token' }; mockClusterClient.callAsInternalUser.mockResolvedValue({ access_token: tokenPair.accessToken, refresh_token: tokenPair.refreshToken, + authentication: authenticationInfo, }); - await expect(tokens.refresh(refreshToken)).resolves.toEqual(tokenPair); + await expect(tokens.refresh(refreshToken)).resolves.toEqual({ + authenticationInfo, + ...tokenPair, + }); expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledTimes(1); expect(mockClusterClient.callAsInternalUser).toHaveBeenCalledWith('shield.getAccessToken', { diff --git a/x-pack/plugins/security/server/authentication/tokens.ts b/x-pack/plugins/security/server/authentication/tokens.ts index 3918b0b190a154..a435452ae112f2 100644 --- a/x-pack/plugins/security/server/authentication/tokens.ts +++ b/x-pack/plugins/security/server/authentication/tokens.ts @@ -5,6 +5,7 @@ */ import { ILegacyClusterClient, Logger } from '../../../../../src/core/server'; +import type { AuthenticationInfo } from '../elasticsearch'; import { getErrorStatusCode } from '../errors'; /** @@ -24,6 +25,13 @@ export interface TokenPair { readonly refreshToken: string; } +/** + * Represents the result of the token refresh operation. + */ +export interface RefreshTokenResult extends TokenPair { + authenticationInfo: AuthenticationInfo; +} + /** * Class responsible for managing access and refresh tokens (refresh, invalidate, etc.) used by * various authentication providers. @@ -44,19 +52,20 @@ export class Tokens { * Tries to exchange provided refresh token to a new pair of access and refresh tokens. * @param existingRefreshToken Refresh token to send to the refresh token API. */ - public async refresh(existingRefreshToken: string): Promise { + public async refresh(existingRefreshToken: string): Promise { try { // Token should be refreshed by the same user that obtained that token. const { access_token: accessToken, refresh_token: refreshToken, + authentication: authenticationInfo, } = await this.options.client.callAsInternalUser('shield.getAccessToken', { body: { grant_type: 'refresh_token', refresh_token: existingRefreshToken }, }); this.logger.debug('Access token has been successfully refreshed.'); - return { accessToken, refreshToken }; + return { accessToken, refreshToken, authenticationInfo }; } catch (err) { this.logger.debug(`Failed to refresh access token: ${err.message}`); diff --git a/x-pack/plugins/security/server/elasticsearch/index.ts b/x-pack/plugins/security/server/elasticsearch/index.ts index 793bdc1c6ad261..23e4876904c317 100644 --- a/x-pack/plugins/security/server/elasticsearch/index.ts +++ b/x-pack/plugins/security/server/elasticsearch/index.ts @@ -4,6 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ +import { AuthenticatedUser } from '../../common/model'; + +export type AuthenticationInfo = Omit; export { ElasticsearchService, ElasticsearchServiceSetup, diff --git a/x-pack/scripts/functional_tests.js b/x-pack/scripts/functional_tests.js index 5e877717fd21e6..9db102e4871165 100644 --- a/x-pack/scripts/functional_tests.js +++ b/x-pack/scripts/functional_tests.js @@ -42,7 +42,7 @@ const onlyNotInCoverageTests = [ require.resolve('../test/security_api_integration/pki.config.ts'), require.resolve('../test/security_api_integration/oidc.config.ts'), require.resolve('../test/security_api_integration/oidc_implicit_flow.config.ts'), - require.resolve('../test/token_api_integration/config.js'), + require.resolve('../test/security_api_integration/token.config.ts'), require.resolve('../test/observability_api_integration/basic/config.ts'), require.resolve('../test/observability_api_integration/trial/config.ts'), require.resolve('../test/encrypted_saved_objects_api_integration/config.ts'), diff --git a/x-pack/test/security_api_integration/tests/pki/pki_auth.ts b/x-pack/test/security_api_integration/tests/pki/pki_auth.ts index 0331f756712cab..93eabe33dc6876 100644 --- a/x-pack/test/security_api_integration/tests/pki/pki_auth.ts +++ b/x-pack/test/security_api_integration/tests/pki/pki_auth.ts @@ -179,7 +179,7 @@ export default function ({ getService }: FtrProviderContext) { authentication_realm: { name: 'pki1', type: 'pki' }, lookup_realm: { name: 'pki1', type: 'pki' }, authentication_provider: { name: 'pki', type: 'pki' }, - authentication_type: 'token', + authentication_type: 'realm', }); checkCookieIsSet(request.cookie(response.headers['set-cookie'][0])!); diff --git a/x-pack/test/token_api_integration/auth/header.js b/x-pack/test/security_api_integration/tests/token/header.ts similarity index 91% rename from x-pack/test/token_api_integration/auth/header.js rename to x-pack/test/security_api_integration/tests/token/header.ts index 0cc233b56d9849..2150d7a6269b0c 100644 --- a/x-pack/test/token_api_integration/auth/header.js +++ b/x-pack/test/security_api_integration/tests/token/header.ts @@ -4,12 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -export default function ({ getService }) { +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertestWithoutAuth'); const es = getService('legacyEs'); async function createToken() { - const { access_token: accessToken } = await es.shield.getAccessToken({ + const { access_token: accessToken } = await (es as any).shield.getAccessToken({ body: { grant_type: 'password', username: 'elastic', diff --git a/x-pack/test/token_api_integration/auth/index.js b/x-pack/test/security_api_integration/tests/token/index.ts similarity index 72% rename from x-pack/test/token_api_integration/auth/index.js rename to x-pack/test/security_api_integration/tests/token/index.ts index e7b5a5b46a503c..e9bf6c641fb1fb 100644 --- a/x-pack/test/token_api_integration/auth/index.js +++ b/x-pack/test/security_api_integration/tests/token/index.ts @@ -4,8 +4,10 @@ * you may not use this file except in compliance with the Elastic License. */ -export default function ({ loadTestFile }) { - describe('token-based auth', function () { +import { FtrProviderContext } from '../../ftr_provider_context'; + +export default function ({ loadTestFile }: FtrProviderContext) { + describe('security APIs - Token', function () { this.tags('ciGroup6'); loadTestFile(require.resolve('./login')); loadTestFile(require.resolve('./logout')); diff --git a/x-pack/test/token_api_integration/auth/login.js b/x-pack/test/security_api_integration/tests/token/login.ts similarity index 94% rename from x-pack/test/token_api_integration/auth/login.js rename to x-pack/test/security_api_integration/tests/token/login.ts index b2dd870e018dad..82abb10847e06e 100644 --- a/x-pack/test/token_api_integration/auth/login.js +++ b/x-pack/test/security_api_integration/tests/token/login.ts @@ -5,11 +5,12 @@ */ import request from 'request'; +import { FtrProviderContext } from '../../ftr_provider_context'; -export default function ({ getService }) { +export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertestWithoutAuth'); - function extractSessionCookie(response) { + function extractSessionCookie(response: { headers: Record }) { const cookie = (response.headers['set-cookie'] || []).find((header) => header.startsWith('sid=') ); diff --git a/x-pack/test/token_api_integration/auth/logout.js b/x-pack/test/security_api_integration/tests/token/logout.ts similarity index 91% rename from x-pack/test/token_api_integration/auth/logout.js rename to x-pack/test/security_api_integration/tests/token/logout.ts index fcc0e8182158f2..ccd8b4586be2db 100644 --- a/x-pack/test/token_api_integration/auth/logout.js +++ b/x-pack/test/security_api_integration/tests/token/logout.ts @@ -5,11 +5,12 @@ */ import request from 'request'; +import { FtrProviderContext } from '../../ftr_provider_context'; -export default function ({ getService }) { +export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertestWithoutAuth'); - function extractSessionCookie(response) { + function extractSessionCookie(response: { headers: Record }) { const cookie = (response.headers['set-cookie'] || []).find((header) => header.startsWith('sid=') ); diff --git a/x-pack/test/token_api_integration/auth/session.js b/x-pack/test/security_api_integration/tests/token/session.ts similarity index 91% rename from x-pack/test/token_api_integration/auth/session.js rename to x-pack/test/security_api_integration/tests/token/session.ts index 1f69b06315a80e..30e004a0fff3c5 100644 --- a/x-pack/test/token_api_integration/auth/session.js +++ b/x-pack/test/security_api_integration/tests/token/session.ts @@ -4,15 +4,16 @@ * you may not use this file except in compliance with the Elastic License. */ -import request from 'request'; +import request, { Cookie } from 'request'; import expect from '@kbn/expect'; +import { FtrProviderContext } from '../../ftr_provider_context'; -const delay = (ms) => new Promise((resolve) => setTimeout(() => resolve(), ms)); +const delay = (ms: number) => new Promise((resolve) => setTimeout(() => resolve(), ms)); -export default function ({ getService }) { +export default function ({ getService }: FtrProviderContext) { const supertest = getService('supertestWithoutAuth'); - function extractSessionCookie(response) { + function extractSessionCookie(response: { headers: Record }) { const cookie = (response.headers['set-cookie'] || []).find((header) => header.startsWith('sid=') ); @@ -68,7 +69,7 @@ export default function ({ getService }) { }); describe('API access with expired access token.', function () { - const expectNewSessionCookie = (originalCookie, newCookie) => { + const expectNewSessionCookie = (originalCookie: Cookie, newCookie: Cookie) => { if (!newCookie) { throw new Error('No session cookie set after token refresh'); } @@ -97,7 +98,7 @@ export default function ({ getService }) { .set('cookie', originalCookie.cookieString()) .expect(200); - const firstNewCookie = extractSessionCookie(firstResponse); + const firstNewCookie = extractSessionCookie(firstResponse)!; expectNewSessionCookie(originalCookie, firstNewCookie); // Request with old cookie should return another valid cookie we can use to authenticate requests @@ -108,7 +109,7 @@ export default function ({ getService }) { .set('Cookie', originalCookie.cookieString()) .expect(200); - const secondNewCookie = extractSessionCookie(secondResponse); + const secondNewCookie = extractSessionCookie(secondResponse)!; expectNewSessionCookie(originalCookie, secondNewCookie); if (secondNewCookie.value === firstNewCookie.value) { @@ -132,7 +133,7 @@ export default function ({ getService }) { }); describe('API access with missing access token document.', () => { - let sessionCookie; + let sessionCookie: Cookie; beforeEach(async () => (sessionCookie = await createSessionCookie())); it('should clear cookie and redirect to login', async function () { @@ -155,7 +156,7 @@ export default function ({ getService }) { const cookies = response.headers['set-cookie']; expect(cookies).to.have.length(1); - const cookie = request.cookie(cookies[0]); + const cookie = request.cookie(cookies[0])!; expect(cookie.key).to.be('sid'); expect(cookie.value).to.be.empty(); expect(cookie.path).to.be('/'); diff --git a/x-pack/test/token_api_integration/config.js b/x-pack/test/security_api_integration/token.config.ts similarity index 81% rename from x-pack/test/token_api_integration/config.js rename to x-pack/test/security_api_integration/token.config.ts index 3e78a98067a8f8..c7afa51edba5e5 100644 --- a/x-pack/test/token_api_integration/config.js +++ b/x-pack/test/security_api_integration/token.config.ts @@ -4,11 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -export default async function ({ readConfigFile }) { +import { FtrConfigProviderContext } from '@kbn/test/types/ftr'; + +export default async function ({ readConfigFile }: FtrConfigProviderContext) { const xPackAPITestsConfig = await readConfigFile(require.resolve('../api_integration/config.ts')); return { - testFiles: [require.resolve('./auth')], + testFiles: [require.resolve('./tests/token')], servers: xPackAPITestsConfig.get('servers'), security: { disableTestUser: true }, services: { @@ -16,7 +18,7 @@ export default async function ({ readConfigFile }) { supertestWithoutAuth: xPackAPITestsConfig.get('services.supertestWithoutAuth'), }, junit: { - reportName: 'Token-auth API Integration Tests', + reportName: 'X-Pack Security API Integration Tests (Token)', }, esTestCluster: { From ece505b0759fcb5cdfb4acaf19e432e407dda9e2 Mon Sep 17 00:00:00 2001 From: Mikhail Shustov Date: Tue, 10 Nov 2020 21:26:35 +0300 Subject: [PATCH 03/57] require schema for UiSettings (#83037) --- .../ui_settings/ui_settings_service.test.ts | 29 +++++++++++++++++++ .../server/ui_settings/ui_settings_service.ts | 6 ++-- 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/core/server/ui_settings/ui_settings_service.test.ts b/src/core/server/ui_settings/ui_settings_service.test.ts index 0c17a3a614d60c..f4e24e3a517c36 100644 --- a/src/core/server/ui_settings/ui_settings_service.test.ts +++ b/src/core/server/ui_settings/ui_settings_service.test.ts @@ -89,6 +89,20 @@ describe('uiSettings', () => { describe('#start', () => { describe('validation', () => { + it('throws if validation schema is not provided', async () => { + const { register } = await service.setup(setupDeps); + register({ + // @ts-expect-error schema is required key + custom: { + value: 42, + }, + }); + + await expect(service.start()).rejects.toMatchInlineSnapshot( + `[Error: Validation schema is not provided for [custom] UI Setting]` + ); + }); + it('validates registered definitions', async () => { const { register } = await service.setup(setupDeps); register({ @@ -125,6 +139,21 @@ describe('uiSettings', () => { `[Error: [ui settings overrides [custom]]: expected value of type [string] but got [number]]` ); }); + + it('do not throw on unknown overrides', async () => { + const coreContext = mockCoreContext.create(); + coreContext.configService.atPath.mockReturnValueOnce( + new BehaviorSubject({ + overrides: { + custom: 42, + }, + }) + ); + const customizedService = new UiSettingsService(coreContext); + await customizedService.setup(setupDeps); + + await customizedService.start(); + }); }); describe('#asScopedToClient', () => { diff --git a/src/core/server/ui_settings/ui_settings_service.ts b/src/core/server/ui_settings/ui_settings_service.ts index 25062490f5b6b5..4f757d18ea7da6 100644 --- a/src/core/server/ui_settings/ui_settings_service.ts +++ b/src/core/server/ui_settings/ui_settings_service.ts @@ -109,15 +109,17 @@ export class UiSettingsService private validatesDefinitions() { for (const [key, definition] of this.uiSettingsDefaults) { - if (definition.schema) { - definition.schema.validate(definition.value, {}, `ui settings defaults [${key}]`); + if (!definition.schema) { + throw new Error(`Validation schema is not provided for [${key}] UI Setting`); } + definition.schema.validate(definition.value, {}, `ui settings defaults [${key}]`); } } private validatesOverrides() { for (const [key, value] of Object.entries(this.overrides)) { const definition = this.uiSettingsDefaults.get(key); + // overrides might contain UiSettings for a disabled plugin if (definition?.schema) { definition.schema.validate(value, {}, `ui settings overrides [${key}]`); } From ccb0b354525a072c9b637f1147637391fdf8fa0b Mon Sep 17 00:00:00 2001 From: Wylie Conlon Date: Tue, 10 Nov 2020 13:45:46 -0500 Subject: [PATCH 04/57] [Lens] Use entire layers, not specific columns (#82550) * [Lens] Use entire layers, not specific columns * Fix types * Move all of state_helpers over * Fix tests * Fix crash and add tests to prevent future issues * Prevent users from dropping duplicate fields * Respond to review feedback * Fix review feedback Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> --- .../config_panel/layer_panel.test.tsx | 37 + .../editor_frame/config_panel/layer_panel.tsx | 19 +- .../__mocks__/state_helpers.ts | 21 - .../bucket_nesting_editor.test.tsx | 61 +- .../dimension_panel/dimension_editor.tsx | 107 +- .../dimension_panel/dimension_panel.test.tsx | 23 +- .../dimension_panel/droppable.test.ts | 280 ++--- .../dimension_panel/droppable.ts | 71 +- .../indexpattern_datasource/indexpattern.tsx | 9 +- .../indexpattern_suggestions.test.tsx | 105 +- .../indexpattern_suggestions.ts | 400 +++---- .../layerpanel.test.tsx | 2 - .../public/indexpattern_datasource/loader.ts | 2 +- .../operations/__mocks__/index.ts | 27 +- .../operations/definitions/cardinality.tsx | 10 +- .../operations/definitions/column_types.ts | 3 +- .../operations/definitions/count.tsx | 10 +- .../definitions/date_histogram.test.tsx | 10 +- .../operations/definitions/date_histogram.tsx | 17 +- .../definitions/filters/filters.tsx | 5 +- .../operations/definitions/index.ts | 15 +- .../operations/definitions/metrics.tsx | 5 +- .../operations/definitions/ranges/ranges.tsx | 37 +- .../operations/definitions/terms/index.tsx | 12 +- .../definitions/terms/terms.test.tsx | 38 +- .../operations/index.ts | 1 + .../layer_helpers.test.ts} | 1008 +++++++++++------ .../operations/layer_helpers.ts | 344 ++++++ .../operations/operations.test.ts | 72 +- .../operations/operations.ts | 97 +- .../indexpattern_datasource/state_helpers.ts | 180 +-- x-pack/plugins/lens/public/types.ts | 8 - .../public/xy_visualization/visualization.tsx | 2 - x-pack/plugins/lens/server/migrations.test.ts | 88 +- x-pack/plugins/lens/server/migrations.ts | 30 +- 35 files changed, 1710 insertions(+), 1446 deletions(-) delete mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/__mocks__/state_helpers.ts rename x-pack/plugins/lens/public/indexpattern_datasource/{state_helpers.test.ts => operations/layer_helpers.test.ts} (52%) create mode 100644 x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx index 56425326c1ce1a..c0cd211a49dd9a 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.test.tsx @@ -371,6 +371,43 @@ describe('LayerPanel', () => { ); }); + it('should determine if the datasource supports dropping of a field onto a pre-filled dimension', () => { + mockVisualization.getConfiguration.mockReturnValue({ + groups: [ + { + groupLabel: 'A', + groupId: 'a', + accessors: ['a'], + filterOperations: () => true, + supportsMoreColumns: true, + dataTestSubj: 'lnsGroup', + }, + ], + }); + + mockDatasource.canHandleDrop.mockImplementation(({ columnId }) => columnId !== 'a'); + + const draggingField = { field: { name: 'dragged' }, indexPatternId: 'a', id: '1' }; + + const component = mountWithIntl( + + + + ); + + expect(mockDatasource.canHandleDrop).toHaveBeenCalledWith( + expect.objectContaining({ columnId: 'a' }) + ); + + expect( + component.find('DragDrop[data-test-subj="lnsGroup"]').first().prop('droppable') + ).toEqual(false); + + component.find('DragDrop[data-test-subj="lnsGroup"]').first().simulate('drop'); + + expect(mockDatasource.onDrop).not.toHaveBeenCalled(); + }); + it('should allow drag to move between groups', () => { (generateId as jest.Mock).mockReturnValue(`newid`); diff --git a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx index 0332f11aa78b34..f780f9c3f22d76 100644 --- a/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx +++ b/x-pack/plugins/lens/public/editor_frame_service/editor_frame/config_panel/layer_panel.tsx @@ -235,6 +235,17 @@ export function LayerPanel( dragging.groupId === group.groupId && dragging.columnId !== accessor && dragging.groupId !== 'y'; // TODO: remove this line when https://github.com/elastic/elastic-charts/issues/868 is fixed + + const isDroppable = isDraggedOperation(dragging) + ? dragType === 'reorder' + ? isFromTheSameGroup + : isFromCompatibleGroup + : layerDatasource.canHandleDrop({ + ...layerDatasourceDropProps, + columnId: accessor, + filterOperations: group.filterOperations, + }); + return ( { layerDatasource.onDrop({ isReorder: true, @@ -303,7 +310,6 @@ export function LayerPanel( ...layerDatasourceConfigProps, columnId: accessor, filterOperations: group.filterOperations, - suggestedPriority: group.suggestedPriority, onClick: () => { if (activeId) { setActiveDimension(initialActiveDimensionState); @@ -450,7 +456,6 @@ export function LayerPanel( core: props.core, columnId: activeId, filterOperations: activeGroup.filterOperations, - suggestedPriority: activeGroup?.suggestedPriority, dimensionGroups: groups, setState: (newState: unknown) => { props.updateAll( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/__mocks__/state_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/__mocks__/state_helpers.ts deleted file mode 100644 index 47687ef10f8824..00000000000000 --- a/x-pack/plugins/lens/public/indexpattern_datasource/__mocks__/state_helpers.ts +++ /dev/null @@ -1,21 +0,0 @@ -/* - * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one - * or more contributor license agreements. Licensed under the Elastic License; - * you may not use this file except in compliance with the Elastic License. - */ - -const actual = jest.requireActual('../state_helpers'); - -jest.spyOn(actual, 'changeColumn'); -jest.spyOn(actual, 'updateLayerIndexPattern'); - -export const { - getColumnOrder, - changeColumn, - deleteColumn, - updateColumnParam, - sortByField, - hasField, - updateLayerIndexPattern, - mergeLayer, -} = actual; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/bucket_nesting_editor.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/bucket_nesting_editor.test.tsx index ee6a86072236c4..ef0b7f15544781 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/bucket_nesting_editor.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/bucket_nesting_editor.test.tsx @@ -31,7 +31,6 @@ describe('BucketNestingEditor', () => { orderDirection: 'asc', }, sourceField: 'a', - suggestedPriority: 0, ...col, }; @@ -46,9 +45,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['a', 'b', 'c'], columns: { - a: mockCol({ suggestedPriority: 0 }), - b: mockCol({ suggestedPriority: 1 }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol(), + b: mockCol(), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }} @@ -67,9 +66,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['b', 'a', 'c'], columns: { - a: mockCol({ suggestedPriority: 0 }), - b: mockCol({ suggestedPriority: 1 }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol(), + b: mockCol(), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }} @@ -89,9 +88,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['b', 'a', 'c'], columns: { - a: mockCol({ suggestedPriority: 0 }), - b: mockCol({ suggestedPriority: 1 }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol(), + b: mockCol(), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }} @@ -109,9 +108,9 @@ describe('BucketNestingEditor', () => { layer: { columnOrder: ['a', 'b', 'c'], columns: { - a: mockCol({ suggestedPriority: 0 }), - b: mockCol({ suggestedPriority: 1 }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol(), + b: mockCol(), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }, @@ -134,9 +133,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['a', 'b', 'c'], columns: { - a: mockCol({ suggestedPriority: 0, operationType: 'avg', isBucketed: false }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: false }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol({ operationType: 'avg', isBucketed: false }), + b: mockCol({ operationType: 'max', isBucketed: false }), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }} @@ -155,9 +154,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['a', 'b', 'c'], columns: { - a: mockCol({ suggestedPriority: 0 }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: false }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: false }), + a: mockCol(), + b: mockCol({ operationType: 'max', isBucketed: false }), + c: mockCol({ operationType: 'min', isBucketed: false }), }, indexPatternId: 'foo', }} @@ -176,9 +175,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['c', 'a', 'b'], columns: { - a: mockCol({ suggestedPriority: 0, operationType: 'count', isBucketed: true }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: true }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: true }), + a: mockCol({ operationType: 'count', isBucketed: true }), + b: mockCol({ operationType: 'max', isBucketed: true }), + c: mockCol({ operationType: 'min', isBucketed: true }), }, indexPatternId: 'foo', }} @@ -200,9 +199,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['c', 'a', 'b'], columns: { - a: mockCol({ suggestedPriority: 0, operationType: 'count', isBucketed: true }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: true }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: true }), + a: mockCol({ operationType: 'count', isBucketed: true }), + b: mockCol({ operationType: 'max', isBucketed: true }), + c: mockCol({ operationType: 'min', isBucketed: true }), }, indexPatternId: 'foo', }} @@ -227,9 +226,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['c', 'a', 'b'], columns: { - a: mockCol({ suggestedPriority: 0, operationType: 'count', isBucketed: true }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: true }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: true }), + a: mockCol({ operationType: 'count', isBucketed: true }), + b: mockCol({ operationType: 'max', isBucketed: true }), + c: mockCol({ operationType: 'min', isBucketed: true }), }, indexPatternId: 'foo', }} @@ -254,9 +253,9 @@ describe('BucketNestingEditor', () => { layer={{ columnOrder: ['c', 'a', 'b'], columns: { - a: mockCol({ suggestedPriority: 0, operationType: 'count', isBucketed: true }), - b: mockCol({ suggestedPriority: 1, operationType: 'max', isBucketed: true }), - c: mockCol({ suggestedPriority: 2, operationType: 'min', isBucketed: true }), + a: mockCol({ operationType: 'count', isBucketed: true }), + b: mockCol({ operationType: 'max', isBucketed: true }), + c: mockCol({ operationType: 'min', isBucketed: true }), }, indexPatternId: 'foo', }} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx index 7cbfbc17493824..cd196745f3315d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_editor.tsx @@ -22,14 +22,16 @@ import { IndexPatternColumn, OperationType } from '../indexpattern'; import { operationDefinitionMap, getOperationDisplay, - buildColumn, - changeField, + insertOrReplaceColumn, + replaceColumn, + deleteColumn, + updateColumnParam, } from '../operations'; -import { deleteColumn, changeColumn, updateColumnParam, mergeLayer } from '../state_helpers'; +import { mergeLayer } from '../state_helpers'; import { FieldSelect } from './field_select'; import { hasField, fieldIsInvalid } from '../utils'; import { BucketNestingEditor } from './bucket_nesting_editor'; -import { IndexPattern } from '../types'; +import { IndexPattern, IndexPatternLayer } from '../types'; import { trackUiEvent } from '../../lens_ui_telemetry'; import { FormatSelector } from './format_selector'; @@ -170,21 +172,13 @@ export function DimensionEditor(props: DimensionEditorProps) { if (selectedColumn?.operationType === operationType) { return; } - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn: buildColumn({ - columns: props.state.layers[props.layerId].columns, - suggestedPriority: props.suggestedPriority, - layerId: props.layerId, - op: operationType, - indexPattern: currentIndexPattern, - previousColumn: selectedColumn, - }), - }) - ); + const newLayer = insertOrReplaceColumn({ + layer: props.state.layers[props.layerId], + indexPattern: currentIndexPattern, + columnId, + op: operationType, + }); + setState(mergeLayer({ state, layerId, newLayer })); trackUiEvent(`indexpattern_dimension_operation_${operationType}`); return; } else if (!selectedColumn || !compatibleWithCurrentField) { @@ -192,18 +186,15 @@ export function DimensionEditor(props: DimensionEditorProps) { if (possibleFields.size === 1) { setState( - changeColumn({ + mergeLayer({ state, layerId, - columnId, - newColumn: buildColumn({ - columns: props.state.layers[props.layerId].columns, - suggestedPriority: props.suggestedPriority, - layerId: props.layerId, - op: operationType, + newLayer: insertOrReplaceColumn({ + layer: props.state.layers[props.layerId], indexPattern: currentIndexPattern, + columnId, + op: operationType, field: currentIndexPattern.getFieldByName(possibleFields.values().next().value), - previousColumn: selectedColumn, }), }) ); @@ -216,30 +207,21 @@ export function DimensionEditor(props: DimensionEditorProps) { setInvalidOperationType(null); - if (selectedColumn?.operationType === operationType) { + if (selectedColumn.operationType === operationType) { return; } - const newColumn: IndexPatternColumn = buildColumn({ - columns: props.state.layers[props.layerId].columns, - suggestedPriority: props.suggestedPriority, - layerId: props.layerId, - op: operationType, + const newLayer = replaceColumn({ + layer: props.state.layers[props.layerId], indexPattern: currentIndexPattern, + columnId, + op: operationType, field: hasField(selectedColumn) ? currentIndexPattern.getFieldByName(selectedColumn.sourceField) : undefined, - previousColumn: selectedColumn, }); - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn, - }) - ); + setState(mergeLayer({ state, layerId, newLayer })); }, }; } @@ -297,30 +279,31 @@ export function DimensionEditor(props: DimensionEditorProps) { incompatibleSelectedOperationType={incompatibleSelectedOperationType} onDeleteColumn={() => { setState( - deleteColumn({ + mergeLayer({ state, layerId, - columnId, + newLayer: deleteColumn({ layer: state.layers[layerId], columnId }), }) ); }} onChoose={(choice) => { - let column: IndexPatternColumn; + let newLayer: IndexPatternLayer; if ( !incompatibleSelectedOperationType && selectedColumn && 'field' in choice && choice.operationType === selectedColumn.operationType ) { - // If we just changed the field are not in an error state and the operation didn't change, - // we use the operations onFieldChange method to calculate the new column. - column = changeField( - selectedColumn, - currentIndexPattern, - currentIndexPattern.getFieldByName(choice.field)! - ); + // Replaces just the field + newLayer = replaceColumn({ + layer: state.layers[layerId], + columnId, + indexPattern: currentIndexPattern, + op: choice.operationType, + field: currentIndexPattern.getFieldByName(choice.field)!, + }); } else { - // Otherwise we'll use the buildColumn method to calculate a new column + // Finds a new operation const compatibleOperations = ('field' in choice && operationSupportMatrix.operationByField[choice.field]) || new Set(); @@ -334,26 +317,16 @@ export function DimensionEditor(props: DimensionEditorProps) { } else if ('field' in choice) { operation = choice.operationType; } - column = buildColumn({ - columns: props.state.layers[props.layerId].columns, + newLayer = insertOrReplaceColumn({ + layer: state.layers[layerId], + columnId, field: currentIndexPattern.getFieldByName(choice.field), indexPattern: currentIndexPattern, - layerId: props.layerId, - suggestedPriority: props.suggestedPriority, op: operation as OperationType, - previousColumn: selectedColumn, }); } - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn: column, - keepParams: false, - }) - ); + setState(mergeLayer({ state, layerId, newLayer })); setInvalidOperationType(null); }} /> diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx index 3ed04b08df58f0..e9eb3fa4542fe5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/dimension_panel.test.tsx @@ -9,7 +9,6 @@ import React from 'react'; import { act } from 'react-dom/test-utils'; import { EuiComboBox, EuiListGroupItemProps, EuiListGroup, EuiRange } from '@elastic/eui'; import { DataPublicPluginStart } from '../../../../../../src/plugins/data/public'; -import { changeColumn } from '../state_helpers'; import { IndexPatternDimensionEditorComponent, IndexPatternDimensionEditorProps, @@ -18,14 +17,14 @@ import { mountWithIntl as mount, shallowWithIntl as shallow } from 'test_utils/e import { IUiSettingsClient, SavedObjectsClientContract, HttpSetup, CoreSetup } from 'kibana/public'; import { IStorageWrapper } from 'src/plugins/kibana_utils/public'; import { IndexPatternPrivateState } from '../types'; -import { IndexPatternColumn } from '../operations'; +import { IndexPatternColumn, replaceColumn } from '../operations'; import { documentField } from '../document_field'; import { OperationMetadata } from '../../types'; import { DateHistogramIndexPatternColumn } from '../operations/definitions/date_histogram'; import { getFieldByNameFactory } from '../pure_helpers'; jest.mock('../loader'); -jest.mock('../state_helpers'); +jest.mock('../operations'); jest.mock('lodash', () => { const original = jest.requireActual('lodash'); @@ -682,7 +681,7 @@ describe('IndexPatternDimensionEditorPanel', () => { // Other parts of this don't matter for this test }), }, - columnOrder: ['col1', 'col2'], + columnOrder: ['col2', 'col1'], }, }, }); @@ -1029,15 +1028,13 @@ describe('IndexPatternDimensionEditorPanel', () => { ); }); - expect(changeColumn).toHaveBeenCalledWith({ - state: initialState, - columnId: 'col1', - layerId: 'first', - newColumn: expect.objectContaining({ - sourceField: 'bytes', - operationType: 'min', - }), - }); + expect(replaceColumn).toHaveBeenCalledWith( + expect.objectContaining({ + columnId: 'col1', + op: 'min', + field: expect.objectContaining({ name: 'bytes' }), + }) + ); }); it('should clear the dimension when removing the selection in field combobox', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts index 1d85c1f8f78ca4..48240a54171084 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.test.ts @@ -17,7 +17,7 @@ import { OperationMetadata } from '../../types'; import { IndexPatternColumn } from '../operations'; import { getFieldByNameFactory } from '../pure_helpers'; -jest.mock('../state_helpers'); +jest.mock('../operations'); const fields = [ { @@ -56,8 +56,8 @@ const fields = [ ]; const expectedIndexPatterns = { - 1: { - id: '1', + foo: { + id: 'foo', title: 'my-fake-index-pattern', timeFieldName: 'timestamp', hasExistence: true, @@ -89,7 +89,7 @@ describe('IndexPatternDimensionEditorPanel', () => { state = { indexPatternRefs: [], indexPatterns: expectedIndexPatterns, - currentIndexPatternId: '1', + currentIndexPatternId: 'foo', isFirstExistenceFetch: false, existingFields: { 'my-fake-index-pattern': { @@ -101,7 +101,7 @@ describe('IndexPatternDimensionEditorPanel', () => { }, layers: { first: { - indexPatternId: '1', + indexPatternId: 'foo', columnOrder: ['col1'], columns: { col1: { @@ -156,84 +156,8 @@ describe('IndexPatternDimensionEditorPanel', () => { jest.clearAllMocks(); }); - function dragDropState(): IndexPatternPrivateState { - return { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: { - foo: { - id: 'foo', - title: 'Foo pattern', - hasRestrictions: false, - fields: [ - { - aggregatable: true, - name: 'bar', - displayName: 'bar', - searchable: true, - type: 'number', - }, - { - aggregatable: true, - name: 'mystring', - displayName: 'mystring', - searchable: true, - type: 'string', - }, - ], - - getFieldByName: getFieldByNameFactory([ - { - aggregatable: true, - name: 'bar', - displayName: 'bar', - searchable: true, - type: 'number', - }, - { - aggregatable: true, - name: 'mystring', - displayName: 'mystring', - searchable: true, - type: 'string', - }, - ]), - }, - }, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - myLayer: { - indexPatternId: 'foo', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Date histogram of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - params: { - interval: '1d', - }, - sourceField: 'timestamp', - }, - }, - }, - }, - }; - } - it('is not droppable if no drag is happening', () => { - expect( - canHandleDrop({ - ...defaultProps, - dragDropContext, - state: dragDropState(), - layerId: 'myLayer', - }) - ).toBe(false); + expect(canHandleDrop({ ...defaultProps, dragDropContext })).toBe(false); }); it('is not droppable if the dragged item has no field', () => { @@ -260,9 +184,7 @@ describe('IndexPatternDimensionEditorPanel', () => { id: 'mystring', }, }, - state: dragDropState(), filterOperations: () => false, - layerId: 'myLayer', }) ).toBe(false); }); @@ -274,14 +196,12 @@ describe('IndexPatternDimensionEditorPanel', () => { dragDropContext: { ...dragDropContext, dragging: { - field: { type: 'number', name: 'bar', aggregatable: true }, + field: { type: 'number', name: 'bytes', aggregatable: true }, indexPatternId: 'foo', id: 'bar', }, }, - state: dragDropState(), filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', }) ).toBe(true); }); @@ -298,9 +218,30 @@ describe('IndexPatternDimensionEditorPanel', () => { id: 'bar', }, }, - state: dragDropState(), filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', + }) + ).toBe(false); + }); + + it('is not droppable if the dragged field is already in use by this operation', () => { + expect( + canHandleDrop({ + ...defaultProps, + dragDropContext: { + ...dragDropContext, + dragging: { + field: { + name: 'timestamp', + displayName: 'timestampLabel', + type: 'date', + aggregatable: true, + searchable: true, + exists: true, + }, + indexPatternId: 'foo', + id: 'bar', + }, + }, }) ).toBe(false); }); @@ -314,14 +255,11 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging: { columnId: 'col1', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col1', }, }, - state: dragDropState(), columnId: 'col2', - filterOperations: (op: OperationMetadata) => true, - layerId: 'myLayer', }) ).toBe(true); }); @@ -335,14 +273,10 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging: { columnId: 'col1', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'bar', }, }, - state: dragDropState(), - columnId: 'col1', - filterOperations: (op: OperationMetadata) => true, - layerId: 'myLayer', }) ).toBe(false); }); @@ -356,25 +290,22 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging: { columnId: 'col1', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'bar', }, }, - state: dragDropState(), columnId: 'col2', filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', }) ).toBe(false); }); it('appends the dropped column when a field is dropped', () => { const dragging = { - field: { type: 'number', name: 'bar', aggregatable: true }, + field: { type: 'number', name: 'bytes', aggregatable: true }, indexPatternId: 'foo', id: 'bar', }; - const testState = dragDropState(); onDrop({ ...defaultProps, @@ -383,24 +314,22 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging, }, droppedItem: dragging, - state: testState, columnId: 'col2', filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ - ...testState, + ...state, layers: { - myLayer: { - ...testState.layers.myLayer, + first: { + ...state.layers.first, columnOrder: ['col1', 'col2'], columns: { - ...testState.layers.myLayer.columns, + ...state.layers.first.columns, col2: expect.objectContaining({ dataType: 'number', - sourceField: 'bar', + sourceField: 'bytes', }), }, }, @@ -410,11 +339,10 @@ describe('IndexPatternDimensionEditorPanel', () => { it('selects the specific operation that was valid on drop', () => { const dragging = { - field: { type: 'string', name: 'mystring', aggregatable: true }, + field: { type: 'string', name: 'source', aggregatable: true }, indexPatternId: 'foo', id: 'bar', }; - const testState = dragDropState(); onDrop({ ...defaultProps, dragDropContext: { @@ -422,24 +350,22 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging, }, droppedItem: dragging, - state: testState, columnId: 'col2', filterOperations: (op: OperationMetadata) => op.isBucketed, - layerId: 'myLayer', }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ - ...testState, + ...state, layers: { - myLayer: { - ...testState.layers.myLayer, - columnOrder: ['col1', 'col2'], + first: { + ...state.layers.first, + columnOrder: ['col2', 'col1'], columns: { - ...testState.layers.myLayer.columns, + ...state.layers.first.columns, col2: expect.objectContaining({ dataType: 'string', - sourceField: 'mystring', + sourceField: 'source', }), }, }, @@ -449,11 +375,10 @@ describe('IndexPatternDimensionEditorPanel', () => { it('updates a column when a field is dropped', () => { const dragging = { - field: { type: 'number', name: 'bar', aggregatable: true }, + field: { type: 'number', name: 'bytes', aggregatable: true }, indexPatternId: 'foo', id: 'bar', }; - const testState = dragDropState(); onDrop({ ...defaultProps, dragDropContext: { @@ -461,20 +386,18 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging, }, droppedItem: dragging, - state: testState, filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ - ...testState, + ...state, layers: { - myLayer: expect.objectContaining({ + first: expect.objectContaining({ columns: expect.objectContaining({ col1: expect.objectContaining({ dataType: 'number', - sourceField: 'bar', + sourceField: 'bytes', }), }), }), @@ -482,13 +405,12 @@ describe('IndexPatternDimensionEditorPanel', () => { }); }); - it('does not set the size of the terms aggregation', () => { + it('keeps the operation when dropping a different compatible field', () => { const dragging = { - field: { type: 'string', name: 'mystring', aggregatable: true }, + field: { name: 'memory', type: 'number', aggregatable: true }, indexPatternId: 'foo', - id: 'bar', + id: '1', }; - const testState = dragDropState(); onDrop({ ...defaultProps, dragDropContext: { @@ -496,27 +418,41 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging, }, droppedItem: dragging, - state: testState, - columnId: 'col2', - filterOperations: (op: OperationMetadata) => op.isBucketed, - layerId: 'myLayer', + state: { + ...state, + layers: { + first: { + indexPatternId: 'foo', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Sum of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'sum', + sourceField: 'bytes', + }, + }, + }, + }, + }, }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ - ...testState, + ...state, layers: { - myLayer: { - ...testState.layers.myLayer, - columnOrder: ['col1', 'col2'], - columns: { - ...testState.layers.myLayer.columns, - col2: expect.objectContaining({ - operationType: 'terms', - params: expect.objectContaining({ size: 3 }), + first: expect.objectContaining({ + columns: expect.objectContaining({ + col1: expect.objectContaining({ + operationType: 'sum', + dataType: 'number', + sourceField: 'memory', }), - }, - }, + }), + }), }, }); }); @@ -525,10 +461,9 @@ describe('IndexPatternDimensionEditorPanel', () => { const dragging = { columnId: 'col1', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'bar', }; - const testState = dragDropState(); onDrop({ ...defaultProps, @@ -537,21 +472,18 @@ describe('IndexPatternDimensionEditorPanel', () => { dragging, }, droppedItem: dragging, - state: testState, columnId: 'col2', - filterOperations: (op: OperationMetadata) => true, - layerId: 'myLayer', }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ - ...testState, + ...state, layers: { - myLayer: { - ...testState.layers.myLayer, + first: { + ...state.layers.first, columnOrder: ['col2'], columns: { - col2: testState.layers.myLayer.columns.col1, + col2: state.layers.first.columns.col1, }, }, }, @@ -562,15 +494,15 @@ describe('IndexPatternDimensionEditorPanel', () => { const dragging = { columnId: 'col2', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col2', }; - const testState = dragDropState(); - testState.layers.myLayer = { + const testState = { ...state }; + testState.layers.first = { indexPatternId: 'foo', columnOrder: ['col1', 'col2', 'col3'], columns: { - col1: testState.layers.myLayer.columns.col1, + col1: testState.layers.first.columns.col1, col2: { label: 'Top values of src', @@ -606,21 +538,18 @@ describe('IndexPatternDimensionEditorPanel', () => { }, droppedItem: dragging, state: testState, - columnId: 'col1', - filterOperations: (op: OperationMetadata) => true, - layerId: 'myLayer', }); expect(setState).toBeCalledTimes(1); expect(setState).toHaveBeenCalledWith({ ...testState, layers: { - myLayer: { - ...testState.layers.myLayer, + first: { + ...testState.layers.first, columnOrder: ['col1', 'col3'], columns: { - col1: testState.layers.myLayer.columns.col2, - col3: testState.layers.myLayer.columns.col3, + col1: testState.layers.first.columns.col2, + col3: testState.layers.first.columns.col3, }, }, }, @@ -631,13 +560,13 @@ describe('IndexPatternDimensionEditorPanel', () => { const dragging = { columnId: 'col1', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col1', }; const testState = { - ...dragDropState(), + ...state, layers: { - myLayer: { + first: { indexPatternId: 'foo', columnOrder: ['col1', 'col2', 'col3'], columns: { @@ -671,18 +600,17 @@ describe('IndexPatternDimensionEditorPanel', () => { droppedItem: dragging, state: testState, filterOperations: (op: OperationMetadata) => op.dataType === 'number', - layerId: 'myLayer', }; const stateWithColumnOrder = (columnOrder: string[]) => { return { ...testState, layers: { - myLayer: { - ...testState.layers.myLayer, + first: { + ...testState.layers.first, columnOrder, columns: { - ...testState.layers.myLayer.columns, + ...testState.layers.first.columns, }, }, }, @@ -704,7 +632,7 @@ describe('IndexPatternDimensionEditorPanel', () => { droppedItem: { columnId: 'col3', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col3', }, }); @@ -718,7 +646,7 @@ describe('IndexPatternDimensionEditorPanel', () => { droppedItem: { columnId: 'col2', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col2', }, }); @@ -732,7 +660,7 @@ describe('IndexPatternDimensionEditorPanel', () => { droppedItem: { columnId: 'col2', groupId: 'a', - layerId: 'myLayer', + layerId: 'first', id: 'col2', }, }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.ts b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.ts index a6ff550af96e98..e4eabafc6938e1 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/dimension_panel/droppable.ts @@ -10,9 +10,9 @@ import { isDraggedOperation, } from '../../types'; import { IndexPatternColumn } from '../indexpattern'; -import { buildColumn, changeField } from '../operations'; -import { changeColumn, mergeLayer } from '../state_helpers'; -import { isDraggedField, hasField } from '../utils'; +import { insertOrReplaceColumn } from '../operations'; +import { mergeLayer } from '../state_helpers'; +import { hasField, isDraggedField } from '../utils'; import { IndexPatternPrivateState, IndexPatternField } from '../types'; import { trackUiEvent } from '../../lens_ui_telemetry'; import { getOperationSupportMatrix } from './operation_support'; @@ -28,9 +28,12 @@ export function canHandleDrop(props: DatasourceDimensionDropProps columns.length); trackUiEvent(hasData ? 'drop_non_empty' : 'drop_empty'); - - setState( - changeColumn({ - state, - layerId, - columnId, - newColumn, - // If the field has changed, the onFieldChange method needs to take care of everything including moving - // over params. If we create a new column above we want changeColumn to move over params. - keepParams: !hasFieldChanged, - }) - ); + setState(mergeLayer({ state, layerId, newLayer })); return true; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx index fa106e90d518ab..e37c31559cd0ca 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern.tsx @@ -51,13 +51,14 @@ import { IndexPatternField, IndexPatternPrivateState, IndexPatternPersistedState import { KibanaContextProvider } from '../../../../../src/plugins/kibana_react/public'; import { DataPublicPluginStart } from '../../../../../src/plugins/data/public'; import { VisualizeFieldContext } from '../../../../../src/plugins/ui_actions/public'; -import { deleteColumn } from './state_helpers'; +import { mergeLayer } from './state_helpers'; import { Datasource, StateSetter } from '../index'; import { ChartsPluginSetup } from '../../../../../src/plugins/charts/public'; +import { deleteColumn } from './operations'; import { FieldBasedIndexPatternColumn } from './operations/definitions/column_types'; import { Dragging } from '../drag_drop/providers'; -export { OperationType, IndexPatternColumn } from './operations'; +export { OperationType, IndexPatternColumn, deleteColumn } from './operations'; export type DraggedField = Dragging & { field: IndexPatternField; @@ -159,10 +160,10 @@ export function getIndexPatternDatasource({ }, removeColumn({ prevState, layerId, columnId }) { - return deleteColumn({ + return mergeLayer({ state: prevState, layerId, - columnId, + newLayer: deleteColumn({ layer: prevState.layers[layerId], columnId }), }); }, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx index 523a1be34ba3d3..c88af50e525d42 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.test.tsx @@ -12,6 +12,7 @@ import { getDatasourceSuggestionsFromCurrentState, getDatasourceSuggestionsForVisualizeField, } from './indexpattern_suggestions'; +import { documentField } from './document_field'; import { getFieldByNameFactory } from './pure_helpers'; jest.mock('./loader'); @@ -60,6 +61,7 @@ const fieldsOne = [ aggregatable: true, searchable: true, }, + documentField, ]; const fieldsTwo = [ @@ -116,6 +118,7 @@ const fieldsTwo = [ }, }, }, + documentField, ]; const expectedIndexPatterns = { @@ -290,13 +293,13 @@ describe('IndexPattern Data Source suggestions', () => { state: expect.objectContaining({ layers: { id1: expect.objectContaining({ - columnOrder: ['id2', 'id3'], + columnOrder: ['id3', 'id2'], columns: { - id2: expect.objectContaining({ + id3: expect.objectContaining({ operationType: 'date_histogram', sourceField: 'timestamp', }), - id3: expect.objectContaining({ + id2: expect.objectContaining({ operationType: 'avg', sourceField: 'bytes', }), @@ -310,10 +313,10 @@ describe('IndexPattern Data Source suggestions', () => { isMultiRow: true, columns: [ expect.objectContaining({ - columnId: 'id2', + columnId: 'id3', }), expect.objectContaining({ - columnId: 'id3', + columnId: 'id2', }), ], layerId: 'id1', @@ -510,13 +513,13 @@ describe('IndexPattern Data Source suggestions', () => { state: expect.objectContaining({ layers: { previousLayer: expect.objectContaining({ - columnOrder: ['id1', 'id2'], + columnOrder: ['id2', 'id1'], columns: { - id1: expect.objectContaining({ + id2: expect.objectContaining({ operationType: 'date_histogram', sourceField: 'timestamp', }), - id2: expect.objectContaining({ + id1: expect.objectContaining({ operationType: 'avg', sourceField: 'bytes', }), @@ -530,10 +533,10 @@ describe('IndexPattern Data Source suggestions', () => { isMultiRow: true, columns: [ expect.objectContaining({ - columnId: 'id1', + columnId: 'id2', }), expect.objectContaining({ - columnId: 'id2', + columnId: 'id1', }), ], layerId: 'previousLayer', @@ -757,9 +760,9 @@ describe('IndexPattern Data Source suggestions', () => { layers: { previousLayer: initialState.layers.previousLayer, currentLayer: expect.objectContaining({ - columnOrder: ['id1', 'colb'], + columnOrder: ['cola', 'colb'], columns: { - id1: expect.objectContaining({ + cola: expect.objectContaining({ operationType: 'date_histogram', sourceField: 'start_date', }), @@ -867,7 +870,7 @@ describe('IndexPattern Data Source suggestions', () => { ); }); - it('replaces a metric column on a number field if only one other metric is already set', () => { + it('suggests both replacing and adding metric if only one other metric is set', () => { const initialState = stateWithNonEmptyTables(); const suggestions = getDatasourceSuggestionsForField(initialState, '1', { name: 'memory', @@ -895,6 +898,26 @@ describe('IndexPattern Data Source suggestions', () => { }), }) ); + + expect(suggestions).toContainEqual( + expect.objectContaining({ + state: expect.objectContaining({ + layers: expect.objectContaining({ + currentLayer: expect.objectContaining({ + columnOrder: ['cola', 'colb', 'id1'], + columns: { + cola: initialState.layers.currentLayer.columns.cola, + colb: initialState.layers.currentLayer.columns.colb, + id1: expect.objectContaining({ + operationType: 'avg', + sourceField: 'memory', + }), + }, + }), + }), + }), + }) + ); }); it('adds a metric column on a number field if no other metrics set', () => { @@ -941,7 +964,20 @@ describe('IndexPattern Data Source suggestions', () => { ); }); - it('adds a metric column on a number field if 2 or more other metric', () => { + it('skips duplicates when the field is already in use', () => { + const initialState = stateWithNonEmptyTables(); + const suggestions = getDatasourceSuggestionsForField(initialState, '1', { + name: 'bytes', + displayName: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }); + + expect(suggestions).not.toContain(expect.objectContaining({ changeType: 'extended' })); + }); + + it('skips duplicates when the document-specific field is already in use', () => { const initialState = stateWithNonEmptyTables(); const modifiedState: IndexPatternPrivateState = { ...initialState, @@ -951,45 +987,20 @@ describe('IndexPattern Data Source suggestions', () => { ...initialState.layers.currentLayer, columns: { ...initialState.layers.currentLayer.columns, - colc: { - dataType: 'number', + colb: { + label: 'Count of records', + dataType: 'document', isBucketed: false, - sourceField: 'dest', - label: 'Unique count of dest', - operationType: 'cardinality', + + operationType: 'count', + sourceField: 'Records', }, }, - columnOrder: ['cola', 'colb', 'colc'], }, }, }; - const suggestions = getDatasourceSuggestionsForField(modifiedState, '1', { - name: 'memory', - displayName: 'memory', - type: 'number', - aggregatable: true, - searchable: true, - }); - - expect(suggestions).toContainEqual( - expect.objectContaining({ - state: expect.objectContaining({ - layers: { - previousLayer: modifiedState.layers.previousLayer, - currentLayer: expect.objectContaining({ - columnOrder: ['cola', 'colb', 'colc', 'id1'], - columns: { - ...modifiedState.layers.currentLayer.columns, - id1: expect.objectContaining({ - operationType: 'avg', - sourceField: 'memory', - }), - }, - }), - }, - }), - }) - ); + const suggestions = getDatasourceSuggestionsForField(modifiedState, '1', documentField); + expect(suggestions).not.toContain(expect.objectContaining({ changeType: 'extended' })); }); }); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts index c12d7d4be226b6..b74d75207e1125 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/indexpattern_suggestions.ts @@ -10,14 +10,14 @@ import { generateId } from '../id_generator'; import { DatasourceSuggestion, TableChangeType } from '../types'; import { columnToOperation } from './indexpattern'; import { - buildColumn, + insertNewColumn, + replaceColumn, + getMetricOperationTypes, getOperationTypesForField, operationDefinitionMap, IndexPatternColumn, OperationType, } from './operations'; -import { operationDefinitions } from './operations/definitions'; -import { TermsIndexPatternColumn } from './operations/definitions/terms'; import { hasField, hasInvalidReference } from './utils'; import { IndexPattern, @@ -137,6 +137,7 @@ export function getDatasourceSuggestionsForVisualizeField( ); } +// TODO: Stop hard-coding the specific operation types function getBucketOperation(field: IndexPatternField) { // We allow numeric bucket types in some cases, but it's generally not the right suggestion, // so we eliminate it here. @@ -160,29 +161,38 @@ function getExistingLayerSuggestionsForField( const suggestions: IndexPatternSugestion[] = []; if (usableAsBucketOperation && !fieldInUse) { - suggestions.push( - buildSuggestion({ - state, - updatedLayer: addFieldAsBucketOperation( - layer, + if ( + usableAsBucketOperation === 'date_histogram' && + layer.columnOrder.some((colId) => layer.columns[colId].operationType === 'date_histogram') + ) { + const previousDate = layer.columnOrder.find( + (colId) => layer.columns[colId].operationType === 'date_histogram' + )!; + suggestions.push( + buildSuggestion({ + state, + updatedLayer: replaceColumn({ + layer, + indexPattern, + field, + op: usableAsBucketOperation, + columnId: previousDate, + }), layerId, - indexPattern, - field, - usableAsBucketOperation - ), - layerId, - changeType: 'extended', - }) - ); - } - - if (!usableAsBucketOperation && operations.length > 0) { - const updatedLayer = addFieldAsMetricOperation(layer, layerId, indexPattern, field); - if (updatedLayer) { + changeType: 'initial', + }) + ); + } else { suggestions.push( buildSuggestion({ state, - updatedLayer, + updatedLayer: insertNewColumn({ + layer, + indexPattern, + field, + op: usableAsBucketOperation, + columnId: generateId(), + }), layerId, changeType: 'extended', }) @@ -190,6 +200,50 @@ function getExistingLayerSuggestionsForField( } } + if (!usableAsBucketOperation && operations.length > 0 && !fieldInUse) { + const [metricOperation] = getMetricOperationTypes(field); + if (metricOperation) { + const layerWithNewMetric = insertNewColumn({ + layer, + indexPattern, + field, + columnId: generateId(), + op: metricOperation.type, + }); + if (layerWithNewMetric) { + suggestions.push( + buildSuggestion({ + state, + layerId, + updatedLayer: layerWithNewMetric, + changeType: 'extended', + }) + ); + } + + const [, metrics] = separateBucketColumns(layer); + if (metrics.length === 1) { + const layerWithReplacedMetric = replaceColumn({ + layer, + indexPattern, + field, + columnId: metrics[0], + op: metricOperation.type, + }); + if (layerWithReplacedMetric) { + suggestions.push( + buildSuggestion({ + state, + layerId, + updatedLayer: layerWithReplacedMetric, + changeType: 'extended', + }) + ); + } + } + } + } + const metricSuggestion = createMetricSuggestion(indexPattern, layerId, state, field); if (metricSuggestion) { suggestions.push(metricSuggestion); @@ -198,100 +252,6 @@ function getExistingLayerSuggestionsForField( return suggestions; } -function addFieldAsMetricOperation( - layer: IndexPatternLayer, - layerId: string, - indexPattern: IndexPattern, - field: IndexPatternField -): IndexPatternLayer | undefined { - const newColumn = getMetricColumn(indexPattern, layerId, field); - const addedColumnId = generateId(); - - const [, metrics] = separateBucketColumns(layer); - - // Add metrics if there are 0 or > 1 metric - if (metrics.length !== 1) { - return { - indexPatternId: indexPattern.id, - columns: { - ...layer.columns, - [addedColumnId]: newColumn, - }, - columnOrder: [...layer.columnOrder, addedColumnId], - }; - } - - // Replacing old column with new column, keeping the old ID - const newColumns = { ...layer.columns, [metrics[0]]: newColumn }; - - return { - indexPatternId: indexPattern.id, - columns: newColumns, - columnOrder: layer.columnOrder, // Order is kept by replacing - }; -} - -function addFieldAsBucketOperation( - layer: IndexPatternLayer, - layerId: string, - indexPattern: IndexPattern, - field: IndexPatternField, - operation: OperationType -): IndexPatternLayer { - const newColumn = buildColumn({ - op: operation, - columns: layer.columns, - layerId, - indexPattern, - suggestedPriority: undefined, - field, - }); - const [buckets, metrics] = separateBucketColumns(layer); - const newColumnId = generateId(); - const updatedColumns = { - ...layer.columns, - [newColumnId]: newColumn, - }; - - if (buckets.length === 0 && operation === 'terms') { - (newColumn as TermsIndexPatternColumn).params.size = 5; - } - - const oldDateHistogramIndex = layer.columnOrder.findIndex( - (columnId) => layer.columns[columnId].operationType === 'date_histogram' - ); - const oldDateHistogramId = - oldDateHistogramIndex > -1 ? layer.columnOrder[oldDateHistogramIndex] : null; - - let updatedColumnOrder: string[] = []; - if (oldDateHistogramId) { - if (operation === 'terms') { - // Insert the new terms bucket above the first date histogram - updatedColumnOrder = [ - ...buckets.slice(0, oldDateHistogramIndex), - newColumnId, - ...buckets.slice(oldDateHistogramIndex, buckets.length), - ...metrics, - ]; - } else if (operation === 'date_histogram') { - // Replace date histogram with new date histogram - delete updatedColumns[oldDateHistogramId]; - updatedColumnOrder = layer.columnOrder.map((columnId) => - columnId !== oldDateHistogramId ? columnId : newColumnId - ); - } - } else { - // Insert the new bucket after existing buckets. Users will see the same data - // they already had, with an extra level of detail. - updatedColumnOrder = [...buckets, newColumnId, ...metrics]; - } - return { - indexPatternId: indexPattern.id, - columns: updatedColumns, - columnOrder: updatedColumnOrder, - }; -} - function getEmptyLayerSuggestionsForField( state: IndexPatternPrivateState, layerId: string, @@ -302,9 +262,9 @@ function getEmptyLayerSuggestionsForField( let newLayer: IndexPatternLayer | undefined; const bucketOperation = getBucketOperation(field); if (bucketOperation) { - newLayer = createNewLayerWithBucketAggregation(layerId, indexPattern, field, bucketOperation); + newLayer = createNewLayerWithBucketAggregation(indexPattern, field, bucketOperation); } else if (indexPattern.timeFieldName && getOperationTypesForField(field).length > 0) { - newLayer = createNewLayerWithMetricAggregation(layerId, indexPattern, field); + newLayer = createNewLayerWithMetricAggregation(indexPattern, field); } const newLayerSuggestions = newLayer @@ -324,77 +284,48 @@ function getEmptyLayerSuggestionsForField( } function createNewLayerWithBucketAggregation( - layerId: string, indexPattern: IndexPattern, field: IndexPatternField, operation: OperationType ): IndexPatternLayer { - const countColumn = buildColumn({ + return insertNewColumn({ op: 'count', - columns: {}, - indexPattern, - layerId, - suggestedPriority: undefined, + layer: insertNewColumn({ + op: operation, + layer: { indexPatternId: indexPattern.id, columns: {}, columnOrder: [] }, + columnId: generateId(), + field, + indexPattern, + }), + columnId: generateId(), field: documentField, - }); - - const col1 = generateId(); - const col2 = generateId(); - - // let column know about count column - const column = buildColumn({ - layerId, - op: operation, indexPattern, - columns: { - [col2]: countColumn, - }, - field, - suggestedPriority: undefined, }); - if (operation === 'terms') { - (column as TermsIndexPatternColumn).params.size = 5; - } - - return { - indexPatternId: indexPattern.id, - columns: { - [col1]: column, - [col2]: countColumn, - }, - columnOrder: [col1, col2], - }; } function createNewLayerWithMetricAggregation( - layerId: string, indexPattern: IndexPattern, field: IndexPatternField -): IndexPatternLayer { +): IndexPatternLayer | undefined { const dateField = indexPattern.getFieldByName(indexPattern.timeFieldName!); + const [metricOperation] = getMetricOperationTypes(field); + if (!metricOperation) { + return; + } - const column = getMetricColumn(indexPattern, layerId, field); - - const dateColumn = buildColumn({ + return insertNewColumn({ op: 'date_histogram', - columns: {}, - suggestedPriority: undefined, + layer: insertNewColumn({ + op: metricOperation.type, + layer: { indexPatternId: indexPattern.id, columns: {}, columnOrder: [] }, + columnId: generateId(), + field, + indexPattern, + }), + columnId: generateId(), field: dateField, indexPattern, - layerId, }); - - const col1 = generateId(); - const col2 = generateId(); - - return { - indexPatternId: indexPattern.id, - columns: { - [col1]: dateColumn, - [col2]: column, - }, - columnOrder: [col1, col2], - }; } export function getDatasourceSuggestionsFromCurrentState( @@ -527,57 +458,33 @@ function createChangedNestingSuggestion(state: IndexPatternPrivateState, layerId }); } -function getMetricColumn(indexPattern: IndexPattern, layerId: string, field: IndexPatternField) { - const operationDefinitionsMap = _.keyBy(operationDefinitions, 'type'); - const [column] = getOperationTypesForField(field) - .map((type) => - operationDefinitionsMap[type].buildColumn({ - field, - indexPattern, - layerId, - columns: {}, - suggestedPriority: 0, - }) - ) - .filter((op) => (op.dataType === 'number' || op.dataType === 'document') && !op.isBucketed); - return column; -} - function createMetricSuggestion( indexPattern: IndexPattern, layerId: string, state: IndexPatternPrivateState, field: IndexPatternField ) { - const column = getMetricColumn(indexPattern, layerId, field); + const [operation] = getMetricOperationTypes(field); - if (!column) { + if (!operation) { return; } - const newId = generateId(); - return buildSuggestion({ layerId, state, changeType: 'initial', - updatedLayer: { - indexPatternId: indexPattern.id, - columns: { - [newId]: - column.dataType !== 'document' - ? column - : buildColumn({ - op: 'count', - columns: {}, - indexPattern, - layerId, - suggestedPriority: undefined, - field: documentField, - }), + updatedLayer: insertNewColumn({ + layer: { + indexPatternId: indexPattern.id, + columns: {}, + columnOrder: [], }, - columnOrder: [newId], - }, + columnId: generateId(), + op: operation.type, + field: operation.type === 'count' ? documentField : field, + indexPattern, + }), }); } @@ -591,6 +498,7 @@ function getNestedTitle([outerBucketLabel, innerBucketLabel]: string[]) { }); } +// Replaces all metrics on the table with a different field-based function function createAlternativeMetricSuggestions( indexPattern: IndexPattern, layerId: string, @@ -598,6 +506,7 @@ function createAlternativeMetricSuggestions( ) { const layer = state.layers[layerId]; const suggestions: Array> = []; + layer.columnOrder.forEach((columnId) => { const column = layer.columns[columnId]; if (!hasField(column)) { @@ -607,39 +516,28 @@ function createAlternativeMetricSuggestions( if (!field) { return; } - const alternativeMetricOperations = getOperationTypesForField(field) - .map((op) => - buildColumn({ - op, - columns: layer.columns, - indexPattern, - layerId, - field, - suggestedPriority: undefined, - }) - ) - .filter( - (fullOperation) => - fullOperation.operationType !== column.operationType && !fullOperation.isBucketed - ); - if (alternativeMetricOperations.length === 0) { - return; - } - const newId = generateId(); - const newColumn = alternativeMetricOperations[0]; - const updatedLayer = { - indexPatternId: indexPattern.id, - columns: { [newId]: newColumn }, - columnOrder: [newId], - }; - suggestions.push( - buildSuggestion({ - state, - layerId, - updatedLayer, - changeType: 'initial', - }) + const possibleOperations = getMetricOperationTypes(field).filter( + ({ type }) => type !== column.operationType ); + if (possibleOperations.length) { + const layerWithNewMetric = replaceColumn({ + layer, + indexPattern, + field, + columnId, + op: possibleOperations[0].type, + }); + if (layerWithNewMetric) { + suggestions.push( + buildSuggestion({ + state, + layerId, + updatedLayer: layerWithNewMetric, + changeType: 'initial', + }) + ); + } + } }); return suggestions; } @@ -651,29 +549,17 @@ function createSuggestionWithDefaultDateHistogram( ) { const layer = state.layers[layerId]; const indexPattern = state.indexPatterns[layer.indexPatternId]; - const newId = generateId(); - const [buckets, metrics] = separateBucketColumns(layer); - const timeColumn = buildColumn({ - layerId, - op: 'date_histogram', - indexPattern, - columns: layer.columns, - field: timeField, - suggestedPriority: undefined, - }); - const updatedLayer = { - indexPatternId: layer.indexPatternId, - columns: { - ...layer.columns, - [newId]: timeColumn, - }, - columnOrder: [...buckets, newId, ...metrics], - }; return buildSuggestion({ state, layerId, - updatedLayer, + updatedLayer: insertNewColumn({ + layer, + indexPattern, + field: timeField, + op: 'date_histogram', + columnId: generateId(), + }), label: i18n.translate('xpack.lens.indexpattern.suggestions.overTimeLabel', { defaultMessage: 'Over time', }), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/layerpanel.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/layerpanel.test.tsx index 40eb52fe67c6d4..b7df3cc5c36871 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/layerpanel.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/layerpanel.test.tsx @@ -13,8 +13,6 @@ import { EuiSelectable } from '@elastic/eui'; import { ChangeIndexPattern } from './change_indexpattern'; import { getFieldByNameFactory } from './pure_helpers'; -jest.mock('./state_helpers'); - interface IndexPatternPickerOption { label: string; checked?: 'on' | 'off'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts index fac5d7350e45e7..64c4122245ce06 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/loader.ts @@ -16,7 +16,7 @@ import { IndexPatternField, IndexPatternLayer, } from './types'; -import { updateLayerIndexPattern } from './state_helpers'; +import { updateLayerIndexPattern } from './operations'; import { DateRange, ExistingFields } from '../../common/types'; import { BASE_API_URL } from '../../common'; import { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts index eeb19bba240060..72dfe85dfc0e9d 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/__mocks__/index.ts @@ -4,20 +4,35 @@ * you may not use this file except in compliance with the Elastic License. */ -const actual = jest.requireActual('../../operations'); +const actualOperations = jest.requireActual('../operations'); +const actualHelpers = jest.requireActual('../layer_helpers'); -jest.spyOn(actual.operationDefinitionMap.date_histogram, 'paramEditor'); -jest.spyOn(actual.operationDefinitionMap.terms, 'onOtherColumnChanged'); +jest.spyOn(actualOperations.operationDefinitionMap.date_histogram, 'paramEditor'); +jest.spyOn(actualOperations.operationDefinitionMap.terms, 'onOtherColumnChanged'); +jest.spyOn(actualHelpers, 'insertOrReplaceColumn'); +jest.spyOn(actualHelpers, 'insertNewColumn'); +jest.spyOn(actualHelpers, 'replaceColumn'); export const { getAvailableOperationsByMetadata, - buildColumn, getOperations, getOperationDisplay, getOperationTypesForField, getOperationResultType, operationDefinitionMap, operationDefinitions, +} = actualOperations; + +export const { + insertOrReplaceColumn, + insertNewColumn, + replaceColumn, + getColumnOrder, + deleteColumn, + updateColumnParam, + sortByField, + hasField, + updateLayerIndexPattern, + mergeLayer, isColumnTransferable, - changeField, -} = actual; +} = actualHelpers; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx index 1cfa63511a45cc..bd8c4b46833962 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/cardinality.tsx @@ -52,20 +52,20 @@ export const cardinalityOperation: OperationDefinition { + onFieldChange: (oldColumn, field) => { return { ...oldColumn, label: ofName(field.displayName), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts index 2e95e3fd4250fc..bd4b452a49e1d5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/column_types.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { Operation, DimensionPriority } from '../../../types'; +import { Operation } from '../../../types'; /** * This is the root type of a column. If you are implementing a new @@ -14,7 +14,6 @@ import { Operation, DimensionPriority } from '../../../types'; export interface BaseIndexPatternColumn extends Operation { // Private operationType: string; - suggestedPriority?: DimensionPriority; customLabel?: boolean; } diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx index cdf1a6b7604930..e33fc681b25794 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/count.tsx @@ -25,7 +25,7 @@ export const countOperation: OperationDefinition { + onFieldChange: (oldColumn, field) => { return { ...oldColumn, label: field.displayName, @@ -41,20 +41,20 @@ export const countOperation: OperationDefinition { it('should create column object with auto interval for primary time field', () => { const column = dateHistogramOperation.buildColumn({ columns: {}, - suggestedPriority: 0, - layerId: 'first', indexPattern: createMockedIndexPattern(), field: { name: 'timestamp', @@ -207,8 +205,6 @@ describe('date_histogram', () => { it('should create column object with auto interval for non-primary time fields', () => { const column = dateHistogramOperation.buildColumn({ columns: {}, - suggestedPriority: 0, - layerId: 'first', indexPattern: createMockedIndexPattern(), field: { name: 'start_date', @@ -225,8 +221,6 @@ describe('date_histogram', () => { it('should create column object with restrictions', () => { const column = dateHistogramOperation.buildColumn({ columns: {}, - suggestedPriority: 0, - layerId: 'first', indexPattern: createMockedIndexPattern(), field: { name: 'timestamp', @@ -334,7 +328,7 @@ describe('date_histogram', () => { const indexPattern = createMockedIndexPattern(); const newDateField = indexPattern.getFieldByName('start_date')!; - const column = dateHistogramOperation.onFieldChange(oldColumn, indexPattern, newDateField); + const column = dateHistogramOperation.onFieldChange(oldColumn, newDateField); expect(column).toHaveProperty('sourceField', 'start_date'); expect(column).toHaveProperty('params.interval', 'd'); expect(column.label).toContain('start_date'); @@ -354,7 +348,7 @@ describe('date_histogram', () => { const indexPattern = createMockedIndexPattern(); const newDateField = indexPattern.getFieldByName('start_date')!; - const column = dateHistogramOperation.onFieldChange(oldColumn, indexPattern, newDateField); + const column = dateHistogramOperation.onFieldChange(oldColumn, newDateField); expect(column).toHaveProperty('sourceField', 'start_date'); expect(column).toHaveProperty('params.interval', 'auto'); expect(column.label).toContain('start_date'); diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx index 19043c03e5a613..659390a42f261f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/date_histogram.tsx @@ -19,7 +19,7 @@ import { EuiTextColor, EuiSpacer, } from '@elastic/eui'; -import { updateColumnParam } from '../../state_helpers'; +import { updateColumnParam } from '../layer_helpers'; import { OperationDefinition } from './index'; import { FieldBasedIndexPatternColumn } from './column_types'; import { IndexPatternAggRestrictions, search } from '../../../../../../../src/plugins/data/public'; @@ -59,7 +59,7 @@ export const dateHistogramOperation: OperationDefinition< }; } }, - buildColumn({ suggestedPriority, field }) { + buildColumn({ field }) { let interval = autoInterval; let timeZone: string | undefined; if (field.aggregationRestrictions && field.aggregationRestrictions.date_histogram) { @@ -70,7 +70,6 @@ export const dateHistogramOperation: OperationDefinition< label: field.displayName, dataType: 'date', operationType: 'date_histogram', - suggestedPriority, sourceField: field.name, isBucketed: true, scale: 'interval', @@ -112,7 +111,7 @@ export const dateHistogramOperation: OperationDefinition< return column; }, - onFieldChange: (oldColumn, indexPattern, field) => { + onFieldChange: (oldColumn, field) => { return { ...oldColumn, label: field.displayName, @@ -168,15 +167,7 @@ export const dateHistogramOperation: OperationDefinition< const isCalendarInterval = calendarOnlyIntervals.has(newInterval.unit); const value = `${isCalendarInterval ? '1' : newInterval.value}${newInterval.unit || 'd'}`; - setState( - updateColumnParam({ - state, - layerId, - currentColumn, - value, - paramName: 'interval', - }) - ); + setState(updateColumnParam({ state, layerId, currentColumn, paramName: 'interval', value })); }; return ( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx index 076c1846e35a70..522e951bfba34c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/filters/filters.tsx @@ -9,7 +9,7 @@ import React, { MouseEventHandler, useState } from 'react'; import { omit } from 'lodash'; import { i18n } from '@kbn/i18n'; import { EuiFormRow, EuiLink, htmlIdGenerator } from '@elastic/eui'; -import { updateColumnParam } from '../../../state_helpers'; +import { updateColumnParam } from '../../layer_helpers'; import { OperationDefinition } from '../index'; import { BaseIndexPatternColumn } from '../column_types'; import { FilterPopover } from './filter_popover'; @@ -75,7 +75,7 @@ export const filtersOperation: OperationDefinition true, - buildColumn({ suggestedPriority, previousColumn }) { + buildColumn({ previousColumn }) { let params = { filters: [defaultFilter] }; if (previousColumn?.operationType === 'terms') { params = { @@ -96,7 +96,6 @@ export const filtersOperation: OperationDefinition { } interface BaseBuildColumnArgs { - suggestedPriority: DimensionPriority | undefined; - layerId: string; columns: Partial>; indexPattern: IndexPattern; } @@ -174,8 +172,7 @@ interface FieldBasedOperationDefinition { buildColumn: ( arg: BaseBuildColumnArgs & { field: IndexPatternField; - // previousColumn?: IndexPatternColumn; - previousColumn?: C; + previousColumn?: IndexPatternColumn; } ) => C; /** @@ -191,15 +188,9 @@ interface FieldBasedOperationDefinition { * index pattern not just a field. * * @param oldColumn The column before the user changed the field. - * @param indexPattern The index pattern that field is on. * @param field The field that the user changed to. */ - onFieldChange: ( - // oldColumn: FieldBasedIndexPatternColumn, - oldColumn: C, - indexPattern: IndexPattern, - field: IndexPatternField - ) => C; + onFieldChange: (oldColumn: C, field: IndexPatternField) => C; } interface OperationDefinitionMap { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx index fef575c61475c5..37a7ef8ee25631 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/metrics.tsx @@ -52,18 +52,17 @@ function buildMetricOperation>({ (!newField.aggregationRestrictions || newField.aggregationRestrictions![type]) ); }, - buildColumn: ({ suggestedPriority, field, previousColumn }) => ({ + buildColumn: ({ field, previousColumn }) => ({ label: ofName(field.displayName), dataType: 'number', operationType: type, - suggestedPriority, sourceField: field.name, isBucketed: false, scale: 'ratio', params: previousColumn && previousColumn.dataType === 'number' ? previousColumn.params : undefined, }), - onFieldChange: (oldColumn, indexPattern, field) => { + onFieldChange: (oldColumn, field) => { return { ...oldColumn, label: ofName(field.displayName), diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx index c6cc6ae13f1788..b1cb2312d5bb8f 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/ranges/ranges.tsx @@ -12,7 +12,8 @@ import { Range } from '../../../../../../../../src/plugins/expressions/common/ex import { RangeEditor } from './range_editor'; import { OperationDefinition } from '../index'; import { FieldBasedIndexPatternColumn } from '../column_types'; -import { updateColumnParam, changeColumn } from '../../../state_helpers'; +import { updateColumnParam } from '../../layer_helpers'; +import { mergeLayer } from '../../../state_helpers'; import { supportedFormats } from '../../../format_column'; import { MODES, AUTO_BARS, DEFAULT_INTERVAL, MIN_HISTOGRAM_BARS, SLICES } from './constants'; import { IndexPattern, IndexPatternField } from '../../../types'; @@ -121,12 +122,11 @@ export const rangeOperation: OperationDefinition { + onFieldChange: (oldColumn, field) => { return { ...oldColumn, label: field.name, @@ -211,23 +211,26 @@ export const rangeOperation: OperationDefinition column && isSortableByColumn(column)) .map(([id]) => id)[0]; + const previousBucketsLength = Object.values(columns).filter((col) => col && col.isBucketed) + .length; + return { label: ofName(field.displayName), dataType: field.type as DataType, operationType: 'terms', scale: 'ordinal', - suggestedPriority, sourceField: field.name, isBucketed: true, params: { - size: DEFAULT_SIZE, + size: previousBucketsLength === 0 ? 5 : DEFAULT_SIZE, orderBy: existingMetricColumn ? { type: 'column', columnId: existingMetricColumn } : { type: 'alphabetical' }, @@ -111,7 +113,7 @@ export const termsOperation: OperationDefinition { + onFieldChange: (oldColumn, field) => { const newParams = { ...oldColumn.params }; if ('format' in newParams && field.type !== 'number') { delete newParams.format; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx index bb1b13ba74cc58..0c6d6d18a74dc2 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/definitions/terms/terms.test.tsx @@ -105,7 +105,7 @@ describe('terms', () => { const indexPattern = createMockedIndexPattern(); const newNumberField = indexPattern.getFieldByName('bytes')!; - const column = termsOperation.onFieldChange(oldColumn, indexPattern, newNumberField); + const column = termsOperation.onFieldChange(oldColumn, newNumberField); expect(column).toHaveProperty('dataType', 'number'); expect(column).toHaveProperty('sourceField', 'bytes'); expect(column).toHaveProperty('params.size', 5); @@ -133,7 +133,7 @@ describe('terms', () => { const indexPattern = createMockedIndexPattern(); const newStringField = indexPattern.fields.find((i) => i.name === 'source')!; - const column = termsOperation.onFieldChange(oldColumn, indexPattern, newStringField); + const column = termsOperation.onFieldChange(oldColumn, newStringField); expect(column).toHaveProperty('dataType', 'string'); expect(column).toHaveProperty('sourceField', 'source'); expect(column.params.format).toBeUndefined(); @@ -236,8 +236,6 @@ describe('terms', () => { describe('buildColumn', () => { it('should use type from the passed field', () => { const termsColumn = termsOperation.buildColumn({ - layerId: 'first', - suggestedPriority: undefined, indexPattern: createMockedIndexPattern(), field: { aggregatable: true, @@ -253,8 +251,6 @@ describe('terms', () => { it('should use existing metric column as order column', () => { const termsColumn = termsOperation.buildColumn({ - layerId: 'first', - suggestedPriority: undefined, indexPattern: createMockedIndexPattern(), columns: { col1: { @@ -279,6 +275,36 @@ describe('terms', () => { }) ); }); + + it('should use the default size when there is an existing bucket', () => { + const termsColumn = termsOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + columns: state.layers.first.columns, + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + }); + expect(termsColumn.params).toEqual(expect.objectContaining({ size: 3 })); + }); + + it('should use a size of 5 when there are no other buckets', () => { + const termsColumn = termsOperation.buildColumn({ + indexPattern: createMockedIndexPattern(), + columns: {}, + field: { + aggregatable: true, + searchable: true, + type: 'boolean', + name: 'test', + displayName: 'test', + }, + }); + expect(termsColumn.params).toEqual(expect.objectContaining({ size: 5 })); + }); }); describe('onOtherColumnChanged', () => { diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts index 31a36c59274da7..f0e02c7ff0faf5 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/index.ts @@ -5,4 +5,5 @@ */ export * from './operations'; +export * from './layer_helpers'; export { OperationType, IndexPatternColumn, FieldBasedIndexPatternColumn } from './definitions'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts similarity index 52% rename from x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.test.ts rename to x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts index 45008b2d9439a2..e1a31dc274837c 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.test.ts @@ -5,22 +5,658 @@ */ import { + insertNewColumn, + replaceColumn, updateColumnParam, - changeColumn, getColumnOrder, deleteColumn, updateLayerIndexPattern, -} from './state_helpers'; -import { operationDefinitionMap } from './operations'; -import { TermsIndexPatternColumn } from './operations/definitions/terms'; -import { DateHistogramIndexPatternColumn } from './operations/definitions/date_histogram'; -import { AvgIndexPatternColumn } from './operations/definitions/metrics'; -import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types'; -import { getFieldByNameFactory } from './pure_helpers'; +} from './layer_helpers'; +import { operationDefinitionMap, OperationType } from '../operations'; +import { TermsIndexPatternColumn } from './definitions/terms'; +import { DateHistogramIndexPatternColumn } from './definitions/date_histogram'; +import { AvgIndexPatternColumn } from './definitions/metrics'; +import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from '../types'; +import { documentField } from '../document_field'; +import { getFieldByNameFactory } from '../pure_helpers'; -jest.mock('./operations'); +jest.mock('../operations'); + +const indexPatternFields = [ + { + name: 'timestamp', + displayName: 'timestampLabel', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'start_date', + displayName: 'start_date', + type: 'date', + aggregatable: true, + searchable: true, + }, + { + name: 'bytes', + displayName: 'bytes', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'memory', + displayName: 'memory', + type: 'number', + aggregatable: true, + searchable: true, + }, + { + name: 'source', + displayName: 'source', + type: 'string', + aggregatable: true, + searchable: true, + }, + { + name: 'dest', + displayName: 'dest', + type: 'string', + aggregatable: true, + searchable: true, + }, + documentField, +]; + +const indexPattern = { + id: '1', + title: 'my-fake-index-pattern', + timeFieldName: 'timestamp', + hasRestrictions: false, + fields: indexPatternFields, + getFieldByName: getFieldByNameFactory(indexPatternFields), +}; describe('state_helpers', () => { + describe('insertNewColumn', () => { + it('should throw for invalid operations', () => { + expect(() => { + insertNewColumn({ + layer: { indexPatternId: '', columns: {}, columnOrder: [] }, + indexPattern, + op: 'missing' as OperationType, + columnId: 'none', + }); + }).toThrow(); + }); + + it('should update order on inserting a bucketed fieldless operation', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'avg', + sourceField: 'bytes', + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'filters', + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col2', 'col1'] })); + }); + + it('should update order on inserting a bucketed field-based operation', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'avg', + sourceField: 'bytes', + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'date_histogram', + field: indexPattern.fields[0], + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col2', 'col1'] })); + }); + + it('should insert a metric after buckets', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'count', + field: documentField, + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2'] })); + }); + + it('should insert new buckets at the end of previous buckets', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col3'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + col3: { + label: 'Count of records', + dataType: 'document', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + }, + }; + expect( + insertNewColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'filters', + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2', 'col3'] })); + }); + + it('should throw if the aggregation does not support the field', () => { + expect(() => { + insertNewColumn({ + layer: { indexPatternId: '1', columnOrder: [], columns: {} }, + columnId: 'col1', + indexPattern, + op: 'terms', + field: indexPattern.fields[0], + }); + }).toThrow(); + }); + + it('should put the terms agg ahead of the date histogram', () => { + expect( + insertNewColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + columnId: 'col2', + indexPattern, + op: 'terms', + field: indexPattern.fields[2], + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col2', 'col1'] })); + }); + + it('should allow two date histograms', () => { + expect( + insertNewColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + columnId: 'col2', + indexPattern, + op: 'date_histogram', + field: indexPattern.fields[0], + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2'] })); + }); + + it('should allow multiple metrics', () => { + expect( + insertNewColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'avg', + sourceField: 'bytes', + }, + col2: { + label: 'Count of records', + dataType: 'document', + isBucketed: false, + + // Private + operationType: 'count', + sourceField: 'Records', + }, + }, + }, + columnId: 'col3', + indexPattern, + op: 'sum', + field: indexPattern.fields[2], + }) + ).toEqual(expect.objectContaining({ columnOrder: ['col1', 'col2', 'col3'] })); + }); + }); + + describe('replaceColumn', () => { + it('should throw if there is no previous column', () => { + expect(() => { + replaceColumn({ + layer: { indexPatternId: '', columns: {}, columnOrder: [] }, + indexPattern, + op: 'count', + field: documentField, + columnId: 'none', + }); + }).toThrow(); + }); + + it('should throw for invalid operations', () => { + expect(() => { + replaceColumn({ + layer: { indexPatternId: '', columns: {}, columnOrder: [] }, + indexPattern, + op: 'missing' as OperationType, + columnId: 'none', + }); + }).toThrow(); + }); + + it('should update order on changing the column', () => { + const layer: IndexPatternLayer = { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: { + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'avg', + sourceField: 'bytes', + }, + col2: { + label: 'Max of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'max', + sourceField: 'bytes', + }, + }, + }; + expect( + replaceColumn({ + layer, + indexPattern, + columnId: 'col2', + op: 'date_histogram', + field: indexPattern.fields[0], // date + }) + ).toEqual( + expect.objectContaining({ + columnOrder: ['col2', 'col1'], + }) + ); + }); + + it('should throw if nothing is changing', () => { + expect(() => { + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + columnId: 'col1', + indexPattern, + op: 'date_histogram', + field: indexPattern.fields[0], + }); + }).toThrow(); + }); + + it('should throw if switching to a field-based operation without providing a field', () => { + expect(() => { + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + columnId: 'col1', + indexPattern, + op: 'date_histogram', + }); + }).toThrow(); + }); + + it('should carry over params from old column if the switching fields', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + columnId: 'col1', + indexPattern, + op: 'date_histogram', + field: indexPattern.fields[1], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + params: { interval: 'h' }, + }) + ); + }); + + it('should transition from field-based to fieldless operation', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Date histogram of timestamp', + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'filters', + }).columns.col1 + ).toEqual( + expect.objectContaining({ + operationType: 'filters', + }) + ); + }); + + it('should transition from fieldless to field-based operation', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'Filters', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'filters', + params: { + filters: [], + }, + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'date_histogram', + field: indexPattern.fields[0], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + operationType: 'date_histogram', + }) + ); + }); + + it('should carry over label on field switch when customLabel flag is set', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'My custom label', + customLabel: true, + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'date_histogram', + field: indexPattern.fields[1], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + label: 'My custom label', + customLabel: true, + }) + ); + }); + + it('should carry over label on operation switch when customLabel flag is set', () => { + expect( + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1'], + columns: { + col1: { + label: 'My custom label', + customLabel: true, + dataType: 'date', + isBucketed: true, + + // Private + operationType: 'date_histogram', + sourceField: 'timestamp', + params: { + interval: 'h', + }, + }, + }, + }, + indexPattern, + columnId: 'col1', + op: 'terms', + field: indexPattern.fields[0], + }).columns.col1 + ).toEqual( + expect.objectContaining({ + label: 'My custom label', + customLabel: true, + }) + ); + }); + + it('should execute adjustments for other columns', () => { + const termsColumn: TermsIndexPatternColumn = { + label: 'Top values of source', + dataType: 'string', + isBucketed: true, + + // Private + operationType: 'terms', + sourceField: 'source', + params: { + orderBy: { type: 'alphabetical' }, + orderDirection: 'asc', + size: 5, + }, + }; + + replaceColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: termsColumn, + col2: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', + }, + }, + }, + indexPattern, + columnId: 'col2', + op: 'avg', + field: indexPattern.fields[2], // bytes field + }); + + expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(termsColumn, { + col1: termsColumn, + col2: expect.objectContaining({ + label: 'Average of bytes', + dataType: 'number', + isBucketed: false, + + // Private + operationType: 'avg', + sourceField: 'bytes', + }), + }); + }); + }); + describe('deleteColumn', () => { it('should remove column', () => { const termsColumn: TermsIndexPatternColumn = { @@ -38,14 +674,9 @@ describe('state_helpers', () => { }, }; - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { + expect( + deleteColumn({ + layer: { indexPatternId: '1', columnOrder: ['col1', 'col2'], columns: { @@ -59,11 +690,8 @@ describe('state_helpers', () => { }, }, }, - }, - }; - - expect( - deleteColumn({ state, columnId: 'col2', layerId: 'first' }).layers.first.columns + columnId: 'col2', + }).columns ).toEqual({ col1: { ...termsColumn, @@ -92,34 +720,22 @@ describe('state_helpers', () => { }, }; - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: termsColumn, - col2: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, + deleteColumn({ + layer: { + indexPatternId: '1', + columnOrder: ['col1', 'col2'], + columns: { + col1: termsColumn, + col2: { + label: 'Count', + dataType: 'number', + isBucketed: false, + sourceField: 'Records', + operationType: 'count', }, }, }, - }; - - deleteColumn({ - state, columnId: 'col2', - layerId: 'first', }); expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(termsColumn, { @@ -216,241 +832,6 @@ describe('state_helpers', () => { }); }); - describe('changeColumn', () => { - it('should update order on changing the column', () => { - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: { - label: 'Average of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'avg', - sourceField: 'bytes', - }, - col2: { - label: 'Max of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'max', - sourceField: 'bytes', - }, - }, - }, - }, - }; - expect( - changeColumn({ - state, - columnId: 'col2', - layerId: 'first', - newColumn: { - label: 'Date histogram of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - params: { - interval: '1d', - }, - sourceField: 'timestamp', - }, - }) - ).toEqual({ - ...state, - layers: { - first: expect.objectContaining({ - columnOrder: ['col2', 'col1'], - }), - }, - }); - }); - - it('should carry over params from old column if the operation type stays the same', () => { - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Date histogram of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - sourceField: 'timestamp', - params: { - interval: 'h', - }, - }, - }, - }, - }, - }; - expect( - changeColumn({ - state, - layerId: 'first', - columnId: 'col2', - newColumn: { - label: 'Date histogram of order_date', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - sourceField: 'order_date', - params: { - interval: 'w', - }, - }, - }).layers.first.columns.col1 - ).toEqual( - expect.objectContaining({ - params: { interval: 'h' }, - }) - ); - }); - - it('should carry over label if customLabel flag is set', () => { - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'My custom label', - customLabel: true, - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - sourceField: 'timestamp', - params: { - interval: 'h', - }, - }, - }, - }, - }, - }; - expect( - changeColumn({ - state, - layerId: 'first', - columnId: 'col2', - newColumn: { - label: 'Date histogram of order_date', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - sourceField: 'order_date', - params: { - interval: 'w', - }, - }, - }).layers.first.columns.col1 - ).toEqual( - expect.objectContaining({ - label: 'My custom label', - customLabel: true, - }) - ); - }); - - it('should execute adjustments for other columns', () => { - const termsColumn: TermsIndexPatternColumn = { - label: 'Top values of source', - dataType: 'string', - isBucketed: true, - - // Private - operationType: 'terms', - sourceField: 'source', - params: { - orderBy: { type: 'alphabetical' }, - orderDirection: 'asc', - size: 5, - }, - }; - - const newColumn: AvgIndexPatternColumn = { - label: 'Average of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'avg', - sourceField: 'bytes', - }; - - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - indexPatterns: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1', 'col2'], - columns: { - col1: termsColumn, - col2: { - label: 'Count', - dataType: 'number', - isBucketed: false, - sourceField: 'Records', - operationType: 'count', - }, - }, - }, - }, - }; - - changeColumn({ - state, - layerId: 'first', - columnId: 'col2', - newColumn, - }); - - expect(operationDefinitionMap.terms.onOtherColumnChanged).toHaveBeenCalledWith(termsColumn, { - col1: termsColumn, - col2: newColumn, - }); - }); - }); - describe('getColumnOrder', () => { it('should work for empty columns', () => { expect( @@ -532,57 +913,6 @@ describe('state_helpers', () => { }) ).toEqual(['col1', 'col3', 'col2']); }); - - it('should reorder aggregations based on suggested priority', () => { - expect( - getColumnOrder({ - indexPatternId: '', - columnOrder: [], - columns: { - col1: { - label: 'Top values of category', - dataType: 'string', - isBucketed: true, - - // Private - operationType: 'terms', - sourceField: 'category', - params: { - size: 5, - orderBy: { - type: 'alphabetical', - }, - orderDirection: 'asc', - }, - suggestedPriority: 2, - }, - col2: { - label: 'Average of bytes', - dataType: 'number', - isBucketed: false, - - // Private - operationType: 'avg', - sourceField: 'bytes', - suggestedPriority: 0, - }, - col3: { - label: 'Date histogram of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - sourceField: 'timestamp', - suggestedPriority: 1, - params: { - interval: '1d', - }, - }, - }, - }) - ).toEqual(['col3', 'col1', 'col2']); - }); }); describe('updateLayerIndexPattern', () => { @@ -635,7 +965,7 @@ describe('state_helpers', () => { type: 'date', }, ]; - const indexPattern: IndexPattern = { + const newIndexPattern: IndexPattern = { id: 'test', title: '', hasRestrictions: true, @@ -645,7 +975,7 @@ describe('state_helpers', () => { it('should switch index pattern id in layer', () => { const layer = { columnOrder: [], columns: {}, indexPatternId: 'original' }; - expect(updateLayerIndexPattern(layer, indexPattern)).toEqual({ + expect(updateLayerIndexPattern(layer, newIndexPattern)).toEqual({ ...layer, indexPatternId: 'test', }); @@ -677,7 +1007,7 @@ describe('state_helpers', () => { }, indexPatternId: 'original', }; - const updatedLayer = updateLayerIndexPattern(layer, indexPattern); + const updatedLayer = updateLayerIndexPattern(layer, newIndexPattern); expect(updatedLayer.columnOrder).toEqual(['col1']); expect(updatedLayer.columns).toEqual({ col1: layer.columns.col1, @@ -708,7 +1038,7 @@ describe('state_helpers', () => { }, indexPatternId: 'original', }; - const updatedLayer = updateLayerIndexPattern(layer, indexPattern); + const updatedLayer = updateLayerIndexPattern(layer, newIndexPattern); expect(updatedLayer.columnOrder).toEqual(['col2']); expect(updatedLayer.columns).toEqual({ col2: layer.columns.col2, @@ -732,7 +1062,7 @@ describe('state_helpers', () => { }, indexPatternId: 'original', }; - const updatedLayer = updateLayerIndexPattern(layer, indexPattern); + const updatedLayer = updateLayerIndexPattern(layer, newIndexPattern); expect(updatedLayer.columnOrder).toEqual(['col1']); expect(updatedLayer.columns).toEqual({ col1: { @@ -771,7 +1101,7 @@ describe('state_helpers', () => { }, indexPatternId: 'original', }; - const updatedLayer = updateLayerIndexPattern(layer, indexPattern); + const updatedLayer = updateLayerIndexPattern(layer, newIndexPattern); expect(updatedLayer.columnOrder).toEqual(['col1']); expect(updatedLayer.columns).toEqual({ col1: layer.columns.col1, @@ -804,7 +1134,7 @@ describe('state_helpers', () => { }, indexPatternId: 'original', }; - const updatedLayer = updateLayerIndexPattern(layer, indexPattern); + const updatedLayer = updateLayerIndexPattern(layer, newIndexPattern); expect(updatedLayer.columnOrder).toEqual(['col1']); expect(updatedLayer.columns).toEqual({ col1: layer.columns.col1, diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts new file mode 100644 index 00000000000000..f071df15421471 --- /dev/null +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/layer_helpers.ts @@ -0,0 +1,344 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import _, { partition } from 'lodash'; +import { + operationDefinitionMap, + operationDefinitions, + OperationType, + IndexPatternColumn, +} from './definitions'; +import { + IndexPattern, + IndexPatternField, + IndexPatternLayer, + IndexPatternPrivateState, +} from '../types'; +import { getSortScoreByPriority } from './operations'; +import { mergeLayer } from '../state_helpers'; + +interface ColumnChange { + op: OperationType; + layer: IndexPatternLayer; + columnId: string; + indexPattern: IndexPattern; + field?: IndexPatternField; +} + +export function insertOrReplaceColumn(args: ColumnChange): IndexPatternLayer { + if (args.layer.columns[args.columnId]) { + return replaceColumn(args); + } + return insertNewColumn(args); +} + +export function insertNewColumn({ + op, + layer, + columnId, + field, + indexPattern, +}: ColumnChange): IndexPatternLayer { + const operationDefinition = operationDefinitionMap[op]; + + if (!operationDefinition) { + throw new Error('No suitable operation found for given parameters'); + } + + const baseOptions = { + columns: layer.columns, + indexPattern, + previousColumn: layer.columns[columnId], + }; + + // TODO: Reference based operations require more setup to create the references + + if (operationDefinition.input === 'none') { + const possibleOperation = operationDefinition.getPossibleOperation(); + if (!possibleOperation) { + throw new Error('Tried to create an invalid operation'); + } + const isBucketed = Boolean(possibleOperation.isBucketed); + if (isBucketed) { + return addBucket(layer, operationDefinition.buildColumn(baseOptions), columnId); + } else { + return addMetric(layer, operationDefinition.buildColumn(baseOptions), columnId); + } + } + + if (!field) { + throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); + } + + const possibleOperation = operationDefinition.getPossibleOperationForField(field); + if (!possibleOperation) { + throw new Error( + `Tried to create an invalid operation ${operationDefinition.type} on ${field.name}` + ); + } + const isBucketed = Boolean(possibleOperation.isBucketed); + if (isBucketed) { + return addBucket(layer, operationDefinition.buildColumn({ ...baseOptions, field }), columnId); + } else { + return addMetric(layer, operationDefinition.buildColumn({ ...baseOptions, field }), columnId); + } +} + +export function replaceColumn({ + layer, + columnId, + indexPattern, + op, + field, +}: ColumnChange): IndexPatternLayer { + const previousColumn = layer.columns[columnId]; + if (!previousColumn) { + throw new Error(`Can't replace column because there is no prior column`); + } + + const isNewOperation = Boolean(op) && op !== previousColumn.operationType; + const operationDefinition = operationDefinitionMap[op || previousColumn.operationType]; + + if (!operationDefinition) { + throw new Error('No suitable operation found for given parameters'); + } + + const baseOptions = { + columns: layer.columns, + indexPattern, + previousColumn, + }; + + if (isNewOperation) { + // TODO: Reference based operations require more setup to create the references + + if (operationDefinition.input === 'none') { + const newColumn = operationDefinition.buildColumn(baseOptions); + + if (previousColumn.customLabel) { + newColumn.customLabel = true; + newColumn.label = previousColumn.label; + } + + return { + ...layer, + columns: adjustColumnReferencesForChangedColumn( + { ...layer.columns, [columnId]: newColumn }, + columnId + ), + }; + } + + if (!field) { + throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); + } + + const newColumn = operationDefinition.buildColumn({ ...baseOptions, field }); + + if (previousColumn.customLabel) { + newColumn.customLabel = true; + newColumn.label = previousColumn.label; + } + + const newColumns = { ...layer.columns, [columnId]: newColumn }; + return { + ...layer, + columnOrder: getColumnOrder({ ...layer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + }; + } else if ( + operationDefinition.input === 'field' && + field && + 'sourceField' in previousColumn && + previousColumn.sourceField !== field.name + ) { + // Same operation, new field + const newColumn = operationDefinition.onFieldChange(previousColumn, field); + + if (previousColumn.customLabel) { + newColumn.customLabel = true; + newColumn.label = previousColumn.label; + } + + const newColumns = { ...layer.columns, [columnId]: newColumn }; + return { + ...layer, + columnOrder: getColumnOrder({ ...layer, columns: newColumns }), + columns: adjustColumnReferencesForChangedColumn(newColumns, columnId), + }; + } else { + throw new Error('nothing changed'); + } +} + +function addBucket( + layer: IndexPatternLayer, + column: IndexPatternColumn, + addedColumnId: string +): IndexPatternLayer { + const [buckets, metrics] = separateBucketColumns(layer); + + const oldDateHistogramIndex = layer.columnOrder.findIndex( + (columnId) => layer.columns[columnId].operationType === 'date_histogram' + ); + + let updatedColumnOrder: string[] = []; + if (oldDateHistogramIndex > -1 && column.operationType === 'terms') { + // Insert the new terms bucket above the first date histogram + updatedColumnOrder = [ + ...buckets.slice(0, oldDateHistogramIndex), + addedColumnId, + ...buckets.slice(oldDateHistogramIndex, buckets.length), + ...metrics, + ]; + } else { + // Insert the new bucket after existing buckets. Users will see the same data + // they already had, with an extra level of detail. + updatedColumnOrder = [...buckets, addedColumnId, ...metrics]; + } + return { + ...layer, + columns: { ...layer.columns, [addedColumnId]: column }, + columnOrder: updatedColumnOrder, + }; +} + +function addMetric( + layer: IndexPatternLayer, + column: IndexPatternColumn, + addedColumnId: string +): IndexPatternLayer { + return { + ...layer, + columns: { + ...layer.columns, + [addedColumnId]: column, + }, + columnOrder: [...layer.columnOrder, addedColumnId], + }; +} + +function separateBucketColumns(layer: IndexPatternLayer) { + return partition(layer.columnOrder, (columnId) => layer.columns[columnId]?.isBucketed); +} + +export function getMetricOperationTypes(field: IndexPatternField) { + return operationDefinitions.sort(getSortScoreByPriority).filter((definition) => { + if (definition.input !== 'field') return; + const metadata = definition.getPossibleOperationForField(field); + return metadata && !metadata.isBucketed && metadata.dataType === 'number'; + }); +} + +export function updateColumnParam({ + state, + layerId, + currentColumn, + paramName, + value, +}: { + state: IndexPatternPrivateState; + layerId: string; + currentColumn: C; + paramName: string; + value: unknown; +}): IndexPatternPrivateState { + const columnId = Object.entries(state.layers[layerId].columns).find( + ([_columnId, column]) => column === currentColumn + )![0]; + + const layer = state.layers[layerId]; + + return mergeLayer({ + state, + layerId, + newLayer: { + columns: { + ...layer.columns, + [columnId]: { + ...currentColumn, + params: { + ...currentColumn.params, + [paramName]: value, + }, + }, + }, + }, + }); +} + +function adjustColumnReferencesForChangedColumn( + columns: Record, + columnId: string +) { + const newColumns = { ...columns }; + Object.keys(newColumns).forEach((currentColumnId) => { + if (currentColumnId !== columnId) { + const currentColumn = newColumns[currentColumnId]; + const operationDefinition = operationDefinitionMap[currentColumn.operationType]; + newColumns[currentColumnId] = operationDefinition.onOtherColumnChanged + ? operationDefinition.onOtherColumnChanged(currentColumn, newColumns) + : currentColumn; + } + }); + return newColumns; +} + +export function deleteColumn({ + layer, + columnId, +}: { + layer: IndexPatternLayer; + columnId: string; +}): IndexPatternLayer { + const hypotheticalColumns = { ...layer.columns }; + delete hypotheticalColumns[columnId]; + + const newLayer = { + ...layer, + columns: adjustColumnReferencesForChangedColumn(hypotheticalColumns, columnId), + }; + return { ...newLayer, columnOrder: getColumnOrder(newLayer) }; +} + +export function getColumnOrder(layer: IndexPatternLayer): string[] { + const [aggregations, metrics] = _.partition( + Object.entries(layer.columns), + ([id, col]) => col.isBucketed + ); + + return aggregations.map(([id]) => id).concat(metrics.map(([id]) => id)); +} + +/** + * Returns true if the given column can be applied to the given index pattern + */ +export function isColumnTransferable(column: IndexPatternColumn, newIndexPattern: IndexPattern) { + return operationDefinitionMap[column.operationType].isTransferable(column, newIndexPattern); +} + +export function updateLayerIndexPattern( + layer: IndexPatternLayer, + newIndexPattern: IndexPattern +): IndexPatternLayer { + const keptColumns: IndexPatternLayer['columns'] = _.pickBy(layer.columns, (column) => + isColumnTransferable(column, newIndexPattern) + ); + const newColumns: IndexPatternLayer['columns'] = _.mapValues(keptColumns, (column) => { + const operationDefinition = operationDefinitionMap[column.operationType]; + return operationDefinition.transfer + ? operationDefinition.transfer(column, newIndexPattern) + : column; + }); + const newColumnOrder = layer.columnOrder.filter((columnId) => newColumns[columnId]); + + return { + ...layer, + indexPatternId: newIndexPattern.id, + columns: newColumns, + columnOrder: newColumnOrder, + }; +} diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts index 9767d4bdca688b..99149c3e745681 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.test.ts @@ -4,10 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import { getOperationTypesForField, getAvailableOperationsByMetadata, buildColumn } from './index'; -import { AvgIndexPatternColumn } from './definitions/metrics'; -import { IndexPatternPrivateState } from '../types'; -import { documentField } from '../document_field'; +import { getOperationTypesForField, getAvailableOperationsByMetadata } from './index'; import { getFieldByNameFactory } from '../pure_helpers'; jest.mock('../loader'); @@ -157,73 +154,6 @@ describe('getOperationTypesForField', () => { }); }); - describe('buildColumn', () => { - const state: IndexPatternPrivateState = { - indexPatternRefs: [], - existingFields: {}, - currentIndexPatternId: '1', - isFirstExistenceFetch: false, - indexPatterns: expectedIndexPatterns, - layers: { - first: { - indexPatternId: '1', - columnOrder: ['col1'], - columns: { - col1: { - label: 'Date histogram of timestamp', - dataType: 'date', - isBucketed: true, - - // Private - operationType: 'date_histogram', - params: { - interval: '1d', - }, - sourceField: 'timestamp', - }, - }, - }, - }, - }; - - it('should build a column for the given field-based operation type if it is passed in', () => { - const column = buildColumn({ - layerId: 'first', - indexPattern: expectedIndexPatterns[1], - columns: state.layers.first.columns, - suggestedPriority: 0, - op: 'count', - field: documentField, - }); - expect(column.operationType).toEqual('count'); - }); - - it('should build a column for the given no-input operation type if it is passed in', () => { - const column = buildColumn({ - layerId: 'first', - indexPattern: expectedIndexPatterns[1], - columns: state.layers.first.columns, - suggestedPriority: 0, - op: 'filters', - }); - expect(column.operationType).toEqual('filters'); - }); - - it('should build a column for the given operation type and field if it is passed in', () => { - const field = expectedIndexPatterns[1].fields[1]; - const column = buildColumn({ - layerId: 'first', - indexPattern: expectedIndexPatterns[1], - columns: state.layers.first.columns, - suggestedPriority: 0, - op: 'avg', - field, - }) as AvgIndexPatternColumn; - expect(column.operationType).toEqual('avg'); - expect(column.sourceField).toEqual(field.name); - }); - }); - describe('getAvailableOperationsByMetaData', () => { it('should put the average operation first', () => { const numberOperation = getAvailableOperationsByMetadata(expectedIndexPatterns[1]).find( diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts index 46dd73ba849a26..8d489df3660887 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/operations/operations.ts @@ -4,17 +4,19 @@ * you may not use this file except in compliance with the Elastic License. */ -import { DimensionPriority, OperationMetadata } from '../../types'; +import _ from 'lodash'; +import { OperationMetadata } from '../../types'; import { operationDefinitionMap, operationDefinitions, GenericOperationDefinition, OperationType, - IndexPatternColumn, } from './definitions'; import { IndexPattern, IndexPatternField } from '../types'; import { documentField } from '../document_field'; +export { operationDefinitionMap } from './definitions'; + /** * Returns all available operation types as a list at runtime. * This will be an array of each member of the union type `OperationType` @@ -24,13 +26,6 @@ export function getOperations(): OperationType[] { return Object.keys(operationDefinitionMap) as OperationType[]; } -/** - * Returns true if the given column can be applied to the given index pattern - */ -export function isColumnTransferable(column: IndexPatternColumn, newIndexPattern: IndexPattern) { - return operationDefinitionMap[column.operationType].isTransferable(column, newIndexPattern); -} - /** * Returns a list of the display names of all operations with any guaranteed order. */ @@ -51,7 +46,10 @@ export function getOperationDisplay() { return display; } -function getSortScoreByPriority(a: GenericOperationDefinition, b: GenericOperationDefinition) { +export function getSortScoreByPriority( + a: GenericOperationDefinition, + b: GenericOperationDefinition +) { return (b.priority || Number.NEGATIVE_INFINITY) - (a.priority || Number.NEGATIVE_INFINITY); } @@ -169,82 +167,3 @@ export function getAvailableOperationsByMetadata(indexPattern: IndexPattern) { return Object.values(operationByMetadata); } - -/** - * Changes the field of the passed in colum. To do so, this method uses the `onFieldChange` function of - * the operation definition of the column. Returns a new column object with the field changed. - * @param column The column object with the old field configured - * @param indexPattern The index pattern associated to the layer of the column - * @param newField The new field the column should be switched to - */ -export function changeField( - column: IndexPatternColumn, - indexPattern: IndexPattern, - newField: IndexPatternField -) { - const operationDefinition = operationDefinitionMap[column.operationType]; - - if (operationDefinition.input === 'field' && 'sourceField' in column) { - return operationDefinition.onFieldChange(column, indexPattern, newField); - } else { - throw new Error( - "Invariant error: Cannot change field if operation isn't a field based operaiton" - ); - } -} - -/** - * Builds a column object based on the context passed in. It tries - * to find the applicable operation definition and then calls the `buildColumn` - * function of that definition. It passes in the given `field` (if available), - * `suggestedPriority`, `layerId` and the currently existing `columns`. - * * If `op` is specified, the specified operation definition is used directly. - * * If `asDocumentOperation` is true, the first matching document-operation is used. - * * If `field` is specified, the first matching field based operation applicable to the field is used. - */ -export function buildColumn({ - op, - columns, - field, - layerId, - indexPattern, - suggestedPriority, - previousColumn, -}: { - op: OperationType; - columns: Partial>; - suggestedPriority: DimensionPriority | undefined; - layerId: string; - indexPattern: IndexPattern; - field?: IndexPatternField; - previousColumn?: IndexPatternColumn; -}): IndexPatternColumn { - const operationDefinition = operationDefinitionMap[op]; - - if (!operationDefinition) { - throw new Error('No suitable operation found for given parameters'); - } - - const baseOptions = { - columns, - suggestedPriority, - layerId, - indexPattern, - previousColumn, - }; - - if (operationDefinition.input === 'none') { - return operationDefinition.buildColumn(baseOptions); - } - - if (!field) { - throw new Error(`Invariant error: ${operationDefinition.type} operation requires field`); - } - - return operationDefinition.buildColumn({ - ...baseOptions, - field, - }); -} - -export { operationDefinitionMap } from './definitions'; diff --git a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts b/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts index 2e92d4ad8f88b5..32e45965f3e754 100644 --- a/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts +++ b/x-pack/plugins/lens/public/indexpattern_datasource/state_helpers.ts @@ -4,162 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -import _ from 'lodash'; -import { isColumnTransferable, operationDefinitionMap, IndexPatternColumn } from './operations'; -import { IndexPattern, IndexPatternPrivateState, IndexPatternLayer } from './types'; - -export function updateColumnParam({ - state, - layerId, - currentColumn, - paramName, - value, -}: { - state: IndexPatternPrivateState; - layerId: string; - currentColumn: C; - paramName: string; - value: unknown; -}): IndexPatternPrivateState { - const columnId = Object.entries(state.layers[layerId].columns).find( - ([_columnId, column]) => column === currentColumn - )![0]; - - const layer = state.layers[layerId]; - - return mergeLayer({ - state, - layerId, - newLayer: { - columns: { - ...layer.columns, - [columnId]: { - ...currentColumn, - params: { - ...currentColumn.params, - [paramName]: value, - }, - }, - }, - }, - }); -} - -function adjustColumnReferencesForChangedColumn( - columns: Record, - columnId: string -) { - const newColumns = { ...columns }; - Object.keys(newColumns).forEach((currentColumnId) => { - if (currentColumnId !== columnId) { - const currentColumn = newColumns[currentColumnId]; - const operationDefinition = operationDefinitionMap[currentColumn.operationType]; - newColumns[currentColumnId] = operationDefinition.onOtherColumnChanged - ? operationDefinition.onOtherColumnChanged(currentColumn, newColumns) - : currentColumn; - } - }); - return newColumns; -} - -export function changeColumn({ - state, - layerId, - columnId, - newColumn, - keepParams = true, -}: { - state: IndexPatternPrivateState; - layerId: string; - columnId: string; - newColumn: C; - keepParams?: boolean; -}): IndexPatternPrivateState { - const oldColumn = state.layers[layerId].columns[columnId]; - - const updatedColumn = - keepParams && - oldColumn && - oldColumn.operationType === newColumn.operationType && - 'params' in oldColumn - ? { ...newColumn, params: oldColumn.params } - : newColumn; - - if (oldColumn && oldColumn.customLabel) { - updatedColumn.customLabel = true; - updatedColumn.label = oldColumn.label; - } - - const layer = { - ...state.layers[layerId], - }; - - const newColumns = adjustColumnReferencesForChangedColumn( - { - ...layer.columns, - [columnId]: updatedColumn, - }, - columnId - ); - - return mergeLayer({ - state, - layerId, - newLayer: { - columnOrder: getColumnOrder({ - ...layer, - columns: newColumns, - }), - columns: newColumns, - }, - }); -} - -export function deleteColumn({ - state, - layerId, - columnId, -}: { - state: IndexPatternPrivateState; - layerId: string; - columnId: string; -}): IndexPatternPrivateState { - const hypotheticalColumns = { ...state.layers[layerId].columns }; - delete hypotheticalColumns[columnId]; - - const newColumns = adjustColumnReferencesForChangedColumn(hypotheticalColumns, columnId); - const layer = { - ...state.layers[layerId], - columns: newColumns, - }; - - return mergeLayer({ - state, - layerId, - newLayer: { - ...layer, - columnOrder: getColumnOrder(layer), - }, - }); -} - -export function getColumnOrder(layer: IndexPatternLayer): string[] { - const [aggregations, metrics] = _.partition( - Object.entries(layer.columns), - ([id, col]) => col.isBucketed - ); - - return aggregations - .sort(([id, col], [id2, col2]) => { - return ( - // Sort undefined orders last - (col.suggestedPriority !== undefined ? col.suggestedPriority : Number.MAX_SAFE_INTEGER) - - (col2.suggestedPriority !== undefined ? col2.suggestedPriority : Number.MAX_SAFE_INTEGER) - ); - }) - .map(([id]) => id) - .concat(metrics.map(([id]) => id)); -} +import { IndexPatternPrivateState, IndexPatternLayer } from './types'; export function mergeLayer({ state, @@ -178,26 +23,3 @@ export function mergeLayer({ }, }; } - -export function updateLayerIndexPattern( - layer: IndexPatternLayer, - newIndexPattern: IndexPattern -): IndexPatternLayer { - const keptColumns: IndexPatternLayer['columns'] = _.pickBy(layer.columns, (column) => - isColumnTransferable(column, newIndexPattern) - ); - const newColumns: IndexPatternLayer['columns'] = _.mapValues(keptColumns, (column) => { - const operationDefinition = operationDefinitionMap[column.operationType]; - return operationDefinition.transfer - ? operationDefinition.transfer(column, newIndexPattern) - : column; - }); - const newColumnOrder = layer.columnOrder.filter((columnId) => newColumns[columnId]); - - return { - ...layer, - indexPatternId: newIndexPattern.id, - columns: newColumns, - columnOrder: newColumnOrder, - }; -} diff --git a/x-pack/plugins/lens/public/types.ts b/x-pack/plugins/lens/public/types.ts index 4ad849c5d441e4..b8bceb5454bc8e 100644 --- a/x-pack/plugins/lens/public/types.ts +++ b/x-pack/plugins/lens/public/types.ts @@ -72,9 +72,6 @@ export interface EditorFrameStart { createInstance: () => Promise; } -// Hints the default nesting to the data source. 0 is the highest priority -export type DimensionPriority = 0 | 1 | 2; - export interface TableSuggestionColumn { columnId: string; operation: Operation; @@ -220,11 +217,6 @@ interface SharedDimensionProps { */ filterOperations: (operation: OperationMetadata) => boolean; - /** Visualizations can hint at the role this dimension would play, which - * affects the default ordering of the query - */ - suggestedPriority?: DimensionPriority; - /** Some dimension editors will allow users to change the operation grouping * from the panel, and this lets the visualization hint that it doesn't want * users to have that level of control diff --git a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx index 7e155de14a39ab..1f135929dac21a 100644 --- a/x-pack/plugins/lens/public/xy_visualization/visualization.tsx +++ b/x-pack/plugins/lens/public/xy_visualization/visualization.tsx @@ -178,7 +178,6 @@ export const getXyVisualization = ({ groupLabel: getAxisName('x', { isHorizontal }), accessors: layer.xAccessor ? [layer.xAccessor] : [], filterOperations: isBucketed, - suggestedPriority: 1, supportsMoreColumns: !layer.xAccessor, dataTestSubj: 'lnsXY_xDimensionPanel', }, @@ -199,7 +198,6 @@ export const getXyVisualization = ({ }), accessors: layer.splitAccessor ? [layer.splitAccessor] : [], filterOperations: isBucketed, - suggestedPriority: 0, supportsMoreColumns: !layer.splitAccessor, dataTestSubj: 'lnsXY_splitDimensionPanel', required: layer.seriesType.includes('percentage'), diff --git a/x-pack/plugins/lens/server/migrations.test.ts b/x-pack/plugins/lens/server/migrations.test.ts index 676494dcab619b..957da5cbb37431 100644 --- a/x-pack/plugins/lens/server/migrations.test.ts +++ b/x-pack/plugins/lens/server/migrations.test.ts @@ -4,8 +4,8 @@ * you may not use this file except in compliance with the Elastic License. */ -import { migrations } from './migrations'; -import { SavedObjectMigrationContext } from 'src/core/server'; +import { migrations, LensDocShape } from './migrations'; +import { SavedObjectMigrationContext, SavedObjectMigrationFn } from 'src/core/server'; describe('Lens migrations', () => { describe('7.7.0 missing dimensions in XY', () => { @@ -507,4 +507,88 @@ describe('Lens migrations', () => { expect(result).toMatchSnapshot(); }); }); + + describe('7.11.0 remove suggested priority', () => { + const context = ({ log: { warning: () => {} } } as unknown) as SavedObjectMigrationContext; + + const example = { + type: 'lens', + attributes: { + state: { + datasourceStates: { + indexpattern: { + currentIndexPatternId: 'ff959d40-b880-11e8-a6d9-e546fe2bba5f', + layers: { + 'bd09dc71-a7e2-42d0-83bd-85df8291f03c': { + indexPatternId: 'ff959d40-b880-11e8-a6d9-e546fe2bba5f', + columns: { + '1d9cc16c-1460-41de-88f8-471932ecbc97': { + label: 'products.created_on', + dataType: 'date', + operationType: 'date_histogram', + sourceField: 'products.created_on', + isBucketed: true, + scale: 'interval', + params: { interval: 'auto' }, + suggestedPriority: 0, + }, + '66115819-8481-4917-a6dc-8ffb10dd02df': { + label: 'Count of records', + dataType: 'number', + operationType: 'count', + suggestedPriority: 1, + isBucketed: false, + scale: 'ratio', + sourceField: 'Records', + }, + }, + columnOrder: [ + '1d9cc16c-1460-41de-88f8-471932ecbc97', + '66115819-8481-4917-a6dc-8ffb10dd02df', + ], + }, + }, + }, + }, + datasourceMetaData: { + filterableIndexPatterns: [ + { id: 'ff959d40-b880-11e8-a6d9-e546fe2bba5f', title: 'kibana_sample_data_ecommerce' }, + ], + }, + visualization: { + legend: { isVisible: true, position: 'right' }, + preferredSeriesType: 'bar_stacked', + layers: [ + { + layerId: 'bd09dc71-a7e2-42d0-83bd-85df8291f03c', + accessors: ['66115819-8481-4917-a6dc-8ffb10dd02df'], + position: 'top', + seriesType: 'bar_stacked', + showGridlines: false, + xAccessor: '1d9cc16c-1460-41de-88f8-471932ecbc97', + }, + ], + }, + query: { query: '', language: 'kuery' }, + filters: [], + }, + title: 'Bar chart', + visualizationType: 'lnsXY', + }, + }; + + it('should remove the suggested priority from all columns', () => { + const result = migrations['7.11.0'](example, context) as ReturnType< + SavedObjectMigrationFn + >; + const resultLayers = result.attributes.state.datasourceStates.indexpattern.layers; + const layersWithSuggestedPriority = Object.values(resultLayers).reduce( + (count, layer) => + count + Object.values(layer.columns).filter((col) => 'suggestedPriority' in col).length, + 0 + ); + + expect(layersWithSuggestedPriority).toEqual(0); + }); + }); }); diff --git a/x-pack/plugins/lens/server/migrations.ts b/x-pack/plugins/lens/server/migrations.ts index fdbfa1e455f60d..3ec3abdfd62609 100644 --- a/x-pack/plugins/lens/server/migrations.ts +++ b/x-pack/plugins/lens/server/migrations.ts @@ -31,7 +31,7 @@ interface LensDocShapePre710 { string, { columnOrder: string[]; - columns: Record; + columns: Record>; indexPatternId: string; } >; @@ -43,7 +43,7 @@ interface LensDocShapePre710 { }; } -interface LensDocShape { +export interface LensDocShape { id?: string; type?: string; visualizationType: string | null; @@ -56,7 +56,7 @@ interface LensDocShape { string, { columnOrder: string[]; - columns: Record; + columns: Record>; } >; }; @@ -310,10 +310,34 @@ const extractReferences: SavedObjectMigrationFn = (doc) => { + const newDoc = cloneDeep(doc); + const datasourceLayers = newDoc.attributes.state.datasourceStates.indexpattern.layers || {}; + newDoc.attributes.state.datasourceStates.indexpattern.layers = Object.fromEntries( + Object.entries(datasourceLayers).map(([layerId, layer]) => { + return [ + layerId, + { + ...layer, + columns: Object.fromEntries( + Object.entries(layer.columns).map(([columnId, column]) => { + const copy = { ...column }; + delete copy.suggestedPriority; + return [columnId, copy]; + }) + ), + }, + ]; + }) + ); + return newDoc; +}; + export const migrations: SavedObjectMigrationMap = { '7.7.0': removeInvalidAccessors, // The order of these migrations matter, since the timefield migration relies on the aggConfigs // sitting directly on the esaggs as an argument and not a nested function (which lens_auto_date was). '7.8.0': (doc, context) => addTimeFieldToEsaggs(removeLensAutoDate(doc, context), context), '7.10.0': extractReferences, + '7.11.0': removeSuggestedPriority, }; From 8842e9b7a96640ea626f90ea7201233eafb217e5 Mon Sep 17 00:00:00 2001 From: Liza Katz Date: Tue, 10 Nov 2020 20:50:09 +0200 Subject: [PATCH 05/57] [Autocomplete] Support useTimeFilter option (#81515) * pass timefilter to autocomplete * ignoreTimeRange advanced setting * Show all results in filter bar autocomplete * Round timerange to minute for autocomplete memoization * Change useTimeFilter param name Update autocomplete tests and docs * Fix maps test useTimeFilter in uptime * docs * useTimeRange in apm * remove date validation * Update src/plugins/data/common/constants.ts Co-authored-by: Lukas Olson * docs Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com> Co-authored-by: Lukas Olson --- ...ns-data-public.querysuggestiongetfnargs.md | 1 + ...c.querysuggestiongetfnargs.usetimerange.md | 11 +++++ ...-plugin-plugins-data-public.ui_settings.md | 1 + ...-plugin-plugins-data-server.ui_settings.md | 1 + src/plugins/data/common/constants.ts | 1 + .../providers/query_suggestion_provider.ts | 1 + .../value_suggestion_provider.test.ts | 49 +++++++++++++++++++ .../providers/value_suggestion_provider.ts | 34 ++++++++++--- src/plugins/data/public/public.api.md | 3 ++ .../public/query/timefilter/timefilter.ts | 11 +++-- .../filter_editor/phrase_suggestor.tsx | 2 + .../autocomplete/value_suggestions_route.ts | 10 ++-- src/plugins/data/server/server.api.md | 1 + src/plugins/data/server/ui_settings.ts | 12 +++++ .../shared/KueryBar/get_bool_filter.ts | 18 +------ .../components/shared/KueryBar/index.tsx | 1 + .../providers/kql_query_suggestion/value.ts | 3 +- .../overview/kuery_bar/kuery_bar.tsx | 13 +---- .../apps/discover/value_suggestions.ts | 3 +- .../functional/apps/maps/vector_styling.js | 6 ++- 20 files changed, 136 insertions(+), 46 deletions(-) create mode 100644 docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.md index 96e43ca8368915..de6f4563b678ac 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.md @@ -23,4 +23,5 @@ export interface QuerySuggestionGetFnArgs | [selectionEnd](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.selectionend.md) | number | | | [selectionStart](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.selectionstart.md) | number | | | [signal](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.signal.md) | AbortSignal | | +| [useTimeRange](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md) | boolean | | diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md new file mode 100644 index 00000000000000..a29cddd81d885a --- /dev/null +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md @@ -0,0 +1,11 @@ + + +[Home](./index.md) > [kibana-plugin-plugins-data-public](./kibana-plugin-plugins-data-public.md) > [QuerySuggestionGetFnArgs](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.md) > [useTimeRange](./kibana-plugin-plugins-data-public.querysuggestiongetfnargs.usetimerange.md) + +## QuerySuggestionGetFnArgs.useTimeRange property + +Signature: + +```typescript +useTimeRange?: boolean; +``` diff --git a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ui_settings.md b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ui_settings.md index 6ed20beb396f15..9a0c37c8edd383 100644 --- a/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ui_settings.md +++ b/docs/development/plugins/data/public/kibana-plugin-plugins-data-public.ui_settings.md @@ -37,5 +37,6 @@ UI_SETTINGS: { readonly INDEXPATTERN_PLACEHOLDER: "indexPattern:placeholder"; readonly FILTERS_PINNED_BY_DEFAULT: "filters:pinnedByDefault"; readonly FILTERS_EDITOR_SUGGEST_VALUES: "filterEditor:suggestValues"; + readonly AUTOCOMPLETE_USE_TIMERANGE: "autocomplete:useTimeRange"; } ``` diff --git a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.ui_settings.md b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.ui_settings.md index 2d4ce75b956df3..c2edc64f292d29 100644 --- a/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.ui_settings.md +++ b/docs/development/plugins/data/server/kibana-plugin-plugins-data-server.ui_settings.md @@ -37,5 +37,6 @@ UI_SETTINGS: { readonly INDEXPATTERN_PLACEHOLDER: "indexPattern:placeholder"; readonly FILTERS_PINNED_BY_DEFAULT: "filters:pinnedByDefault"; readonly FILTERS_EDITOR_SUGGEST_VALUES: "filterEditor:suggestValues"; + readonly AUTOCOMPLETE_USE_TIMERANGE: "autocomplete:useTimeRange"; } ``` diff --git a/src/plugins/data/common/constants.ts b/src/plugins/data/common/constants.ts index 43120583bd3a41..a70c5eedbf8478 100644 --- a/src/plugins/data/common/constants.ts +++ b/src/plugins/data/common/constants.ts @@ -49,4 +49,5 @@ export const UI_SETTINGS = { INDEXPATTERN_PLACEHOLDER: 'indexPattern:placeholder', FILTERS_PINNED_BY_DEFAULT: 'filters:pinnedByDefault', FILTERS_EDITOR_SUGGEST_VALUES: 'filterEditor:suggestValues', + AUTOCOMPLETE_USE_TIMERANGE: 'autocomplete:useTimeRange', } as const; diff --git a/src/plugins/data/public/autocomplete/providers/query_suggestion_provider.ts b/src/plugins/data/public/autocomplete/providers/query_suggestion_provider.ts index 16500ac9e239a0..da523e740c3d6d 100644 --- a/src/plugins/data/public/autocomplete/providers/query_suggestion_provider.ts +++ b/src/plugins/data/public/autocomplete/providers/query_suggestion_provider.ts @@ -39,6 +39,7 @@ export interface QuerySuggestionGetFnArgs { selectionStart: number; selectionEnd: number; signal?: AbortSignal; + useTimeRange?: boolean; boolFilter?: any; } diff --git a/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.test.ts b/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.test.ts index 6b0c0f07cf6c95..0ef5b7db958e45 100644 --- a/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.test.ts +++ b/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.test.ts @@ -21,6 +21,26 @@ import { stubIndexPattern, stubFields } from '../../stubs'; import { setupValueSuggestionProvider, ValueSuggestionsGetFn } from './value_suggestion_provider'; import { IUiSettingsClient, CoreSetup } from 'kibana/public'; +jest.mock('../../services', () => ({ + getQueryService: () => ({ + timefilter: { + timefilter: { + createFilter: () => { + return { + time: 'fake', + }; + }, + getTime: () => { + return { + to: 'now', + from: 'now-15m', + }; + }, + }, + }, + }), +})); + describe('FieldSuggestions', () => { let getValueSuggestions: ValueSuggestionsGetFn; let http: any; @@ -94,6 +114,7 @@ describe('FieldSuggestions', () => { indexPattern: stubIndexPattern, field, query: '', + useTimeRange: false, }); expect(http.fetch).toHaveBeenCalled(); @@ -107,6 +128,7 @@ describe('FieldSuggestions', () => { indexPattern: stubIndexPattern, field, query: '', + useTimeRange: false, }; await getValueSuggestions(args); @@ -123,6 +145,7 @@ describe('FieldSuggestions', () => { indexPattern: stubIndexPattern, field, query: '', + useTimeRange: false, }; const { now } = Date; @@ -146,50 +169,76 @@ describe('FieldSuggestions', () => { indexPattern: stubIndexPattern, field: fields[0], query: '', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: stubIndexPattern, field: fields[0], query: 'query', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: stubIndexPattern, field: fields[1], query: '', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: stubIndexPattern, field: fields[1], query: 'query', + useTimeRange: false, }); const customIndexPattern = { ...stubIndexPattern, title: 'customIndexPattern', + useTimeRange: false, }; await getValueSuggestions({ indexPattern: customIndexPattern, field: fields[0], query: '', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: customIndexPattern, field: fields[0], query: 'query', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: customIndexPattern, field: fields[1], query: '', + useTimeRange: false, }); await getValueSuggestions({ indexPattern: customIndexPattern, field: fields[1], query: 'query', + useTimeRange: false, }); expect(http.fetch).toHaveBeenCalledTimes(8); }); + + it('should apply timefilter', async () => { + const [field] = stubFields.filter( + ({ type, aggregatable }) => type === 'string' && aggregatable + ); + + await getValueSuggestions({ + indexPattern: stubIndexPattern, + field, + query: '', + useTimeRange: true, + }); + const callParams = http.fetch.mock.calls[0][1]; + + expect(JSON.parse(callParams.body).filters).toHaveLength(1); + expect(http.fetch).toHaveBeenCalled(); + }); }); }); diff --git a/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.ts b/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.ts index a6a45a26f06b39..fe9f939a0261d2 100644 --- a/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.ts +++ b/src/plugins/data/public/autocomplete/providers/value_suggestion_provider.ts @@ -17,15 +17,16 @@ * under the License. */ +import dateMath from '@elastic/datemath'; import { memoize } from 'lodash'; import { CoreSetup } from 'src/core/public'; -import { IIndexPattern, IFieldType, UI_SETTINGS } from '../../../common'; +import { IIndexPattern, IFieldType, UI_SETTINGS, buildQueryFromFilters } from '../../../common'; +import { getQueryService } from '../../services'; -function resolver(title: string, field: IFieldType, query: string, boolFilter: any) { +function resolver(title: string, field: IFieldType, query: string, filters: any[]) { // Only cache results for a minute const ttl = Math.floor(Date.now() / 1000 / 60); - - return [ttl, query, title, field.name, JSON.stringify(boolFilter)].join('|'); + return [ttl, query, title, field.name, JSON.stringify(filters)].join('|'); } export type ValueSuggestionsGetFn = (args: ValueSuggestionsGetFnArgs) => Promise; @@ -34,18 +35,31 @@ interface ValueSuggestionsGetFnArgs { indexPattern: IIndexPattern; field: IFieldType; query: string; + useTimeRange?: boolean; boolFilter?: any[]; signal?: AbortSignal; } +const getAutocompleteTimefilter = (indexPattern: IIndexPattern) => { + const { timefilter } = getQueryService().timefilter; + const timeRange = timefilter.getTime(); + + // Use a rounded timerange so that memoizing works properly + const roundedTimerange = { + from: dateMath.parse(timeRange.from)!.startOf('minute').toISOString(), + to: dateMath.parse(timeRange.to)!.endOf('minute').toISOString(), + }; + return timefilter.createFilter(indexPattern, roundedTimerange); +}; + export const getEmptyValueSuggestions = (() => Promise.resolve([])) as ValueSuggestionsGetFn; export const setupValueSuggestionProvider = (core: CoreSetup): ValueSuggestionsGetFn => { const requestSuggestions = memoize( - (index: string, field: IFieldType, query: string, boolFilter: any = [], signal?: AbortSignal) => + (index: string, field: IFieldType, query: string, filters: any = [], signal?: AbortSignal) => core.http.fetch(`/api/kibana/suggestions/values/${index}`, { method: 'POST', - body: JSON.stringify({ query, field: field.name, boolFilter }), + body: JSON.stringify({ query, field: field.name, filters }), signal, }), resolver @@ -55,12 +69,15 @@ export const setupValueSuggestionProvider = (core: CoreSetup): ValueSuggestionsG indexPattern, field, query, + useTimeRange, boolFilter, signal, }: ValueSuggestionsGetFnArgs): Promise => { const shouldSuggestValues = core!.uiSettings.get( UI_SETTINGS.FILTERS_EDITOR_SUGGEST_VALUES ); + useTimeRange = + useTimeRange ?? core!.uiSettings.get(UI_SETTINGS.AUTOCOMPLETE_USE_TIMERANGE); const { title } = indexPattern; if (field.type === 'boolean') { @@ -69,6 +86,9 @@ export const setupValueSuggestionProvider = (core: CoreSetup): ValueSuggestionsG return []; } - return await requestSuggestions(title, field, query, boolFilter, signal); + const timeFilter = useTimeRange ? getAutocompleteTimefilter(indexPattern) : undefined; + const filterQuery = timeFilter ? buildQueryFromFilters([timeFilter], indexPattern).filter : []; + const filters = [...(boolFilter ? boolFilter : []), ...filterQuery]; + return await requestSuggestions(title, field, query, filters, signal); }; }; diff --git a/src/plugins/data/public/public.api.md b/src/plugins/data/public/public.api.md index 31b05bd4763a24..643bc46465360d 100644 --- a/src/plugins/data/public/public.api.md +++ b/src/plugins/data/public/public.api.md @@ -1840,6 +1840,8 @@ export interface QuerySuggestionGetFnArgs { selectionStart: number; // (undocumented) signal?: AbortSignal; + // (undocumented) + useTimeRange?: boolean; } // Warning: (ae-missing-release-tag) "QuerySuggestionTypes" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -2309,6 +2311,7 @@ export const UI_SETTINGS: { readonly INDEXPATTERN_PLACEHOLDER: "indexPattern:placeholder"; readonly FILTERS_PINNED_BY_DEFAULT: "filters:pinnedByDefault"; readonly FILTERS_EDITOR_SUGGEST_VALUES: "filterEditor:suggestValues"; + readonly AUTOCOMPLETE_USE_TIMERANGE: "autocomplete:useTimeRange"; }; diff --git a/src/plugins/data/public/query/timefilter/timefilter.ts b/src/plugins/data/public/query/timefilter/timefilter.ts index 49a8c68f6916f6..7278ceaaddcced 100644 --- a/src/plugins/data/public/query/timefilter/timefilter.ts +++ b/src/plugins/data/public/query/timefilter/timefilter.ts @@ -24,9 +24,14 @@ import { PublicMethodsOf } from '@kbn/utility-types'; import { areRefreshIntervalsDifferent, areTimeRangesDifferent } from './lib/diff_time_picker_vals'; import { getForceNow } from './lib/get_force_now'; import { TimefilterConfig, InputTimeRange, TimeRangeBounds } from './types'; -import { calculateBounds, getTime, RefreshInterval, TimeRange } from '../../../common'; +import { + calculateBounds, + getTime, + IIndexPattern, + RefreshInterval, + TimeRange, +} from '../../../common'; import { TimeHistoryContract } from './time_history'; -import { IndexPattern } from '../../index_patterns'; // TODO: remove! @@ -170,7 +175,7 @@ export class Timefilter { } }; - public createFilter = (indexPattern: IndexPattern, timeRange?: TimeRange) => { + public createFilter = (indexPattern: IIndexPattern, timeRange?: TimeRange) => { return getTime(indexPattern, timeRange ? timeRange : this._time, { forceNow: this.getForceNow(), }); diff --git a/src/plugins/data/public/ui/filter_bar/filter_editor/phrase_suggestor.tsx b/src/plugins/data/public/ui/filter_bar/filter_editor/phrase_suggestor.tsx index 719827a98cc634..c420734a43d41c 100644 --- a/src/plugins/data/public/ui/filter_bar/filter_editor/phrase_suggestor.tsx +++ b/src/plugins/data/public/ui/filter_bar/filter_editor/phrase_suggestor.tsx @@ -85,6 +85,8 @@ export class PhraseSuggestorUI extends React.Com field, query, signal: this.abortController.signal, + // Show all results in filter bar autocomplete + useTimeRange: false, }); this.setState({ suggestions, isLoading: false }); diff --git a/src/plugins/data/server/autocomplete/value_suggestions_route.ts b/src/plugins/data/server/autocomplete/value_suggestions_route.ts index 6a5b7d1d5b4141..89ee0995f41408 100644 --- a/src/plugins/data/server/autocomplete/value_suggestions_route.ts +++ b/src/plugins/data/server/autocomplete/value_suggestions_route.ts @@ -45,7 +45,7 @@ export function registerValueSuggestionsRoute( { field: schema.string(), query: schema.string(), - boolFilter: schema.maybe(schema.any()), + filters: schema.maybe(schema.any()), }, { unknowns: 'allow' } ), @@ -53,7 +53,7 @@ export function registerValueSuggestionsRoute( }, async (context, request, response) => { const config = await config$.pipe(first()).toPromise(); - const { field: fieldName, query, boolFilter } = request.body; + const { field: fieldName, query, filters } = request.body; const { index } = request.params; const { client } = context.core.elasticsearch.legacy; const signal = getRequestAbortedSignal(request.events.aborted$); @@ -66,7 +66,7 @@ export function registerValueSuggestionsRoute( const indexPattern = await findIndexPatternById(context.core.savedObjects.client, index); const field = indexPattern && getFieldByName(fieldName, indexPattern); - const body = await getBody(autocompleteSearchOptions, field || fieldName, query, boolFilter); + const body = await getBody(autocompleteSearchOptions, field || fieldName, query, filters); try { const result = await client.callAsCurrentUser('search', { index, body }, { signal }); @@ -88,7 +88,7 @@ async function getBody( { timeout, terminate_after }: Record, field: IFieldType | string, query: string, - boolFilter: Filter[] = [] + filters: Filter[] = [] ) { const isFieldObject = (f: any): f is IFieldType => Boolean(f && f.name); @@ -108,7 +108,7 @@ async function getBody( terminate_after, query: { bool: { - filter: boolFilter, + filter: filters, }, }, aggs: { diff --git a/src/plugins/data/server/server.api.md b/src/plugins/data/server/server.api.md index 131b3e4c39c6bb..2984ca336819ae 100644 --- a/src/plugins/data/server/server.api.md +++ b/src/plugins/data/server/server.api.md @@ -1181,6 +1181,7 @@ export const UI_SETTINGS: { readonly INDEXPATTERN_PLACEHOLDER: "indexPattern:placeholder"; readonly FILTERS_PINNED_BY_DEFAULT: "filters:pinnedByDefault"; readonly FILTERS_EDITOR_SUGGEST_VALUES: "filterEditor:suggestValues"; + readonly AUTOCOMPLETE_USE_TIMERANGE: "autocomplete:useTimeRange"; }; // Warning: (ae-missing-release-tag) "usageProvider" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal) diff --git a/src/plugins/data/server/ui_settings.ts b/src/plugins/data/server/ui_settings.ts index 763a086d7688d2..9393700a0e771f 100644 --- a/src/plugins/data/server/ui_settings.ts +++ b/src/plugins/data/server/ui_settings.ts @@ -684,5 +684,17 @@ export function getUiSettings(): Record> { }), schema: schema.boolean(), }, + [UI_SETTINGS.AUTOCOMPLETE_USE_TIMERANGE]: { + name: i18n.translate('data.advancedSettings.autocompleteIgnoreTimerange', { + defaultMessage: 'Use time range', + description: 'Restrict autocomplete results to the current time range', + }), + value: true, + description: i18n.translate('data.advancedSettings.autocompleteIgnoreTimerangeText', { + defaultMessage: + 'Disable this property to get autocomplete suggestions from your full dataset, rather than from the current time range.', + }), + schema: schema.boolean(), + }, }; } diff --git a/x-pack/plugins/apm/public/components/shared/KueryBar/get_bool_filter.ts b/x-pack/plugins/apm/public/components/shared/KueryBar/get_bool_filter.ts index 74d7ace20dae09..f0d12fd16bf7a0 100644 --- a/x-pack/plugins/apm/public/components/shared/KueryBar/get_bool_filter.ts +++ b/x-pack/plugins/apm/public/components/shared/KueryBar/get_bool_filter.ts @@ -26,23 +26,7 @@ export function getBoolFilter({ serviceName?: string; urlParams: IUrlParams; }) { - const { start, end } = urlParams; - - if (!start || !end) { - throw new Error('Date range was not defined'); - } - - const boolFilter: ESFilter[] = [ - { - range: { - '@timestamp': { - gte: new Date(start).getTime(), - lte: new Date(end).getTime(), - format: 'epoch_millis', - }, - }, - }, - ]; + const boolFilter: ESFilter[] = []; if (serviceName) { boolFilter.push({ diff --git a/x-pack/plugins/apm/public/components/shared/KueryBar/index.tsx b/x-pack/plugins/apm/public/components/shared/KueryBar/index.tsx index 157e014bee4247..dce8e49deec414 100644 --- a/x-pack/plugins/apm/public/components/shared/KueryBar/index.tsx +++ b/x-pack/plugins/apm/public/components/shared/KueryBar/index.tsx @@ -111,6 +111,7 @@ export function KueryBar() { query: inputValue, selectionStart, selectionEnd: selectionStart, + useTimeRange: true, })) || [] ) .filter((suggestion) => !startsWith(suggestion.text, 'span.')) diff --git a/x-pack/plugins/data_enhanced/public/autocomplete/providers/kql_query_suggestion/value.ts b/x-pack/plugins/data_enhanced/public/autocomplete/providers/kql_query_suggestion/value.ts index 441e5a6f775dd5..cab3a657b7b6b0 100644 --- a/x-pack/plugins/data_enhanced/public/autocomplete/providers/kql_query_suggestion/value.ts +++ b/x-pack/plugins/data_enhanced/public/autocomplete/providers/kql_query_suggestion/value.ts @@ -27,7 +27,7 @@ const wrapAsSuggestions = (start: number, end: number, query: string, values: st export const setupGetValueSuggestions: KqlQuerySuggestionProvider = () => { return async ( - { indexPatterns, boolFilter, signal }, + { indexPatterns, boolFilter, useTimeRange, signal }, { start, end, prefix, suffix, fieldName, nestedPath } ): Promise => { const fullFieldName = nestedPath ? `${nestedPath}.${fieldName}` : fieldName; @@ -49,6 +49,7 @@ export const setupGetValueSuggestions: KqlQuerySuggestionProvider = () => { field, query, boolFilter, + useTimeRange, signal, }).then((valueSuggestions) => { const quotedValues = valueSuggestions.map((value) => diff --git a/x-pack/plugins/uptime/public/components/overview/kuery_bar/kuery_bar.tsx b/x-pack/plugins/uptime/public/components/overview/kuery_bar/kuery_bar.tsx index 61402008363da6..f42f0e7385eb6d 100644 --- a/x-pack/plugins/uptime/public/components/overview/kuery_bar/kuery_bar.tsx +++ b/x-pack/plugins/uptime/public/components/overview/kuery_bar/kuery_bar.tsx @@ -67,7 +67,7 @@ export function KueryBar({ let currentRequestCheck: string; const [getUrlParams, updateUrlParams] = useUrlParams(); - const { search: kuery, dateRangeStart, dateRangeEnd } = getUrlParams(); + const { search: kuery } = getUrlParams(); useEffect(() => { updateSearchText(kuery); @@ -105,16 +105,7 @@ export function KueryBar({ query: inputValue, selectionStart: selectionStart || 0, selectionEnd: selectionStart || 0, - boolFilter: [ - { - range: { - '@timestamp': { - gte: dateRangeStart, - lte: dateRangeEnd, - }, - }, - }, - ], + useTimeRange: true, })) || [] ).filter((suggestion: QuerySuggestion) => !suggestion.text.startsWith('span.')); if (currentRequest !== currentRequestCheck) { diff --git a/x-pack/test/functional/apps/discover/value_suggestions.ts b/x-pack/test/functional/apps/discover/value_suggestions.ts index 54720b94172f61..5d759276552285 100644 --- a/x-pack/test/functional/apps/discover/value_suggestions.ts +++ b/x-pack/test/functional/apps/discover/value_suggestions.ts @@ -10,13 +10,14 @@ import { FtrProviderContext } from '../../ftr_provider_context'; export default function ({ getService, getPageObjects }: FtrProviderContext) { const esArchiver = getService('esArchiver'); const queryBar = getService('queryBar'); - const PageObjects = getPageObjects(['common']); + const PageObjects = getPageObjects(['common', 'timePicker']); describe('value suggestions', function describeIndexTests() { before(async function () { await esArchiver.loadIfNeeded('logstash_functional'); await esArchiver.load('dashboard/drilldowns'); await PageObjects.common.navigateToApp('discover'); + await PageObjects.timePicker.setDefaultAbsoluteRange(); }); after(async () => { diff --git a/x-pack/test/functional/apps/maps/vector_styling.js b/x-pack/test/functional/apps/maps/vector_styling.js index e4c5eaf892c769..56a8188c8341b7 100644 --- a/x-pack/test/functional/apps/maps/vector_styling.js +++ b/x-pack/test/functional/apps/maps/vector_styling.js @@ -7,13 +7,17 @@ import expect from '@kbn/expect'; export default function ({ getService, getPageObjects }) { - const PageObjects = getPageObjects(['maps']); + const PageObjects = getPageObjects(['maps', 'timePicker']); const security = getService('security'); describe('vector styling', () => { before(async () => { await security.testUser.setRoles(['test_logstash_reader', 'global_maps_all']); await PageObjects.maps.loadSavedMap('document example'); + await PageObjects.timePicker.setAbsoluteRange( + 'Mar 1, 2015 @ 00:00:00.000', + 'Mar 1, 2016 @ 00:00:00.000' + ); }); after(async () => { await PageObjects.maps.refreshAndClearUnsavedChangesWarning(); From c7f085ff0bf22f0b53f5b1268c6b846a05f32954 Mon Sep 17 00:00:00 2001 From: Constance Date: Tue, 10 Nov 2020 10:52:20 -0800 Subject: [PATCH 06/57] [Enterprise Search] Tech debt/cleanup: remove I/E/T Typescript prefixes (#83027) * [All] Remove prefixes on simple self-contained type defs - Types should not be exported - Types should not be used outside each affected file * [All][kea] Remove ts prefixes and unnecessary exports Kea now takes care of type checking for us, so there should virtually never be a need to export our values and actions interfaces going forward * [shared] Remove createHref type prefixes * [shared] Remove breadcrumb prefixes * [shared] Remove telemetry prefixes * [shared] remove types.ts Opionionated change: it was only being used for IFlashMessage, and at this point I think it's more useful to go in one level deeper to grab the type you need * [server] remove route dependencies prefixes * [server] Various type cleanups - plugin.ts - remove unnecessary export - MockRouter - remove prefix for request type, change IMockRouter to match Kibana's IRouter - check_access - remove prefixes - callEnterpriseSearchConfigAPI - remove prefixes - EnterpriseSearchRequestHandler - remove prefixes * [common] Remove InitialAppData prefix + remove unnecessary export from public/plugin.ts * [common] Remove Meta prefixes * [common] Remove configured limits prefixes * [AS] Remove Account and Role prefixes * [AS] Remove Engine prefixes * [AS] Remove credentials prefixes * [AS] Remove log settings prefixes * [WS] Remove account/organization/initial data prefixes * [WS] Remove group(s), user, & content source prefixes + GroupLogic and GroupsLogic refactor - remove unnecessary defs in actions, it's already defined in the Actions interface above and in some cases (e.g. old History param) is causing out of date issues * [WS] Misc type fixes - TSpacerSize -> SpaceSizeTypes - SourcePriority - remove prefixes - IComponentLoader - this isn't used anywhere else and appears to be component props so it probably should live only within component_loader.tsx - Remove recent feed activity prefix * [WS][Opinionated] Move interfaces not used in server/ out of common/ and to public/ --- .../common/types/app_search.ts | 4 +- .../enterprise_search/common/types/index.ts | 28 +++---- .../common/types/workplace_search.ts | 58 ++------------ .../__mocks__/mount_async.mock.tsx | 4 +- .../applications/app_search/app_logic.ts | 18 ++--- .../form_components/key_read_write_access.tsx | 6 +- .../credentials_flyout/header.test.tsx | 4 +- .../credentials_list.test.tsx | 6 +- .../credentials_list/credentials_list.tsx | 20 ++--- .../credentials/credentials_list/key.tsx | 4 +- .../credentials/credentials_logic.ts | 40 +++++----- .../components/credentials/types.ts | 10 +-- .../credentials/utils/api_token_sort.test.ts | 4 +- .../credentials/utils/api_token_sort.ts | 4 +- .../utils/get_engines_display_text.test.tsx | 4 +- .../utils/get_engines_display_text.tsx | 4 +- .../utils/get_mode_display_text.test.ts | 4 +- .../utils/get_mode_display_text.ts | 4 +- .../components/engines/engines_overview.tsx | 8 +- .../components/engines/engines_table.tsx | 18 ++--- .../log_retention/log_retention_logic.test.ts | 30 ++++---- .../log_retention/log_retention_logic.ts | 26 +++---- .../settings/log_retention/types.ts | 26 +++---- .../utils/convert_log_retention.ts | 30 ++++---- .../public/applications/app_search/index.tsx | 10 +-- .../public/applications/app_search/types.ts | 4 +- .../app_search/utils/role/index.ts | 22 +++--- .../components/product_card/product_card.tsx | 4 +- .../product_selector/product_selector.tsx | 4 +- .../applications/enterprise_search/index.tsx | 4 +- .../public/applications/index.tsx | 4 +- .../flash_messages/flash_messages_logic.ts | 6 +- .../flash_messages/handle_api_errors.ts | 9 +-- .../shared/flash_messages/index.ts | 8 +- .../shared/hidden_text/hidden_text.tsx | 8 +- .../applications/shared/http/http_logic.ts | 10 +-- .../public/applications/shared/http/index.ts | 2 +- .../shared/kibana/kibana_logic.ts | 14 ++-- .../kibana_chrome/generate_breadcrumbs.ts | 16 ++-- .../shared/kibana_chrome/generate_title.ts | 10 +-- .../shared/kibana_chrome/set_chrome.tsx | 12 +-- .../applications/shared/layout/layout.tsx | 4 +- .../applications/shared/layout/side_nav.tsx | 12 +-- .../shared/licensing/licensing_logic.ts | 10 +-- .../react_router_helpers/create_href.ts | 8 +- .../shared/react_router_helpers/eui_link.tsx | 16 ++-- .../shared/react_router_helpers/index.ts | 2 +- .../react_router_helpers/link_events.ts | 10 +-- .../shared/setup_guide/setup_guide.tsx | 4 +- .../shared/table_header/table_header.tsx | 4 +- .../shared/telemetry/send_telemetry.tsx | 11 ++- .../shared/telemetry/telemetry_logic.ts | 24 +++--- .../shared/truncate/truncated_content.tsx | 4 +- .../public/applications/shared/types.ts | 7 -- .../workplace_search/app_logic.ts | 20 ++--- .../components/shared/api_key/api_key.tsx | 4 +- .../component_loader/component_loader.tsx | 6 +- .../content_section/content_section.tsx | 8 +- .../credential_item/credential_item.tsx | 4 +- .../source_config_fields.tsx | 4 +- .../shared/source_icon/source_icon.tsx | 4 +- .../shared/source_row/source_row.tsx | 8 +- .../shared/sources_table/sources_table.tsx | 8 +- .../table_pagination_bar.tsx | 4 +- .../components/shared/user_icon/user_icon.tsx | 4 +- .../components/shared/user_row/user_row.tsx | 8 +- .../view_content_header.tsx | 4 +- .../applications/workplace_search/index.tsx | 6 +- .../applications/workplace_search/types.ts | 52 +++++++++++-- .../groups/__mocks__/group_logic.mock.ts | 12 ++- .../groups/__mocks__/groups_logic.mock.ts | 12 ++- .../components/filterable_users_list.test.tsx | 4 +- .../components/filterable_users_list.tsx | 10 +-- .../components/filterable_users_popover.tsx | 8 +- .../groups/components/group_manager_modal.tsx | 8 +- .../views/groups/components/group_row.tsx | 4 +- .../components/group_row_sources_dropdown.tsx | 8 +- .../components/group_row_users_dropdown.tsx | 4 +- .../group_source_prioritization.tsx | 4 +- .../groups/components/group_sources.test.tsx | 4 +- .../views/groups/components/group_sources.tsx | 8 +- .../groups/components/group_users.test.tsx | 4 +- .../views/groups/components/group_users.tsx | 8 +- .../components/group_users_table.test.tsx | 4 +- .../groups/components/group_users_table.tsx | 4 +- .../groups/components/source_option_item.tsx | 8 +- .../views/groups/components/sources_list.tsx | 8 +- .../groups/components/user_option_item.tsx | 8 +- .../views/groups/group_logic.ts | 75 +++++++++---------- .../views/groups/groups_logic.ts | 72 +++++++++--------- .../overview/__mocks__/overview_logic.mock.ts | 4 +- .../views/overview/onboarding_card.tsx | 4 +- .../views/overview/overview_logic.ts | 14 ++-- .../views/overview/recent_activity.tsx | 6 +- .../views/overview/statistic_card.tsx | 4 +- .../enterprise_search/public/plugin.ts | 7 +- .../server/__mocks__/router.mock.ts | 14 ++-- .../server/collectors/app_search/telemetry.ts | 8 +- .../collectors/enterprise_search/telemetry.ts | 8 +- .../server/collectors/lib/telemetry.ts | 4 +- .../collectors/workplace_search/telemetry.ts | 8 +- .../server/lib/check_access.ts | 6 +- .../lib/enterprise_search_config_api.ts | 12 +-- .../lib/enterprise_search_request_handler.ts | 12 +-- .../enterprise_search/server/plugin.ts | 4 +- .../server/routes/app_search/credentials.ts | 4 +- .../server/routes/app_search/engines.ts | 8 +- .../server/routes/app_search/index.ts | 4 +- .../server/routes/app_search/settings.ts | 4 +- .../routes/enterprise_search/config_data.ts | 4 +- .../routes/enterprise_search/telemetry.ts | 8 +- .../server/routes/workplace_search/groups.ts | 19 ++--- .../server/routes/workplace_search/index.ts | 4 +- .../routes/workplace_search/overview.ts | 4 +- 114 files changed, 597 insertions(+), 625 deletions(-) delete mode 100644 x-pack/plugins/enterprise_search/public/applications/shared/types.ts diff --git a/x-pack/plugins/enterprise_search/common/types/app_search.ts b/x-pack/plugins/enterprise_search/common/types/app_search.ts index 203b77834bc15d..9f754412ec730b 100644 --- a/x-pack/plugins/enterprise_search/common/types/app_search.ts +++ b/x-pack/plugins/enterprise_search/common/types/app_search.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export interface IAccount { +export interface Account { accountId: string; onboardingComplete: boolean; role: { @@ -21,7 +21,7 @@ export interface IAccount { }; } -export interface IConfiguredLimits { +export interface ConfiguredLimits { engine: { maxDocumentByteSize: number; maxEnginesPerMetaEngine: number; diff --git a/x-pack/plugins/enterprise_search/common/types/index.ts b/x-pack/plugins/enterprise_search/common/types/index.ts index 1006d391387593..39d9aa8607bc25 100644 --- a/x-pack/plugins/enterprise_search/common/types/index.ts +++ b/x-pack/plugins/enterprise_search/common/types/index.ts @@ -5,39 +5,39 @@ */ import { - IAccount as IAppSearchAccount, - IConfiguredLimits as IAppSearchConfiguredLimits, + Account as AppSearchAccount, + ConfiguredLimits as AppSearchConfiguredLimits, } from './app_search'; import { - IWorkplaceSearchInitialData, - IConfiguredLimits as IWorkplaceSearchConfiguredLimits, + WorkplaceSearchInitialData, + ConfiguredLimits as WorkplaceSearchConfiguredLimits, } from './workplace_search'; -export interface IInitialAppData { +export interface InitialAppData { readOnlyMode?: boolean; ilmEnabled?: boolean; isFederatedAuth?: boolean; - configuredLimits?: IConfiguredLimits; + configuredLimits?: ConfiguredLimits; access?: { hasAppSearchAccess: boolean; hasWorkplaceSearchAccess: boolean; }; - appSearch?: IAppSearchAccount; - workplaceSearch?: IWorkplaceSearchInitialData; + appSearch?: AppSearchAccount; + workplaceSearch?: WorkplaceSearchInitialData; } -export interface IConfiguredLimits { - appSearch: IAppSearchConfiguredLimits; - workplaceSearch: IWorkplaceSearchConfiguredLimits; +export interface ConfiguredLimits { + appSearch: AppSearchConfiguredLimits; + workplaceSearch: WorkplaceSearchConfiguredLimits; } -export interface IMetaPage { +export interface MetaPage { current: number; size: number; total_pages: number; total_results: number; } -export interface IMeta { - page: IMetaPage; +export interface Meta { + page: MetaPage; } diff --git a/x-pack/plugins/enterprise_search/common/types/workplace_search.ts b/x-pack/plugins/enterprise_search/common/types/workplace_search.ts index 886597fcd98911..883cc6939b4bcb 100644 --- a/x-pack/plugins/enterprise_search/common/types/workplace_search.ts +++ b/x-pack/plugins/enterprise_search/common/types/workplace_search.ts @@ -4,7 +4,7 @@ * you may not use this file except in compliance with the Elastic License. */ -export interface IAccount { +export interface Account { id: string; groups: string[]; isAdmin: boolean; @@ -14,65 +14,19 @@ export interface IAccount { viewedOnboardingPage: boolean; } -export interface IOrganization { +export interface Organization { name: string; defaultOrgName: string; } -export interface IWorkplaceSearchInitialData { - organization: IOrganization; - account: IAccount; +export interface WorkplaceSearchInitialData { + organization: Organization; + account: Account; } -export interface IConfiguredLimits { +export interface ConfiguredLimits { customApiSource: { maxDocumentByteSize: number; totalFields: number; }; } - -export interface IGroup { - id: string; - name: string; - createdAt: string; - updatedAt: string; - contentSources: IContentSource[]; - users: IUser[]; - usersCount: number; - color?: string; -} - -export interface IGroupDetails extends IGroup { - contentSources: IContentSourceDetails[]; - canEditGroup: boolean; - canDeleteGroup: boolean; -} - -export interface IUser { - id: string; - name: string | null; - initials: string; - pictureUrl: string | null; - color: string; - email: string; - role?: string; - groupIds: string[]; -} - -export interface IContentSource { - id: string; - serviceType: string; - name: string; -} - -export interface IContentSourceDetails extends IContentSource { - status: string; - statusMessage: string; - documentCount: string; - isFederatedSource: boolean; - searchable: boolean; - supportedByLicense: boolean; - errorReason: number; - allowsReauth: boolean; - boost: number; -} diff --git a/x-pack/plugins/enterprise_search/public/applications/__mocks__/mount_async.mock.tsx b/x-pack/plugins/enterprise_search/public/applications/__mocks__/mount_async.mock.tsx index a33e116c7ca726..a3d817e1da9048 100644 --- a/x-pack/plugins/enterprise_search/public/applications/__mocks__/mount_async.mock.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/__mocks__/mount_async.mock.tsx @@ -20,13 +20,13 @@ import { mountWithIntl } from './'; * const wrapper = mountAsync(); */ -interface IOptions { +interface Options { i18n?: boolean; } export const mountAsync = async ( children: React.ReactElement, - options: IOptions + options: Options ): Promise => { let wrapper: ReactWrapper | undefined; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/app_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/app_logic.ts index 932e84af45c2bf..2b475073c6ea53 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/app_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/app_logic.ts @@ -6,24 +6,24 @@ import { kea, MakeLogicType } from 'kea'; -import { IInitialAppData } from '../../../common/types'; -import { IConfiguredLimits, IAccount, IRole } from './types'; +import { InitialAppData } from '../../../common/types'; +import { ConfiguredLimits, Account, Role } from './types'; import { getRoleAbilities } from './utils/role'; -export interface IAppValues { +interface AppValues { hasInitialized: boolean; ilmEnabled: boolean; - configuredLimits: Partial; - account: Partial; - myRole: Partial; + configuredLimits: Partial; + account: Partial; + myRole: Partial; } -export interface IAppActions { - initializeAppData(props: IInitialAppData): Required; +interface AppActions { + initializeAppData(props: InitialAppData): Required; setOnboardingComplete(): boolean; } -export const AppLogic = kea>({ +export const AppLogic = kea>({ path: ['enterprise_search', 'app_search', 'app_logic'], actions: { initializeAppData: (props) => props, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/form_components/key_read_write_access.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/form_components/key_read_write_access.tsx index a02b00b6ad3770..d96c57b3c8bc3c 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/form_components/key_read_write_access.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/form_components/key_read_write_access.tsx @@ -10,7 +10,7 @@ import { EuiCheckbox, EuiText, EuiTitle, EuiSpacer, EuiPanel } from '@elastic/eu import { i18n } from '@kbn/i18n'; import { CredentialsLogic } from '../../credentials_logic'; -import { ITokenReadWrite } from '../../types'; +import { TokenReadWrite } from '../../types'; export const FormKeyReadWriteAccess: React.FC = () => { const { setTokenReadWrite } = useActions(CredentialsLogic); @@ -37,7 +37,7 @@ export const FormKeyReadWriteAccess: React.FC = () => { name="read" id="read" checked={activeApiToken.read} - onChange={(e) => setTokenReadWrite(e.target as ITokenReadWrite)} + onChange={(e) => setTokenReadWrite(e.target as TokenReadWrite)} label={i18n.translate( 'xpack.enterpriseSearch.appSearch.credentials.formReadWrite.readLabel', { defaultMessage: 'Read Access' } @@ -47,7 +47,7 @@ export const FormKeyReadWriteAccess: React.FC = () => { name="write" id="write" checked={activeApiToken.write} - onChange={(e) => setTokenReadWrite(e.target as ITokenReadWrite)} + onChange={(e) => setTokenReadWrite(e.target as TokenReadWrite)} label={i18n.translate( 'xpack.enterpriseSearch.appSearch.credentials.formReadWrite.writeLabel', { defaultMessage: 'Write Access' } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/header.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/header.test.tsx index a8d9505136faab..803789ebee568d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/header.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_flyout/header.test.tsx @@ -11,12 +11,12 @@ import { shallow } from 'enzyme'; import { EuiFlyoutHeader } from '@elastic/eui'; import { ApiTokenTypes } from '../constants'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; import { CredentialsFlyoutHeader } from './header'; describe('CredentialsFlyoutHeader', () => { - const apiToken: IApiToken = { + const apiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.test.tsx index 97d29b9333f4b5..4f5ded0a3ccc17 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.test.tsx @@ -10,7 +10,7 @@ import React from 'react'; import { shallow } from 'enzyme'; import { EuiBasicTable, EuiCopy, EuiEmptyPrompt } from '@elastic/eui'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; import { ApiTokenTypes } from '../constants'; import { HiddenText } from '../../../../shared/hidden_text'; @@ -18,7 +18,7 @@ import { Key } from './key'; import { CredentialsList } from './credentials_list'; describe('Credentials', () => { - const apiToken: IApiToken = { + const apiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, @@ -77,7 +77,7 @@ describe('Credentials', () => { }); const wrapper = shallow(); const { items } = wrapper.find(EuiBasicTable).props(); - expect(items.map((i: IApiToken) => i.id)).toEqual([undefined, 1, 2]); + expect(items.map((i: ApiToken) => i.id)).toEqual([undefined, 1, 2]); }); }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.tsx index f9752dca582e1c..9240bade4975e1 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/credentials_list.tsx @@ -14,7 +14,7 @@ import { i18n } from '@kbn/i18n'; import { CredentialsLogic } from '../credentials_logic'; import { Key } from './key'; import { HiddenText } from '../../../../shared/hidden_text'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; import { TOKEN_TYPE_DISPLAY_NAMES } from '../constants'; import { apiTokenSort } from '../utils/api_token_sort'; import { getModeDisplayText, getEnginesDisplayText } from '../utils'; @@ -26,21 +26,21 @@ export const CredentialsList: React.FC = () => { const items = useMemo(() => apiTokens.slice().sort(apiTokenSort), [apiTokens]); - const columns: Array> = [ + const columns: Array> = [ { name: 'Name', width: '12%', - render: (token: IApiToken) => token.name, + render: (token: ApiToken) => token.name, }, { name: 'Type', width: '15%', - render: (token: IApiToken) => TOKEN_TYPE_DISPLAY_NAMES[token.type], + render: (token: ApiToken) => TOKEN_TYPE_DISPLAY_NAMES[token.type], }, { name: 'Key', width: '36%', - render: (token: IApiToken) => { + render: (token: ApiToken) => { const { key } = token; if (!key) return null; return ( @@ -64,12 +64,12 @@ export const CredentialsList: React.FC = () => { { name: 'Modes', width: '10%', - render: (token: IApiToken) => getModeDisplayText(token), + render: (token: ApiToken) => getModeDisplayText(token), }, { name: 'Engines', width: '18%', - render: (token: IApiToken) => getEnginesDisplayText(token), + render: (token: ApiToken) => getEnginesDisplayText(token), }, { actions: [ @@ -83,7 +83,7 @@ export const CredentialsList: React.FC = () => { type: 'icon', icon: 'pencil', color: 'primary', - onClick: (token: IApiToken) => showCredentialsForm(token), + onClick: (token: ApiToken) => showCredentialsForm(token), }, { name: i18n.translate('xpack.enterpriseSearch.actions.delete', { @@ -95,7 +95,7 @@ export const CredentialsList: React.FC = () => { type: 'icon', icon: 'trash', color: 'danger', - onClick: (token: IApiToken) => deleteApiKey(token.name), + onClick: (token: ApiToken) => deleteApiKey(token.name), }, ], }, @@ -108,7 +108,7 @@ export const CredentialsList: React.FC = () => { hidePerPageOptions: true, }; - const onTableChange = ({ page }: CriteriaWithPagination) => { + const onTableChange = ({ page }: CriteriaWithPagination) => { const { index: current } = page; fetchCredentials(current + 1); }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/key.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/key.tsx index 5c0c24ec733a47..fa2d124cbccdf6 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/key.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_list/key.tsx @@ -8,14 +8,14 @@ import React from 'react'; import { EuiButtonIcon } from '@elastic/eui'; import { i18n } from '@kbn/i18n'; -interface IProps { +interface Props { copy: () => void; toggleIsHidden: () => void; isHidden: boolean; text: React.ReactNode; } -export const Key: React.FC = ({ copy, toggleIsHidden, isHidden, text }) => { +export const Key: React.FC = ({ copy, toggleIsHidden, isHidden, text }) => { const hideIcon = isHidden ? 'eye' : 'eyeClosed'; const hideIconLabel = isHidden ? i18n.translate('xpack.enterpriseSearch.appSearch.credentials.showApiKey', { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts index 7b8b864b3a317c..166cbae9a45129 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/credentials_logic.ts @@ -17,11 +17,11 @@ import { } from '../../../shared/flash_messages'; import { AppLogic } from '../../app_logic'; -import { IMeta } from '../../../../../common/types'; -import { IEngine } from '../../types'; -import { IApiToken, ICredentialsDetails, ITokenReadWrite } from './types'; +import { Meta } from '../../../../../common/types'; +import { Engine } from '../../types'; +import { ApiToken, CredentialsDetails, TokenReadWrite } from './types'; -export const defaultApiToken: IApiToken = { +export const defaultApiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, @@ -29,21 +29,21 @@ export const defaultApiToken: IApiToken = { access_all_engines: true, }; -interface ICredentialsLogicActions { +interface CredentialsLogicActions { addEngineName(engineName: string): string; onApiKeyDelete(tokenName: string): string; - onApiTokenCreateSuccess(apiToken: IApiToken): IApiToken; + onApiTokenCreateSuccess(apiToken: ApiToken): ApiToken; onApiTokenError(formErrors: string[]): string[]; - onApiTokenUpdateSuccess(apiToken: IApiToken): IApiToken; + onApiTokenUpdateSuccess(apiToken: ApiToken): ApiToken; removeEngineName(engineName: string): string; setAccessAllEngines(accessAll: boolean): boolean; - setCredentialsData(meta: IMeta, apiTokens: IApiToken[]): { meta: IMeta; apiTokens: IApiToken[] }; - setCredentialsDetails(details: ICredentialsDetails): ICredentialsDetails; + setCredentialsData(meta: Meta, apiTokens: ApiToken[]): { meta: Meta; apiTokens: ApiToken[] }; + setCredentialsDetails(details: CredentialsDetails): CredentialsDetails; setNameInputBlurred(isBlurred: boolean): boolean; - setTokenReadWrite(tokenReadWrite: ITokenReadWrite): ITokenReadWrite; + setTokenReadWrite(tokenReadWrite: TokenReadWrite): TokenReadWrite; setTokenName(name: string): string; setTokenType(tokenType: string): string; - showCredentialsForm(apiToken?: IApiToken): IApiToken; + showCredentialsForm(apiToken?: ApiToken): ApiToken; hideCredentialsForm(): { value: boolean }; resetCredentials(): { value: boolean }; initializeCredentialsData(): { value: boolean }; @@ -54,25 +54,25 @@ interface ICredentialsLogicActions { onEngineSelect(engineName: string): string; } -interface ICredentialsLogicValues { - activeApiToken: IApiToken; +interface CredentialsLogicValues { + activeApiToken: ApiToken; activeApiTokenExists: boolean; activeApiTokenRawName: string; - apiTokens: IApiToken[]; + apiTokens: ApiToken[]; dataLoading: boolean; - engines: IEngine[]; + engines: Engine[]; formErrors: string[]; isCredentialsDataComplete: boolean; isCredentialsDetailsComplete: boolean; fullEngineAccessChecked: boolean; - meta: Partial; + meta: Partial; nameInputBlurred: boolean; shouldShowCredentialsForm: boolean; } -export const CredentialsLogic = kea< - MakeLogicType ->({ +type CredentialsLogicType = MakeLogicType; // If we leave this inline, Prettier does some horrifying indenting nonsense :/ + +export const CredentialsLogic = kea({ path: ['enterprise_search', 'app_search', 'credentials_logic'], actions: () => ({ addEngineName: (engineName) => engineName, @@ -267,7 +267,7 @@ export const CredentialsLogic = kea< onApiTokenChange: async () => { const { id, name, engines, type, read, write } = values.activeApiToken; - const data: IApiToken = { + const data: ApiToken = { name, type, }; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/types.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/types.ts index 9ca4d086d55c81..23f78b44c0db57 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/types.ts @@ -4,14 +4,14 @@ * you may not use this file except in compliance with the Elastic License. */ -import { IEngine } from '../../types'; +import { Engine } from '../../types'; import { ApiTokenTypes } from './constants'; -export interface ICredentialsDetails { - engines: IEngine[]; +export interface CredentialsDetails { + engines: Engine[]; } -export interface IApiToken { +export interface ApiToken { access_all_engines?: boolean; key?: string; engines?: string[]; @@ -22,7 +22,7 @@ export interface IApiToken { write?: boolean; } -export interface ITokenReadWrite { +export interface TokenReadWrite { name: 'read' | 'write'; checked: boolean; } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.test.ts index 84818322b35709..2287125bb5eb89 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.test.ts @@ -7,10 +7,10 @@ import { apiTokenSort } from '.'; import { ApiTokenTypes } from '../constants'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; describe('apiTokenSort', () => { - const apiToken: IApiToken = { + const apiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.ts index 80a46f30e09308..b9fb501ccabe2d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/api_token_sort.ts @@ -4,9 +4,9 @@ * you may not use this file except in compliance with the Elastic License. */ -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; -export const apiTokenSort = (apiTokenA: IApiToken, apiTokenB: IApiToken): number => { +export const apiTokenSort = (apiTokenA: ApiToken, apiTokenB: ApiToken): number => { if (!apiTokenA.id) { return -1; } diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.test.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.test.tsx index b06ed63f8616c2..bee19a44facd39 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.test.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.test.tsx @@ -8,10 +8,10 @@ import React from 'react'; import { shallow } from 'enzyme'; import { getEnginesDisplayText } from './get_engines_display_text'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; import { ApiTokenTypes } from '../constants'; -const apiToken: IApiToken = { +const apiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.tsx index 1b216c46307db3..fb23551302f3bc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_engines_display_text.tsx @@ -6,9 +6,9 @@ import React from 'react'; import { ApiTokenTypes, ALL } from '../constants'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; -export const getEnginesDisplayText = (apiToken: IApiToken): JSX.Element | string => { +export const getEnginesDisplayText = (apiToken: ApiToken): JSX.Element | string => { const { type, access_all_engines: accessAll, engines = [] } = apiToken; if (type === ApiTokenTypes.Admin) { return '--'; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.test.ts index b2083f22c8e1c5..46b4c9b2c786cc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.test.ts @@ -5,11 +5,11 @@ */ import { ApiTokenTypes } from '../constants'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; import { getModeDisplayText } from './get_mode_display_text'; -const apiToken: IApiToken = { +const apiToken: ApiToken = { name: '', type: ApiTokenTypes.Private, read: true, diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.ts index 9c8758d83882d9..b150aa2cfc3bee 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/credentials/utils/get_mode_display_text.ts @@ -5,9 +5,9 @@ */ import { ApiTokenTypes, READ_ONLY, READ_WRITE, SEARCH_DISPLAY, WRITE_ONLY } from '../constants'; -import { IApiToken } from '../types'; +import { ApiToken } from '../types'; -export const getModeDisplayText = (apiToken: IApiToken): string => { +export const getModeDisplayText = (apiToken: ApiToken): string => { const { read = false, write = false, type } = apiToken; switch (type) { diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.tsx index 559fef695d63b5..0381c3806fec7e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_overview.tsx @@ -28,11 +28,11 @@ import { EnginesTable } from './engines_table'; import './engines_overview.scss'; -interface IGetEnginesParams { +interface GetEnginesParams { type: string; pageIndex: number; } -interface ISetEnginesCallbacks { +interface SetEnginesCallbacks { setResults: React.Dispatch>; setResultsTotal: React.Dispatch>; } @@ -49,12 +49,12 @@ export const EnginesOverview: React.FC = () => { const [metaEnginesPage, setMetaEnginesPage] = useState(1); const [metaEnginesTotal, setMetaEnginesTotal] = useState(0); - const getEnginesData = async ({ type, pageIndex }: IGetEnginesParams) => { + const getEnginesData = async ({ type, pageIndex }: GetEnginesParams) => { return await http.get('/api/app_search/engines', { query: { type, pageIndex }, }); }; - const setEnginesData = async (params: IGetEnginesParams, callbacks: ISetEnginesCallbacks) => { + const setEnginesData = async (params: GetEnginesParams, callbacks: SetEnginesCallbacks) => { const response = await getEnginesData(params); callbacks.setResults(response.results); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.tsx index a9cf64b3dffda3..9591bbda1f7c25 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/engines/engines_table.tsx @@ -16,28 +16,28 @@ import { getEngineRoute } from '../../routes'; import { ENGINES_PAGE_SIZE } from '../../../../../common/constants'; -interface IEnginesTableData { +interface EnginesTableData { name: string; created_at: string; document_count: number; field_count: number; } -interface IEnginesTablePagination { +interface EnginesTablePagination { totalEngines: number; pageIndex: number; onPaginate(pageIndex: number): void; } -interface IEnginesTableProps { - data: IEnginesTableData[]; - pagination: IEnginesTablePagination; +interface EnginesTableProps { + data: EnginesTableData[]; + pagination: EnginesTablePagination; } -interface IOnChange { +interface OnChange { page: { index: number; }; } -export const EnginesTable: React.FC = ({ +export const EnginesTable: React.FC = ({ data, pagination: { totalEngines, pageIndex, onPaginate }, }) => { @@ -52,7 +52,7 @@ export const EnginesTable: React.FC = ({ }), }); - const columns: Array> = [ + const columns: Array> = [ { field: 'name', name: i18n.translate('xpack.enterpriseSearch.appSearch.enginesOverview.table.column.name', { @@ -144,7 +144,7 @@ export const EnginesTable: React.FC = ({ totalItemCount: totalEngines, hidePerPageOptions: true, }} - onChange={({ page }: IOnChange) => { + onChange={({ page }: OnChange) => { const { index } = page; onPaginate(index + 1); // Note on paging - App Search's API pages start at 1, EuiBasicTables' pages start at 0 }} diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.test.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.test.ts index 367c7b085123fc..c86d7e3e915e2d 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.test.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.test.ts @@ -17,7 +17,7 @@ jest.mock('../../../../shared/flash_messages', () => ({ })); import { flashAPIErrors } from '../../../../shared/flash_messages'; -import { ELogRetentionOptions } from './types'; +import { LogRetentionOptions } from './types'; import { LogRetentionLogic } from './log_retention_logic'; describe('LogRetentionLogic', () => { @@ -87,11 +87,11 @@ describe('LogRetentionLogic', () => { it('should be set to the provided value', () => { mount(); - LogRetentionLogic.actions.setOpenedModal(ELogRetentionOptions.Analytics); + LogRetentionLogic.actions.setOpenedModal(LogRetentionOptions.Analytics); expect(LogRetentionLogic.values).toEqual({ ...DEFAULT_VALUES, - openedModal: ELogRetentionOptions.Analytics, + openedModal: LogRetentionOptions.Analytics, }); }); }); @@ -194,10 +194,10 @@ describe('LogRetentionLogic', () => { describe('openedModal', () => { it('should be reset to null', () => { mount({ - openedModal: ELogRetentionOptions.Analytics, + openedModal: LogRetentionOptions.Analytics, }); - LogRetentionLogic.actions.saveLogRetention(ELogRetentionOptions.Analytics, true); + LogRetentionLogic.actions.saveLogRetention(LogRetentionOptions.Analytics, true); expect(LogRetentionLogic.values).toEqual({ ...DEFAULT_VALUES, @@ -211,7 +211,7 @@ describe('LogRetentionLogic', () => { const promise = Promise.resolve(TYPICAL_SERVER_LOG_RETENTION); http.put.mockReturnValue(promise); - LogRetentionLogic.actions.saveLogRetention(ELogRetentionOptions.Analytics, true); + LogRetentionLogic.actions.saveLogRetention(LogRetentionOptions.Analytics, true); expect(http.put).toHaveBeenCalledWith('/api/app_search/log_settings', { body: JSON.stringify({ @@ -233,7 +233,7 @@ describe('LogRetentionLogic', () => { const promise = Promise.reject('An error occured'); http.put.mockReturnValue(promise); - LogRetentionLogic.actions.saveLogRetention(ELogRetentionOptions.Analytics, true); + LogRetentionLogic.actions.saveLogRetention(LogRetentionOptions.Analytics, true); try { await promise; @@ -252,7 +252,7 @@ describe('LogRetentionLogic', () => { isLogRetentionUpdating: false, }); - LogRetentionLogic.actions.toggleLogRetention(ELogRetentionOptions.Analytics); + LogRetentionLogic.actions.toggleLogRetention(LogRetentionOptions.Analytics); expect(LogRetentionLogic.values).toEqual({ ...DEFAULT_VALUES, @@ -264,17 +264,17 @@ describe('LogRetentionLogic', () => { it('will call setOpenedModal if already enabled', () => { mount({ logRetention: { - [ELogRetentionOptions.Analytics]: { + [LogRetentionOptions.Analytics]: { enabled: true, }, }, }); jest.spyOn(LogRetentionLogic.actions, 'setOpenedModal'); - LogRetentionLogic.actions.toggleLogRetention(ELogRetentionOptions.Analytics); + LogRetentionLogic.actions.toggleLogRetention(LogRetentionOptions.Analytics); expect(LogRetentionLogic.actions.setOpenedModal).toHaveBeenCalledWith( - ELogRetentionOptions.Analytics + LogRetentionOptions.Analytics ); }); }); @@ -337,17 +337,17 @@ describe('LogRetentionLogic', () => { it('will call saveLogRetention if NOT already enabled', () => { mount({ logRetention: { - [ELogRetentionOptions.Analytics]: { + [LogRetentionOptions.Analytics]: { enabled: false, }, }, }); jest.spyOn(LogRetentionLogic.actions, 'saveLogRetention'); - LogRetentionLogic.actions.toggleLogRetention(ELogRetentionOptions.Analytics); + LogRetentionLogic.actions.toggleLogRetention(LogRetentionOptions.Analytics); expect(LogRetentionLogic.actions.saveLogRetention).toHaveBeenCalledWith( - ELogRetentionOptions.Analytics, + LogRetentionOptions.Analytics, true ); }); @@ -359,7 +359,7 @@ describe('LogRetentionLogic', () => { jest.spyOn(LogRetentionLogic.actions, 'saveLogRetention'); jest.spyOn(LogRetentionLogic.actions, 'setOpenedModal'); - LogRetentionLogic.actions.toggleLogRetention(ELogRetentionOptions.API); + LogRetentionLogic.actions.toggleLogRetention(LogRetentionOptions.API); expect(LogRetentionLogic.actions.saveLogRetention).not.toHaveBeenCalled(); expect(LogRetentionLogic.actions.setOpenedModal).not.toHaveBeenCalled(); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.ts index 28830f2edb1d99..31fc41213492db 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/components/settings/log_retention/log_retention_logic.ts @@ -6,31 +6,31 @@ import { kea, MakeLogicType } from 'kea'; -import { ELogRetentionOptions, ILogRetention, ILogRetentionServer } from './types'; +import { LogRetentionOptions, LogRetention, LogRetentionServer } from './types'; import { HttpLogic } from '../../../../shared/http'; import { flashAPIErrors } from '../../../../shared/flash_messages'; import { convertLogRetentionFromServerToClient } from './utils/convert_log_retention'; -interface ILogRetentionActions { +interface LogRetentionActions { clearLogRetentionUpdating(): { value: boolean }; closeModals(): { value: boolean }; fetchLogRetention(): { value: boolean }; saveLogRetention( - option: ELogRetentionOptions, + option: LogRetentionOptions, enabled: boolean - ): { option: ELogRetentionOptions; enabled: boolean }; - setOpenedModal(option: ELogRetentionOptions): { option: ELogRetentionOptions }; - toggleLogRetention(option: ELogRetentionOptions): { option: ELogRetentionOptions }; - updateLogRetention(logRetention: ILogRetention): { logRetention: ILogRetention }; + ): { option: LogRetentionOptions; enabled: boolean }; + setOpenedModal(option: LogRetentionOptions): { option: LogRetentionOptions }; + toggleLogRetention(option: LogRetentionOptions): { option: LogRetentionOptions }; + updateLogRetention(logRetention: LogRetention): { logRetention: LogRetention }; } -interface ILogRetentionValues { - logRetention: ILogRetention | null; +interface LogRetentionValues { + logRetention: LogRetention | null; isLogRetentionUpdating: boolean; - openedModal: ELogRetentionOptions | null; + openedModal: LogRetentionOptions | null; } -export const LogRetentionLogic = kea>({ +export const LogRetentionLogic = kea>({ path: ['enterprise_search', 'app_search', 'log_retention_logic'], actions: () => ({ clearLogRetentionUpdating: true, @@ -72,7 +72,7 @@ export const LogRetentionLogic = kea ({ - [ELogRetentionOptions.Analytics]: convertLogRetentionSettingsFromServerToClient( - logRetention[ELogRetentionOptions.Analytics] + logRetention: LogRetentionServer +): LogRetention => ({ + [LogRetentionOptions.Analytics]: convertLogRetentionSettingsFromServerToClient( + logRetention[LogRetentionOptions.Analytics] ), - [ELogRetentionOptions.API]: convertLogRetentionSettingsFromServerToClient( - logRetention[ELogRetentionOptions.API] + [LogRetentionOptions.API]: convertLogRetentionSettingsFromServerToClient( + logRetention[LogRetentionOptions.API] ), }); @@ -29,7 +29,7 @@ const convertLogRetentionSettingsFromServerToClient = ({ disabled_at: disabledAt, enabled, retention_policy: retentionPolicy, -}: ILogRetentionServerSettings): ILogRetentionSettings => ({ +}: LogRetentionServerSettings): LogRetentionSettings => ({ disabledAt, enabled, retentionPolicy: @@ -39,7 +39,7 @@ const convertLogRetentionSettingsFromServerToClient = ({ const convertLogRetentionPolicyFromServerToClient = ({ min_age_days: minAgeDays, is_default: isDefault, -}: ILogRetentionServerPolicy): ILogRetentionPolicy => ({ +}: LogRetentionServerPolicy): LogRetentionPolicy => ({ isDefault, minAgeDays, }); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx index 4571ef10286e49..743cf63fb4bc35 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/index.tsx @@ -12,7 +12,7 @@ import { getAppSearchUrl } from '../shared/enterprise_search_url'; import { KibanaLogic } from '../shared/kibana'; import { HttpLogic } from '../shared/http'; import { AppLogic } from './app_logic'; -import { IInitialAppData } from '../../../common/types'; +import { InitialAppData } from '../../../common/types'; import { APP_SEARCH_PLUGIN } from '../../../common/constants'; import { Layout, SideNav, SideNavLink } from '../shared/layout'; @@ -36,7 +36,7 @@ import { Settings, SETTINGS_TITLE } from './components/settings'; import { Credentials, CREDENTIALS_TITLE } from './components/credentials'; import { ROLE_MAPPINGS_TITLE } from './components/role_mappings'; -export const AppSearch: React.FC = (props) => { +export const AppSearch: React.FC = (props) => { const { config } = useValues(KibanaLogic); return !config.host ? : ; }; @@ -52,7 +52,7 @@ export const AppSearchUnconfigured: React.FC = () => ( ); -export const AppSearchConfigured: React.FC = (props) => { +export const AppSearchConfigured: React.FC = (props) => { const { initializeAppData } = useActions(AppLogic); const { hasInitialized } = useValues(AppLogic); const { errorConnecting, readOnlyMode } = useValues(HttpLogic); @@ -100,11 +100,11 @@ export const AppSearchConfigured: React.FC = (props) => { ); }; -interface IAppSearchNavProps { +interface AppSearchNavProps { subNav?: React.ReactNode; } -export const AppSearchNav: React.FC = ({ subNav }) => { +export const AppSearchNav: React.FC = ({ subNav }) => { const { myRole: { canViewSettings, canViewAccountCredentials, canViewRoleMappings }, } = useValues(AppLogic); diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/types.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/types.ts index 568a0a33659824..ce7d906555d346 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/types.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/types.ts @@ -5,9 +5,9 @@ */ export * from '../../../common/types/app_search'; -export { IRole, TRole, TAbility } from './utils/role'; +export { Role, RoleTypes, AbilityTypes } from './utils/role'; -export interface IEngine { +export interface Engine { name: string; type: string; language: string; diff --git a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/index.ts b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/index.ts index a935fa657738c2..21a80bc0c208fc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/app_search/utils/role/index.ts @@ -4,18 +4,18 @@ * you may not use this file except in compliance with the Elastic License. */ -import { IAccount } from '../../types'; +import { Account } from '../../types'; -export type TRole = 'owner' | 'admin' | 'dev' | 'editor' | 'analyst'; -export type TAbility = 'manage' | 'edit' | 'view'; +export type RoleTypes = 'owner' | 'admin' | 'dev' | 'editor' | 'analyst'; +export type AbilityTypes = 'manage' | 'edit' | 'view'; -export interface IRole { +export interface Role { id: string; - roleType: TRole; - availableRoleTypes: TRole[]; + roleType: RoleTypes; + availableRoleTypes: RoleTypes[]; credentialTypes: string[]; canAccessAllEngines: boolean; - can(action: TAbility, subject: string): boolean; + can(action: AbilityTypes, subject: string): boolean; canViewMetaEngines: boolean; canViewAccountCredentials: boolean; canViewEngineAnalytics: boolean; @@ -48,10 +48,10 @@ export interface IRole { * Transforms the `role` data we receive from the Enterprise Search * server into a more convenient format for front-end use */ -export const getRoleAbilities = (role: IAccount['role']): IRole => { +export const getRoleAbilities = (role: Account['role']): Role => { // Role ability function helpers const myRole = { - can: (action: TAbility, subject: string): boolean => { + can: (action: AbilityTypes, subject: string): boolean => { return ( role?.ability?.manage?.includes(subject) || (Array.isArray(role.ability[action]) && role.ability[action].includes(subject)) @@ -63,8 +63,8 @@ export const getRoleAbilities = (role: IAccount['role']): IRole => { // Clone top-level role props, and move some props out of `ability` and into the top-level for convenience const topLevelProps = { id: role.id, - roleType: role.roleType as TRole, - availableRoleTypes: role.ability.availableRoleTypes as TRole[], + roleType: role.roleType as RoleTypes, + availableRoleTypes: role.ability.availableRoleTypes as RoleTypes[], credentialTypes: role.ability.credentialTypes, }; diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_card/product_card.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_card/product_card.tsx index ee778f49ef5b60..de553acf11f7ba 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_card/product_card.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_card/product_card.tsx @@ -16,7 +16,7 @@ import { KibanaLogic } from '../../../shared/kibana'; import './product_card.scss'; -interface IProductCard { +interface ProductCardProps { // Expects product plugin constants (@see common/constants.ts) product: { ID: string; @@ -27,7 +27,7 @@ interface IProductCard { image: string; } -export const ProductCard: React.FC = ({ product, image }) => { +export const ProductCard: React.FC = ({ product, image }) => { const { sendEnterpriseSearchTelemetry } = useActions(TelemetryLogic); const { config } = useValues(KibanaLogic); diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_selector/product_selector.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_selector/product_selector.tsx index 235ececd8b6fca..579baf41f2ce31 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_selector/product_selector.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/components/product_selector/product_selector.tsx @@ -30,14 +30,14 @@ import { SetupGuideCta } from '../setup_guide'; import AppSearchImage from '../../assets/app_search.png'; import WorkplaceSearchImage from '../../assets/workplace_search.png'; -interface IProductSelectorProps { +interface ProductSelectorProps { access: { hasAppSearchAccess?: boolean; hasWorkplaceSearchAccess?: boolean; }; } -export const ProductSelector: React.FC = ({ access }) => { +export const ProductSelector: React.FC = ({ access }) => { const { hasAppSearchAccess, hasWorkplaceSearchAccess } = access; const { config } = useValues(KibanaLogic); diff --git a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/index.tsx b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/index.tsx index 048baabe6a1ddc..b562cd577c56b5 100644 --- a/x-pack/plugins/enterprise_search/public/applications/enterprise_search/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/enterprise_search/index.tsx @@ -9,7 +9,7 @@ import { Route, Switch } from 'react-router-dom'; import { useValues } from 'kea'; import { KibanaLogic } from '../shared/kibana'; -import { IInitialAppData } from '../../../common/types'; +import { InitialAppData } from '../../../common/types'; import { HttpLogic } from '../shared/http'; @@ -21,7 +21,7 @@ import { SetupGuide } from './components/setup_guide'; import './index.scss'; -export const EnterpriseSearch: React.FC = ({ access = {} }) => { +export const EnterpriseSearch: React.FC = ({ access = {} }) => { const { errorConnecting } = useValues(HttpLogic); const { config } = useValues(KibanaLogic); diff --git a/x-pack/plugins/enterprise_search/public/applications/index.tsx b/x-pack/plugins/enterprise_search/public/applications/index.tsx index b9c94e351089db..3436df851c8d81 100644 --- a/x-pack/plugins/enterprise_search/public/applications/index.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/index.tsx @@ -14,7 +14,7 @@ import { I18nProvider } from '@kbn/i18n/react'; import { AppMountParameters, CoreStart } from 'src/core/public'; import { PluginsStart, ClientConfigType, ClientData } from '../plugin'; -import { IInitialAppData } from '../../common/types'; +import { InitialAppData } from '../../common/types'; import { mountKibanaLogic } from './shared/kibana'; import { mountLicensingLogic } from './shared/licensing'; @@ -29,7 +29,7 @@ import { externalUrl } from './shared/enterprise_search_url'; */ export const renderApp = ( - App: React.FC, + App: React.FC, { params, core, plugins }: { params: AppMountParameters; core: CoreStart; plugins: PluginsStart }, { config, data }: { config: ClientConfigType; data: ClientData } ) => { diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts index 5a05a03adeb6b0..7271a1dbbea395 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/flash_messages_logic.ts @@ -15,12 +15,12 @@ export interface IFlashMessage { description?: ReactNode; } -export interface IFlashMessagesValues { +interface FlashMessagesValues { messages: IFlashMessage[]; queuedMessages: IFlashMessage[]; historyListener: Function | null; } -export interface IFlashMessagesActions { +interface FlashMessagesActions { setFlashMessages(messages: IFlashMessage | IFlashMessage[]): { messages: IFlashMessage[] }; clearFlashMessages(): void; setQueuedMessages(messages: IFlashMessage | IFlashMessage[]): { messages: IFlashMessage[] }; @@ -31,7 +31,7 @@ export interface IFlashMessagesActions { const convertToArray = (messages: IFlashMessage | IFlashMessage[]) => !Array.isArray(messages) ? [messages] : messages; -export const FlashMessagesLogic = kea>({ +export const FlashMessagesLogic = kea>({ path: ['enterprise_search', 'flash_messages_logic'], actions: { setFlashMessages: (messages) => ({ messages: convertToArray(messages) }), diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts index 2bd04d1d87f7db..c4b287ee08354f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/handle_api_errors.ts @@ -17,7 +17,7 @@ import { FlashMessagesLogic, IFlashMessage } from './'; * `errors` property in the response's data, which will contain messages we can * display to the user. */ -interface IErrorResponse { +interface ErrorResponse { statusCode: number; error: string; message: string; @@ -25,17 +25,14 @@ interface IErrorResponse { errors: string[]; }; } -interface IOptions { +interface Options { isQueued?: boolean; } /** * Converts API/HTTP errors into user-facing Flash Messages */ -export const flashAPIErrors = ( - error: HttpResponse, - { isQueued }: IOptions = {} -) => { +export const flashAPIErrors = (error: HttpResponse, { isQueued }: Options = {}) => { const defaultErrorMessage = 'An unexpected error occurred'; const errorFlashMessages: IFlashMessage[] = Array.isArray(error?.body?.attributes?.errors) diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts index 21c1a60efa6b73..8792c26f9bad42 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/flash_messages/index.ts @@ -5,12 +5,6 @@ */ export { FlashMessages } from './flash_messages'; -export { - FlashMessagesLogic, - IFlashMessage, - IFlashMessagesValues, - IFlashMessagesActions, - mountFlashMessagesLogic, -} from './flash_messages_logic'; +export { FlashMessagesLogic, IFlashMessage, mountFlashMessagesLogic } from './flash_messages_logic'; export { flashAPIErrors } from './handle_api_errors'; export { setSuccessMessage, setErrorMessage, setQueuedSuccessMessage } from './set_message_helpers'; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/hidden_text/hidden_text.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/hidden_text/hidden_text.tsx index 9b0833dfce5412..69176b8a139e79 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/hidden_text/hidden_text.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/hidden_text/hidden_text.tsx @@ -7,18 +7,18 @@ import React, { useState, ReactElement } from 'react'; import { i18n } from '@kbn/i18n'; -interface IChildrenProps { +interface ChildrenProps { toggle: () => void; isHidden: boolean; hiddenText: React.ReactNode; } -interface IProps { +interface Props { text: string; - children(props: IChildrenProps): ReactElement; + children(props: ChildrenProps): ReactElement; } -export const HiddenText: React.FC = ({ text, children }) => { +export const HiddenText: React.FC = ({ text, children }) => { const [isHidden, toggleIsHidden] = useState(true); const hiddenLabel = i18n.translate('xpack.enterpriseSearch.hiddenText', { diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.ts b/x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.ts index d16e507bfb3bc4..76cefa4bc5a2c6 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/http/http_logic.ts @@ -10,20 +10,20 @@ import { HttpSetup, HttpInterceptorResponseError, HttpResponse } from 'src/core/ import { READ_ONLY_MODE_HEADER } from '../../../../common/constants'; -export interface IHttpValues { +interface HttpValues { http: HttpSetup; httpInterceptors: Function[]; errorConnecting: boolean; readOnlyMode: boolean; } -export interface IHttpActions { +interface HttpActions { initializeHttpInterceptors(): void; setHttpInterceptors(httpInterceptors: Function[]): { httpInterceptors: Function[] }; setErrorConnecting(errorConnecting: boolean): { errorConnecting: boolean }; setReadOnlyMode(readOnlyMode: boolean): { readOnlyMode: boolean }; } -export const HttpLogic = kea>({ +export const HttpLogic = kea>({ path: ['enterprise_search', 'http_logic'], actions: { initializeHttpInterceptors: () => null, @@ -108,12 +108,12 @@ export const HttpLogic = kea>({ /** * Mount/props helper */ -interface IHttpLogicProps { +interface HttpLogicProps { http: HttpSetup; errorConnecting?: boolean; readOnlyMode?: boolean; } -export const mountHttpLogic = (props: IHttpLogicProps) => { +export const mountHttpLogic = (props: HttpLogicProps) => { HttpLogic(props); const unmount = HttpLogic.mount(); return unmount; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/http/index.ts b/x-pack/plugins/enterprise_search/public/applications/shared/http/index.ts index 46a52415f85643..5e41ea032d503b 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/http/index.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/http/index.ts @@ -4,4 +4,4 @@ * you may not use this file except in compliance with the Elastic License. */ -export { HttpLogic, IHttpValues, IHttpActions, mountHttpLogic } from './http_logic'; +export { HttpLogic, mountHttpLogic } from './http_logic'; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts index 89ed07f302b035..28f500a2c8a39f 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/kibana/kibana_logic.ts @@ -11,9 +11,9 @@ import { History } from 'history'; import { ApplicationStart, ChromeBreadcrumb } from 'src/core/public'; import { HttpLogic } from '../http'; -import { createHref, ICreateHrefOptions } from '../react_router_helpers'; +import { createHref, CreateHrefOptions } from '../react_router_helpers'; -interface IKibanaLogicProps { +interface KibanaLogicProps { config: { host?: string }; history: History; navigateToUrl: ApplicationStart['navigateToUrl']; @@ -21,17 +21,17 @@ interface IKibanaLogicProps { setDocTitle(title: string): void; renderHeaderActions(HeaderActions: FC): void; } -export interface IKibanaValues extends IKibanaLogicProps { - navigateToUrl(path: string, options?: ICreateHrefOptions): Promise; +export interface KibanaValues extends KibanaLogicProps { + navigateToUrl(path: string, options?: CreateHrefOptions): Promise; } -export const KibanaLogic = kea>({ +export const KibanaLogic = kea>({ path: ['enterprise_search', 'kibana_logic'], reducers: ({ props }) => ({ config: [props.config || {}, {}], history: [props.history, {}], navigateToUrl: [ - (url: string, options?: ICreateHrefOptions) => { + (url: string, options?: CreateHrefOptions) => { const deps = { history: props.history, http: HttpLogic.values.http }; const href = createHref(url, deps, options); return props.navigateToUrl(href); @@ -44,7 +44,7 @@ export const KibanaLogic = kea>({ }), }); -export const mountKibanaLogic = (props: IKibanaLogicProps) => { +export const mountKibanaLogic = (props: KibanaLogicProps) => { KibanaLogic(props); const unmount = KibanaLogic.mount(); return unmount; diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts index e22334aeea3712..25d4850425a6e8 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_breadcrumbs.ts @@ -23,15 +23,15 @@ import { letBrowserHandleEvent, createHref } from '../react_router_helpers'; * Types */ -interface IBreadcrumb { +interface Breadcrumb { text: string; path?: string; // Used to navigate outside of the React Router basename, // i.e. if we need to go from App Search to Enterprise Search shouldNotCreateHref?: boolean; } -export type TBreadcrumbs = IBreadcrumb[]; -export type TBreadcrumbTrail = string[]; // A trail of breadcrumb text +export type Breadcrumbs = Breadcrumb[]; +export type BreadcrumbTrail = string[]; // A trail of breadcrumb text /** * Generate an array of breadcrumbs based on: @@ -50,7 +50,7 @@ export type TBreadcrumbTrail = string[]; // A trail of breadcrumb text * > Source Prioritization (linked to `/groups/{example-group-id}/source_prioritization`) */ -export const useGenerateBreadcrumbs = (trail: TBreadcrumbTrail): TBreadcrumbs => { +export const useGenerateBreadcrumbs = (trail: BreadcrumbTrail): Breadcrumbs => { const { history } = useValues(KibanaLogic); const pathArray = stripLeadingSlash(history.location.pathname).split('/'); @@ -65,7 +65,7 @@ export const useGenerateBreadcrumbs = (trail: TBreadcrumbTrail): TBreadcrumbs => * https://elastic.github.io/eui/#/navigation/breadcrumbs */ -export const useEuiBreadcrumbs = (breadcrumbs: TBreadcrumbs): EuiBreadcrumb[] => { +export const useEuiBreadcrumbs = (breadcrumbs: Breadcrumbs): EuiBreadcrumb[] => { const { navigateToUrl, history } = useValues(KibanaLogic); const { http } = useValues(HttpLogic); @@ -89,7 +89,7 @@ export const useEuiBreadcrumbs = (breadcrumbs: TBreadcrumbs): EuiBreadcrumb[] => * Product-specific breadcrumb helpers */ -export const useEnterpriseSearchBreadcrumbs = (breadcrumbs: TBreadcrumbs = []) => +export const useEnterpriseSearchBreadcrumbs = (breadcrumbs: Breadcrumbs = []) => useEuiBreadcrumbs([ { text: ENTERPRISE_SEARCH_PLUGIN.NAME, @@ -99,10 +99,10 @@ export const useEnterpriseSearchBreadcrumbs = (breadcrumbs: TBreadcrumbs = []) = ...breadcrumbs, ]); -export const useAppSearchBreadcrumbs = (breadcrumbs: TBreadcrumbs = []) => +export const useAppSearchBreadcrumbs = (breadcrumbs: Breadcrumbs = []) => useEnterpriseSearchBreadcrumbs([{ text: APP_SEARCH_PLUGIN.NAME, path: '/' }, ...breadcrumbs]); -export const useWorkplaceSearchBreadcrumbs = (breadcrumbs: TBreadcrumbs = []) => +export const useWorkplaceSearchBreadcrumbs = (breadcrumbs: Breadcrumbs = []) => useEnterpriseSearchBreadcrumbs([ { text: WORKPLACE_SEARCH_PLUGIN.NAME, path: '/' }, ...breadcrumbs, diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_title.ts b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_title.ts index de5f72de791925..a0e34106fe2a2e 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_title.ts +++ b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/generate_title.ts @@ -15,24 +15,24 @@ import { * https://github.com/elastic/kibana/blob/master/docs/development/core/public/kibana-plugin-core-public.chromedoctitle.md */ -export type TTitle = string[]; +type Title = string[]; /** * Given an array of page titles, return a final formatted document title * @param pages - e.g., ['Curations', 'some Engine', 'App Search'] * @returns - e.g., 'Curations - some Engine - App Search' */ -export const generateTitle = (pages: TTitle) => pages.join(' - '); +export const generateTitle = (pages: Title) => pages.join(' - '); /** * Product-specific helpers */ -export const enterpriseSearchTitle = (page: TTitle = []) => +export const enterpriseSearchTitle = (page: Title = []) => generateTitle([...page, ENTERPRISE_SEARCH_PLUGIN.NAME]); -export const appSearchTitle = (page: TTitle = []) => +export const appSearchTitle = (page: Title = []) => generateTitle([...page, APP_SEARCH_PLUGIN.NAME]); -export const workplaceSearchTitle = (page: TTitle = []) => +export const workplaceSearchTitle = (page: Title = []) => generateTitle([...page, WORKPLACE_SEARCH_PLUGIN.NAME]); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/set_chrome.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/set_chrome.tsx index a43e7053bb1e19..0e694a3d2bbc85 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/set_chrome.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/kibana_chrome/set_chrome.tsx @@ -14,7 +14,7 @@ import { useEnterpriseSearchBreadcrumbs, useAppSearchBreadcrumbs, useWorkplaceSearchBreadcrumbs, - TBreadcrumbTrail, + BreadcrumbTrail, } from './generate_breadcrumbs'; import { enterpriseSearchTitle, appSearchTitle, workplaceSearchTitle } from './generate_title'; @@ -33,11 +33,11 @@ import { enterpriseSearchTitle, appSearchTitle, workplaceSearchTitle } from './g * Title output: Workplace Search - Elastic */ -interface ISetChromeProps { - trail?: TBreadcrumbTrail; +interface SetChromeProps { + trail?: BreadcrumbTrail; } -export const SetEnterpriseSearchChrome: React.FC = ({ trail = [] }) => { +export const SetEnterpriseSearchChrome: React.FC = ({ trail = [] }) => { const { setBreadcrumbs, setDocTitle } = useValues(KibanaLogic); const title = reverseArray(trail); @@ -54,7 +54,7 @@ export const SetEnterpriseSearchChrome: React.FC = ({ trail = [ return null; }; -export const SetAppSearchChrome: React.FC = ({ trail = [] }) => { +export const SetAppSearchChrome: React.FC = ({ trail = [] }) => { const { setBreadcrumbs, setDocTitle } = useValues(KibanaLogic); const title = reverseArray(trail); @@ -71,7 +71,7 @@ export const SetAppSearchChrome: React.FC = ({ trail = [] }) => return null; }; -export const SetWorkplaceSearchChrome: React.FC = ({ trail = [] }) => { +export const SetWorkplaceSearchChrome: React.FC = ({ trail = [] }) => { const { setBreadcrumbs, setDocTitle } = useValues(KibanaLogic); const title = reverseArray(trail); diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx index ef8216e8b6711c..0ee7de242dfe23 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/layout.tsx @@ -12,7 +12,7 @@ import { i18n } from '@kbn/i18n'; import './layout.scss'; -interface ILayoutProps { +interface LayoutProps { navigation: React.ReactNode; restrictWidth?: boolean; readOnlyMode?: boolean; @@ -23,7 +23,7 @@ export interface INavContext { } export const NavContext = React.createContext({}); -export const Layout: React.FC = ({ +export const Layout: React.FC = ({ children, navigation, restrictWidth, diff --git a/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx b/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx index facfd0bfcb16d5..6c4e1d084c16dc 100644 --- a/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx +++ b/x-pack/plugins/enterprise_search/public/applications/shared/layout/side_nav.tsx @@ -23,7 +23,7 @@ import './side_nav.scss'; * Side navigation - product & icon + links wrapper */ -interface ISideNavProps { +interface SideNavProps { // Expects product plugin constants (@see common/constants.ts) product: { NAME: string; @@ -31,7 +31,7 @@ interface ISideNavProps { }; } -export const SideNav: React.FC = ({ product, children }) => { +export const SideNav: React.FC = ({ product, children }) => { return (