From 5531bbec642558b6cd859a581a0f494bce779b5d Mon Sep 17 00:00:00 2001 From: Adrian Mroz Date: Mon, 17 May 2021 15:26:09 +0200 Subject: [PATCH 1/6] Dimension has now limits property --- .../components/split-menu/limit-dropdown.tsx | 6 ++-- src/client/deserializers/dimension.ts | 6 +++- src/common/limit/limit.ts | 2 +- src/common/models/dimension/dimension.ts | 30 ++++++++++++++++++- .../version-4/split-definition.ts | 4 +-- .../bar-chart/bar-chart.ts | 4 +-- .../line-chart/line-chart.ts | 4 +-- 7 files changed, 44 insertions(+), 12 deletions(-) diff --git a/src/client/components/split-menu/limit-dropdown.tsx b/src/client/components/split-menu/limit-dropdown.tsx index 71f4f8820..a58b15e6b 100644 --- a/src/client/components/split-menu/limit-dropdown.tsx +++ b/src/client/components/split-menu/limit-dropdown.tsx @@ -15,7 +15,7 @@ */ import * as React from "react"; -import { AVAILABLE_LIMITS } from "../../../common/limit/limit"; +import { DEFAULT_LIMITS } from "../../../common/limit/limit"; import { Unary } from "../../../common/utils/functional/functional"; import { STRINGS } from "../../config/constants"; import { Dropdown } from "../dropdown/dropdown"; @@ -25,8 +25,8 @@ function formatLimit(limit: number): string { } function calculateLimits(includeNone: boolean) { - if (!includeNone) return AVAILABLE_LIMITS; - return [...AVAILABLE_LIMITS, null]; + if (!includeNone) return DEFAULT_LIMITS; + return [...DEFAULT_LIMITS, null]; } export interface LimitDropdownProps { diff --git a/src/client/deserializers/dimension.ts b/src/client/deserializers/dimension.ts index 7d722ce58..c5a782940 100644 --- a/src/client/deserializers/dimension.ts +++ b/src/client/deserializers/dimension.ts @@ -20,7 +20,7 @@ import { bucketingStrategies, ClientDimension, SerializedDimension } from "../.. import { TimeShift } from "../../common/models/time-shift/time-shift"; export function deserialize(dimension: SerializedDimension): ClientDimension { - const { description, url, name, title, expression, sortStrategy } = dimension; + const { description, url, name, title, expression, limits, sortStrategy } = dimension; switch (dimension.kind) { case "string": { @@ -30,6 +30,7 @@ export function deserialize(dimension: SerializedDimension): ClientDimension { url, name, title, + limits, sortStrategy, expression: Expression.fromJS(expression), multiValue: dimension.multiValue @@ -42,6 +43,7 @@ export function deserialize(dimension: SerializedDimension): ClientDimension { url, name, title, + limits, sortStrategy, expression: Expression.fromJS(expression) }; @@ -53,6 +55,7 @@ export function deserialize(dimension: SerializedDimension): ClientDimension { url, name, title, + limits, sortStrategy, expression: Expression.fromJS(expression), granularities, @@ -68,6 +71,7 @@ export function deserialize(dimension: SerializedDimension): ClientDimension { url, name, title, + limits, sortStrategy, expression: Expression.fromJS(expression), bucketedBy: bucketedBy && Duration.fromJS(bucketedBy), diff --git a/src/common/limit/limit.ts b/src/common/limit/limit.ts index 504fe85b4..207ff3b38 100644 --- a/src/common/limit/limit.ts +++ b/src/common/limit/limit.ts @@ -14,4 +14,4 @@ * limitations under the License. */ -export const AVAILABLE_LIMITS = [5, 10, 25, 50, 100]; +export const DEFAULT_LIMITS = [5, 10, 25, 50, 100]; diff --git a/src/common/models/dimension/dimension.ts b/src/common/models/dimension/dimension.ts index b1e728a29..4178f218c 100644 --- a/src/common/models/dimension/dimension.ts +++ b/src/common/models/dimension/dimension.ts @@ -21,6 +21,7 @@ import { DEFAULT_LATEST_PERIOD_DURATIONS, DEFAULT_TIME_SHIFT_DURATIONS } from "../../../client/components/filter-menu/time-filter-menu/presets"; +import { DEFAULT_LIMITS } from "../../limit/limit"; import { makeTitle, verifyUrlSafeName } from "../../utils/general/general"; import { GranularityJS } from "../granularity/granularity"; import { Bucket } from "../split/split"; @@ -37,6 +38,7 @@ interface BaseDimension { title: string; description?: string; expression: Expression; + limits: number[]; url?: string; sortStrategy?: string; } @@ -99,6 +101,7 @@ export interface DimensionJS { granularities?: GranularityJS[]; timeShiftDurations?: string[]; latestPeriodDurations?: string[]; + limits?: number[]; bucketedBy?: GranularityJS; bucketingStrategy?: BucketingStrategy; sortStrategy?: string; @@ -121,6 +124,7 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { const kind = rawKind ? readKind(rawKind) : typeToKind(config.type); const expression = readExpression(config); const title = rawTitle || makeTitle(name); + const limits = readLimits(config); switch (kind) { case "string": { const { multiValue } = config; @@ -130,6 +134,7 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { name, title, expression, + limits, multiValue: Boolean(multiValue), sortStrategy, kind @@ -142,6 +147,7 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { name, title, expression, + limits, sortStrategy, kind }; @@ -159,6 +165,7 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { url, description, expression, + limits, bucketedBy: bucketedBy && Duration.fromJS(bucketedBy as string), bucketingStrategy: bucketingStrategy && bucketingStrategies[bucketingStrategy], granularities: readGranularities(config, "time"), @@ -180,6 +187,7 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { url, description, expression, + limits, bucketedBy: bucketedBy && Number(bucketedBy), bucketingStrategy: bucketingStrategy && bucketingStrategies[bucketingStrategy], granularities: readGranularities(config, "number") @@ -188,6 +196,14 @@ export function fromConfig(config: DimensionJS & LegacyDimension): Dimension { } } +function readLimits({ name, limits }: DimensionJS): number[] { + if (!limits) return DEFAULT_LIMITS; + if (!Array.isArray(limits)) { + throw new Error(`must have list of valid limits in dimension '${name}'`); + } + return limits; +} + function readLatestPeriodDurations({ name, latestPeriodDurations }: DimensionJS): Duration[] { if (!latestPeriodDurations) return DEFAULT_LATEST_PERIODS; if (!Array.isArray(latestPeriodDurations) || latestPeriodDurations.length !== 5) { @@ -226,6 +242,7 @@ interface SerializedStringDimension { title: string; description?: string; expression: ExpressionJS; + limits: number[]; url?: string; sortStrategy?: string; multiValue: boolean; @@ -237,6 +254,7 @@ interface SerializedBooleanDimension { title: string; description?: string; expression: ExpressionJS; + limits: number[]; url?: string; sortStrategy?: string; } @@ -247,6 +265,7 @@ interface SerializedNumberDimension { title: string; description?: string; expression: ExpressionJS; + limits: number[]; url?: string; sortStrategy?: string; granularities?: number[]; @@ -260,6 +279,7 @@ interface SerializedTimeDimension { title: string; description?: string; expression: ExpressionJS; + limits: number[]; url?: string; sortStrategy?: string; granularities?: string[]; @@ -276,7 +296,7 @@ export type SerializedDimension = | SerializedTimeDimension; export function serialize(dimension: Dimension): SerializedDimension { - const { name, description, expression, title, sortStrategy, url } = dimension; + const { name, description, expression, title, sortStrategy, url, limits } = dimension; // NOTE: If we move kind to destructuring above, typescript would not infer proper Dimension variant switch (dimension.kind) { @@ -288,6 +308,7 @@ export function serialize(dimension: Dimension): SerializedDimension { name, title, expression: expression.toJS(), + limits, multiValue, sortStrategy, kind: dimension.kind @@ -300,6 +321,7 @@ export function serialize(dimension: Dimension): SerializedDimension { name, title, expression: expression.toJS(), + limits, sortStrategy, kind: dimension.kind }; @@ -311,6 +333,7 @@ export function serialize(dimension: Dimension): SerializedDimension { name, title, expression: expression.toJS(), + limits, sortStrategy, bucketingStrategy, bucketedBy: bucketedBy && bucketedBy.toJS(), @@ -328,6 +351,7 @@ export function serialize(dimension: Dimension): SerializedDimension { name, title, expression: expression.toJS(), + limits, sortStrategy, bucketingStrategy, bucketedBy, @@ -357,6 +381,7 @@ export function createDimension(kind: DimensionKind, name: string, expression: E name, title: makeTitle(name), expression, + limits: DEFAULT_LIMITS, multiValue: Boolean(multiValue) }; case "boolean": @@ -364,6 +389,7 @@ export function createDimension(kind: DimensionKind, name: string, expression: E kind, name, title: makeTitle(name), + limits: DEFAULT_LIMITS, expression }; case "time": @@ -372,6 +398,7 @@ export function createDimension(kind: DimensionKind, name: string, expression: E timeShiftDurations: DEFAULT_TIME_SHIFTS, kind, name, + limits: DEFAULT_LIMITS, title: makeTitle(name), expression }; @@ -379,6 +406,7 @@ export function createDimension(kind: DimensionKind, name: string, expression: E return { kind, name, + limits: DEFAULT_LIMITS, title: makeTitle(name), expression }; diff --git a/src/common/view-definitions/version-4/split-definition.ts b/src/common/view-definitions/version-4/split-definition.ts index 25d53a967..4a338d6a9 100644 --- a/src/common/view-definitions/version-4/split-definition.ts +++ b/src/common/view-definitions/version-4/split-definition.ts @@ -15,7 +15,7 @@ */ import { Duration } from "chronoshift"; -import { AVAILABLE_LIMITS } from "../../limit/limit"; +import { DEFAULT_LIMITS } from "../../limit/limit"; import { SeriesDerivation } from "../../models/series/concrete-series"; import { DimensionSort, SeriesSort, Sort, SortDirection, SortType } from "../../models/sort/sort"; import { Split, SplitType } from "../../models/split/split"; @@ -98,7 +98,7 @@ function fromSort(sort: Sort): SplitSortDefinition { function toLimit(limit: unknown): number | null { if (limit === null) return null; if (isNumber(limit) && isFiniteNumber(limit)) return limit; - return AVAILABLE_LIMITS[0]; + return DEFAULT_LIMITS[0]; } const numberSplitConversion: SplitDefinitionConversion = { diff --git a/src/common/visualization-manifests/bar-chart/bar-chart.ts b/src/common/visualization-manifests/bar-chart/bar-chart.ts index c5032e0c1..c18595692 100644 --- a/src/common/visualization-manifests/bar-chart/bar-chart.ts +++ b/src/common/visualization-manifests/bar-chart/bar-chart.ts @@ -17,7 +17,7 @@ import { List } from "immutable"; import { clamp } from "../../../client/utils/dom/dom"; -import { AVAILABLE_LIMITS } from "../../limit/limit"; +import { DEFAULT_LIMITS } from "../../limit/limit"; import { NORMAL_COLORS } from "../../models/colors/colors"; import { canBucketByDefault, Dimension } from "../../models/dimension/dimension"; import { allDimensions, findDimensionByName } from "../../models/dimension/dimensions"; @@ -48,7 +48,7 @@ function isTimeSplitValid(timeSplit: Split): boolean { const clampNominalSplitLimit = (split: Split) => split .update("limit", limit => - clamp(limit, AVAILABLE_LIMITS[0], NORMAL_COLORS.length)); + clamp(limit, DEFAULT_LIMITS[0], NORMAL_COLORS.length)); const fixTimeSplit = (split: Split) => { const { reference } = split; diff --git a/src/common/visualization-manifests/line-chart/line-chart.ts b/src/common/visualization-manifests/line-chart/line-chart.ts index d035463a2..a99263d0c 100644 --- a/src/common/visualization-manifests/line-chart/line-chart.ts +++ b/src/common/visualization-manifests/line-chart/line-chart.ts @@ -17,7 +17,7 @@ import { List } from "immutable"; import { clamp } from "../../../client/utils/dom/dom"; -import { AVAILABLE_LIMITS } from "../../limit/limit"; +import { DEFAULT_LIMITS } from "../../limit/limit"; import { NORMAL_COLORS } from "../../models/colors/colors"; import { getDimensionsByKind } from "../../models/data-cube/data-cube"; import { findDimensionByName } from "../../models/dimension/dimensions"; @@ -112,7 +112,7 @@ const rulesEvaluator = visualizationDependentEvaluatorBuilder timeSplit = timeSplit.changeLimit(null); } - const colorSplit = splits.getSplit(1).update("limit", limit => clamp(limit, AVAILABLE_LIMITS[0], COLORS_COUNT)); + const colorSplit = splits.getSplit(1).update("limit", limit => clamp(limit, DEFAULT_LIMITS[0], COLORS_COUNT)); return Resolve.automatic(8, { splits: new Splits({ splits: List([colorSplit, timeSplit]) }) From 94dc682e527a09fa11136aba56aebfeef2945a26 Mon Sep 17 00:00:00 2001 From: Adrian Mroz Date: Mon, 17 May 2021 15:26:38 +0200 Subject: [PATCH 2/6] Handle limits property in url decoders --- .../version-3/view-definition-converter-3.ts | 2 +- .../version-4/split-definition.ts | 47 ++++++++++--------- .../version-4/view-definition-converter-4.ts | 2 +- 3 files changed, 27 insertions(+), 24 deletions(-) diff --git a/src/common/view-definitions/version-3/view-definition-converter-3.ts b/src/common/view-definitions/version-3/view-definition-converter-3.ts index 7980ce2f1..dcf04357b 100644 --- a/src/common/view-definitions/version-3/view-definition-converter-3.ts +++ b/src/common/view-definitions/version-3/view-definition-converter-3.ts @@ -41,7 +41,7 @@ export class ViewDefinitionConverter3 implements ViewDefinitionConverter filterDefinitionConverter.toFilterClause(fc, dataCube))); const splitDefinitions = List(definition.splits); - const splits = new Splits({ splits: splitDefinitions.map(splitConverter.toSplitCombine) }); + const splits = new Splits({ splits: splitDefinitions.map(sd => splitConverter.toSplitCombine(sd, dataCube)) }); const pinnedDimensions = OrderedSet(definition.pinnedDimensions || []); const pinnedSort = definition.pinnedSort; diff --git a/src/common/view-definitions/version-4/split-definition.ts b/src/common/view-definitions/version-4/split-definition.ts index 4a338d6a9..5b659e4e8 100644 --- a/src/common/view-definitions/version-4/split-definition.ts +++ b/src/common/view-definitions/version-4/split-definition.ts @@ -15,7 +15,9 @@ */ import { Duration } from "chronoshift"; -import { DEFAULT_LIMITS } from "../../limit/limit"; +import { ClientDataCube } from "../../models/data-cube/data-cube"; +import { Dimension } from "../../models/dimension/dimension"; +import { findDimensionByName } from "../../models/dimension/dimensions"; import { SeriesDerivation } from "../../models/series/concrete-series"; import { DimensionSort, SeriesSort, Sort, SortDirection, SortType } from "../../models/sort/sort"; import { Split, SplitType } from "../../models/split/split"; @@ -52,7 +54,7 @@ export interface TimeSplitDefinition extends BaseSplitDefinition { export type SplitDefinition = NumberSplitDefinition | StringSplitDefinition | TimeSplitDefinition; interface SplitDefinitionConversion { - toSplitCombine(split: In): Split; + toSplitCombine(split: In, dimension: Dimension): Split; fromSplitCombine(splitCombine: Split): In; } @@ -95,21 +97,21 @@ function fromSort(sort: Sort): SplitSortDefinition { return { ref, ...rest }; } -function toLimit(limit: unknown): number | null { +function toLimit(limit: unknown, dimension: Dimension): number | null { if (limit === null) return null; if (isNumber(limit) && isFiniteNumber(limit)) return limit; - return DEFAULT_LIMITS[0]; + return dimension.limits[0]; } const numberSplitConversion: SplitDefinitionConversion = { - toSplitCombine(split: NumberSplitDefinition): Split { - const { dimension, limit, sort, granularity } = split; + toSplitCombine(split: NumberSplitDefinition, dimension: Dimension): Split { + const { limit, sort, granularity } = split; return new Split({ type: SplitType.number, - reference: dimension, + reference: dimension.name, bucket: granularity, - sort: sort && toSort(sort, dimension), - limit: toLimit(limit) + sort: sort && toSort(sort, dimension.name), + limit: toLimit(limit, dimension) }); }, @@ -129,14 +131,14 @@ const numberSplitConversion: SplitDefinitionConversion = }; const timeSplitConversion: SplitDefinitionConversion = { - toSplitCombine(split: TimeSplitDefinition): Split { - const { dimension, limit, sort, granularity } = split; + toSplitCombine(split: TimeSplitDefinition, dimension: Dimension): Split { + const { limit, sort, granularity } = split; return new Split({ type: SplitType.time, - reference: dimension, + reference: dimension.name, bucket: Duration.fromJS(granularity), - sort: sort && toSort(sort, dimension), - limit: toLimit(limit) + sort: sort && toSort(sort, dimension.name), + limit: toLimit(limit, dimension) }); }, @@ -157,12 +159,12 @@ const timeSplitConversion: SplitDefinitionConversion = { }; const stringSplitConversion: SplitDefinitionConversion = { - toSplitCombine(split: StringSplitDefinition): Split { - const { dimension, limit, sort } = split; + toSplitCombine(split: StringSplitDefinition, dimension: Dimension): Split { + const { limit, sort } = split; return new Split({ - reference: dimension, - sort: sort && toSort(sort, dimension), - limit: toLimit(limit) + reference: dimension.name, + sort: sort && toSort(sort, dimension.name), + limit: toLimit(limit, dimension) }); }, @@ -183,14 +185,15 @@ const splitConversions: { [type in SplitType]: SplitDefinitionConversion filterDefinitionConverter.toFilterClause(fc, dataCube))); const splitDefinitions = List(definition.splits); - const splits = new Splits({ splits: splitDefinitions.map(splitConverter.toSplitCombine) }); + const splits = new Splits({ splits: splitDefinitions.map(sd => splitConverter.toSplitCombine(sd, dataCube)) }); const pinnedDimensions = OrderedSet(definition.pinnedDimensions || []); const pinnedSort = definition.pinnedSort; From 7c33eaf3d9866469540a18d4e41d28f61f6d58c4 Mon Sep 17 00:00:00 2001 From: Adrian Mroz Date: Mon, 17 May 2021 15:27:05 +0200 Subject: [PATCH 3/6] Hardcode small limit in resolvers. To be fixed in #756 --- src/common/visualization-manifests/bar-chart/bar-chart.ts | 4 ++-- src/common/visualization-manifests/line-chart/line-chart.ts | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/visualization-manifests/bar-chart/bar-chart.ts b/src/common/visualization-manifests/bar-chart/bar-chart.ts index c18595692..f559007cf 100644 --- a/src/common/visualization-manifests/bar-chart/bar-chart.ts +++ b/src/common/visualization-manifests/bar-chart/bar-chart.ts @@ -17,7 +17,6 @@ import { List } from "immutable"; import { clamp } from "../../../client/utils/dom/dom"; -import { DEFAULT_LIMITS } from "../../limit/limit"; import { NORMAL_COLORS } from "../../models/colors/colors"; import { canBucketByDefault, Dimension } from "../../models/dimension/dimension"; import { allDimensions, findDimensionByName } from "../../models/dimension/dimensions"; @@ -46,9 +45,10 @@ function isTimeSplitValid(timeSplit: Split): boolean { return isTimeSortValid && isTimeLimitValid; } +// TODO: This magic 5 will disappear in #756 const clampNominalSplitLimit = (split: Split) => split .update("limit", limit => - clamp(limit, DEFAULT_LIMITS[0], NORMAL_COLORS.length)); + clamp(limit, 5, NORMAL_COLORS.length)); const fixTimeSplit = (split: Split) => { const { reference } = split; diff --git a/src/common/visualization-manifests/line-chart/line-chart.ts b/src/common/visualization-manifests/line-chart/line-chart.ts index a99263d0c..1b2edf824 100644 --- a/src/common/visualization-manifests/line-chart/line-chart.ts +++ b/src/common/visualization-manifests/line-chart/line-chart.ts @@ -17,7 +17,6 @@ import { List } from "immutable"; import { clamp } from "../../../client/utils/dom/dom"; -import { DEFAULT_LIMITS } from "../../limit/limit"; import { NORMAL_COLORS } from "../../models/colors/colors"; import { getDimensionsByKind } from "../../models/data-cube/data-cube"; import { findDimensionByName } from "../../models/dimension/dimensions"; @@ -112,7 +111,8 @@ const rulesEvaluator = visualizationDependentEvaluatorBuilder timeSplit = timeSplit.changeLimit(null); } - const colorSplit = splits.getSplit(1).update("limit", limit => clamp(limit, DEFAULT_LIMITS[0], COLORS_COUNT)); + // TODO: This magic 5 will disappear in #756 + const colorSplit = splits.getSplit(1).update("limit", limit => clamp(limit, 5, COLORS_COUNT)); return Resolve.automatic(8, { splits: new Splits({ splits: List([colorSplit, timeSplit]) }) From 83ba3483ac8314b191a9f4460289dcc19f2a40e0 Mon Sep 17 00:00:00 2001 From: Adrian Mroz Date: Mon, 17 May 2021 15:27:36 +0200 Subject: [PATCH 4/6] Use limits in Split menu --- .../components/split-menu/limit-dropdown.tsx | 17 +++++++++-------- src/client/components/split-menu/split-menu.tsx | 5 +++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/client/components/split-menu/limit-dropdown.tsx b/src/client/components/split-menu/limit-dropdown.tsx index a58b15e6b..8b41f0e44 100644 --- a/src/client/components/split-menu/limit-dropdown.tsx +++ b/src/client/components/split-menu/limit-dropdown.tsx @@ -15,7 +15,6 @@ */ import * as React from "react"; -import { DEFAULT_LIMITS } from "../../../common/limit/limit"; import { Unary } from "../../../common/utils/functional/functional"; import { STRINGS } from "../../config/constants"; import { Dropdown } from "../dropdown/dropdown"; @@ -24,22 +23,24 @@ function formatLimit(limit: number): string { return limit === null ? "None" : String(limit); } -function calculateLimits(includeNone: boolean) { - if (!includeNone) return DEFAULT_LIMITS; - return [...DEFAULT_LIMITS, null]; +// TODO: Review again when fixing time split menu in #756 +function calculateLimits(limits: number[], includeNone: boolean) { + if (!includeNone) return limits; + return [...limits, null]; } export interface LimitDropdownProps { - limit: number; + selectedLimit: number; + limits: number[]; includeNone: boolean; onLimitSelect: Unary; } -export const LimitDropdown: React.SFC = ({ onLimitSelect, limit, includeNone }) => { +export const LimitDropdown: React.SFC = ({ onLimitSelect, limits, selectedLimit, includeNone }) => { return label={STRINGS.limit} - items={calculateLimits(includeNone)} - selectedItem={limit} + items={calculateLimits(limits, includeNone)} + selectedItem={selectedLimit} renderItem={formatLimit} onSelect={onLimitSelect} />; diff --git a/src/client/components/split-menu/split-menu.tsx b/src/client/components/split-menu/split-menu.tsx index 4d5c2c097..1ee3a7a77 100644 --- a/src/client/components/split-menu/split-menu.tsx +++ b/src/client/components/split-menu/split-menu.tsx @@ -157,8 +157,9 @@ export class SplitMenu extends React.Component { {this.renderSortDropdown()} + selectedLimit={limit} + includeNone={isContinuous(dimension)} + limits={dimension.limits}/>