Skip to content

Commit

Permalink
convert most of the usages
Browse files Browse the repository at this point in the history
  • Loading branch information
thmsobrmlr committed Sep 26, 2023
1 parent 02cc117 commit 1654a0a
Show file tree
Hide file tree
Showing 19 changed files with 70 additions and 73 deletions.
4 changes: 2 additions & 2 deletions frontend/src/lib/components/UnitPicker/CustomUnitModal.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ function chooseFormativeElementValue(
trendsFilter: TrendsFilter | null | undefined
): string {
if (formativeElement === 'prefix') {
return trendsFilter?.aggregation_axis_prefix || ''
return trendsFilter?.aggregationAxisPrefix || ''
}

if (formativeElement === 'postfix') {
return trendsFilter?.aggregation_axis_postfix || ''
return trendsFilter?.aggregationAxisPostfix || ''
}

return ''
Expand Down
28 changes: 14 additions & 14 deletions frontend/src/lib/components/UnitPicker/UnitPicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ export function UnitPicker(): JSX.Element {
const { reportAxisUnitsChanged } = useActions(eventUsageLogic)

const [isVisible, setIsVisible] = useState(false)
const [localAxisFormat, setLocalAxisFormat] = useState(trendsFilter?.aggregation_axis_format || undefined)
const [localAxisFormat, setLocalAxisFormat] = useState(trendsFilter?.aggregationAxisFormat || undefined)
const [customUnitModal, setCustomUnitModal] = useState<'prefix' | 'postfix' | null>(null)

const customUnitModalRef = useRef<HTMLDivElement | null>(null)
Expand All @@ -50,9 +50,9 @@ export function UnitPicker(): JSX.Element {
setLocalAxisFormat(format)

updateInsightFilter({
aggregation_axis_format: format,
aggregation_axis_prefix: prefix,
aggregation_axis_postfix: postfix,
aggregationAxisFormat: format,
aggregationAxisPrefix: prefix,
aggregationAxisPostfix: postfix,
})

reportAxisUnitsChanged({
Expand All @@ -72,11 +72,11 @@ export function UnitPicker(): JSX.Element {
if (localAxisFormat) {
displayValue = aggregationDisplayMap[localAxisFormat]
}
if (trendsFilter?.aggregation_axis_prefix?.length) {
displayValue = `Prefix: ${trendsFilter?.aggregation_axis_prefix}`
if (trendsFilter?.aggregationAxisPrefix?.length) {
displayValue = `Prefix: ${trendsFilter?.aggregationAxisPrefix}`
}
if (trendsFilter?.aggregation_axis_postfix?.length) {
displayValue = `Postfix: ${trendsFilter?.aggregation_axis_postfix}`
if (trendsFilter?.aggregationAxisPostfix?.length) {
displayValue = `Postfix: ${trendsFilter?.aggregationAxisPostfix}`
}
return displayValue
}, [localAxisFormat, trendsFilter])
Expand Down Expand Up @@ -121,23 +121,23 @@ export function UnitPicker(): JSX.Element {
<LemonButton
onClick={() => setCustomUnitModal('prefix')}
status="stealth"
active={!!trendsFilter?.aggregation_axis_prefix}
active={!!trendsFilter?.aggregationAxisPrefix}
fullWidth
>
Custom prefix
{trendsFilter?.aggregation_axis_prefix
? `: ${trendsFilter?.aggregation_axis_prefix}...`
{trendsFilter?.aggregationAxisPrefix
? `: ${trendsFilter?.aggregationAxisPrefix}...`
: '...'}
</LemonButton>
<LemonButton
onClick={() => setCustomUnitModal('postfix')}
status="stealth"
active={!!trendsFilter?.aggregation_axis_postfix}
active={!!trendsFilter?.aggregationAxisPostfix}
fullWidth
>
Custom postfix
{trendsFilter?.aggregation_axis_postfix
? `: ${trendsFilter?.aggregation_axis_postfix}...`
{trendsFilter?.aggregationAxisPostfix
? `: ${trendsFilter?.aggregationAxisPostfix}...`
: '...'}
</LemonButton>
</>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/lib/lemon-ui/LemonRow/LemonRow.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Spinner } from '../Spinner/Spinner'
import React from 'react'

// Fix for function type inference in forwardRef, so that function components wrapped with forwardRef can be generic.
// For some reason the @types/react definitons as React 16 and TS 4.9 don't work, because `P` (the props) is wrapped in
// For some reason the @types/react definitions as React 16 and TS 4.9 don't work, because `P` (the props) is wrapped in
// `Pick` (inside `React.PropsWithoutRef`), which breaks TypeScript's ability to reason about it as a generic type.
// `Omit` has the same effect. It's probably fine to just use `P` directly in `ForwardRefExoticComponent`.
declare module 'react' {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,12 +312,12 @@ describe('filtersToQueryNode', () => {
kind: NodeKind.TrendsQuery,
trendsFilter: {
smoothing_intervals: 1,
show_legend: true,
showLegend: true,
hidden_legend_indexes: [0, 10],
compare: true,
aggregation_axis_format: 'numeric',
aggregation_axis_prefix: '£',
aggregation_axis_postfix: '%',
aggregationAxisFormat: 'numeric',
aggregationAxisPrefix: '£',
aggregationAxisPostfix: '%',
formula: 'A+B',
shown_as: ShownAsValue.VOLUME,
display: ChartDisplayType.ActionsAreaGraph,
Expand Down Expand Up @@ -369,7 +369,7 @@ describe('filtersToQueryNode', () => {
funnel_viz_type: FunnelVizType.Steps,
funnel_from_step: 1,
funnel_to_step: 2,
funnel_step_reference: FunnelStepReference.total,
funnelStepReference: FunnelStepReference.total,
breakdown_attribution_type: BreakdownAttributionType.AllSteps,
breakdown_attribution_value: 1,
bin_count: 'auto',
Expand Down Expand Up @@ -473,7 +473,6 @@ describe('filtersToQueryNode', () => {
insight: InsightType.STICKINESS,
compare: true,
show_legend: true,
hidden_legend_keys: { 0: true, 10: true },
shown_as: ShownAsValue.STICKINESS,
display: ChartDisplayType.ActionsLineGraph,
}
Expand All @@ -485,7 +484,6 @@ describe('filtersToQueryNode', () => {
stickinessFilter: {
compare: true,
show_legend: true,
hidden_legend_indexes: [0, 10],
shown_as: ShownAsValue.STICKINESS,
display: ChartDisplayType.ActionsLineGraph,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ export function InsightDisplayConfig({ disableTable }: InsightDisplayConfigProps
(showPercentStackView && isPercentStackViewOn ? 1 : 0) +
(!isPercentStackViewOn &&
showUnit &&
trendsFilter?.aggregation_axis_format &&
trendsFilter.aggregation_axis_format !== 'numeric'
trendsFilter?.aggregationAxisFormat &&
trendsFilter.aggregationAxisFormat !== 'numeric'
? 1
: 0) +
(hasLegend && showLegend ? 1 : 0)
Expand Down
12 changes: 6 additions & 6 deletions frontend/src/queries/nodes/InsightViz/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,29 +96,29 @@ export const getShownAs = (query: InsightQueryNode): ShownAsValue | undefined =>

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
}
}

export const getShowPercentStackView = (query: InsightQueryNode): boolean | undefined => {
if (isTrendsQuery(query)) {
return query.trendsFilter?.show_percent_stack_view
return query.trendsFilter?.showPercentStackView
} else {
return undefined
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export function FunnelBarGraph({
const { ref: graphRef, width } = useResizeObserver()

const steps = visibleStepsWithConversionMetrics
const stepReference = funnelsFilter?.funnel_step_reference || FunnelStepReference.total
const stepReference = funnelsFilter?.funnelStepReference || FunnelStepReference.total

const showPersonsModal = canOpenPersonModal && showPersonsModalProp

Expand Down
2 changes: 2 additions & 0 deletions frontend/src/scenes/funnels/ValueInspectorButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,4 +35,6 @@ export const ValueInspectorButton = React.forwardRef<HTMLElement, ValueInspector
)
}
)

// @ts-expect-error
ValueInspectorButton.displayName = 'ValueInspectorButton'
8 changes: 4 additions & 4 deletions frontend/src/scenes/funnels/funnelDataLogic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ export const funnelDataLogic = kea<funnelDataLogicType>([
stepsWithConversionMetrics: [
(s) => [s.steps, s.funnelsFilter],
(steps, funnelsFilter): FunnelStepWithConversionMetrics[] => {
const stepReference = funnelsFilter?.funnel_step_reference || FunnelStepReference.total
const stepReference = funnelsFilter?.funnelStepReference || FunnelStepReference.total
return stepsWithConversionMetrics(steps, stepReference)
},
],
Expand Down Expand Up @@ -356,7 +356,7 @@ export const funnelDataLogic = kea<funnelDataLogicType>([
],

/*
* Advanced options: funnel_order_type, funnel_step_reference, exclusions
* Advanced options: funnel_order_type, funnelStepReference, exclusions
*/
advancedOptionsUsedCount: [
(s) => [s.funnelsFilter],
Expand All @@ -366,8 +366,8 @@ export const funnelDataLogic = kea<funnelDataLogicType>([
count = count + 1
}
if (
funnelsFilter?.funnel_step_reference &&
funnelsFilter?.funnel_step_reference !== FunnelStepReference.total
funnelsFilter?.funnelStepReference &&
funnelsFilter?.funnelStepReference !== FunnelStepReference.total
) {
count = count + 1
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function FunnelsAdvanced({ insightProps }: EditorFilterProps): JSX.Elemen
onClick={() => {
updateInsightFilter({
funnel_order_type: undefined,
funnel_step_reference: undefined,
funnelStepReference: undefined,
exclusions: undefined,
})
}}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export function PercentStackViewFilter(): JSX.Element {
className="p-1 px-2"
checked={!!showPercentStackView}
onChange={(checked) => {
updateInsightFilter({ show_percent_stack_view: checked })
updateInsightFilter({ showPercentStackView: checked })
}}
label={<span className="font-normal">Show as % of total</span>}
size="small"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,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 @@ -37,7 +37,7 @@ export const valueOnSeriesFilterLogic = kea<valueOnSeriesFilterLogicType>([

listeners(({ actions }) => ({
setValueOnSeries: ({ checked }) => {
actions.updateInsightFilter({ show_values_on_series: checked })
actions.updateInsightFilter({ showValuesOnSeries: checked })
},
})),
])
10 changes: 4 additions & 6 deletions frontend/src/scenes/insights/aggregationAxisFormat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ export const INSIGHT_UNIT_OPTIONS: LemonSelectOptionLeaf<AggregationAxisFormat>[
]

export const formatAggregationAxisValue = (
trendsFilter: TrendsFilter | null | undefined | Partial<TrendsFilterType>,
trendsFilter: TrendsFilter | null | undefined,
value: number | string
): string => {
value = Number(value)
let formattedValue = humanFriendlyNumber(value)
if (trendsFilter?.aggregation_axis_format) {
switch (trendsFilter?.aggregation_axis_format) {
if (trendsFilter?.aggregationAxisFormat) {
switch (trendsFilter?.aggregationAxisFormat) {
case 'duration':
formattedValue = humanFriendlyDuration(value)
break
Expand All @@ -39,9 +39,7 @@ export const formatAggregationAxisValue = (
break
}
}
return `${trendsFilter?.aggregation_axis_prefix || ''}${formattedValue}${
trendsFilter?.aggregation_axis_postfix || ''
}`
return `${trendsFilter?.aggregationAxisPrefix || ''}${formattedValue}${trendsFilter?.aggregationAxisPostfix || ''}`
}

export const formatPercentStackAxisValue = (
Expand Down
35 changes: 17 additions & 18 deletions frontend/src/scenes/insights/aggregationAxisFormats.test.ts
Original file line number Diff line number Diff line change
@@ -1,34 +1,33 @@
import { formatAggregationAxisValue } from 'scenes/insights/aggregationAxisFormat'
import { FilterType } from '~/types'
import { TrendsFilter } from '~/queries/schema'

describe('formatAggregationAxisValue', () => {
const formatTestcases = [
{ candidate: 34, filters: { aggregation_axis_format: 'duration' }, expected: '34s' },
{ candidate: 340, filters: { aggregation_axis_format: 'duration' }, expected: '5m 40s' },
{ candidate: 3940, filters: { aggregation_axis_format: 'duration' }, expected: '1h 5m 40s' },
{ candidate: 3.944, filters: { aggregation_axis_format: 'percentage' }, expected: '3.94%' },
{ candidate: 3.956, filters: { aggregation_axis_format: 'percentage' }, expected: '3.96%' },
{ candidate: 3940, filters: { aggregation_axis_format: 'percentage' }, expected: '3,940%' },
{ candidate: 34, filters: { aggregation_axis_format: 'numeric' }, expected: '34' },
{ candidate: 394, filters: { aggregation_axis_format: 'numeric' }, expected: '394' },
{ candidate: 3940, filters: { aggregation_axis_format: 'numeric' }, expected: '3,940' },
const formatTestcases: { candidate: number; filters: TrendsFilter; expected: string }[] = [
{ candidate: 34, filters: { aggregationAxisFormat: 'duration' }, expected: '34s' },
{ candidate: 340, filters: { aggregationAxisFormat: 'duration' }, expected: '5m 40s' },
{ candidate: 3940, filters: { aggregationAxisFormat: 'duration' }, expected: '1h 5m 40s' },
{ candidate: 3.944, filters: { aggregationAxisFormat: 'percentage' }, expected: '3.94%' },
{ candidate: 3.956, filters: { aggregationAxisFormat: 'percentage' }, expected: '3.96%' },
{ candidate: 3940, filters: { aggregationAxisFormat: 'percentage' }, expected: '3,940%' },
{ candidate: 34, filters: { aggregationAxisFormat: 'numeric' }, expected: '34' },
{ candidate: 394, filters: { aggregationAxisFormat: 'numeric' }, expected: '394' },
{ candidate: 3940, filters: { aggregationAxisFormat: 'numeric' }, expected: '3,940' },
{ candidate: 3940, filters: {}, expected: '3,940' },
{ candidate: 3940, filters: { aggregation_axis_format: 'unexpected' }, expected: '3,940' },
// @ts-expect-error
{ candidate: 3940, filters: { aggregationAxisFormat: 'unexpected' }, expected: '3,940' },
{
candidate: 3940,
filters: {
aggregation_axis_format: 'numeric',
aggregation_axis_prefix: '£',
aggregation_axis_postfix: '💖',
aggregationAxisFormat: 'numeric',
aggregationAxisPrefix: '£',
aggregationAxisPostfix: '💖',
},
expected: '£3,940💖',
},
]
formatTestcases.forEach((testcase) => {
it(`correctly formats "${testcase.candidate}" as ${testcase.expected} when filters are ${testcase.filters}`, () => {
expect(formatAggregationAxisValue(testcase.filters as Partial<FilterType>, testcase.candidate)).toEqual(
testcase.expected
)
expect(formatAggregationAxisValue(testcase.filters, testcase.candidate)).toEqual(testcase.expected)
})
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export function FunnelStepReferencePicker(): JSX.Element | null {
const { insightFilter } = useValues(funnelDataLogic(insightProps))
const { updateInsightFilter } = useActions(funnelDataLogic(insightProps))

const { funnel_step_reference } = (insightFilter || {}) as FunnelsFilter
const { funnelStepReference } = (insightFilter || {}) as FunnelsFilter

const options = [
{
Expand All @@ -27,8 +27,8 @@ export function FunnelStepReferencePicker(): JSX.Element | null {

return (
<LemonSelect
value={funnel_step_reference || FunnelStepReference.total}
onChange={(stepRef) => stepRef && updateInsightFilter({ funnel_step_reference: stepRef })}
value={funnelStepReference || FunnelStepReference.total}
onChange={(stepRef) => stepRef && updateInsightFilter({ funnelStepReference: stepRef })}
dropdownMatchSelectWidth={false}
data-attr="funnel-step-reference-selector"
options={options}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/scenes/insights/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,15 +180,15 @@ describe('formatAggregationValue', () => {

it('uses render count when there is a value and property format is a no-op', () => {
const fakeRenderCount = (x: number): string =>
formatAggregationAxisValue({ aggregation_axis_format: 'duration' }, x)
formatAggregationAxisValue({ aggregationAxisFormat: 'duration' }, x)
const noOpFormatProperty = jest.fn((_, y) => y)
const actual = formatAggregationValue('some name', 500, fakeRenderCount, noOpFormatProperty)
expect(actual).toEqual('8m 20s')
})

it('uses render count when there is a value and property format converts number to string', () => {
const fakeRenderCount = (x: number): string =>
formatAggregationAxisValue({ aggregation_axis_format: 'duration' }, x)
formatAggregationAxisValue({ aggregationAxisFormat: 'duration' }, x)
const noOpFormatProperty = jest.fn((_, y) => String(y))
const actual = formatAggregationValue('some name', 500, fakeRenderCount, noOpFormatProperty)
expect(actual).toEqual('8m 20s')
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/scenes/insights/views/LineGraph/PieChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function PieChart({
const tooltipEl = ensureTooltipElement()
if (tooltip.opacity === 0) {
// remove highlight from the legend
if (trendsFilter?.show_legend) {
if (trendsFilter?.showLegend) {
highlightSeries(null)
}
tooltipEl.style.opacity = '0'
Expand Down
Loading

0 comments on commit 1654a0a

Please sign in to comment.