Skip to content

Commit

Permalink
chore(query-nodes): camel case show_legend and show_values_on_series (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr authored Jan 19, 2024
1 parent 823bd0b commit 81e70e6
Show file tree
Hide file tree
Showing 21 changed files with 153 additions and 83 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -325,7 +325,7 @@ describe('filtersToQueryNode', () => {
kind: NodeKind.TrendsQuery,
trendsFilter: {
smoothingIntervals: 1,
show_legend: true,
showLegend: true,
hidden_legend_indexes: [0, 10],
compare: true,
aggregationAxisFormat: 'numeric',
Expand Down Expand Up @@ -500,7 +500,7 @@ describe('filtersToQueryNode', () => {
kind: NodeKind.StickinessQuery,
stickinessFilter: {
compare: true,
show_legend: true,
showLegend: true,
hidden_legend_indexes: [0, 10],
display: ChartDisplayType.ActionsLineGraph,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ export const filtersToQueryNode = (filters: Partial<FilterType>): InsightQueryNo
if (isTrendsFilter(filters) && isTrendsQuery(query)) {
query.trendsFilter = objectCleanWithEmpty({
smoothingIntervals: filters.smoothing_intervals,
show_legend: filters.show_legend,
showLegend: filters.show_legend,
hidden_legend_indexes: cleanHiddenLegendIndexes(filters.hidden_legend_keys),
compare: filters.compare,
aggregationAxisFormat: filters.aggregation_axis_format,
Expand All @@ -268,7 +268,7 @@ export const filtersToQueryNode = (filters: Partial<FilterType>): InsightQueryNo
decimalPlaces: filters.decimal_places,
formula: filters.formula,
display: filters.display,
show_values_on_series: filters.show_values_on_series,
showValuesOnSeries: filters.show_values_on_series,
showPercentStackView: filters.show_percent_stack_view,
showLabelsOnSeries: filters.show_labels_on_series,
})
Expand Down Expand Up @@ -333,17 +333,17 @@ export const filtersToQueryNode = (filters: Partial<FilterType>): InsightQueryNo
query.stickinessFilter = objectCleanWithEmpty({
display: filters.display,
compare: filters.compare,
show_legend: filters.show_legend,
showLegend: filters.show_legend,
hidden_legend_indexes: cleanHiddenLegendIndexes(filters.hidden_legend_keys),
show_values_on_series: filters.show_values_on_series,
showValuesOnSeries: filters.show_values_on_series,
})
}

// lifecycle filter
if (isLifecycleFilter(filters) && isLifecycleQuery(query)) {
query.lifecycleFilter = objectCleanWithEmpty({
toggledLifecycles: filters.toggledLifecycles,
show_values_on_series: filters.show_values_on_series,
showValuesOnSeries: filters.show_values_on_series,
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,12 @@ describe('queryNodeToFilter', () => {
formula: 'A + B',
display: ChartDisplayType.ActionsBar,
// breakdown_histogram_bin_count?: TrendsFilterLegacy['breakdown_histogram_bin_count']
// show_legend?: TrendsFilterLegacy['show_legend']
showLegend: true,
aggregationAxisFormat: 'numeric',
aggregationAxisPrefix: 'M',
aggregationAxisPostfix: '$',
decimalPlaces: 5,
// show_values_on_series?: TrendsFilterLegacy['show_values_on_series']
showValuesOnSeries: true,
showLabelsOnSeries: true,
showPercentStackView: true,
// hidden_legend_indexes?: TrendsFilterLegacy['hidden_legend_indexes']
Expand All @@ -84,6 +84,8 @@ describe('queryNodeToFilter', () => {
aggregation_axis_postfix: '$',
show_labels_on_series: true,
show_percent_stack_view: true,
show_legend: true,
show_values_on_series: true,
}
expect(result).toEqual(filters)
})
Expand Down
12 changes: 12 additions & 0 deletions frontend/src/queries/nodes/InsightQuery/utils/queryNodeToFilter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,13 +157,25 @@ export const queryNodeToFilter = (query: InsightQueryNode): Partial<FilterType>
legacyProps.aggregation_axis_prefix = insightFilter.aggregationAxisPrefix
legacyProps.show_labels_on_series = insightFilter.showLabelsOnSeries
legacyProps.show_percent_stack_view = insightFilter.showPercentStackView
legacyProps.show_legend = insightFilter.showLegend
legacyProps.show_values_on_series = insightFilter.showValuesOnSeries
delete insightFilter.smoothingIntervals
delete insightFilter.decimalPlaces
delete insightFilter.aggregationAxisFormat
delete insightFilter.aggregationAxisPostfix
delete insightFilter.aggregationAxisPrefix
delete insightFilter.showLabelsOnSeries
delete insightFilter.showPercentStackView
delete insightFilter.showLegend
delete insightFilter.showValuesOnSeries
} else if (isStickinessQuery(query)) {
legacyProps.show_legend = insightFilter.showLegend
legacyProps.show_values_on_series = insightFilter.showValuesOnSeries
delete insightFilter.showLegend
delete insightFilter.showValuesOnSeries
} else if (isLifecycleQuery(query)) {
legacyProps.show_values_on_series = insightFilter.showValuesOnSeries
delete insightFilter.showValuesOnSeries
}
Object.assign(filters, insightFilter)
Object.assign(filters, legacyProps)
Expand Down
10 changes: 5 additions & 5 deletions frontend/src/queries/nodes/InsightViz/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,21 +85,21 @@ export const getBreakdown = (query: InsightQueryNode): BreakdownFilter | undefin

export const getShowLegend = (query: InsightQueryNode): boolean | undefined => {
if (isStickinessQuery(query)) {
return query.stickinessFilter?.show_legend
return query.stickinessFilter?.showLegend
} else if (isTrendsQuery(query)) {
return query.trendsFilter?.show_legend
return query.trendsFilter?.showLegend
} else {
return undefined
}
}

export const getShowValueOnSeries = (query: InsightQueryNode): boolean | undefined => {
if (isLifecycleQuery(query)) {
return query.lifecycleFilter?.show_values_on_series
return query.lifecycleFilter?.showValuesOnSeries
} else if (isStickinessQuery(query)) {
return query.stickinessFilter?.show_values_on_series
return query.stickinessFilter?.showValuesOnSeries
} else if (isTrendsQuery(query)) {
return query.trendsFilter?.show_values_on_series
return query.trendsFilter?.showValuesOnSeries
} else {
return undefined
}
Expand Down
45 changes: 42 additions & 3 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1995,6 +1995,21 @@
"type": "string"
},
"LifecycleFilter": {
"additionalProperties": false,
"properties": {
"showValuesOnSeries": {
"type": "boolean"
},
"toggledLifecycles": {
"items": {
"$ref": "#/definitions/LifecycleToggle"
},
"type": "array"
}
},
"type": "object"
},
"LifecycleFilterLegacy": {
"additionalProperties": false,
"description": "`LifecycleFilterType` minus everything inherited from `FilterType`",
"properties": {
Expand Down Expand Up @@ -3499,6 +3514,30 @@
"type": "string"
},
"StickinessFilter": {
"additionalProperties": false,
"properties": {
"compare": {
"type": "boolean"
},
"display": {
"$ref": "#/definitions/ChartDisplayType"
},
"hidden_legend_indexes": {
"items": {
"type": "number"
},
"type": "array"
},
"showLegend": {
"type": "boolean"
},
"showValuesOnSeries": {
"type": "boolean"
}
},
"type": "object"
},
"StickinessFilterLegacy": {
"additionalProperties": false,
"description": "`StickinessFilterType` minus everything inherited from `FilterType` and persons modal related params and `hidden_legend_keys` replaced by `hidden_legend_indexes`",
"properties": {
Expand Down Expand Up @@ -3774,13 +3813,13 @@
"showLabelsOnSeries": {
"type": "boolean"
},
"showPercentStackView": {
"showLegend": {
"type": "boolean"
},
"show_legend": {
"showPercentStackView": {
"type": "boolean"
},
"show_values_on_series": {
"showValuesOnSeries": {
"type": "boolean"
},
"smoothingIntervals": {
Expand Down
21 changes: 17 additions & 4 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,13 +518,13 @@ export type TrendsFilter = {
compare?: TrendsFilterLegacy['compare']
formula?: TrendsFilterLegacy['formula']
display?: TrendsFilterLegacy['display']
show_legend?: TrendsFilterLegacy['show_legend']
showLegend?: TrendsFilterLegacy['show_legend']
breakdown_histogram_bin_count?: TrendsFilterLegacy['breakdown_histogram_bin_count'] // TODO: fully move into BreakdownFilter
aggregationAxisFormat?: TrendsFilterLegacy['aggregation_axis_format']
aggregationAxisPrefix?: TrendsFilterLegacy['aggregation_axis_prefix']
aggregationAxisPostfix?: TrendsFilterLegacy['aggregation_axis_postfix']
decimalPlaces?: TrendsFilterLegacy['decimal_places']
show_values_on_series?: TrendsFilterLegacy['show_values_on_series']
showValuesOnSeries?: TrendsFilterLegacy['show_values_on_series']
showLabelsOnSeries?: TrendsFilterLegacy['show_labels_on_series']
showPercentStackView?: TrendsFilterLegacy['show_percent_stack_view']
hidden_legend_indexes?: TrendsFilterLegacy['hidden_legend_indexes']
Expand Down Expand Up @@ -611,11 +611,19 @@ export interface PathsQuery extends InsightsQueryBase {

/** `StickinessFilterType` minus everything inherited from `FilterType` and persons modal related params
* and `hidden_legend_keys` replaced by `hidden_legend_indexes` */
export type StickinessFilter = Omit<
export type StickinessFilterLegacy = Omit<
StickinessFilterType & { hidden_legend_indexes?: number[] },
keyof FilterType | 'hidden_legend_keys' | 'stickiness_days' | 'shown_as'
>

export type StickinessFilter = {
compare?: StickinessFilterLegacy['compare']
display?: StickinessFilterLegacy['display']
showLegend?: StickinessFilterLegacy['show_legend']
showValuesOnSeries?: StickinessFilterLegacy['show_values_on_series']
hidden_legend_indexes?: StickinessFilterLegacy['hidden_legend_indexes']
}

export interface StickinessQueryResponse extends QueryResponse {
results: Record<string, any>[]
}
Expand All @@ -631,11 +639,16 @@ export interface StickinessQuery extends Omit<InsightsQueryBase, 'aggregation_gr
}

/** `LifecycleFilterType` minus everything inherited from `FilterType` */
export type LifecycleFilter = Omit<LifecycleFilterType, keyof FilterType | 'shown_as'> & {
export type LifecycleFilterLegacy = Omit<LifecycleFilterType, keyof FilterType | 'shown_as'> & {
/** Lifecycles that have been removed from display are not included in this array */
toggledLifecycles?: LifecycleToggle[]
} // using everything except what it inherits from FilterType

export type LifecycleFilter = {
showValuesOnSeries?: LifecycleFilterLegacy['show_values_on_series']
toggledLifecycles?: LifecycleFilterLegacy['toggledLifecycles']
}

export interface QueryRequest {
/** Client provided query ID. Can be used to retrieve the status or cancel the query. */
client_query_id?: string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export function ShowLegendFilter(): JSX.Element | null {
const { updateInsightFilter } = useActions(insightVizDataLogic(insightProps))

const toggleShowLegend = (): void => {
updateInsightFilter({ show_legend: !showLegend })
updateInsightFilter({ showLegend: !showLegend })
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export function ValueOnSeriesFilter(): JSX.Element {
className="p-1 px-2"
checked={valueOnSeries}
onChange={(checked) => {
updateInsightFilter({ show_values_on_series: checked })
updateInsightFilter({ showValuesOnSeries: checked })
}}
label={<span className="font-normal">Show values on series</span>}
size="small"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ describe('insightNavLogic', () => {
event: '$pageview',
},
],
trendsFilter: { show_values_on_series: true },
trendsFilter: { showValuesOnSeries: true },
},
}
const funnelsQuery: InsightVizNode = {
Expand Down Expand Up @@ -229,7 +229,7 @@ describe('insightNavLogic', () => {
builtInsightDataLogic.actions.setQuery(trendsQuery)
}).toMatchValues({
queryPropertyCache: expect.objectContaining({
commonFilter: { show_values_on_series: true },
commonFilter: { showValuesOnSeries: true },
}),
})

Expand All @@ -238,7 +238,7 @@ describe('insightNavLogic', () => {
}).toMatchValues({
queryPropertyCache: expect.objectContaining({
commonFilter: {
show_values_on_series: true,
showValuesOnSeries: true,
funnel_order_type: 'strict',
funnel_viz_type: 'steps',
},
Expand Down Expand Up @@ -283,7 +283,7 @@ describe('insightNavLogic', () => {
kind: 'LifecycleQuery',
series: [{ kind: 'EventsNode', name: '$pageview', event: '$pageview' }],
filterTestAccounts: true,
lifecycleFilter: { show_values_on_series: true },
lifecycleFilter: { showValuesOnSeries: true },
},
} as Node),
])
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/InsightNav/insightNavLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -339,8 +339,8 @@ const mergeCachedProperties = (query: InsightQueryNode, cache: QueryPropertyCach
// TODO: fix an issue where switching between trends and funnels with the option enabled would
// result in an error before uncommenting
// ...(getCompare(node) ? { compare: getCompare(node) } : {}),
...(getShowValueOnSeries(node) ? { show_values_on_series: getShowValueOnSeries(node) } : {}),
...(getShowPercentStackView(node) ? { show_percent_stack_view: getShowPercentStackView(node) } : {}),
...(getShowValueOnSeries(node) ? { showValuesOnSeries: getShowValueOnSeries(node) } : {}),
...(getShowPercentStackView(node) ? { showPercentStackView: getShowPercentStackView(node) } : {}),
...(getDisplay(node) ? { display: getDisplay(node) } : {}),
}
}
Expand Down
16 changes: 0 additions & 16 deletions frontend/src/scenes/insights/insightLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,22 +248,6 @@ describe('insightLogic', () => {
})

describe('insight legend', () => {
it('toggles insight legend', async () => {
logic = insightLogic({
dashboardItemId: undefined,
filters: { show_legend: false },
})
logic.mount()

await expectLogic(logic, () => {
logic.actions.toggleInsightLegend()
})
.toDispatchActions(['toggleInsightLegend', 'setFilters'])
.toMatchValues({
filters: partial({ show_legend: true }),
})
})

it('initialize insight with hidden keys', async () => {
logic = insightLogic({
dashboardItemId: undefined,
Expand Down
8 changes: 0 additions & 8 deletions frontend/src/scenes/insights/insightLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,6 @@ export const insightLogic = kea<insightLogicType>([
callback,
}),
setInsightMetadata: (metadata: Partial<InsightModel>) => ({ metadata }),
toggleInsightLegend: true,
toggleVisibility: (index: number) => ({ index }),
highlightSeries: (seriesIndex: number | null) => ({ seriesIndex }),
}),
Expand Down Expand Up @@ -694,13 +693,6 @@ export const insightLogic = kea<insightLogicType>([
loadInsightSuccess: async ({ insight }) => {
actions.reportInsightViewed(insight, insight?.filters || {})
},
toggleInsightLegend: () => {
const newFilters: Partial<TrendsFilterType> = {
...values.filters,
show_legend: !(values.filters as Partial<TrendsFilterType>).show_legend,
}
actions.setFilters(newFilters)
},
toggleVisibility: ({ index }) => {
const currentIsHidden = !!values.hiddenLegendKeys?.[index]
const newFilters: Partial<TrendsFilterType> = {
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/scenes/insights/insightVizDataLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ describe('insightVizDataLogic', () => {
// merges with existing insight filter
expectLogic(builtInsightDataLogic, () => {
builtInsightVizDataLogic.actions.updateInsightFilter({
show_values_on_series: true,
showValuesOnSeries: true,
})
}).toMatchValues({
query: {
Expand All @@ -213,15 +213,15 @@ describe('insightVizDataLogic', () => {
...trendsQueryDefault,
trendsFilter: {
display: 'ActionsAreaGraph',
show_values_on_series: true,
showValuesOnSeries: true,
},
},
},
})

expect(builtInsightVizDataLogic.values.insightFilter).toEqual({
display: 'ActionsAreaGraph',
show_values_on_series: true,
showValuesOnSeries: true,
})
})

Expand Down
Loading

0 comments on commit 81e70e6

Please sign in to comment.