Skip to content

Commit

Permalink
Merge pull request #12 from stratoula/woof-meow-3
Browse files Browse the repository at this point in the history
Fixes based on review
  • Loading branch information
ppisljar authored Jan 25, 2024
2 parents 5287cb9 + 2351e02 commit 53a4461
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 25 deletions.
1 change: 1 addition & 0 deletions src/plugins/unified_histogram/public/chart/histogram.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ export function Histogram({
{...lensProps}
disableTriggers={disableTriggers}
disabledActions={disabledActions}
shouldUseSizeTransitionVeil={false}
onFilter={onFilter}
onBrushEnd={onBrushEnd}
withDefaultActions={withDefaultActions}
Expand Down
25 changes: 25 additions & 0 deletions src/plugins/unified_histogram/public/layout/helpers.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

import { AggregateQuery } from '@kbn/es-query';

const TRANSFORMATIONAL_COMMANDS = ['stats', 'project', 'keep'];

export const shouldDisplayHistogram = (query: AggregateQuery) => {
let queryHasTransformationalCommands = false;
if ('esql' in query) {
TRANSFORMATIONAL_COMMANDS.forEach((command: string) => {
if (query.esql.toLowerCase().includes(command)) {
queryHasTransformationalCommands = true;
return;
}
});
}

return !queryHasTransformationalCommands;
};
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { LensSuggestionsApi, Suggestion } from '@kbn/lens-plugin/public';
import { isEqual } from 'lodash';
import { useEffect, useMemo, useRef, useState } from 'react';
import { computeInterval } from './compute_interval';
const TRANSFORMATIONAL_COMMANDS = ['stats', 'project', 'keep'];
import { shouldDisplayHistogram } from '../helpers';

export const useLensSuggestions = ({
dataView,
Expand Down Expand Up @@ -74,17 +74,8 @@ export const useLensSuggestions = ({
getAggregateQueryMode(query) === 'esql' &&
timeRange
) {
let queryHasTransformationalCommands = false;
if ('esql' in query) {
TRANSFORMATIONAL_COMMANDS.forEach((command: string) => {
if (query.esql.toLowerCase().includes(command)) {
queryHasTransformationalCommands = true;
return;
}
});
}

if (queryHasTransformationalCommands) return undefined;
const isOnHistogramMode = shouldDisplayHistogram(query);
if (!isOnHistogramMode) return undefined;

const interval = computeInterval(timeRange, data);
const language = getAggregateQueryMode(query);
Expand Down
14 changes: 10 additions & 4 deletions src/plugins/unified_histogram/public/layout/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import type {
LensSuggestionsApi,
Suggestion,
} from '@kbn/lens-plugin/public';
import type { AggregateQuery, Filter, Query, TimeRange } from '@kbn/es-query';
import { AggregateQuery, Filter, isOfAggregateQueryType, Query, TimeRange } from '@kbn/es-query';
import {
ResizableLayout,
ResizableLayoutMode,
Expand All @@ -39,6 +39,7 @@ import type {
UnifiedHistogramInput$,
} from '../types';
import { useLensSuggestions } from './hooks/use_lens_suggestions';
import { shouldDisplayHistogram } from './helpers';

const ChartMemoized = React.memo(Chart);

Expand Down Expand Up @@ -242,9 +243,14 @@ export const UnifiedHistogramLayout = ({
});

// apply table to current suggestion

const usedSuggestion = useMemo(() => {
if (table) {
if (
currentSuggestion &&
table &&
query &&
isOfAggregateQueryType(query) &&
!shouldDisplayHistogram(query)
) {
const { layers } = currentSuggestion.datasourceState as TextBasedPersistedState;

const newState = {
Expand All @@ -264,7 +270,7 @@ export const UnifiedHistogramLayout = ({
} else {
return currentSuggestion;
}
}, [currentSuggestion, table]);
}, [currentSuggestion, query, table]);

const chart = suggestionUnsupported ? undefined : originalChart;
const isChartAvailable = checkChartAvailability({ chart, dataView, isPlainRecord });
Expand Down
16 changes: 8 additions & 8 deletions test/functional/apps/discover/group3/_request_counts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,21 +110,21 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expect(searchCount).to.be(expectedRequests);
});

it(`should send ${expectedRefreshRequest} requests (documents + chart) when refreshing`, async () => {
await expectSearches(type, expectedRefreshRequest, async () => {
it(`should send ${expectedRequests} requests (documents + chart) when refreshing`, async () => {
await expectSearches(type, expectedRequests, async () => {
await queryBar.clickQuerySubmitButton();
});
});

it(`should send ${expectedRefreshRequest} requests (documents + chart) when changing the query`, async () => {
await expectSearches(type, expectedRefreshRequest, async () => {
it(`should send ${expectedRequests} requests (documents + chart) when changing the query`, async () => {
await expectSearches(type, expectedRequests, async () => {
await setQuery(query1);
await queryBar.clickQuerySubmitButton();
});
});

it(`should send ${expectedRefreshRequest} requests (documents + chart) when changing the time range`, async () => {
await expectSearches(type, expectedRefreshRequest, async () => {
it(`should send ${expectedRequests} requests (documents + chart) when changing the time range`, async () => {
await expectSearches(type, expectedRequests, async () => {
await PageObjects.timePicker.setAbsoluteRange(
'Sep 21, 2015 @ 06:31:44.000',
'Sep 23, 2015 @ 00:00:00.000'
Expand All @@ -150,7 +150,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
await setQuery(query2);
await queryBar.clickQuerySubmitButton();
await waitForLoadingToFinish();
await expectSearches(type, savedSearchesRequests ?? expectedRequests, async () => {
await expectSearches(type, expectedRequests, async () => {
await PageObjects.discover.revertUnsavedChanges();
});
// clearing the saved search
Expand Down Expand Up @@ -250,7 +250,7 @@ export default function ({ getService, getPageObjects }: FtrProviderContext) {
expectedRefreshRequest: 2,
});

it(`should send 3 requests (documents + chart) when toggling the chart visibility`, async () => {
it(`should send 2 requests (documents + chart) when toggling the chart visibility`, async () => {
await expectSearches(type, 3, async () => {
await PageObjects.discover.toggleChartVisibility();
});
Expand Down
2 changes: 2 additions & 0 deletions x-pack/plugins/lens/public/embeddable/embeddable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ interface LensBaseEmbeddableInput extends EmbeddableInput {
style?: React.CSSProperties;
className?: string;
noPadding?: boolean;
shouldUseSizeTransitionVeil?: boolean;
onBrushEnd?: (data: Simplify<BrushTriggerEvent['data'] & PreventableEvent>) => void;
onLoad?: (
isLoading: boolean,
Expand Down Expand Up @@ -1151,6 +1152,7 @@ export class Embeddable
this.logError('runtime');
}}
noPadding={this.visDisplayOptions.noPadding}
shouldUseSizeTransitionVeil={this.input.shouldUseSizeTransitionVeil}
/>
</KibanaThemeProvider>
<MessagesBadge
Expand Down
4 changes: 3 additions & 1 deletion x-pack/plugins/lens/public/embeddable/expression_wrapper.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export interface ExpressionWrapperProps {
executionContext?: KibanaExecutionContext;
lensInspector: LensInspector;
noPadding?: boolean;
shouldUseSizeTransitionVeil?: boolean;
}

export function ExpressionWrapper({
Expand All @@ -71,6 +72,7 @@ export function ExpressionWrapper({
executionContext,
lensInspector,
noPadding,
shouldUseSizeTransitionVeil,
}: ExpressionWrapperProps) {
if (!expression) return null;
return (
Expand All @@ -92,7 +94,7 @@ export function ExpressionWrapper({
syncTooltips={syncTooltips}
syncCursor={syncCursor}
executionContext={executionContext}
shouldUseSizeTransitionVeil={true}
shouldUseSizeTransitionVeil={shouldUseSizeTransitionVeil ?? true}
renderError={(errorMessage, error) => {
const messages = getOriginalRequestErrorMessages(error || null);
addUserMessages(messages);
Expand Down

0 comments on commit 53a4461

Please sign in to comment.