Skip to content

Commit

Permalink
[Lens] [metric visualization] a column normalized by unit doesn't dis…
Browse files Browse the repository at this point in the history
…play properly on dashboard (#142741) (#142934)

* [Lens] [metric visualization] a column normalized by unit doesn't display properly on dashboard

Closes: #142615

* add test

* [CI] Auto-commit changed files from 'node scripts/build_plugin_list_docs'

* bundle size reduction

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit 4854f42)
  • Loading branch information
alexwizp authored Oct 7, 2022
1 parent 942e082 commit 9f168b7
Show file tree
Hide file tree
Showing 11 changed files with 96 additions and 37 deletions.
4 changes: 2 additions & 2 deletions x-pack/plugins/lens/common/embeddable_factory/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@
* 2.0.
*/

import { SerializableRecord, Serializable } from '@kbn/utility-types';
import { SavedObjectReference } from '@kbn/core/types';
import type { SerializableRecord, Serializable } from '@kbn/utility-types';
import type { SavedObjectReference } from '@kbn/core/types';
import type {
EmbeddableStateWithType,
EmbeddableRegistryDefinition,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,6 @@ import type { TimeRange } from '@kbn/es-query';
import { createDatatableUtilitiesMock } from '@kbn/data-plugin/common/mocks';
import { functionWrapper } from '@kbn/expressions-plugin/common/expression_functions/specs/tests/utils';

// mock the specific inner variable:
// there are intra dependencies in the data plugin we might break trying to mock the whole thing
jest.mock('@kbn/data-plugin/common/query/timefilter/get_time', () => {
const localMoment = jest.requireActual('moment');
return {
calculateBounds: jest.fn(({ from, to }) => ({
min: localMoment(from),
max: localMoment(to),
})),
};
});

import { getTimeScale } from './time_scale';
import type { TimeScaleArgs } from './types';

Expand Down Expand Up @@ -425,6 +413,35 @@ describe('time_scale', () => {
expect(result.rows.map(({ scaledMetric }) => scaledMetric)).toEqual([75]);
});

it('should work with relative time range', async () => {
const result = await timeScaleWrapped(
{
...emptyTable,
rows: [
{
date: moment().subtract('1d').valueOf(),
metric: 300,
},
],
},
{
inputColumnId: 'metric',
outputColumnId: 'scaledMetric',
targetUnit: 'd',
},
{
getSearchContext: () => ({
timeRange: {
from: 'now-10d',
to: 'now',
},
}),
} as unknown as ExecutionContext
);

expect(result.rows.map(({ scaledMetric }) => scaledMetric)).toEqual([30]);
});

it('should apply fn for non-histogram fields (with Reduced time range)', async () => {
const result = await timeScaleWrapped(
{
Expand Down
49 changes: 43 additions & 6 deletions x-pack/plugins/lens/common/expressions/time_scale/time_scale_fn.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,39 @@ const unitInMs: Record<TimeScaleUnit, number> = {
d: 1000 * 60 * 60 * 24,
};

// the datemath plugin always parses dates by using the current default moment time zone.
// to use the configured time zone, we are temporary switching it just for the calculation.

// The code between this call and the reset in the finally block is not allowed to get async,
// otherwise the timezone setting can leak out of this function.
const withChangedTimeZone = <TReturnedValue = unknown>(
timeZone: string | undefined,
action: () => TReturnedValue
): TReturnedValue => {
if (timeZone) {
const defaultTimezone = moment().zoneName();
try {
moment.tz.setDefault(timeZone);
return action();
} finally {
// reset default moment timezone
moment.tz.setDefault(defaultTimezone);
}
} else {
return action();
}
};

const getTimeBounds = (timeRange: TimeRange, timeZone?: string, getForceNow?: () => Date) =>
withChangedTimeZone(timeZone, () => calculateBounds(timeRange, { forceNow: getForceNow?.() }));

export const timeScaleFn =
(
getDatatableUtilities: (
context: ExecutionContext
) => DatatableUtilitiesService | Promise<DatatableUtilitiesService>,
getTimezone: (context: ExecutionContext) => string | Promise<string>
getTimezone: (context: ExecutionContext) => string | Promise<string>,
getForceNow?: () => Date
): TimeScaleExpressionFunction['fn'] =>
async (
input,
Expand Down Expand Up @@ -69,7 +96,9 @@ export const timeScaleFn =
timeZone: contextTimeZone,
});
const intervalDuration = timeInfo?.interval && parseInterval(timeInfo.interval);
timeBounds = timeInfo?.timeRange && calculateBounds(timeInfo.timeRange);

timeBounds =
timeInfo?.timeRange && getTimeBounds(timeInfo.timeRange, timeInfo?.timeZone, getForceNow);

getStartEndOfBucketMeta = (row) => {
const startOfBucket = moment.tz(row[dateColumnId], timeInfo?.timeZone ?? contextTimeZone);
Expand All @@ -89,8 +118,18 @@ export const timeScaleFn =
}
} else {
const timeRange = context.getSearchContext().timeRange as TimeRange;
const endOfBucket = moment.tz(timeRange.to, contextTimeZone);
let startOfBucket = moment.tz(timeRange.from, contextTimeZone);
timeBounds = getTimeBounds(timeRange, contextTimeZone, getForceNow);

if (!timeBounds.max || !timeBounds.min) {
throw new Error(
i18n.translate('xpack.lens.functions.timeScale.timeBoundsMissingMessage', {
defaultMessage: 'Could not parse "Time Range"',
})
);
}

const endOfBucket = timeBounds.max;
let startOfBucket = timeBounds.min;

if (reducedTimeRange) {
const reducedStartOfBucket = endOfBucket.clone().subtract(parseInterval(reducedTimeRange));
Expand All @@ -100,8 +139,6 @@ export const timeScaleFn =
}
}

timeBounds = calculateBounds(timeRange);

getStartEndOfBucketMeta = () => ({
startOfBucket,
endOfBucket,
Expand Down
File renamed without changes.
1 change: 0 additions & 1 deletion x-pack/plugins/lens/common/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

export * from './constants';
export * from './types';
export * from './visualizations';

// Note: do not import the expression folder here or the page bundle will be bloated with all
// the package
6 changes: 3 additions & 3 deletions x-pack/plugins/lens/common/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@
*/

import type { Filter, FilterMeta } from '@kbn/es-query';
import { Position } from '@elastic/charts';
import { $Values } from '@kbn/utility-types';
import type { Position } from '@elastic/charts';
import type { $Values } from '@kbn/utility-types';
import type { CustomPaletteParams, PaletteOutput } from '@kbn/coloring';
import type { IFieldFormat, SerializedFieldFormat } from '@kbn/field-formats-plugin/common';
import type { ColorMode } from '@kbn/charts-plugin/common';
import { LegendSize } from '@kbn/visualizations-plugin/common';
import type { LegendSize } from '@kbn/visualizations-plugin/common';
import {
CategoryDisplay,
layerTypes,
Expand Down
14 changes: 7 additions & 7 deletions x-pack/plugins/lens/public/embeddable/embeddable_factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ import {
IContainer,
ErrorEmbeddable,
} from '@kbn/embeddable-plugin/public';
import { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import { Start as InspectorStart } from '@kbn/inspector-plugin/public';
import type { UiActionsStart } from '@kbn/ui-actions-plugin/public';
import type { Start as InspectorStart } from '@kbn/inspector-plugin/public';
import type { SpacesPluginStart } from '@kbn/spaces-plugin/public';
import { LensByReferenceInput, LensEmbeddableInput } from './embeddable';
import { Document } from '../persistence/saved_object_store';
import { LensAttributeService } from '../lens_attribute_service';
import type { LensByReferenceInput, LensEmbeddableInput } from './embeddable';
import type { Document } from '../persistence/saved_object_store';
import type { LensAttributeService } from '../lens_attribute_service';
import { DOC_TYPE } from '../../common/constants';
import { ErrorMessage } from '../editor_frame_service/types';
import type { ErrorMessage } from '../editor_frame_service/types';
import { extract, inject } from '../../common/embeddable_factory';
import { DatasourceMap, VisualizationMap } from '../types';
import type { DatasourceMap, VisualizationMap } from '../types';

export interface LensEmbeddableStartServices {
data: DataPublicPluginStart;
Expand Down
10 changes: 7 additions & 3 deletions x-pack/plugins/lens/public/expressions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
*/

import type { ExpressionsSetup } from '@kbn/expressions-plugin/public';

import { getDatatable } from '../common/expressions/datatable/datatable';
import { datatableColumn } from '../common/expressions/datatable/datatable_column';
import { mapToColumns } from '../common/expressions/map_to_columns/map_to_columns';
Expand All @@ -14,11 +15,14 @@ import { counterRate } from '../common/expressions/counter_rate';
import { getTimeScale } from '../common/expressions/time_scale/time_scale';
import { collapse } from '../common/expressions';

type TimeScaleArguments = Parameters<typeof getTimeScale>;

export const setupExpressions = (
expressions: ExpressionsSetup,
formatFactory: Parameters<typeof getDatatable>[0],
getDatatableUtilities: Parameters<typeof getTimeScale>[0],
getTimeZone: Parameters<typeof getTimeScale>[1]
getDatatableUtilities: TimeScaleArguments[0],
getTimeZone: TimeScaleArguments[1],
getForceNow: TimeScaleArguments[2]
) => {
[
collapse,
Expand All @@ -27,6 +31,6 @@ export const setupExpressions = (
mapToColumns,
datatableColumn,
getDatatable(formatFactory),
getTimeScale(getDatatableUtilities, getTimeZone),
getTimeScale(getDatatableUtilities, getTimeZone, getForceNow),
].forEach((expressionFn) => expressions.registerFunction(expressionFn));
};
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/public/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,8 @@ export class LensPlugin {
async () => {
const { getTimeZone } = await import('./utils');
return getTimeZone(core.uiSettings);
}
},
() => startServices().plugins.data.nowProvider.get()
);

const getPresentationUtilContext = () =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ import {
NumberDisplay,
PieChartTypes,
PieVisualizationState,
isPartitionShape,
} from '../../../common';
import { isPartitionShape } from '../../../common/visualizations';
import type { PieChartType } from '../../../common/types';
import { PartitionChartsMeta } from './partition_charts_meta';

Expand Down
3 changes: 2 additions & 1 deletion x-pack/plugins/lens/server/migrations/common_migrations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ import {
VisState850,
LensDocShape850,
} from './types';
import { DOCUMENT_FIELD_NAME, layerTypes, LegacyMetricState, isPartitionShape } from '../../common';
import { DOCUMENT_FIELD_NAME, layerTypes, LegacyMetricState } from '../../common';
import { isPartitionShape } from '../../common/visualizations';
import { LensDocShape } from './saved_object_migrations';

export const commonRenameOperationsForFormula = (
Expand Down

0 comments on commit 9f168b7

Please sign in to comment.