Skip to content

Commit

Permalink
Restore ability to filter out internal and test users
Browse files Browse the repository at this point in the history
  • Loading branch information
@danielbachhuber
danielbachhuber committed Dec 13, 2024
1 parent dbfb992 commit 905ac6b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 54 deletions.
80 changes: 33 additions & 47 deletions frontend/src/scenes/experiments/Metrics/PrimaryGoalTrends.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ export function PrimaryGoalTrends(): JSX.Element {

const metricIdx = 0
const currentMetric = experiment.metrics[metricIdx] as ExperimentTrendsQuery
// :FLAG: CLEAN UP AFTER MIGRATION
const isDataWarehouseMetric =
featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL] &&
currentMetric.count_query.series[0].kind === NodeKind.DataWarehouseNode

return (
<>
Expand Down Expand Up @@ -63,18 +59,10 @@ export function PrimaryGoalTrends(): JSX.Element {
MathAvailability.All
)

if (series[0].kind === NodeKind.DataWarehouseNode) {
setTrendsMetric({
metricIdx,
series,
filterTestAccounts: false,
})
} else {
setTrendsMetric({
metricIdx,
series,
})
}
setTrendsMetric({
metricIdx,
series,
})
} else {
if (actions?.length) {
setExperiment({
Expand Down Expand Up @@ -113,37 +101,35 @@ export function PrimaryGoalTrends(): JSX.Element {
showNumericalPropsOnly={true}
{...commonActionFilterProps}
/>
{!isDataWarehouseMetric && (
<div className="mt-4 space-y-4">
<TestAccountFilterSwitch
checked={(() => {
// :FLAG: CLEAN UP AFTER MIGRATION
if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) {
const val = currentMetric.count_query?.filterTestAccounts
return hasFilters ? !!val : false
}
return hasFilters ? !!experiment.filters.filter_test_accounts : false
})()}
onChange={(checked: boolean) => {
// :FLAG: CLEAN UP AFTER MIGRATION
if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) {
setTrendsMetric({
metricIdx,
filterTestAccounts: checked,
})
} else {
setExperiment({
filters: {
...experiment.filters,
filter_test_accounts: checked,
},
})
}
}}
fullWidth
/>
</div>
)}
<div className="mt-4 space-y-4">
<TestAccountFilterSwitch
checked={(() => {
// :FLAG: CLEAN UP AFTER MIGRATION
if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) {
const val = currentMetric.count_query?.filterTestAccounts
return hasFilters ? !!val : false
}
return hasFilters ? !!experiment.filters.filter_test_accounts : false
})()}
onChange={(checked: boolean) => {
// :FLAG: CLEAN UP AFTER MIGRATION
if (featureFlags[FEATURE_FLAGS.EXPERIMENTS_HOGQL]) {
setTrendsMetric({
metricIdx,
filterTestAccounts: checked,
})
} else {
setExperiment({
filters: {
...experiment.filters,
filter_test_accounts: checked,
},
})
}
}}
fullWidth
/>
</div>
{isExperimentRunning && (
<LemonBanner type="info" className="mt-3 mb-3">
Preview insights are generated based on {EXPERIMENT_DEFAULT_DURATION} days of data. This can cause a
Expand Down
44 changes: 37 additions & 7 deletions posthog/hogql_queries/experiments/test/test_experiment_trends_query_runner.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
OBJECT_STORAGE_SECRET_ACCESS_KEY,
XDIST_SUFFIX,
)
from posthog.test.base import APIBaseTest, ClickhouseTestMixin, _create_event, flush_persons_and_events
from posthog.test.base import APIBaseTest, ClickhouseTestMixin, _create_event, _create_person, flush_persons_and_events
from freezegun import freeze_time
from typing import cast
from django.utils import timezone
Expand Down Expand Up @@ -198,10 +198,12 @@ def create_data_warehouse_table_with_usage(self):

path_to_s3_object = "s3://" + OBJECT_STORAGE_BUCKET + f"/{TEST_BUCKET}"

id = pa.array(["1", "2", "3", "4", "5"])
date = pa.array(["2023-01-01", "2023-01-02", "2023-01-03", "2023-01-06", "2023-01-07"])
user_id = pa.array(["user_control_0", "user_test_1", "user_test_2", "user_test_3", "user_extra"])
usage = pa.array([1000, 500, 750, 800, 900])
id = pa.array(["1", "2", "3", "4", "5", "6"])
date = pa.array(["2023-01-01", "2023-01-02", "2023-01-03", "2023-01-04", "2023-01-06", "2023-01-07"])
user_id = pa.array(
["user_control_0", "user_test_1", "user_test_2", "internal_test_1", "user_test_3", "user_extra"]
)
usage = pa.array([1000, 500, 750, 100000, 800, 900])
names = ["id", "ds", "userid", "usage"]

pq.write_to_dataset(
Expand Down Expand Up @@ -244,6 +246,15 @@ def create_data_warehouse_table_with_usage(self):
field_name="events",
configuration={"experiments_optimized": True, "experiments_timestamp_key": "ds"},
)

DataWarehouseJoin.objects.create(
team=self.team,
source_table_name=table_name,
source_table_key="userid",
joining_table_name="persons",
joining_table_key="properties.$user_id",
field_name="person",
)
return table_name

@freeze_time("2020-01-01T12:00:00Z")
Expand Down Expand Up @@ -816,7 +827,8 @@ def test_query_runner_with_data_warehouse_series_no_end_date_and_nested_id(self)
math_property="usage",
math_property_type="data_warehouse_properties",
)
]
],
filterTestAccounts=True,
)
exposure_query = TrendsQuery(series=[EventsNode(event="$feature_flag_called")])

Expand Down Expand Up @@ -846,6 +858,24 @@ def test_query_runner_with_data_warehouse_series_no_end_date_and_nested_id(self)
timestamp=datetime(2023, 1, i + 1),
)

_create_person(
team=self.team,
distinct_ids=["internal_test_1"],
properties={"$email": "[email protected]"},
)

_create_event(
team=self.team,
event="$feature_flag_called",
distinct_id="internal_test_1",
properties={
feature_flag_property: "test",
"$feature_flag_response": "test",
"$feature_flag": feature_flag.key,
},
timestamp=datetime(2023, 1, 3),
)

# "user_test_3" first exposure (feature_flag_property="control") is on 2023-01-03
# "user_test_3" relevant exposure (feature_flag_property="test") is on 2023-01-04
# "user_test_3" other event (feature_flag_property="control" is on 2023-01-05
Expand Down Expand Up @@ -906,7 +936,7 @@ def test_query_runner_with_data_warehouse_series_no_end_date_and_nested_id(self)
)

# Assert the expected join condition in the clickhouse SQL
expected_join_condition = f"and(equals(events.team_id, {query_runner.count_query_runner.team.id}), equals(event, %(hogql_val_8)s), greaterOrEquals(timestamp, assumeNotNull(parseDateTime64BestEffortOrNull(%(hogql_val_9)s, 6, %(hogql_val_10)s))), lessOrEquals(timestamp, assumeNotNull(parseDateTime64BestEffortOrNull(%(hogql_val_11)s, 6, %(hogql_val_12)s))))) AS e__events ON"
expected_join_condition = f"and(equals(events.team_id, {query_runner.count_query_runner.team.id}), equals(event, %(hogql_val_12)s), greaterOrEquals(timestamp, assumeNotNull(parseDateTime64BestEffortOrNull(%(hogql_val_13)s, 6, %(hogql_val_14)s))), lessOrEquals(timestamp, assumeNotNull(parseDateTime64BestEffortOrNull(%(hogql_val_15)s, 6, %(hogql_val_16)s))))) AS e__events ON"
self.assertIn(expected_join_condition, str(response.clickhouse))

result = query_runner.calculate()
Expand Down

0 comments on commit 905ac6b

Please sign in to comment.