Skip to content

Commit

Permalink
feat(insights): Add dashboard filters to hogql insights (#18076)
Browse files Browse the repository at this point in the history
* WIP

* Kinda sorta working

* Add dashboard filters to cache key

* Make mypy happy

* Add support for properties

* Use get_query_runner

* Add tests for dashboard_query

* Add tests for merging of filters

* Add test for hash when dashboard filters are applied

* Silently accept having no valid query runner

* Remove index type from DashboardFilter

* Fix type of dashboard_query

* Fix test typing
  • Loading branch information
robbie-c authored Oct 20, 2023
1 parent af0c46a commit b3c4d41
Show file tree
Hide file tree
Showing 11 changed files with 240 additions and 10 deletions.
25 changes: 25 additions & 0 deletions frontend/src/queries/schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,31 @@
],
"type": "string"
},
"DashboardFilter": {
"additionalProperties": false,
"properties": {
"date_from": {
"type": ["string", "null"]
},
"date_to": {
"type": ["string", "null"]
},
"properties": {
"anyOf": [
{
"items": {
"$ref": "#/definitions/AnyPropertyFilter"
},
"type": "array"
},
{
"type": "null"
}
]
}
},
"type": "object"
},
"DataNode": {
"additionalProperties": false,
"properties": {
Expand Down
6 changes: 6 additions & 0 deletions frontend/src/queries/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -699,3 +699,9 @@ export interface BreakdownFilter {
breakdown_group_type_index?: number | null
breakdown_histogram_bin_count?: number // trends breakdown histogram bin count
}

export interface DashboardFilter {
date_from?: string | null
date_to?: string | null
properties?: AnyPropertyFilter[] | null
}
4 changes: 2 additions & 2 deletions frontend/src/scenes/dashboard/dashboardLogic.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import { eventUsageLogic } from 'lib/utils/eventUsageLogic'
import {
DashboardTile,
DashboardType,
FilterType,
InsightColor,
InsightModel,
InsightShortId,
Expand All @@ -24,6 +23,7 @@ import { dayjs, now } from 'lib/dayjs'
import { teamLogic } from 'scenes/teamLogic'
import { MOCK_TEAM_ID } from 'lib/api.mock'
import api from 'lib/api'
import { DashboardFilter } from '~/queries/schema'

const dashboardJson = _dashboardJson as any as DashboardType

Expand Down Expand Up @@ -63,7 +63,7 @@ export const tileFromInsight = (insight: InsightModel, id: number = tileId++): D
export const dashboardResult = (
dashboardId: number,
tiles: DashboardTile[],
filters: Partial<Pick<FilterType, 'date_from' | 'date_to' | 'properties'>> = {}
filters: Partial<DashboardFilter> = {}
): DashboardType => {
return {
...dashboardJson,
Expand Down
12 changes: 9 additions & 3 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,13 @@ import { BehavioralFilterKey, BehavioralFilterType } from 'scenes/cohorts/Cohort
import { LogicWrapper } from 'kea'
import { AggregationAxisFormat } from 'scenes/insights/aggregationAxisFormat'
import { Layout } from 'react-grid-layout'
import { DatabaseSchemaQueryResponseField, HogQLQuery, InsightVizNode, Node } from './queries/schema'
import type {
DashboardFilter,
DatabaseSchemaQueryResponseField,
HogQLQuery,
InsightVizNode,
Node,
} from './queries/schema'
import { QueryContext } from '~/queries/types'

import { JSONContent } from 'scenes/notebooks/Notebook/utils'
Expand Down Expand Up @@ -1345,7 +1351,7 @@ export type DashboardTemplateScope = 'team' | 'global' | 'feature_flag'

export interface DashboardType extends DashboardBasicType {
tiles: DashboardTile[]
filters: Record<string, any>
filters: DashboardFilter
}

export interface DashboardTemplateType {
Expand All @@ -1354,7 +1360,7 @@ export interface DashboardTemplateType {
created_at?: string
template_name: string
dashboard_description?: string
dashboard_filters?: Record<string, JsonType>
dashboard_filters?: DashboardFilter
tiles: DashboardTile[]
variables?: DashboardTemplateVariableType[]
tags?: string[]
Expand Down
1 change: 1 addition & 0 deletions posthog/api/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,7 @@ def to_representation(self, instance: Insight):

dashboard: Optional[Dashboard] = self.context.get("dashboard")
representation["filters"] = instance.dashboard_filters(dashboard=dashboard)
representation["query"] = instance.dashboard_query(dashboard=dashboard)

if "insight" not in representation["filters"] and not representation["query"]:
representation["filters"]["insight"] = "TRENDS"
Expand Down
22 changes: 22 additions & 0 deletions posthog/hogql_queries/apply_dashboard_filters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from posthog.hogql_queries.query_runner import get_query_runner
from posthog.models import Team
from posthog.schema import DashboardFilter


# Apply the filters from the django-style Dashboard object
def apply_dashboard_filters(query: dict, filters: dict, team: Team) -> dict:
kind = query.get("kind", None)

if kind == "DataTableNode":
source = apply_dashboard_filters(query["source"], filters, team)
return {**query, "source": source}

try:
query_runner = get_query_runner(query, team)
except ValueError:
return query
try:
return query_runner.apply_dashboard_filters(DashboardFilter(**filters)).dict()
except NotImplementedError:
# TODO when we implement apply_dashboard_filters on more query runners, we can remove the try/catch
return query
15 changes: 14 additions & 1 deletion posthog/hogql_queries/hogql_query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from posthog.hogql.timings import HogQLTimings
from posthog.hogql_queries.query_runner import QueryRunner
from posthog.models import Team
from posthog.schema import HogQLQuery, HogQLQueryResponse
from posthog.schema import HogQLQuery, HogQLQueryResponse, DashboardFilter, HogQLFilters, DateRange


class HogQLQueryRunner(QueryRunner):
Expand Down Expand Up @@ -66,3 +66,16 @@ def _is_stale(self, cached_result_package):

def _refresh_frequency(self):
return timedelta(minutes=1)

def apply_dashboard_filters(self, dashboard_filter: DashboardFilter) -> HogQLQuery:
self.query.filters = self.query.filters or HogQLFilters()
self.query.filters.dateRange = self.query.filters.dateRange or DateRange()

if dashboard_filter.date_to or dashboard_filter.date_from:
self.query.filters.dateRange.date_to = dashboard_filter.date_to
self.query.filters.dateRange.date_from = dashboard_filter.date_from

if dashboard_filter.properties:
self.query.filters.properties = (self.query.filters.properties or []) + dashboard_filter.properties

return self.query
4 changes: 4 additions & 0 deletions posthog/hogql_queries/query_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
EventsQuery,
WebStatsTableQuery,
HogQLQuery,
DashboardFilter,
)
from posthog.utils import generate_cache_key, get_safe_cache

Expand Down Expand Up @@ -240,3 +241,6 @@ def _is_stale(self, cached_result_package):
@abstractmethod
def _refresh_frequency(self):
raise NotImplementedError()

def apply_dashboard_filters(self, dashboard_filter: DashboardFilter) -> RunnableQueryNode:
raise NotImplementedError()
22 changes: 18 additions & 4 deletions posthog/models/insight.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class Meta:
unique_together = ("team", "short_id")

def dashboard_filters(self, dashboard: Optional[Dashboard] = None):
# TODO dashboard filtering needs to know how to override query date ranges😱
# query date range is set in a different function, see dashboard_query
if dashboard and not self.query:
dashboard_filters = {**dashboard.filters}
dashboard_properties = dashboard_filters.pop("properties") if dashboard_filters.get("properties") else None
Expand Down Expand Up @@ -154,6 +154,13 @@ def dashboard_filters(self, dashboard: Optional[Dashboard] = None):
else:
return self.filters

def dashboard_query(self, dashboard: Optional[Dashboard]) -> Optional[dict]:
if not dashboard or not self.query:
return self.query
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters

return apply_dashboard_filters(self.query, dashboard.filters, self.team)

@property
def url(self):
return absolute_uri(f"/insights/{self.short_id}")
Expand All @@ -174,12 +181,19 @@ class Meta:
def generate_insight_cache_key(insight: Insight, dashboard: Optional[Dashboard]) -> str:
try:
if insight.query is not None:
# TODO: dashboard filtering needs to know how to override queries and date ranges 😱
q = insight.query
dashboard_filters = dashboard.filters if dashboard else None

if dashboard_filters:
from posthog.hogql_queries.apply_dashboard_filters import apply_dashboard_filters

q = apply_dashboard_filters(insight.query, dashboard_filters, insight.team)
else:
q = insight.query

if q.get("source"):
q = q["source"]

return generate_cache_key("{}_{}".format(q, insight.team_id))
return generate_cache_key("{}_{}_{}".format(q, dashboard_filters, insight.team_id))

dashboard_insight_filter = get_filter(data=insight.dashboard_filters(dashboard=dashboard), team=insight.team)
candidate_filters_hash = generate_cache_key("{}_{}".format(dashboard_insight_filter.toJSON(), insight.team_id))
Expand Down
115 changes: 115 additions & 0 deletions posthog/models/test/test_insight_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,3 +118,118 @@ def test_query_hash_varies_with_query_content(self) -> None:
filters_hash_two = generate_insight_cache_key(insight_two, None)

assert filters_hash_one != filters_hash_two

def test_dashboard_with_query_insight_and_filters(self) -> None:
browser_equals_firefox = {
"key": "$browser",
"label": None,
"operator": "exact",
"type": "event",
"value": ["Firefox"],
}
browser_equals_chrome = {
"key": "$browser",
"label": None,
"operator": "exact",
"type": "event",
"value": ["Chrome"],
}

# use test cases and a for loop because django TestCase doesn't have parametrization
test_cases = [
(
# test that query filters are equal when there are no dashboard filters
{"dateRange": {"date_from": "-14d", "date_to": "-7d"}},
{},
{"dateRange": {"date_from": "-14d", "date_to": "-7d"}, "properties": None},
),
(
# test that dashboard filters are used when there are no query filters
{},
{"date_from": "-14d", "date_to": "-7d"},
{"dateRange": {"date_from": "-14d", "date_to": "-7d"}, "properties": None},
),
(
# test that dashboard filters take priority
{"dateRange": {"date_from": "-2d", "date_to": "-1d"}},
{"date_from": "-4d", "date_to": "-3d"},
{"dateRange": {"date_from": "-4d", "date_to": "-3d"}, "properties": None},
),
(
# test that dashboard filters take priority, even if only one value is set, the other is set to None
{"dateRange": {"date_from": "-14d", "date_to": "-7d"}},
{"date_from": "all"},
{"dateRange": {"date_from": "all", "date_to": None}, "properties": None},
),
(
# test that if no filters are set then none are outputted
{},
{},
{"dateRange": {"date_from": None, "date_to": None}, "properties": None},
),
(
# test that properties from the query are used when there are no dashboard properties
{"properties": [browser_equals_firefox]},
{},
{"dateRange": {"date_from": None, "date_to": None}, "properties": [browser_equals_firefox]},
),
(
# test that properties from the dashboard are used when there are no query properties
{},
{"properties": [browser_equals_chrome]},
{"dateRange": {"date_from": None, "date_to": None}, "properties": [browser_equals_chrome]},
),
(
# test that properties are merged when set in both query and dashboard
{"properties": [browser_equals_firefox]},
{"properties": [browser_equals_chrome]},
{
"dateRange": {"date_from": None, "date_to": None},
"properties": [browser_equals_firefox, browser_equals_chrome],
},
),
]

for query_filters, dashboard_filters, expected_filters in test_cases:
query_insight = Insight.objects.create(
team=self.team,
query={
"kind": "DataTableNode",
"source": {
"filters": query_filters,
"kind": "HogQLQuery",
"modifiers": None,
"query": "select * from events where {filters}",
"response": None,
"values": None,
},
},
)
dashboard = Dashboard.objects.create(team=self.team, filters=dashboard_filters)

data = query_insight.dashboard_query(dashboard)
assert data
actual = data["source"]["filters"]
assert expected_filters == actual

def test_query_hash_varies_with_dashboard_filters(self) -> None:
query = {
"kind": "DataTableNode",
"source": {
"filters": {"dateRange": {"date_from": "-14d", "date_to": "-7d"}},
"kind": "HogQLQuery",
"modifiers": None,
"query": "select * from events where {filters}",
"response": None,
"values": None,
},
}
dashboard_filters = {"date_from": "-4d", "date_to": "-3d"}

query_insight = Insight.objects.create(team=self.team, query=query)
dashboard = Dashboard.objects.create(team=self.team, filters=dashboard_filters)

hash_sans_dashboard = generate_insight_cache_key(query_insight, None)
hash_with_dashboard = generate_insight_cache_key(query_insight, dashboard)

assert hash_sans_dashboard != hash_with_dashboard
24 changes: 24 additions & 0 deletions posthog/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,6 +947,30 @@ class AnyResponseType(RootModel):
]


class DashboardFilter(BaseModel):
model_config = ConfigDict(
extra="forbid",
)
date_from: Optional[str] = None
date_to: Optional[str] = None
properties: Optional[
List[
Union[
EventPropertyFilter,
PersonPropertyFilter,
ElementPropertyFilter,
SessionPropertyFilter,
CohortPropertyFilter,
RecordingDurationFilter,
GroupPropertyFilter,
FeaturePropertyFilter,
HogQLPropertyFilter,
EmptyPropertyFilter,
]
]
] = None


class DatabaseSchemaQuery(BaseModel):
model_config = ConfigDict(
extra="forbid",
Expand Down

0 comments on commit b3c4d41

Please sign in to comment.