Skip to content

Commit

Permalink
feat(bi): Added support for number and bool vars (#25633)
Browse files Browse the repository at this point in the history 
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
@Gilbert09 @github-actions
Gilbert09 and github-actions[bot] authored Oct 18, 2024
1 parent a147c50 commit 6ca6a43
Show file tree
Hide file tree
Showing 6 changed files with 97 additions and 37 deletions.
Binary file modified frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--dark.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified frontend/__snapshots__/exporter-exporter--trends-line-insight-detailed--light.png
View file
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion frontend/src/queries/nodes/DataVisualization/Components/Variables/AddVariableButton.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const AddVariableButton = (): JSX.Element => {
]
: variables.map((n) => ({
label: n.name,
onClick: () => addVariable({ variableId: n.id, code_name: '' }),
onClick: () => addVariable({ variableId: n.id, code_name: n.code_name }),
})),
},
]}
Expand Down
69 changes: 55 additions & 14 deletions frontend/src/queries/nodes/DataVisualization/Components/Variables/Variables.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import './Variables.scss'

import { IconCopy, IconGear, IconTrash } from '@posthog/icons'
import { LemonButton, LemonDivider, LemonInput, Popover } from '@posthog/lemon-ui'
import { LemonButton, LemonDivider, LemonInput, LemonSegmentedButton, Popover } from '@posthog/lemon-ui'
import { useActions, useValues } from 'kea'
import { FEATURE_FLAGS } from 'lib/constants'
import { LemonField } from 'lib/lemon-ui/LemonField'
Expand Down Expand Up @@ -88,7 +88,15 @@ const VariableInput = ({
onChange,
onRemove,
}: VariableInputProps): JSX.Element => {
const [localInputValue, setLocalInputValue] = useState(variable.value ?? variable.default_value ?? '')
const [localInputValue, setLocalInputValue] = useState(() => {
const val = variable.value ?? variable.default_value

if (variable.type === 'Number' && !val) {
return 0
}

return val ?? ''
})

const inputRef = useRef<HTMLInputElement>(null)
const codeRef = useRef<HTMLElement>(null)
Expand All @@ -102,17 +110,50 @@ const VariableInput = ({
return (
<div>
<div className="flex gap-1 p-1">
<LemonInput
inputRef={inputRef}
placeholder="Value..."
className="flex flex-1"
value={localInputValue.toString()}
onChange={(value) => setLocalInputValue(value)}
onPressEnter={() => {
onChange(variable.id, localInputValue)
closePopover()
}}
/>
{variable.type === 'String' && (
<LemonInput
inputRef={inputRef}
placeholder="Value..."
className="flex flex-1"
value={localInputValue.toString()}
onChange={(value) => setLocalInputValue(value)}
onPressEnter={() => {
onChange(variable.id, localInputValue)
closePopover()
}}
/>
)}
{variable.type === 'Number' && (
<LemonInput
type="number"
inputRef={inputRef}
placeholder="Value..."
className="flex flex-1"
value={Number(localInputValue)}
onChange={(value) => setLocalInputValue(value ?? 0)}
onPressEnter={() => {
onChange(variable.id, localInputValue)
closePopover()
}}
/>
)}
{variable.type === 'Boolean' && (
<LemonSegmentedButton
className="grow"
value={localInputValue ? 'true' : 'false'}
onChange={(value) => setLocalInputValue(value === 'true')}
options={[
{
value: 'true',
label: 'true',
},
{
value: 'false',
label: 'false',
},
]}
/>
)}
<LemonButton
type="primary"
onClick={() => {
Expand Down Expand Up @@ -213,7 +254,7 @@ const VariableComponent = ({
onClick={() => setPopoverOpen(!isPopoverOpen)}
disabledReason={variableOverridesAreSet && 'Discard dashboard variables to change'}
>
{variable.value ?? variable.default_value}
{variable.value?.toString() ?? variable.default_value?.toString()}
</LemonButton>
</LemonField.Pure>
</div>
Expand Down
57 changes: 36 additions & 21 deletions frontend/src/queries/nodes/DataVisualization/Components/Variables/variablesLogic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { getVariablesFromQuery, haveVariablesOrFiltersChanged } from 'scenes/ins
import { DataVisualizationNode, HogQLVariable } from '~/queries/schema'

import { dataVisualizationLogic } from '../../dataVisualizationLogic'
import { Variable } from '../../types'
import { Variable, VariableType } from '../../types'
import { variableDataLogic } from './variableDataLogic'
import type { variablesLogicType } from './variablesLogicType'

Expand All @@ -17,6 +17,18 @@ export interface VariablesLogicProps {
readOnly: boolean
}

const convertValueToCorrectType = (value: string, type: VariableType): number | string | boolean => {
if (type === 'Number') {
return Number(value)
}

if (type === 'Boolean' && typeof value === 'string') {
return value.toLowerCase() === 'true'
}

return value
}

export const variablesLogic = kea<variablesLogicType>([
path(['queries', 'nodes', 'DataVisualization', 'Components', 'Variables', 'variablesLogic']),
props({ key: '' } as VariablesLogicProps),
Expand All @@ -32,15 +44,18 @@ export const variablesLogic = kea<variablesLogicType>([
['featureFlags'],
],
}),
actions({
actions(({ values }) => ({
addVariable: (variable: HogQLVariable) => ({ variable }),
addVariables: (variables: HogQLVariable[]) => ({ variables }),
removeVariable: (variableId: string) => ({ variableId }),
updateVariableValue: (variableId: string, value: any) => ({ variableId, value }),
updateVariableValue: (variableId: string, value: any) => ({
variableId,
value,
allVariables: values.variables,
}),
setEditorQuery: (query: string) => ({ query }),
resetVariables: true,
updateSourceQuery: true,
}),
})),
reducers({
internalSelectedVariables: [
[] as HogQLVariable[],
Expand All @@ -52,30 +67,32 @@ export const variablesLogic = kea<variablesLogicType>([

return [...state, { ...variable }]
},
addVariables: (state, { variables }) => {
return [...state, ...variables.map((n) => ({ ...n }))]
addVariables: (_state, { variables }) => {
return [...variables.map((n) => ({ ...n }))]
},
updateVariableValue: (state, { variableId, value }) => {
updateVariableValue: (state, { variableId, value, allVariables }) => {
const variableIndex = state.findIndex((n) => n.variableId === variableId)
if (variableIndex < 0) {
return state
}

const variableType = allVariables.find((n) => n.id === variableId)?.type
const valueWithType = convertValueToCorrectType(value, variableType ?? 'String')

const variablesInState = [...state]
variablesInState[variableIndex] = { ...variablesInState[variableIndex], value }
variablesInState[variableIndex] = { ...variablesInState[variableIndex], value: valueWithType }

return variablesInState
},
removeVariable: (state, { variableId }) => {
const stateCopy = [...state]
const index = stateCopy.findIndex((n) => n.variableId === variableId)
if (index >= 0) {
stateCopy.splice(index)
stateCopy.splice(index, 1)
}

return stateCopy
},
resetVariables: () => [],
},
],
editorQuery: [
Expand All @@ -88,9 +105,9 @@ export const variablesLogic = kea<variablesLogicType>([
}),
selectors({
variablesForInsight: [
(s) => [s.variables, s.internalSelectedVariables, s.variablesLoading],
(variables, internalSelectedVariables, variablesLoading): Variable[] => {
if (!variables.length || !internalSelectedVariables.length || variablesLoading) {
(s) => [s.variables, s.internalSelectedVariables],
(variables, internalSelectedVariables): Variable[] => {
if (!variables.length || !internalSelectedVariables.length) {
return []
}

Expand Down Expand Up @@ -128,16 +145,16 @@ export const variablesLogic = kea<variablesLogicType>([
return
}

const variables = values.variablesForInsight
const variables = values.internalSelectedVariables

const query: DataVisualizationNode = {
...values.query,
source: {
...values.query.source,
variables: variables.reduce((acc, cur) => {
if (cur.id) {
acc[cur.id] = {
variableId: cur.id,
if (cur.variableId) {
acc[cur.variableId] = {
variableId: cur.variableId,
value: cur.value,
code_name: cur.code_name,
}
Expand Down Expand Up @@ -176,7 +193,7 @@ export const variablesLogic = kea<variablesLogicType>([
return
}

const variableAlreadySelected = values.variablesForInsight.find((n) => n.code_name === match)
const variableAlreadySelected = values.internalSelectedVariables.find((n) => n.code_name === match)
if (!variableAlreadySelected) {
actions.addVariable({ variableId: variableExists.id, code_name: variableExists.code_name })
}
Expand All @@ -187,8 +204,6 @@ export const variablesLogic = kea<variablesLogicType>([
return
}

actions.resetVariables()

const variables = Object.values(query.source.variables ?? {})

if (variables.length) {
Expand Down
6 changes: 5 additions & 1 deletion posthog/hogql/variables.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,11 @@ def visit_placeholder(self, node):
if not matching_insight_variable:
raise QueryError(f"Variable {variable_code_name} does not exist")

value = matching_variable.value or matching_insight_variable[0].default_value
value = (
matching_variable.value
if matching_variable.value is not None
else matching_insight_variable[0].default_value
)

return ast.Constant(value=value)

Expand Down

0 comments on commit 6ca6a43

Please sign in to comment.