diff --git a/ee/api/rbac/test/test_access_control.py b/ee/api/rbac/test/test_access_control.py index 17ce7e3d0e6cd..51e8b8d99f0d1 100644 --- a/ee/api/rbac/test/test_access_control.py +++ b/ee/api/rbac/test/test_access_control.py @@ -433,7 +433,7 @@ def test_query_counts_stable_when_listing_resources_including_access_control_inf for i in range(10): FeatureFlag.objects.create(team=self.team, created_by=self.other_user, key=f"flag-{i}") - baseline = 42 # This is a lot! There is currently an n+1 issue with the legacy access control system + baseline = 44 # This is a lot! There is currently an n+1 issue with the legacy access control system with self.assertNumQueries(baseline + 6): # org, roles, preloaded permissions acs, preloaded acs for the list self.client.get("/api/projects/@current/feature_flags/") diff --git a/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr b/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr index 3a20414b1d99d..19b73603a76ba 100644 --- a/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr +++ b/ee/clickhouse/models/test/__snapshots__/test_cohort.ambr @@ -388,8 +388,9 @@ HAVING ifNull(equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) WHERE and(equals(e.team_id, 99999), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), toDateTime64('explicit_redacted_timestamp.999999', 6, 'UTC')), equals(e.event, 'signup')) GROUP BY if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id) - HAVING ifNull(equals(min_timestamp, min_timestamp_with_condition), isNull(min_timestamp) - and isNull(min_timestamp_with_condition))) + HAVING and(ifNull(equals(min_timestamp, min_timestamp_with_condition), isNull(min_timestamp) + and isNull(min_timestamp_with_condition)), ifNull(notEquals(min_timestamp, fromUnixTimestamp(0)), isNotNull(min_timestamp) + or isNotNull(fromUnixTimestamp(0))))) GROUP BY actor_id) AS source ORDER BY source.id ASC LIMIT 100 SETTINGS optimize_aggregation_in_order=1, @@ -502,8 +503,9 @@ HAVING ifNull(equals(argMax(person_distinct_id_overrides.is_deleted, person_distinct_id_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS e__override ON equals(e.distinct_id, e__override.distinct_id) WHERE and(equals(e.team_id, 99999), lessOrEquals(toTimeZone(e.timestamp, 'UTC'), toDateTime64('explicit_redacted_timestamp.999999', 6, 'UTC')), equals(e.event, 'signup')) GROUP BY if(not(empty(e__override.distinct_id)), e__override.person_id, e.person_id) - HAVING ifNull(equals(min_timestamp, min_timestamp_with_condition), isNull(min_timestamp) - and isNull(min_timestamp_with_condition))) + HAVING and(ifNull(equals(min_timestamp, min_timestamp_with_condition), isNull(min_timestamp) + and isNull(min_timestamp_with_condition)), ifNull(notEquals(min_timestamp, fromUnixTimestamp(0)), isNotNull(min_timestamp) + or isNotNull(fromUnixTimestamp(0))))) GROUP BY actor_id) AS source ORDER BY source.id ASC LIMIT 100 SETTINGS optimize_aggregation_in_order=1, diff --git a/ee/clickhouse/views/test/test_clickhouse_experiments.py b/ee/clickhouse/views/test/test_clickhouse_experiments.py index adc3728417115..5f5694509493d 100644 --- a/ee/clickhouse/views/test/test_clickhouse_experiments.py +++ b/ee/clickhouse/views/test/test_clickhouse_experiments.py @@ -1450,7 +1450,7 @@ def test_used_in_experiment_is_populated_correctly_for_feature_flag_list(self) - ).json() # TODO: Make sure permission bool doesn't cause n + 1 - with self.assertNumQueries(17): + with self.assertNumQueries(19): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) result = response.json() diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png index 1ae7812afb7db..7250565b760d9 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png index 43c2f59e6f6fd..dc1776058b2d8 100644 Binary files a/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png and b/frontend/__snapshots__/scenes-app-feature-flags--edit-multi-variate-feature-flag--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark--webkit.png index a5d8432adc196..22a86b804b6ed 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark.png index d5c9a2182aaea..fd2d7fd374de8 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light--webkit.png index 0312c56346753..c6da633867026 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light.png index 8dbd843dc4000..958e0523463cf 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-historical-trends-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark--webkit.png index d558a1062a524..851596968e6d7 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark.png index b65437f5e454b..280b9e1784dcf 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light--webkit.png index 5020b84933b30..d88ed6cd5a2f6 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png index 770765035c1bb..84e0ed62cd37a 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-breakdown-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark--webkit.png index 58644857039e6..567476eb10e1f 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark.png index 8d31e36741742..42464e68da5bf 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light--webkit.png index d74e1302c72a0..a73d880883bf1 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light.png index dda19e8f3daf5..eac24324443e7 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-left-to-right-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark--webkit.png index 68f84afeec82c..d80ba50ca0399 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark.png index 87434a3d31513..d73ff3f5b6e08 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light--webkit.png index 0a1d0cdcc6d8a..4d9c833bda89f 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light.png index c97a1cabc6087..ab92c489a6b92 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-time-to-convert-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark--webkit.png index 4bcefb555e091..e497c1bcf68a2 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png index 1221fb1754a65..897e0b55de77e 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light--webkit.png index 10b999e6691bd..7fa5fae65aead 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png index e4f321d46aa9a..7b4c1282e09da 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-breakdown-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark--webkit.png index 376e4a1f6d8c5..b1311511ee61b 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark.png index bf663c4779c56..453160c03339d 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light--webkit.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light--webkit.png index 7863163f569e6..e77b825e20ec5 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light--webkit.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light--webkit.png differ diff --git a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light.png b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light.png index 7a5eb1e75347f..af16d4d17f67e 100644 Binary files a/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light.png and b/frontend/__snapshots__/scenes-app-insights--funnel-top-to-bottom-edit--light.png differ diff --git a/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--dark.png b/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--dark.png index 715d9deea2965..bb28b3ff789c0 100644 Binary files a/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--dark.png and b/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--dark.png differ diff --git a/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--light.png b/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--light.png index 59e9f7d75609a..4b7c309dfe1d3 100644 Binary files a/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--light.png and b/frontend/__snapshots__/scenes-app-surveys--surveys-global-settings--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-project--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project--light.png b/frontend/__snapshots__/scenes-other-settings--settings-project--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-project--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--dark.png index e55241e3a6281..73833d2cd9c1f 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--light.png b/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--light.png index 713a2be249795..e78fbda83b147 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-project-with-replay-features--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-all-options--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-password-only--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-github--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-google--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-enforced-saml--light.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png index 0a2693ee599a3..d07533f830841 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--dark.png differ diff --git a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png index 6f0a1b47f2908..9c4af2666d055 100644 Binary files a/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png and b/frontend/__snapshots__/scenes-other-settings--settings-session-timeout-sso-only--light.png differ diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx index db74d7cb22f3e..22b5a18575e17 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/AccessControlObject.tsx @@ -8,6 +8,8 @@ import { LemonSelect, LemonSelectProps, LemonTable, + LemonTag, + Tooltip, } from '@posthog/lemon-ui' import { BindLogic, useActions, useAsyncActions, useValues } from 'kea' import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' @@ -136,8 +138,12 @@ function AccessControlObjectUsers(): JSX.Element | null { key: 'level', title: 'Level', width: 0, - render: function LevelRender(_, { access_level, organization_member }) { - return ( + render: function LevelRender(_, { access_level, organization_member, resource }) { + return resource === 'organization' ? ( + + Organization admin + + ) : (
{ - return ( + render: (_, { organization_member, resource }) => { + return resource === 'organization' ? null : ( diff --git a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts index 5df6af39be81f..9f9b5cdf9be1b 100644 --- a/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts +++ b/frontend/src/layout/navigation-3000/sidepanel/panels/access_control/accessControlLogic.ts @@ -3,6 +3,7 @@ import { actions, afterMount, connect, kea, key, listeners, path, props, selecto import { loaders } from 'kea-loaders' import api from 'lib/api' import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic' +import { OrganizationMembershipLevel } from 'lib/constants' import { toSentenceCase } from 'lib/utils' import posthog from 'posthog-js' import { membersLogic } from 'scenes/organization/membersLogic' @@ -156,6 +157,7 @@ export const accessControlLogic = kea([ return props.resource as AccessControlResourceType }, ], + endpoint: [ () => [(_, props) => props], (props): string => { @@ -166,6 +168,7 @@ export const accessControlLogic = kea([ return `api/projects/@current/${props.resource}s/${props.resource_id}/access_controls` }, ], + humanReadableResource: [ () => [(_, props) => props], (props): string => { @@ -213,6 +216,7 @@ export const accessControlLogic = kea([ return options }, ], + accessControlDefault: [ (s) => [s.accessControls, s.accessControlDefaultLevel], (accessControls, accessControlDefaultLevel): AccessControlTypeProject => { @@ -227,12 +231,39 @@ export const accessControlLogic = kea([ }, ], + organizationAdmins: [ + (s) => [s.sortedMembers], + (members): OrganizationMemberType[] => { + return members?.filter((member) => member.level >= OrganizationMembershipLevel.Admin) ?? [] + }, + ], + + organizationAdminsAsAccessControlMembers: [ + (s) => [s.organizationAdmins], + (organizationAdmins): AccessControlTypeMember[] => { + return organizationAdmins.map((admin) => ({ + organization_member: admin.id, + access_level: 'admin', + created_by: null, + created_at: new Date().toISOString(), + updated_at: new Date().toISOString(), + resource: 'organization', + })) + }, + ], + accessControlMembers: [ - (s) => [s.accessControls], - (accessControls): AccessControlTypeMember[] => { - return (accessControls?.access_controls || []).filter( - (accessControl) => !!accessControl.organization_member - ) as AccessControlTypeMember[] + (s) => [s.accessControls, s.organizationAdminsAsAccessControlMembers], + (accessControls, organizationAdminsAsAccessControlMembers): AccessControlTypeMember[] => { + const members = (accessControls?.access_controls || []) + .filter((accessControl) => !!accessControl.organization_member) + .filter( + (accessControl) => + !organizationAdminsAsAccessControlMembers.some( + (admin) => admin.organization_member === accessControl.organization_member + ) + ) as AccessControlTypeMember[] + return organizationAdminsAsAccessControlMembers.concat(members) }, ], @@ -267,11 +298,13 @@ export const accessControlLogic = kea([ ], addableMembers: [ - (s) => [s.sortedMembers, s.accessControlMembers], - (members, accessControlMembers): any[] => { + (s) => [s.sortedMembers, s.accessControlMembers, s.organizationAdmins], + (members, accessControlMembers, organizationAdmins): any[] => { return members ? members.filter( - (member) => !accessControlMembers.find((ac) => ac.organization_member === member.id) + (member) => + !accessControlMembers.find((ac) => ac.organization_member === member.id) && + !organizationAdmins.find((admin) => admin.id === member.id) ) : [] }, diff --git a/frontend/src/lib/constants.tsx b/frontend/src/lib/constants.tsx index 3024ebdb0dbee..9def3543d8865 100644 --- a/frontend/src/lib/constants.tsx +++ b/frontend/src/lib/constants.tsx @@ -196,7 +196,6 @@ export const FEATURE_FLAGS = { SETTINGS_SESSION_TABLE_VERSION: 'settings-session-table-version', // owner: @robbie-c INSIGHT_FUNNELS_USE_UDF: 'insight-funnels-use-udf', // owner: @aspicer #team-product-analytics INSIGHT_FUNNELS_USE_UDF_TRENDS: 'insight-funnels-use-udf-trends', // owner: @aspicer #team-product-analytics - FIRST_TIME_FOR_USER_MATH: 'first-time-for-user-math', // owner: @skoob13 #team-product-analytics MULTITAB_EDITOR: 'multitab-editor', // owner: @EDsCODE #team-data-warehouse BATCH_EXPORTS_POSTHOG_HTTP: 'posthog-http-batch-exports', DATA_MODELING: 'data-modeling', // owner: @EDsCODE #team-data-warehouse diff --git a/frontend/src/lib/lemon-ui/LemonField/LemonField.tsx b/frontend/src/lib/lemon-ui/LemonField/LemonField.tsx index 7c72d1ed6a20f..4b2e5ad05c16a 100644 --- a/frontend/src/lib/lemon-ui/LemonField/LemonField.tsx +++ b/frontend/src/lib/lemon-ui/LemonField/LemonField.tsx @@ -26,6 +26,14 @@ export type LemonPureFieldProps = { htmlFor?: string } +const LemonFieldError = ({ error }: { error: string }): JSX.Element => { + return ( +
+ {error} +
+ ) +} + const LemonPureField = ({ label, info, @@ -66,15 +74,7 @@ const LemonPureField = ({ ) : null} {children} {help ?
{help}
: null} - {typeof error === 'string' ? ( - renderError ? ( - renderError(error) - ) : ( -
- {error} -
- ) - ) : null} + {typeof error === 'string' ? renderError ? renderError(error) : : null}
) } @@ -113,3 +113,4 @@ export const LemonField = ({ /** A field without Kea form functionality. Within a form use `LemonField`. */ LemonField.Pure = LemonPureField +LemonField.Error = LemonFieldError diff --git a/frontend/src/lib/lemon-ui/LemonInput/LemonInput.scss b/frontend/src/lib/lemon-ui/LemonInput/LemonInput.scss index a8d6566d7e84d..f19c62da8e67d 100644 --- a/frontend/src/lib/lemon-ui/LemonInput/LemonInput.scss +++ b/frontend/src/lib/lemon-ui/LemonInput/LemonInput.scss @@ -31,7 +31,7 @@ background-color: inherit; } - .Field--error &, + .Field--error &:not(.ignore-error-border), &.LemonInput--status-danger { border-color: var(--danger) !important; // The error border overrides hover/focus higlighting } diff --git a/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx b/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx index 5715d8ef88218..e5165dcae5017 100644 --- a/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx +++ b/frontend/src/queries/nodes/InsightViz/InsightDisplayConfig.tsx @@ -77,6 +77,8 @@ export function InsightDisplayConfig(): JSX.Element { ((isTrends || isStickiness) && !(display && NON_TIME_SERIES_DISPLAY_TYPES.includes(display))) const showSmoothing = isTrends && !isValidBreakdown(breakdownFilter) && (!display || display === ChartDisplayType.ActionsLineGraph) + const showMultipleYAxesConfig = isTrends || isStickiness + const showAlertThresholdLinesConfig = isTrends const { showValuesOnSeries, mightContainFractionalNumbers } = useValues(trendsDataLogic(insightProps)) @@ -89,8 +91,10 @@ export function InsightDisplayConfig(): JSX.Element { ...(supportsValueOnSeries ? [{ label: () => }] : []), ...(supportsPercentStackView ? [{ label: () => }] : []), ...(hasLegend ? [{ label: () => }] : []), - { label: () => }, - { label: () => }, + ...(showAlertThresholdLinesConfig + ? [{ label: () => }] + : []), + ...(showMultipleYAxesConfig ? [{ label: () => }] : []), ], }, ] diff --git a/frontend/src/queries/schema.json b/frontend/src/queries/schema.json index 2d977ec781edc..c0ba85e060f0d 100644 --- a/frontend/src/queries/schema.json +++ b/frontend/src/queries/schema.json @@ -4200,6 +4200,9 @@ "additionalProperties": false, "description": "Sync with plugin-server/src/types.ts", "properties": { + "cohort_name": { + "type": "string" + }, "key": { "const": "id", "type": "string" @@ -6112,6 +6115,10 @@ }, "searchQuery": { "type": "string" + }, + "status": { + "enum": ["archived", "active", "resolved", "pending_release", "all"], + "type": "string" } }, "required": ["dateRange", "kind"], @@ -13304,6 +13311,9 @@ "showLegend": { "type": "boolean" }, + "showMultipleYAxes": { + "type": "boolean" + }, "showValuesOnSeries": { "type": "boolean" }, @@ -13352,6 +13362,9 @@ "show_legend": { "type": "boolean" }, + "show_multiple_y_axes": { + "type": "boolean" + }, "show_values_on_series": { "type": "boolean" } diff --git a/frontend/src/queries/schema/schema-general.ts b/frontend/src/queries/schema/schema-general.ts index 27c94a8338f19..1821c73a97813 100644 --- a/frontend/src/queries/schema/schema-general.ts +++ b/frontend/src/queries/schema/schema-general.ts @@ -1178,6 +1178,7 @@ export type StickinessFilter = { display?: StickinessFilterLegacy['display'] showLegend?: StickinessFilterLegacy['show_legend'] showValuesOnSeries?: StickinessFilterLegacy['show_values_on_series'] + showMultipleYAxes?: StickinessFilterLegacy['show_multiple_y_axes'] hiddenLegendIndexes?: integer[] stickinessCriteria?: { operator: StickinessOperator @@ -1652,6 +1653,7 @@ export interface ErrorTrackingQuery extends DataNode issueId?: ErrorTrackingIssue['id'] orderBy?: 'last_seen' | 'first_seen' | 'occurrences' | 'users' | 'sessions' dateRange: DateRange + status?: ErrorTrackingIssue['status'] | 'all' assignee?: ErrorTrackingIssueAssignee | null filterGroup?: PropertyGroupFilter filterTestAccounts?: boolean diff --git a/frontend/src/queries/utils.ts b/frontend/src/queries/utils.ts index 1bf5c953221ca..aa93a2a280c68 100644 --- a/frontend/src/queries/utils.ts +++ b/frontend/src/queries/utils.ts @@ -366,6 +366,8 @@ export const getYAxisScaleType = (query: InsightQueryNode): string | undefined = export const getShowMultipleYAxes = (query: InsightQueryNode): boolean | undefined => { if (isTrendsQuery(query)) { return query.trendsFilter?.showMultipleYAxes + } else if (isStickinessQuery(query)) { + return query.stickinessFilter?.showMultipleYAxes } return undefined } diff --git a/frontend/src/scenes/App.tsx b/frontend/src/scenes/App.tsx index 33e5804d9aa88..e875d4a8fe534 100644 --- a/frontend/src/scenes/App.tsx +++ b/frontend/src/scenes/App.tsx @@ -17,8 +17,6 @@ import { GlobalModals } from '~/layout/GlobalModals' import { breadcrumbsLogic } from '~/layout/navigation/Breadcrumbs/breadcrumbsLogic' import { Navigation } from '~/layout/navigation-3000/Navigation' import { themeLogic } from '~/layout/navigation-3000/themeLogic' -import { actionsModel } from '~/models/actionsModel' -import { cohortsModel } from '~/models/cohortsModel' import type { appLogicType } from './AppType' import { preflightLogic } from './PreflightCheck/preflightLogic' @@ -28,7 +26,7 @@ window.process = MOCK_NODE_PROCESS export const appLogic = kea([ path(['scenes', 'App']), - connect([teamLogic, organizationLogic, actionsModel, cohortsModel]), + connect([teamLogic, organizationLogic]), actions({ enableDelayedSpinner: true, ignoreFeatureFlags: true, diff --git a/frontend/src/scenes/billing/billing-utils.ts b/frontend/src/scenes/billing/billing-utils.ts index 46a18bb11c6ec..db75da625e7ee 100644 --- a/frontend/src/scenes/billing/billing-utils.ts +++ b/frontend/src/scenes/billing/billing-utils.ts @@ -102,8 +102,8 @@ export const convertAmountToUsage = ( let totalAmount = parseFloat(tier.unit_amount_usd) let flatFee = parseFloat(tier.flat_amount_usd || '0') for (const addonTiers of allAddonsTiers) { - totalAmount += parseFloat(addonTiers[index].unit_amount_usd) - flatFee += parseFloat(addonTiers[index].flat_amount_usd || '0') + totalAmount += parseFloat(addonTiers[index]?.unit_amount_usd || '0') + flatFee += parseFloat(addonTiers[index]?.flat_amount_usd || '0') } return { ...tier, diff --git a/frontend/src/scenes/dashboard/DashboardCollaborators.tsx b/frontend/src/scenes/dashboard/DashboardCollaborators.tsx index 100b7a62352a2..a58610040459f 100644 --- a/frontend/src/scenes/dashboard/DashboardCollaborators.tsx +++ b/frontend/src/scenes/dashboard/DashboardCollaborators.tsx @@ -68,6 +68,13 @@ export function DashboardCollaboration({ dashboardId }: { dashboardId: Dashboard return ( + {newAccessControl && ( + + We've upgraded our access control system but this dashboard is using a legacy version that can't be + automatically upgraded. Please reach out to support if you're interested in migrating to the new + system. + + )} {(!canEditDashboard || !canRestrictDashboard) && ( {canEditDashboard diff --git a/frontend/src/scenes/error-tracking/ErrorTrackingFilters.tsx b/frontend/src/scenes/error-tracking/ErrorTrackingFilters.tsx index a1f158458d111..e7de5a64ebfae 100644 --- a/frontend/src/scenes/error-tracking/ErrorTrackingFilters.tsx +++ b/frontend/src/scenes/error-tracking/ErrorTrackingFilters.tsx @@ -15,11 +15,15 @@ const errorTrackingDateOptions = dateMapping.filter((dm) => dm.key != 'Yesterday export const ErrorTrackingFilters = (): JSX.Element => { return ( -
- - - - +
+
+ + +
+
+ + +
) } diff --git a/frontend/src/scenes/error-tracking/ErrorTrackingListOptions.tsx b/frontend/src/scenes/error-tracking/ErrorTrackingListOptions.tsx index 895c2ebb645e9..5b80fa7c4fee7 100644 --- a/frontend/src/scenes/error-tracking/ErrorTrackingListOptions.tsx +++ b/frontend/src/scenes/error-tracking/ErrorTrackingListOptions.tsx @@ -8,43 +8,64 @@ import { errorTrackingSceneLogic } from './errorTrackingSceneLogic' export const ErrorTrackingListOptions = (): JSX.Element => { const { assignee } = useValues(errorTrackingLogic) const { setAssignee } = useActions(errorTrackingLogic) - const { orderBy } = useValues(errorTrackingSceneLogic) - const { setOrderBy } = useActions(errorTrackingSceneLogic) + const { orderBy, status } = useValues(errorTrackingSceneLogic) + const { setOrderBy, setStatus } = useActions(errorTrackingSceneLogic) return ( -
-
-
- Sort by: - -
+
+
+ Status: + +
+
+ Sort by: +
Assigned to: diff --git a/frontend/src/scenes/error-tracking/errorTrackingSceneLogic.ts b/frontend/src/scenes/error-tracking/errorTrackingSceneLogic.ts index f54425aea3864..c335b3ff1614d 100644 --- a/frontend/src/scenes/error-tracking/errorTrackingSceneLogic.ts +++ b/frontend/src/scenes/error-tracking/errorTrackingSceneLogic.ts @@ -31,6 +31,7 @@ export const errorTrackingSceneLogic = kea([ actions({ setOrderBy: (orderBy: ErrorTrackingQuery['orderBy']) => ({ orderBy }), + setStatus: (status: ErrorTrackingQuery['status']) => ({ status }), setSelectedIssueIds: (ids: string[]) => ({ ids }), }), @@ -42,6 +43,13 @@ export const errorTrackingSceneLogic = kea([ setOrderBy: (_, { orderBy }) => orderBy, }, ], + status: [ + 'active' as ErrorTrackingQuery['status'], + { persist: true }, + { + setStatus: (_, { status }) => status, + }, + ], selectedIssueIds: [ [] as string[], { @@ -52,10 +60,11 @@ export const errorTrackingSceneLogic = kea([ selectors(({ values }) => ({ query: [ - (s) => [s.orderBy, s.dateRange, s.assignee, s.filterTestAccounts, s.filterGroup, s.searchQuery], - (orderBy, dateRange, assignee, filterTestAccounts, filterGroup, searchQuery): DataTableNode => + (s) => [s.orderBy, s.status, s.dateRange, s.assignee, s.filterTestAccounts, s.filterGroup, s.searchQuery], + (orderBy, status, dateRange, assignee, filterTestAccounts, filterGroup, searchQuery): DataTableNode => errorTrackingQuery({ orderBy, + status, dateRange, assignee, filterTestAccounts, @@ -84,6 +93,7 @@ export const errorTrackingSceneLogic = kea([ ] => { const searchParams: Params = { orderBy: values.orderBy, + status: values.status, filterTestAccounts: values.filterTestAccounts, } @@ -111,6 +121,7 @@ export const errorTrackingSceneLogic = kea([ return { setOrderBy: () => buildURL(), + setStatus: () => buildURL(), setDateRange: () => buildURL(), setFilterGroup: () => buildURL(), setSearchQuery: () => buildURL(), @@ -123,6 +134,9 @@ export const errorTrackingSceneLogic = kea([ if (params.orderBy && !equal(params.orderBy, values.orderBy)) { actions.setOrderBy(params.orderBy) } + if (params.status && !equal(params.status, values.status)) { + actions.setStatus(params.status) + } if (params.dateRange && !equal(params.dateRange, values.dateRange)) { actions.setDateRange(params.dateRange) } diff --git a/frontend/src/scenes/error-tracking/queries.ts b/frontend/src/scenes/error-tracking/queries.ts index 1cd849212a358..11cee4d4833bf 100644 --- a/frontend/src/scenes/error-tracking/queries.ts +++ b/frontend/src/scenes/error-tracking/queries.ts @@ -12,6 +12,7 @@ import { AnyPropertyFilter, BaseMathType, ChartDisplayType, PropertyGroupFilter, export const errorTrackingQuery = ({ orderBy, + status, dateRange, assignee, filterTestAccounts, @@ -20,7 +21,10 @@ export const errorTrackingQuery = ({ customVolume, columns, limit = 50, -}: Pick & { +}: Pick< + ErrorTrackingQuery, + 'orderBy' | 'status' | 'dateRange' | 'assignee' | 'filterTestAccounts' | 'limit' | 'searchQuery' +> & { filterGroup: UniversalFiltersGroup customVolume?: ErrorTrackingSparklineConfig | null columns: ('error' | 'volume' | 'occurrences' | 'sessions' | 'users' | 'assignee')[] @@ -30,6 +34,7 @@ export const errorTrackingQuery = ({ source: { kind: NodeKind.ErrorTrackingQuery, orderBy, + status, dateRange, assignee, customVolume, diff --git a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx index d644ec025ed63..56a52612749d9 100644 --- a/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx +++ b/frontend/src/scenes/feature-flags/FeatureFlagReleaseConditions.tsx @@ -24,7 +24,6 @@ import { getFilterLabel } from 'lib/taxonomy' import { capitalizeFirstLetter, dateFilterToText, dateStringToComponents, humanFriendlyNumber } from 'lib/utils' import { urls } from 'scenes/urls' -import { cohortsModel } from '~/models/cohortsModel' import { groupsModel } from '~/models/groupsModel' import { AnyPropertyFilter, FeatureFlagGroupType, PropertyOperator } from '~/types' @@ -34,13 +33,7 @@ import { FeatureFlagReleaseConditionsLogicProps, } from './FeatureFlagReleaseConditionsLogic' -function PropertyValueComponent({ - property, - cohortsById, -}: { - property: AnyPropertyFilter - cohortsById: Record -}): JSX.Element { +function PropertyValueComponent({ property }: { property: AnyPropertyFilter }): JSX.Element { if (property.type === 'cohort') { return ( } targetBlank > - {(property.value && cohortsById[property.value]?.name) || `ID ${property.value}`} + {property.cohort_name || `ID ${property.value}`} ) } @@ -135,7 +128,6 @@ export function FeatureFlagReleaseConditions({ const { earlyAccessFeaturesList, hasEarlyAccessFeatures, featureFlagKey, nonEmptyVariants } = useValues(featureFlagLogic) - const { cohortsById } = useValues(cohortsModel) const { groupsAccessStatus } = useValues(groupsAccessLogic) const featureFlagVariants = nonEmptyFeatureFlagVariants || nonEmptyVariants @@ -249,7 +241,7 @@ export function FeatureFlagReleaseConditions({ {allOperatorsToHumanName(property.operator)} ) : null} - +
))} diff --git a/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx b/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx index 87fe3eb905f8a..e18e743cd7f18 100644 --- a/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx +++ b/frontend/src/scenes/insights/EditorFilters/FunnelsQuerySteps.tsx @@ -1,8 +1,6 @@ import { useActions, useValues } from 'kea' import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' -import { FEATURE_FLAGS } from 'lib/constants' import { LemonLabel } from 'lib/lemon-ui/LemonLabel/LemonLabel' -import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import { MathAvailability } from 'scenes/insights/filters/ActionFilter/ActionFilterRow/ActionFilterRow' import { insightVizDataLogic } from 'scenes/insights/insightVizDataLogic' import { keyForInsightLogicProps } from 'scenes/insights/sharedUtils' @@ -25,15 +23,10 @@ export function FunnelsQuerySteps({ insightProps }: EditorFilterProps): JSX.Elem const { series, querySource } = useValues(insightVizDataLogic(insightProps)) const { updateQuerySource } = useActions(insightVizDataLogic(insightProps)) - const { featureFlags } = useValues(featureFlagLogic) - const mathAvailability = featureFlags[FEATURE_FLAGS.FIRST_TIME_FOR_USER_MATH] - ? MathAvailability.FunnelsOnly - : MathAvailability.None - const actionFilters = isInsightQueryNode(querySource) ? queryNodeToFilter(querySource) : null const setActionFilters = (payload: Partial): void => { updateQuerySource({ - series: actionsAndEventsToSeries(payload as any, true, mathAvailability), + series: actionsAndEventsToSeries(payload as any, true, MathAvailability.FunnelsOnly), } as FunnelsQuery) } @@ -62,7 +55,7 @@ export function FunnelsQuerySteps({ insightProps }: EditorFilterProps): JSX.Elem filters={actionFilters} setFilters={setActionFilters} typeKey={keyForInsightLogicProps('new')(insightProps)} - mathAvailability={mathAvailability} + mathAvailability={MathAvailability.FunnelsOnly} hideDeleteBtn={filterSteps.length === 1} buttonCopy="Add step" showSeriesIndicator={showSeriesIndicator} diff --git a/frontend/src/scenes/settings/settingsSceneLogic.test.ts b/frontend/src/scenes/settings/settingsSceneLogic.test.ts index 1315090ac76bb..ba7274f50bfce 100644 --- a/frontend/src/scenes/settings/settingsSceneLogic.test.ts +++ b/frontend/src/scenes/settings/settingsSceneLogic.test.ts @@ -3,6 +3,12 @@ import { expectLogic } from 'kea-test-utils' import { FEATURE_FLAGS } from 'lib/constants' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' +// Mock the survey preview functions +jest.mock('posthog-js/dist/surveys-preview', () => ({ + renderFeedbackWidgetPreview: jest.fn(), + renderSurveysPreview: jest.fn(), +})) + import { initKeaTests } from '~/test/init' import { settingsSceneLogic } from './settingsSceneLogic' diff --git a/frontend/src/scenes/surveys/Survey.tsx b/frontend/src/scenes/surveys/Survey.tsx index fd040372b99cd..f191afdf93ad1 100644 --- a/frontend/src/scenes/surveys/Survey.tsx +++ b/frontend/src/scenes/surveys/Survey.tsx @@ -28,14 +28,15 @@ export const scene: SceneExport = { } export function SurveyComponent({ id }: { id?: string } = {}): JSX.Element { - const { editingSurvey } = useActions(surveyLogic) + const { editingSurvey, setSelectedPageIndex } = useActions(surveyLogic) const { isEditingSurvey, surveyMissing } = useValues(surveyLogic) useEffect(() => { return () => { editingSurvey(false) + setSelectedPageIndex(0) } - }, [editingSurvey]) + }, [editingSurvey, setSelectedPageIndex]) if (surveyMissing) { return diff --git a/frontend/src/scenes/surveys/SurveyAppearancePreview.tsx b/frontend/src/scenes/surveys/SurveyAppearancePreview.tsx index bb7d29b8c35a0..b357f8c4f8ce2 100644 --- a/frontend/src/scenes/surveys/SurveyAppearancePreview.tsx +++ b/frontend/src/scenes/surveys/SurveyAppearancePreview.tsx @@ -1,6 +1,7 @@ import { useValues } from 'kea' import { renderFeedbackWidgetPreview, renderSurveysPreview } from 'posthog-js/dist/surveys-preview' import { useEffect, useRef } from 'react' +import { sanitizeSurveyAppearance } from 'scenes/surveys/utils' import { Survey } from '~/types' @@ -22,7 +23,10 @@ export function SurveyAppearancePreview({ survey, previewPageIndex, onPreviewSub useEffect(() => { if (surveyPreviewRef.current) { renderSurveysPreview({ - survey, + survey: { + ...survey, + appearance: sanitizeSurveyAppearance(survey.appearance), + }, parentElement: surveyPreviewRef.current, previewPageIndex, forceDisableHtml: !surveysHTMLAvailable, @@ -32,7 +36,10 @@ export function SurveyAppearancePreview({ survey, previewPageIndex, onPreviewSub if (feedbackWidgetPreviewRef.current) { renderFeedbackWidgetPreview({ - survey, + survey: { + ...survey, + appearance: sanitizeSurveyAppearance(survey.appearance), + }, root: feedbackWidgetPreviewRef.current, forceDisableHtml: !surveysHTMLAvailable, }) diff --git a/frontend/src/scenes/surveys/SurveyCustomization.tsx b/frontend/src/scenes/surveys/SurveyCustomization.tsx index 9b1b13a8ce21b..b8f5f95bc73c3 100644 --- a/frontend/src/scenes/surveys/SurveyCustomization.tsx +++ b/frontend/src/scenes/surveys/SurveyCustomization.tsx @@ -1,14 +1,18 @@ import { LemonButton, LemonCheckbox, LemonDialog, LemonInput, LemonSelect } from '@posthog/lemon-ui' +import clsx from 'clsx' import { useValues } from 'kea' +import { DeepPartialMap, ValidationErrorType } from 'kea-forms' import { PayGateMini } from 'lib/components/PayGateMini/PayGateMini' import { upgradeModalLogic } from 'lib/components/UpgradeModal/upgradeModalLogic' import { LemonField } from 'lib/lemon-ui/LemonField' -import { AvailableFeature, SurveyAppearance as SurveyAppearanceType } from '~/types' +import { AvailableFeature, SurveyAppearance, SurveyAppearance as SurveyAppearanceType } from '~/types' import { defaultSurveyAppearance, WEB_SAFE_FONTS } from './constants' import { surveysLogic } from './surveysLogic' +const IGNORE_ERROR_BORDER_CLASS = 'ignore-error-border' + interface CustomizationProps { appearance: SurveyAppearanceType customizeRatingButtons: boolean @@ -17,6 +21,7 @@ interface CustomizationProps { deleteBranchingLogic?: () => void onAppearanceChange: (appearance: SurveyAppearanceType) => void isCustomFontsEnabled?: boolean + validationErrors?: DeepPartialMap | null } interface WidgetCustomizationProps extends Omit {} @@ -29,6 +34,7 @@ export function Customization({ onAppearanceChange, deleteBranchingLogic, isCustomFontsEnabled = false, + validationErrors, }: CustomizationProps): JSX.Element { const { surveysStylingAvailable } = useValues(surveysLogic) const surveyShufflingQuestionsAvailable = true @@ -50,14 +56,22 @@ export function Customization({ value={appearance?.backgroundColor} onChange={(backgroundColor) => onAppearanceChange({ ...appearance, backgroundColor })} disabled={!surveysStylingAvailable} + className={clsx( + validationErrors?.backgroundColor ? 'border-danger' : IGNORE_ERROR_BORDER_CLASS + )} /> + {validationErrors?.backgroundColor && ( + + )} onAppearanceChange({ ...appearance, borderColor })} disabled={!surveysStylingAvailable} + className={clsx(validationErrors?.borderColor ? 'border-danger' : IGNORE_ERROR_BORDER_CLASS)} /> + {validationErrors?.borderColor && } <> @@ -91,7 +105,13 @@ export function Customization({ onAppearanceChange({ ...appearance, ratingButtonColor }) } disabled={!surveysStylingAvailable} + className={clsx( + validationErrors?.ratingButtonColor ? 'border-danger' : IGNORE_ERROR_BORDER_CLASS + )} /> + {validationErrors?.ratingButtonColor && ( + + )} + {validationErrors?.ratingButtonActiveColor && ( + + )} )} @@ -109,7 +137,13 @@ export function Customization({ value={appearance?.submitButtonColor} onChange={(submitButtonColor) => onAppearanceChange({ ...appearance, submitButtonColor })} disabled={!surveysStylingAvailable} + className={clsx( + validationErrors?.submitButtonColor ? 'border-danger' : IGNORE_ERROR_BORDER_CLASS + )} /> + {validationErrors?.submitButtonColor && ( + + )} @@ -119,7 +153,13 @@ export function Customization({ onAppearanceChange({ ...appearance, submitButtonTextColor }) } disabled={!surveysStylingAvailable} + className={clsx( + validationErrors?.submitButtonTextColor ? 'border-danger' : IGNORE_ERROR_BORDER_CLASS + )} /> + {validationErrors?.submitButtonTextColor && ( + + )} {customizePlaceholderText && ( @@ -146,6 +187,7 @@ export function Customization({ } onChange={(placeholder) => onAppearanceChange({ ...appearance, placeholder })} disabled={!surveysStylingAvailable} + className="ignore-error-border" /> )} @@ -164,6 +206,7 @@ export function Customization({ value: font, } })} + className="ignore-error-border" /> )} @@ -251,7 +294,7 @@ export function Customization({ }) } }} - className="w-12" + className="w-12 ignore-error-border" />{' '} seconds.
diff --git a/frontend/src/scenes/surveys/SurveyEdit.tsx b/frontend/src/scenes/surveys/SurveyEdit.tsx index 6b42d6e2fc90b..03f043237bd3c 100644 --- a/frontend/src/scenes/surveys/SurveyEdit.tsx +++ b/frontend/src/scenes/surveys/SurveyEdit.tsx @@ -73,6 +73,7 @@ export default function SurveyEdit(): JSX.Element { dataCollectionType, surveyUsesLimit, surveyUsesAdaptiveLimit, + surveyErrors, } = useValues(surveyLogic) const { setSurveyValue, @@ -561,6 +562,7 @@ export default function SurveyEdit(): JSX.Element { isCustomFontsEnabled={ !!featureFlags[FEATURE_FLAGS.SURVEYS_CUSTOM_FONTS] } + validationErrors={surveyErrors?.appearance} /> )} diff --git a/frontend/src/scenes/surveys/SurveySettings.tsx b/frontend/src/scenes/surveys/SurveySettings.tsx index 83703bcfce52c..d32f17601eb5a 100644 --- a/frontend/src/scenes/surveys/SurveySettings.tsx +++ b/frontend/src/scenes/surveys/SurveySettings.tsx @@ -1,61 +1,117 @@ -import { LemonSwitch, Link } from '@posthog/lemon-ui' +import { LemonButton, LemonDivider } from '@posthog/lemon-ui' import { useActions, useValues } from 'kea' +import { DeepPartialMap, ValidationErrorType } from 'kea-forms' import { LemonDialog } from 'lib/lemon-ui/LemonDialog' +import { LemonField } from 'lib/lemon-ui/LemonField' +import { useState } from 'react' +import { surveysLogic } from 'scenes/surveys/surveysLogic' +import { sanitizeSurveyAppearance, validateColor } from 'scenes/surveys/utils' import { teamLogic } from 'scenes/teamLogic' -import { urls } from 'scenes/urls' -export type SurveySettingsProps = { - inModal?: boolean -} +import { SurveyAppearance } from '~/types' -export function SurveySettings({ inModal = false }: SurveySettingsProps): JSX.Element { - const { updateCurrentTeam } = useActions(teamLogic) +import { defaultSurveyAppearance, NEW_SURVEY } from './constants' +import { SurveyAppearancePreview } from './SurveyAppearancePreview' +import { Customization } from './SurveyCustomization' + +export function SurveySettings(): JSX.Element { const { currentTeam } = useValues(teamLogic) + const { updateCurrentTeam } = useActions(teamLogic) + const { globalSurveyAppearanceConfigAvailable } = useValues(surveysLogic) + const [validationErrors, setValidationErrors] = useState | null>(null) + + const [editableSurveyConfig, setEditableSurveyConfig] = useState( + currentTeam?.survey_config?.appearance || defaultSurveyAppearance + ) + + const [templatedSurvey, setTemplatedSurvey] = useState(NEW_SURVEY) + + if (templatedSurvey.appearance === defaultSurveyAppearance) { + templatedSurvey.appearance = editableSurveyConfig + } + + const updateSurveySettings = (): void => { + const sanitizedAppearance = sanitizeSurveyAppearance(editableSurveyConfig) + const errors = { + backgroundColor: validateColor(sanitizedAppearance?.backgroundColor, 'background color'), + borderColor: validateColor(sanitizedAppearance?.borderColor, 'border color'), + ratingButtonActiveColor: validateColor( + sanitizedAppearance?.ratingButtonActiveColor, + 'rating button active color' + ), + ratingButtonColor: validateColor(sanitizedAppearance?.ratingButtonColor, 'rating button color'), + submitButtonColor: validateColor(sanitizedAppearance?.submitButtonColor, 'button color'), + submitButtonTextColor: validateColor(sanitizedAppearance?.submitButtonTextColor, 'button text color'), + } + + // Check if there are any validation errors + const hasErrors = Object.values(errors).some((error) => error !== undefined) + setValidationErrors(errors) + + if (hasErrors || !sanitizedAppearance) { + return + } + + // If no errors, proceed with the update + updateCurrentTeam({ + survey_config: { + ...currentTeam?.survey_config, + appearance: sanitizedAppearance, + }, + }) + } return ( -
-
- { - updateCurrentTeam({ - surveys_opt_in: checked, + <> +
+ + These settings apply to new surveys in this organization. + + +
+ {globalSurveyAppearanceConfigAvailable && ( + + Save settings + + )} +
+ +
+ { + setEditableSurveyConfig({ + ...editableSurveyConfig, + ...appearance, + }) + setTemplatedSurvey({ + ...templatedSurvey, + ...{ appearance: appearance }, }) }} - label="Enable surveys popup" - bordered={!inModal} - fullWidth={inModal} - labelClassName={inModal ? 'text-base font-semibold' : ''} - checked={!!currentTeam?.surveys_opt_in} + validationErrors={validationErrors} /> - -

- Please note your website needs to have the{' '} - PostHog snippet or at least version 1.81.1 of{' '} - - posthog-js - {' '} - directly installed. For more details, check out our{' '} - - docs - - . -

+
+
+ {globalSurveyAppearanceConfigAvailable && ( + + )} +
-
+ ) } export function openSurveysSettingsDialog(): void { LemonDialog.open({ title: 'Surveys settings', - content: , + content: , width: 600, primaryButton: { children: 'Done', diff --git a/frontend/src/scenes/surveys/Surveys.tsx b/frontend/src/scenes/surveys/Surveys.tsx index 71109a271d944..8c5b52c79665a 100644 --- a/frontend/src/scenes/surveys/Surveys.tsx +++ b/frontend/src/scenes/surveys/Surveys.tsx @@ -23,26 +23,21 @@ import { dayjs } from 'lib/dayjs' import { useFeatureFlag } from 'lib/hooks/useFeatureFlag' import { LemonBanner } from 'lib/lemon-ui/LemonBanner' import { More } from 'lib/lemon-ui/LemonButton/More' -import { LemonField } from 'lib/lemon-ui/LemonField' import { LemonTableColumn } from 'lib/lemon-ui/LemonTable' import { createdAtColumn, createdByColumn } from 'lib/lemon-ui/LemonTable/columnUtils' import { LemonTableLink } from 'lib/lemon-ui/LemonTable/LemonTableLink' import { LemonTabs } from 'lib/lemon-ui/LemonTabs' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' import stringWithWBR from 'lib/utils/stringWithWBR' -import { useState } from 'react' import { LinkedHogFunctions } from 'scenes/pipeline/hogfunctions/list/LinkedHogFunctions' import { SceneExport } from 'scenes/sceneTypes' -import { SurveyAppearancePreview } from 'scenes/surveys/SurveyAppearancePreview' -import { Customization } from 'scenes/surveys/SurveyCustomization' -import { teamLogic } from 'scenes/teamLogic' import { urls } from 'scenes/urls' import { userLogic } from 'scenes/userLogic' import { ActivityScope, ProductKey, ProgressStatus, Survey } from '~/types' -import { defaultSurveyAppearance, NEW_SURVEY, SurveyQuestionLabel } from './constants' -import { openSurveysSettingsDialog } from './SurveySettings' +import { SurveyQuestionLabel } from './constants' +import { openSurveysSettingsDialog, SurveySettings } from './SurveySettings' import { getSurveyStatus, surveysLogic, SurveysTabs } from './surveysLogic' export const scene: SceneExport = { @@ -67,19 +62,7 @@ export function Surveys(): JSX.Element { const { deleteSurvey, updateSurvey, setSearchTerm, setSurveysFilters, setTab } = useActions(surveysLogic) const { user } = useValues(userLogic) - const { updateCurrentTeam } = useActions(teamLogic) - const { currentTeam } = useValues(teamLogic) const { featureFlags } = useValues(featureFlagLogic) - - const [editableSurveyConfig, setEditableSurveyConfig] = useState( - currentTeam?.survey_config?.appearance || defaultSurveyAppearance - ) - - const [templatedSurvey, setTemplatedSurvey] = useState(NEW_SURVEY) - - if (templatedSurvey.appearance === defaultSurveyAppearance) { - templatedSurvey.appearance = editableSurveyConfig - } const shouldShowEmptyState = !surveysLoading && surveys.length === 0 const showLinkedHogFunctions = useFeatureFlag('HOG_FUNCTIONS_LINKED') const settingLevel = featureFlags[FEATURE_FLAGS.ENVIRONMENTS] ? 'environment' : 'project' @@ -137,61 +120,7 @@ export function Surveys(): JSX.Element { globalSurveyAppearanceConfigAvailable ? { key: SurveysTabs.Settings, label: 'Settings' } : null, ]} /> - {tab === SurveysTabs.Settings && ( - <> -
- - These settings apply to new surveys in this organization. - - -
- {globalSurveyAppearanceConfigAvailable && ( - { - updateCurrentTeam({ - survey_config: { - ...currentTeam?.survey_config, - appearance: { - ...currentTeam?.survey_config?.appearance, - ...editableSurveyConfig, - }, - }, - }) - }} - > - Save settings - - )} -
- -
- { - setEditableSurveyConfig({ - ...editableSurveyConfig, - ...appearance, - }) - setTemplatedSurvey({ - ...templatedSurvey, - ...{ appearance: appearance }, - }) - }} - /> -
-
- {globalSurveyAppearanceConfigAvailable && ( - - )} -
-
- - )} + {tab === SurveysTabs.Settings && } {tab === SurveysTabs.Notifications && ( <>

Get notified whenever a survey result is submitted

diff --git a/frontend/src/scenes/surveys/surveyLogic.tsx b/frontend/src/scenes/surveys/surveyLogic.tsx index 69de63ea251d1..ee04556074003 100644 --- a/frontend/src/scenes/surveys/surveyLogic.tsx +++ b/frontend/src/scenes/surveys/surveyLogic.tsx @@ -32,7 +32,7 @@ import { import { defaultSurveyAppearance, defaultSurveyFieldValues, NEW_SURVEY, NewSurvey } from './constants' import type { surveyLogicType } from './surveyLogicType' import { surveysLogic } from './surveysLogic' -import { sanitizeHTML } from './utils' +import { sanitizeHTML, sanitizeSurveyAppearance, validateColor } from './utils' export enum SurveyEditSection { Steps = 'steps', @@ -645,6 +645,8 @@ export const surveyLogic = kea([ // When errors occur, scroll to the error, but wait for errors to be set in the DOM first if (hasFormErrors(values.flagPropertyErrors) || values.urlMatchTypeValidationError) { actions.setSelectedSection(SurveyEditSection.DisplayConditions) + } else if (hasFormErrors(values.survey.appearance)) { + actions.setSelectedSection(SurveyEditSection.Customization) } else { actions.setSelectedSection(SurveyEditSection.Steps) } @@ -1220,42 +1222,57 @@ export const surveyLogic = kea([ forms(({ actions, props, values }) => ({ survey: { defaults: { ...NEW_SURVEY } as NewSurvey | Survey, - errors: ({ name, questions }) => ({ - // NOTE: When more validation errors are added, the submitSurveyFailure listener should be updated - // to scroll to the right error section - name: !name && 'Please enter a name.', - questions: questions.map((question) => { - const questionErrors = { - question: !question.question && 'Please enter a question label.', - } - - if (question.type === SurveyQuestionType.Rating) { - return { - ...questionErrors, - display: !question.display && 'Please choose a display type.', - scale: !question.scale && 'Please choose a scale.', - lowerBoundLabel: !question.lowerBoundLabel && 'Please enter a lower bound label.', - upperBoundLabel: !question.upperBoundLabel && 'Please enter an upper bound label.', + errors: ({ name, questions, appearance }) => { + const sanitizedAppearance = sanitizeSurveyAppearance(appearance) + return { + name: !name && 'Please enter a name.', + questions: questions.map((question) => { + const questionErrors = { + question: !question.question && 'Please enter a question label.', } - } else if ( - question.type === SurveyQuestionType.SingleChoice || - question.type === SurveyQuestionType.MultipleChoice - ) { - return { - ...questionErrors, - choices: question.choices.some((choice) => !choice.trim()) - ? 'Please ensure all choices are non-empty.' - : undefined, + + if (question.type === SurveyQuestionType.Rating) { + return { + ...questionErrors, + display: !question.display && 'Please choose a display type.', + scale: !question.scale && 'Please choose a scale.', + lowerBoundLabel: !question.lowerBoundLabel && 'Please enter a lower bound label.', + upperBoundLabel: !question.upperBoundLabel && 'Please enter an upper bound label.', + } + } else if ( + question.type === SurveyQuestionType.SingleChoice || + question.type === SurveyQuestionType.MultipleChoice + ) { + return { + ...questionErrors, + choices: question.choices.some((choice) => !choice.trim()) + ? 'Please ensure all choices are non-empty.' + : undefined, + } } - } - return questionErrors - }), - // release conditions controlled using a PureField in the form - targeting_flag_filters: values.flagPropertyErrors, - // controlled using a PureField in the form - urlMatchType: values.urlMatchTypeValidationError, - }), + return questionErrors + }), + // release conditions controlled using a PureField in the form + targeting_flag_filters: values.flagPropertyErrors, + // controlled using a PureField in the form + urlMatchType: values.urlMatchTypeValidationError, + appearance: sanitizedAppearance && { + backgroundColor: validateColor(sanitizedAppearance.backgroundColor, 'background color'), + borderColor: validateColor(sanitizedAppearance.borderColor, 'border color'), + ratingButtonActiveColor: validateColor( + sanitizedAppearance.ratingButtonActiveColor, + 'rating button active color' + ), + ratingButtonColor: validateColor(sanitizedAppearance.ratingButtonColor, 'rating button color'), + submitButtonColor: validateColor(sanitizedAppearance.submitButtonColor, 'button color'), + submitButtonTextColor: validateColor( + sanitizedAppearance.submitButtonTextColor, + 'button text color' + ), + }, + } + }, submit: (surveyPayload) => { if (values.hasCycle) { actions.reportSurveyCycleDetected(values.survey) @@ -1265,12 +1282,17 @@ export const surveyLogic = kea([ ) } + const payload = { + ...surveyPayload, + appearance: sanitizeSurveyAppearance(surveyPayload.appearance), + } + // when the survey is being submitted, we should turn off editing mode actions.editingSurvey(false) if (props.id && props.id !== 'new') { - actions.updateSurvey(surveyPayload) + actions.updateSurvey(payload) } else { - actions.createSurvey(surveyPayload) + actions.createSurvey(payload) } }, }, diff --git a/frontend/src/scenes/surveys/utils.test.ts b/frontend/src/scenes/surveys/utils.test.ts new file mode 100644 index 0000000000000..2b2a52162dbef --- /dev/null +++ b/frontend/src/scenes/surveys/utils.test.ts @@ -0,0 +1,142 @@ +import { SurveyAppearance } from '~/types' + +import { sanitizeColor, sanitizeSurveyAppearance, validateColor } from './utils' + +describe('survey utils', () => { + beforeAll(() => { + // Mock CSS.supports + global.CSS = { + supports: (property: string, value: string): boolean => { + // Basic color validation - this is a simplified version + if (property === 'color') { + // Helper to validate RGB/HSL number ranges + const isValidRGBNumber = (n: string): boolean => { + const num = parseInt(n) + return !isNaN(num) && num >= 0 && num <= 255 + } + + const isValidAlpha = (n: string): boolean => { + const num = parseFloat(n) + return !isNaN(num) && num >= 0 && num <= 1 + } + + // Hex colors (3, 4, 6 or 8 digits) + if (value.match(/^#([0-9A-Fa-f]{3}){1,2}$/) || value.match(/^#([0-9A-Fa-f]{4}){1,2}$/)) { + return true + } + + // RGB colors + const rgbMatch = value.match(/^rgb\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*\)$/) + if (rgbMatch) { + return ( + isValidRGBNumber(rgbMatch[1]) && + isValidRGBNumber(rgbMatch[2]) && + isValidRGBNumber(rgbMatch[3]) + ) + } + + // RGBA colors + const rgbaMatch = value.match(/^rgba\(\s*(\d+)\s*,\s*(\d+)\s*,\s*(\d+)\s*,\s*([\d.]+)\s*\)$/) + if (rgbaMatch) { + return ( + isValidRGBNumber(rgbaMatch[1]) && + isValidRGBNumber(rgbaMatch[2]) && + isValidRGBNumber(rgbaMatch[3]) && + isValidAlpha(rgbaMatch[4]) + ) + } + + // HSL colors + if (value.match(/^hsl\(\s*\d+\s*,\s*\d+%\s*,\s*\d+%\s*\)$/)) { + return true + } + + // HSLA colors + if (value.match(/^hsla\(\s*\d+\s*,\s*\d+%\s*,\s*\d+%\s*,\s*[\d.]+\s*\)$/)) { + return true + } + + // Named colors - extend the list with more common colors + return ['red', 'blue', 'green', 'transparent', 'black', 'white'].includes(value) + } + return false + }, + } as unknown as typeof CSS + }) + + describe('validateColor', () => { + it('returns undefined for valid colors in different formats', () => { + // Hex colors + expect(validateColor('#ff0000', 'test')).toBeUndefined() + expect(validateColor('#f00', 'test')).toBeUndefined() + expect(validateColor('#ff000080', 'test')).toBeUndefined() // With alpha + + // RGB/RGBA colors + expect(validateColor('rgb(255, 0, 0)', 'test')).toBeUndefined() + expect(validateColor('rgba(255, 0, 0, 0.5)', 'test')).toBeUndefined() + + // HSL/HSLA colors + expect(validateColor('hsl(0, 100%, 50%)', 'test')).toBeUndefined() + expect(validateColor('hsla(0, 100%, 50%, 0.5)', 'test')).toBeUndefined() + + // Named colors + expect(validateColor('red', 'test')).toBeUndefined() + expect(validateColor('transparent', 'test')).toBeUndefined() + }) + + it('returns error message for invalid colors', () => { + expect(validateColor('not-a-color', 'test')).toBe( + 'Invalid color value for test. Please use a valid CSS color.' + ) + }) + + it('returns undefined for undefined input', () => { + expect(validateColor(undefined, 'test')).toBeUndefined() + }) + }) + + describe('sanitizeColor', () => { + it('returns undefined for falsy values', () => { + expect(sanitizeColor(undefined)).toBeUndefined() + expect(sanitizeColor('')).toBeUndefined() + }) + + it('adds # prefix to valid hex colors without it', () => { + expect(sanitizeColor('ff0000')).toBe('#ff0000') + expect(sanitizeColor('123456')).toBe('#123456') + }) + + it('returns original value for already valid colors', () => { + expect(sanitizeColor('#ff0000')).toBe('#ff0000') + expect(sanitizeColor('rgb(255, 0, 0)')).toBe('rgb(255, 0, 0)') + expect(sanitizeColor('red')).toBe('red') + }) + }) + + describe('sanitizeSurveyAppearance', () => { + it('returns null for null input', () => { + expect(sanitizeSurveyAppearance(null)).toBeNull() + }) + + it('sanitizes all color fields in the appearance object', () => { + const input: SurveyAppearance = { + backgroundColor: 'ff0000', + borderColor: '00ff00', + ratingButtonActiveColor: '0000ff', + ratingButtonColor: 'ffffff', + submitButtonColor: '000000', + submitButtonTextColor: 'cccccc', + // Add other required fields from SurveyAppearance type as needed + } + + const result = sanitizeSurveyAppearance(input) + + expect(result?.backgroundColor).toBe('#ff0000') + expect(result?.borderColor).toBe('#00ff00') + expect(result?.ratingButtonActiveColor).toBe('#0000ff') + expect(result?.ratingButtonColor).toBe('#ffffff') + expect(result?.submitButtonColor).toBe('#000000') + expect(result?.submitButtonTextColor).toBe('#cccccc') + }) + }) +}) diff --git a/frontend/src/scenes/surveys/utils.ts b/frontend/src/scenes/surveys/utils.ts index 4493e1ac943e6..0bd730d8ff35e 100644 --- a/frontend/src/scenes/surveys/utils.ts +++ b/frontend/src/scenes/surveys/utils.ts @@ -1,7 +1,47 @@ import DOMPurify from 'dompurify' +import { SurveyAppearance } from '~/types' + const sanitizeConfig = { ADD_ATTR: ['target'] } export function sanitizeHTML(html: string): string { return DOMPurify.sanitize(html, sanitizeConfig) } + +export function sanitizeColor(color: string | undefined): string | undefined { + if (!color) { + return undefined + } + + // test if the color is valid by adding a # to the beginning of the string + if (!validateColor(`#${color}`, 'color')) { + return `#${color}` + } + + return color +} + +export function validateColor(color: string | undefined, fieldName: string): string | undefined { + if (!color) { + return undefined + } + // Test if the color value is valid using CSS.supports + const isValidColor = CSS.supports('color', color) + return !isValidColor ? `Invalid color value for ${fieldName}. Please use a valid CSS color.` : undefined +} + +export function sanitizeSurveyAppearance(appearance: SurveyAppearance | null): SurveyAppearance | null { + if (!appearance) { + return null + } + + return { + ...appearance, + backgroundColor: sanitizeColor(appearance.backgroundColor), + borderColor: sanitizeColor(appearance.borderColor), + ratingButtonActiveColor: sanitizeColor(appearance.ratingButtonActiveColor), + ratingButtonColor: sanitizeColor(appearance.ratingButtonColor), + submitButtonColor: sanitizeColor(appearance.submitButtonColor), + submitButtonTextColor: sanitizeColor(appearance.submitButtonTextColor), + } +} diff --git a/frontend/src/scenes/trends/mathsLogic.test.ts b/frontend/src/scenes/trends/mathsLogic.test.ts deleted file mode 100644 index c3ac0031d6076..0000000000000 --- a/frontend/src/scenes/trends/mathsLogic.test.ts +++ /dev/null @@ -1,30 +0,0 @@ -import { FEATURE_FLAGS } from '~/lib/constants' -import { BaseMathType } from '~/types' - -import { filterMathTypesUnderFeatureFlags, MathCategory, MathDefinition } from './mathsLogic' - -describe('mathsLogic', () => { - it('should filter out math types that are not enabled', () => { - const mathDefinitions: Record = { - test: { name: 'test', category: MathCategory.EventCount, shortName: 'test', description: 'test' }, - [BaseMathType.FirstTimeForUser]: { - name: 'test', - category: MathCategory.EventCount, - shortName: 'test', - description: 'test', - }, - } - - expect( - filterMathTypesUnderFeatureFlags(mathDefinitions, { - [FEATURE_FLAGS.FIRST_TIME_FOR_USER_MATH]: true, - }) - ).toEqual(mathDefinitions) - - const res = filterMathTypesUnderFeatureFlags(mathDefinitions, { - [FEATURE_FLAGS.FIRST_TIME_FOR_USER_MATH]: false, - }) - expect(res).not.toHaveProperty(BaseMathType.FirstTimeForUser) - expect(res).toHaveProperty('test', mathDefinitions.test) - }) -}) diff --git a/frontend/src/scenes/trends/mathsLogic.tsx b/frontend/src/scenes/trends/mathsLogic.tsx index 18ce6ad0fce1a..e041fc96f627b 100644 --- a/frontend/src/scenes/trends/mathsLogic.tsx +++ b/frontend/src/scenes/trends/mathsLogic.tsx @@ -1,5 +1,4 @@ import { connect, kea, path, selectors } from 'kea' -import { FEATURE_FLAGS } from 'lib/constants' import { groupsAccessLogic } from 'lib/introductions/groupsAccessLogic' import { featureFlagLogic } from 'lib/logic/featureFlagLogic' @@ -37,8 +36,8 @@ export const FUNNEL_MATH_DEFINITIONS: Record = { shortName: 'first event', description: ( <> - Only the first time the user performed this event will count towards the funnel, and only if it matches - the event filters. + Matches only the first time the user performed this event type. If this event does not match the event + filters, or is outside the selected date range, the user will be excluded from the funnel.

@@ -54,8 +53,9 @@ export const FUNNEL_MATH_DEFINITIONS: Record = { shortName: 'first matching event', description: ( <> - The first time the user performed this event that matches the event filters will count towards the - funnel. + Matches only the first time the user performed this event type with the selected filters. If this event + is outside the selected date range, or the user never performed the event with the selected filters, the + user will be excluded from the funnel.

@@ -145,7 +145,8 @@ export const BASE_MATH_DEFINITIONS: Record = { shortName: 'first time', description: ( <> - Only the first time the user performed this event will count, and only if it matches the event filters. + Matches only the first time the user performed this event type. If this event does not match the event + filters, or is outside the selected date range, the user will be excluded from the insight.

@@ -161,7 +162,9 @@ export const BASE_MATH_DEFINITIONS: Record = { shortName: 'first matching event', description: ( <> - The first time the user performed this event that matches the event filters will count. + Matches only the first time the user performed this event type with the selected filters. If this event + is outside the selected date range, or the user never performed the event with the selected filters, the + user will be excluded from the insight.

@@ -388,8 +391,8 @@ export const mathsLogic = kea([ }), selectors({ mathDefinitions: [ - (s) => [s.groupsMathDefinitions, s.featureFlags], - (groupsMathDefinitions, featureFlags): Record => { + (s) => [s.groupsMathDefinitions], + (groupsMathDefinitions): Record => { const allMathDefinitions: Record = { ...BASE_MATH_DEFINITIONS, ...groupsMathDefinitions, @@ -397,27 +400,27 @@ export const mathsLogic = kea([ ...COUNT_PER_ACTOR_MATH_DEFINITIONS, ...HOGQL_MATH_DEFINITIONS, } - return filterMathTypesUnderFeatureFlags(allMathDefinitions, featureFlags) + return allMathDefinitions }, ], funnelMathDefinitions: [ - (s) => [s.featureFlags], - (featureFlags): Record => { + () => [], + (): Record => { const funnelMathDefinitions: Record = { ...FUNNEL_MATH_DEFINITIONS, } - return filterMathTypesUnderFeatureFlags(funnelMathDefinitions, featureFlags) + return funnelMathDefinitions }, ], // Static means the options do not have nested selectors (like math function) staticMathDefinitions: [ - (s) => [s.groupsMathDefinitions, s.needsUpgradeForGroups, s.featureFlags], - (groupsMathDefinitions, needsUpgradeForGroups, featureFlags): Record => { + (s) => [s.groupsMathDefinitions, s.needsUpgradeForGroups], + (groupsMathDefinitions, needsUpgradeForGroups): Record => { const staticMathDefinitions: Record = { ...BASE_MATH_DEFINITIONS, ...(!needsUpgradeForGroups ? groupsMathDefinitions : {}), } - return filterMathTypesUnderFeatureFlags(staticMathDefinitions, featureFlags) + return staticMathDefinitions }, ], staticActorsOnlyMathDefinitions: [ @@ -462,14 +465,3 @@ export const mathsLogic = kea([ ], }), ]) - -export function filterMathTypesUnderFeatureFlags( - mathDefinitions: Record, - featureFlags: Record -): Record { - const copy = { ...mathDefinitions } - if (!featureFlags[FEATURE_FLAGS.FIRST_TIME_FOR_USER_MATH]) { - delete copy[BaseMathType.FirstTimeForUser] - } - return copy -} diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 0997ee58623bf..a1b9358c6cdfd 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -842,6 +842,7 @@ export interface CohortPropertyFilter extends BasePropertyFilter { value: number /** @default 'in' */ operator: PropertyOperator + cohort_name?: string } export interface GroupPropertyFilter extends BasePropertyFilter { @@ -2348,6 +2349,7 @@ export interface StickinessFilterType extends FilterType { show_legend?: boolean // used to show/hide legend next to insights graph hidden_legend_keys?: Record // used to toggle visibilities in table and legend show_values_on_series?: boolean + show_multiple_y_axes?: boolean // persons only stickiness_days?: number diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index ec77db9f83121..28fa38383152d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -15180,8 +15180,8 @@ packages: resolution: {integrity: sha512-hAZsKq7Yy11Zu1DE0OzWjw7nnLZmJZYTDZZyEFHZdUhV8FkH5MCfoU1XMaxXovpyW5nq5scPqq0ZDP9Zyl04oQ==} engines: {node: '>= 10.0.0'} - unlayer-types@1.219.0: - resolution: {integrity: sha512-hMjlu7cQqZF8CdPggPJ3kfl/djDOhS+YM7R8Y+NLqfqtFxNtejr5ym9NcG94wL3wVnXgTYy07NFS9ICxHqVZYQ==} + unlayer-types@1.221.0: + resolution: {integrity: sha512-/HcJ4yFRESZrtfdFLbKCuzg/sPPmxjGv1az13IQKXdTHLqJimTFrZhLYNPl+dgmDADbulIFwfh9hgZcI8JY6ZA==} unpipe@1.0.0: resolution: {integrity: sha512-pjy2bYhSsufwWlKwPc+l3cN7+wuJlK6uz0YdJEOlQDbl6jo/YlPi4mb8agUkVC8BF7V8NuzeyPNqRksA3hztKQ==} @@ -32684,7 +32684,7 @@ snapshots: react-email-editor@1.7.11(react@18.2.0): dependencies: react: 18.2.0 - unlayer-types: 1.219.0 + unlayer-types: 1.221.0 react-error-boundary@3.1.4(react@18.2.0): dependencies: @@ -34751,7 +34751,7 @@ snapshots: universalify@2.0.0: {} - unlayer-types@1.219.0: {} + unlayer-types@1.221.0: {} unpipe@1.0.0: {} diff --git a/posthog/api/feature_flag.py b/posthog/api/feature_flag.py index 2a98af4711595..481b84f904b5e 100644 --- a/posthog/api/feature_flag.py +++ b/posthog/api/feature_flag.py @@ -2,7 +2,7 @@ from typing import Any, Optional, cast from datetime import datetime -from django.db.models import QuerySet, Q, deletion +from django.db.models import QuerySet, Q, deletion, Prefetch from django.conf import settings from drf_spectacular.utils import OpenApiParameter from drf_spectacular.types import OpenApiTypes @@ -457,6 +457,41 @@ def get_status(self, feature_flag: FeatureFlag) -> str: flag_status, _ = checker.get_status() return flag_status.name + def to_representation(self, instance): + representation = super().to_representation(instance) + filters = representation.get("filters", {}) + groups = filters.get("groups", []) + + # Get all cohort IDs used in the feature flag + cohort_ids = set() + for group in groups: + for property in group.get("properties", []): + if property.get("type") == "cohort": + cohort_ids.add(property.get("value")) + + # Use prefetched cohorts if available + if hasattr(instance.team, "available_cohorts"): + cohorts = { + str(cohort.id): cohort.name + for cohort in instance.team.available_cohorts + if str(cohort.id) in map(str, cohort_ids) + } + else: + # Fallback to database query if cohorts weren't prefetched + cohorts = { + str(cohort.id): cohort.name + for cohort in Cohort.objects.filter(id__in=cohort_ids, team__project_id=self.context["project_id"]) + } + + # Add cohort names to the response + for group in groups: + for property in group.get("properties", []): + if property.get("type") == "cohort": + property["cohort_name"] = cohorts.get(str(property.get("value"))) + + representation["filters"] = filters + return representation + def _create_usage_dashboard(feature_flag: FeatureFlag, user): from posthog.helpers.dashboard_templates import create_feature_flag_dashboard @@ -559,6 +594,13 @@ def safely_get_queryset(self, queryset) -> QuerySet: .prefetch_related("features") .prefetch_related("analytics_dashboards") .prefetch_related("surveys_linked_flag") + .prefetch_related( + Prefetch( + "team__cohort_set", + queryset=Cohort.objects.filter(deleted=False).only("id", "name"), + to_attr="available_cohorts", + ) + ) ) survey_targeting_flags = Survey.objects.filter( diff --git a/posthog/api/test/test_feature_flag.py b/posthog/api/test/test_feature_flag.py index bcbdfd1f9dda2..09f93776a938e 100644 --- a/posthog/api/test/test_feature_flag.py +++ b/posthog/api/test/test_feature_flag.py @@ -1718,7 +1718,7 @@ def test_getting_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(14, 15)): + with self.assertNumQueries(FuzzyInt(16, 17)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1733,7 +1733,7 @@ def test_getting_flags_is_not_nplus1(self) -> None: format="json", ).json() - with self.assertNumQueries(FuzzyInt(14, 15)): + with self.assertNumQueries(FuzzyInt(16, 17)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1757,7 +1757,7 @@ def test_getting_flags_with_no_creator(self) -> None: name="Flag role access", ) - with self.assertNumQueries(FuzzyInt(14, 15)): + with self.assertNumQueries(FuzzyInt(16, 17)): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual(len(response.json()["results"]), 2) @@ -4705,6 +4705,40 @@ def test_cant_create_flag_with_group_data_that_fails_to_query(self): }, ) + def test_feature_flag_includes_cohort_names(self): + cohort = Cohort.objects.create( + team=self.team, + name="test_cohort", + groups=[{"properties": [{"key": "email", "value": "@posthog.com", "type": "person"}]}], + ) + response = self.client.post( + f"/api/projects/{self.team.id}/feature_flags/", + { + "name": "Alpha feature", + "key": "alpha-feature", + "filters": { + "groups": [{"properties": [{"key": "id", "type": "cohort", "value": cohort.pk}]}], + }, + }, + format="json", + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + # Get the flag + response = self.client.get( + f"/api/projects/{self.team.id}/feature_flags/{response.json()['id']}/", + ) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual( + response.json()["filters"]["groups"][0]["properties"][0], + { + "key": "id", + "type": "cohort", + "value": cohort.pk, + "cohort_name": "test_cohort", + }, + ) + class TestCohortGenerationForFeatureFlag(APIBaseTest, ClickhouseTestMixin): def test_creating_static_cohort_with_deleted_flag(self): diff --git a/posthog/api/test/test_survey.py b/posthog/api/test/test_survey.py index 38626ede897ce..e1068046b3830 100644 --- a/posthog/api/test/test_survey.py +++ b/posthog/api/test/test_survey.py @@ -391,7 +391,7 @@ def test_used_in_survey_is_populated_correctly_for_feature_flag_list(self) -> No format="json", ).json() - with self.assertNumQueries(20): + with self.assertNumQueries(22): response = self.client.get(f"/api/projects/{self.team.id}/feature_flags") self.assertEqual(response.status_code, status.HTTP_200_OK) result = response.json() diff --git a/posthog/hogql_queries/error_tracking_query_runner.py b/posthog/hogql_queries/error_tracking_query_runner.py index 5921c5b4ef9e9..5fb2cbe4ec831 100644 --- a/posthog/hogql_queries/error_tracking_query_runner.py +++ b/posthog/hogql_queries/error_tracking_query_runner.py @@ -218,12 +218,6 @@ def results(self, columns: list[str], query_results: list): "aggregations": self.extract_aggregations(result_dict), } ) - else: - logger.error( - "error tracking issue not found", - issue_id=result_dict["id"], - exc_info=True, - ) return results @@ -256,12 +250,14 @@ def properties(self): return self.query.filterGroup.values[0].values if self.query.filterGroup else None def error_tracking_issues(self, ids): + status = self.query.status queryset = ErrorTrackingIssue.objects.select_related("assignment").filter(team=self.team, id__in=ids) - queryset = ( - queryset.filter(id=self.query.issueId) - if self.query.issueId - else queryset.filter(status__in=[ErrorTrackingIssue.Status.ACTIVE]) - ) + + if self.query.issueId: + queryset = queryset.filter(id=self.query.issueId) + elif status and not status == "all": + queryset = queryset.filter(status=status) + if self.query.assignee: queryset = ( queryset.filter(assignment__user_id=self.query.assignee.id) diff --git a/posthog/hogql_queries/insights/funnels/test/test_funnel_aggregation_operations.py b/posthog/hogql_queries/insights/funnels/test/test_funnel_aggregation_operations.py index 5cfef9b468674..d2402a7d2675e 100644 --- a/posthog/hogql_queries/insights/funnels/test/test_funnel_aggregation_operations.py +++ b/posthog/hogql_queries/insights/funnels/test/test_funnel_aggregation_operations.py @@ -172,12 +172,24 @@ def test_first_time_for_user_aggregation_query_having(self): query = cast(ast.SelectQuery, query.select_from.table) assert query.having is not None - assert isinstance(query.having, ast.CompareOperation) - assert query.having.op == ast.CompareOperationOp.Eq - assert isinstance(query.having.left, ast.Field) - assert query.having.left.chain == ["min_timestamp"] - assert isinstance(query.having.right, ast.Field) - assert query.having.right.chain == ["min_timestamp_with_condition"] + assert isinstance(query.having, ast.And) + + assert len(query.having.exprs) == 2 + + first = query.having.exprs[0] + assert isinstance(first, ast.CompareOperation) + assert first.op == ast.CompareOperationOp.Eq + assert isinstance(first.left, ast.Field) + assert first.left.chain == ["min_timestamp"] + assert isinstance(first.right, ast.Field) + assert first.right.chain == ["min_timestamp_with_condition"] + + second = query.having.exprs[1] + assert isinstance(second, ast.CompareOperation) + assert second.op == ast.CompareOperationOp.NotEq + assert isinstance(second.left, ast.Field) + assert second.left.chain == ["min_timestamp"] + assert isinstance(second.right, ast.Call) def test_first_time_for_user_aggregation_query_sampling_factor(self): funnels_query = FunnelsQuery( diff --git a/posthog/hogql_queries/insights/trends/breakdown.py b/posthog/hogql_queries/insights/trends/breakdown.py index b1fba204f44b7..f21ed00eeb170 100644 --- a/posthog/hogql_queries/insights/trends/breakdown.py +++ b/posthog/hogql_queries/insights/trends/breakdown.py @@ -229,6 +229,7 @@ def get_actors_query_where_filter(self, lookup_values: str | int | list[int | st actors_filter = self._get_actors_query_where_expr( breakdown_value=breakdown.property, breakdown_type=breakdown.type, + normalize_url=breakdown.normalize_url, lookup_value=str( lookup_value ), # numeric values are only in cohorts, so it's a safe convertion here @@ -248,6 +249,7 @@ def get_actors_query_where_filter(self, lookup_values: str | int | list[int | st self._breakdown_filter.breakdown ), # all other value types were excluded already breakdown_type=self._breakdown_filter.breakdown_type, + normalize_url=self._breakdown_filter.breakdown_normalize_url, lookup_value=str( lookup_values ), # numeric values are only in cohorts, so it's a safe convertion here @@ -265,6 +267,7 @@ def _get_actors_query_where_expr( breakdown_value: str, breakdown_type: BreakdownType | MultipleBreakdownType | None, lookup_value: str, + normalize_url: bool | None = None, histogram_bin_count: int | None = None, group_type_index: int | None = None, ): @@ -325,7 +328,7 @@ def _get_actors_query_where_expr( raise ValueError("Breakdown value must be a valid JSON array if the the bin count is selected.") return ast.CompareOperation( - left=self.get_replace_null_values_transform(left), + left=self._get_breakdown_values_transform(left, normalize_url=normalize_url), op=ast.CompareOperationOp.Eq, right=ast.Constant(value=lookup_value), ) diff --git a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py index b17a0e1d4f395..14ad8b5105817 100644 --- a/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py +++ b/posthog/hogql_queries/insights/trends/test/test_trends_query_runner.py @@ -5026,41 +5026,49 @@ def test_trends_aggregation_first_matching_event_for_user(self): distinct_ids=["p1"], properties={}, ) - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-06T12:00:00Z", - properties={"$browser": "Firefox"}, - ) - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-08T12:00:00Z", - properties={"$browser": "Chrome"}, - ) - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-09T12:00:00Z", - properties={"$browser": "Chrome"}, - ) - _create_event( + _create_person( team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-10T12:00:00Z", - properties={"$browser": "Firefox"}, + distinct_ids=["p2"], + properties={}, ) _create_event( team=self.team, event="$pageview", - distinct_id="p1", - timestamp="2020-01-11T12:00:00Z", + distinct_id=f"p1", + timestamp="2020-01-06T12:00:00Z", properties={"$browser": "Firefox"}, ) + + for i in range(1, 3): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-08T12:00:00Z", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-09T12:00:00Z", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-10T12:00:00Z", + properties={"$browser": "Firefox"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-11T12:00:00Z", + properties={"$browser": "Firefox"}, + ) + flush_persons_and_events() response = self._run_trends_query( @@ -5081,7 +5089,7 @@ def test_trends_aggregation_first_matching_event_for_user(self): assert response.results[0]["count"] == 1 assert response.results[0]["data"] == [0, 0, 1, 0] - def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): + def test_trends_aggregation_first_matching_event_for_user_with_breakdown_and_filter_being_the_same(self): _create_person( team=self.team, distinct_ids=["p1"], @@ -5092,45 +5100,20 @@ def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): distinct_ids=["p2"], properties={}, ) - - # these events outside date range should be ignored - _create_event( + _create_person( team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-06T12:00:00Z", - properties={"$browser": "Chrome"}, + distinct_ids=["p3"], + properties={}, ) + _create_event( team=self.team, event="$pageview", - distinct_id="p2", + distinct_id=f"p1", timestamp="2020-01-06T12:00:00Z", properties={"$browser": "Firefox"}, ) - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-06T13:00:00Z", - properties={"$browser": "Firefox"}, - ) - # only this event for p1 should be included - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-08T12:00:00Z", - properties={"$browser": "Chrome"}, - ) - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-09T12:00:00Z", - properties={"$browser": "Chrome"}, - ) _create_event( team=self.team, event="$pageview", @@ -5139,7 +5122,6 @@ def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): properties={"$browser": "Firefox"}, ) - # only this event for p2 should be included _create_event( team=self.team, event="$pageview", @@ -5147,14 +5129,6 @@ def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): timestamp="2020-01-10T12:00:00Z", properties={"$browser": "Firefox"}, ) - - _create_event( - team=self.team, - event="$pageview", - distinct_id="p1", - timestamp="2020-01-11T12:00:00Z", - properties={"$browser": "Firefox"}, - ) _create_event( team=self.team, event="$pageview", @@ -5163,6 +5137,22 @@ def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): properties={"$browser": "Firefox"}, ) + for i in range(1, 4): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-08T12:00:00Z", + properties={"$browser": "Chrome"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-09T12:00:00Z", + properties={"$browser": "Chrome"}, + ) + flush_persons_and_events() response = self._run_trends_query( @@ -5173,23 +5163,92 @@ def test_trends_aggregation_first_matching_event_for_user_with_breakdown(self): EventsNode( event="$pageview", math=BaseMathType.FIRST_MATCHING_EVENT_FOR_USER, + properties=[EventPropertyFilter(key="$browser", operator=PropertyOperator.EXACT, value="Firefox")], ) ], TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH), BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="$browser"), ) - # one for each breakdown value + assert len(response.results) == 1 + + # firefox + assert response.results[0]["breakdown_value"] == "Firefox" + assert response.results[0]["count"] == 1 + # match on 10th (p2) for third day in time range + assert response.results[0]["data"] == [0, 0, 1, 0] + + def test_trends_aggregation_first_matching_event_for_user_with_breakdown_and_filter_being_different(self): + _create_person( + team=self.team, + distinct_ids=["p1"], + properties={}, + ) + _create_person( + team=self.team, + distinct_ids=["p2"], + properties={}, + ) + + for i in range(1, 3): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-08T12:00:00Z", + properties={"$browser": "Chrome", "breakdown_prop": i}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-10T12:00:00Z", + properties={"$browser": "Firefox", "breakdown_prop": i}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-11T12:00:00Z", + properties={"$browser": "Firefox", "breakdown_prop": i}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-09T12:00:00Z", + properties={"$browser": "Chrome", "breakdown_prop": i}, + ) + + flush_persons_and_events() + + response = self._run_trends_query( + "2020-01-08", + "2020-01-11", + IntervalType.DAY, + [ + EventsNode( + event="$pageview", + math=BaseMathType.FIRST_MATCHING_EVENT_FOR_USER, + properties=[EventPropertyFilter(key="$browser", operator=PropertyOperator.EXACT, value="Firefox")], + ) + ], + TrendsFilter(display=ChartDisplayType.ACTIONS_LINE_GRAPH), + BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="breakdown_prop"), + ) + + response.results.sort(key=lambda x: x["breakdown_value"]) + assert len(response.results) == 2 - # chrome - assert response.results[0]["breakdown_value"] == "Chrome" + # 1 + assert response.results[0]["breakdown_value"] == "1" assert response.results[0]["count"] == 1 - # match on 8th (p1) for first day in time range - assert response.results[0]["data"] == [1, 0, 0, 0] + # match on 10th (p2) for third day in time range + assert response.results[0]["data"] == [0, 0, 1, 0] - # firefox - assert response.results[1]["breakdown_value"] == "Firefox" + # 2 + assert response.results[1]["breakdown_value"] == "2" assert response.results[1]["count"] == 1 # match on 10th (p2) for third day in time range assert response.results[1]["data"] == [0, 0, 1, 0] diff --git a/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py b/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py index c7baa7cd837f5..e18329a5bfeab 100644 --- a/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py +++ b/posthog/hogql_queries/insights/trends/trends_actors_query_builder.py @@ -206,9 +206,15 @@ def _get_events_query(self) -> ast.SelectQuery: ast.SelectQuery(select=[]), date_from, date_to, - filters=self._events_where_expr(with_date_range_expr=False, with_event_or_action_expr=False), + filters=self._events_where_expr( + with_date_range_expr=False, with_event_or_action_expr=False, with_breakdown_expr=False + ), + filters_with_breakdown=self._events_where_expr( + with_date_range_expr=False, with_event_or_action_expr=False + ), event_or_action_filter=self._event_or_action_where_expr(), ratio=self._ratio_expr(), + is_first_matching_event=self.trends_aggregation_operations.is_first_matching_event(), ) query_builder.append_select(actor_col) query_builder.extend_select(columns, aggregate=True) diff --git a/posthog/hogql_queries/insights/utils/aggregations.py b/posthog/hogql_queries/insights/utils/aggregations.py index 3c453c79c4616..b8a8e8eba6d9c 100644 --- a/posthog/hogql_queries/insights/utils/aggregations.py +++ b/posthog/hogql_queries/insights/utils/aggregations.py @@ -69,20 +69,32 @@ def __init__( event_or_action_filter: ast.Expr | None = None, ratio: ast.RatioExpr | None = None, is_first_matching_event: bool = False, + filters_with_breakdown: ast.Expr | None = None, ): - query.select = self._select_expr(date_from, filters, is_first_matching_event) + self._filters = filters + self._filters_with_breakdown = filters_with_breakdown + self._is_first_matching_event = is_first_matching_event + query.select = self._select_expr(date_from) query.select_from = self._select_from_expr(ratio) query.where = self._where_expr(date_to, event_or_action_filter) query.group_by = self._group_by_expr() query.having = self._having_expr() super().__init__(query) - def _select_expr(self, date_from: ast.Expr, filters: ast.Expr | None = None, is_first_matching_event: bool = False): - aggregation_filters = date_from if filters is None else ast.And(exprs=[date_from, filters]) + def _select_expr(self, date_from: ast.Expr): + min_timestamp_with_condition_filters = ( + self._filters_with_breakdown if self._filters_with_breakdown is not None else self._filters + ) + aggregation_filters = ( + date_from + if min_timestamp_with_condition_filters is None + else ast.And(exprs=[date_from, min_timestamp_with_condition_filters]) + ) + min_timestamp_expr = ( ast.Call(name="min", args=[ast.Field(chain=["timestamp"])]) - if not is_first_matching_event - else ast.Call(name="minIf", args=[ast.Field(chain=["timestamp"]), aggregation_filters]) + if not self._is_first_matching_event or self._filters is None + else ast.Call(name="minIf", args=[ast.Field(chain=["timestamp"]), self._filters]) ) return [ @@ -118,16 +130,27 @@ def _group_by_expr(self) -> list[ast.Expr]: return [ast.Field(chain=["person_id"])] def _having_expr(self) -> ast.Expr: - return ast.CompareOperation( + left = ast.CompareOperation( op=ast.CompareOperationOp.Eq, left=ast.Field(chain=["min_timestamp"]), right=ast.Field(chain=["min_timestamp_with_condition"]), ) + right = ast.CompareOperation( + op=ast.CompareOperationOp.NotEq, + left=ast.Field(chain=["min_timestamp"]), + right=ast.Call(name="fromUnixTimestamp", args=[ast.Constant(value=0)]), + ) + + return ast.And(exprs=[left, right]) def _transform_column(self, column: ast.Expr): - return ast.Call( - name="argMin", - args=[column, ast.Field(chain=["timestamp"])], + return ( + ast.Call(name="argMin", args=[column, ast.Field(chain=["timestamp"])]) + if not self._is_first_matching_event or self._filters is None + else ast.Call( + name="argMinIf", + args=[column, ast.Field(chain=["timestamp"]), self._filters], + ) ) def append_select(self, expr: ast.Expr, aggregate: bool = False): diff --git a/posthog/hogql_queries/insights/utils/test/test_aggregations.py b/posthog/hogql_queries/insights/utils/test/test_aggregations.py index 4f84afabc0dcc..9498487315b01 100644 --- a/posthog/hogql_queries/insights/utils/test/test_aggregations.py +++ b/posthog/hogql_queries/insights/utils/test/test_aggregations.py @@ -98,12 +98,24 @@ def test_query(self): assert isinstance(query.group_by[0], ast.Field) assert query.group_by[0].chain == ["person_id"] - assert isinstance(query.having, ast.CompareOperation) - assert query.having.op == ast.CompareOperationOp.Eq - assert isinstance(query.having.left, ast.Field) - assert query.having.left.chain == ["min_timestamp"] - assert isinstance(query.having.right, ast.Field) - assert query.having.right.chain == ["min_timestamp_with_condition"] + assert isinstance(query.having, ast.And) + + assert len(query.having.exprs) == 2 + + first = query.having.exprs[0] + assert isinstance(first, ast.CompareOperation) + assert first.op == ast.CompareOperationOp.Eq + assert isinstance(first.left, ast.Field) + assert first.left.chain == ["min_timestamp"] + assert isinstance(first.right, ast.Field) + assert first.right.chain == ["min_timestamp_with_condition"] + + second = query.having.exprs[1] + assert isinstance(second, ast.CompareOperation) + assert second.op == ast.CompareOperationOp.NotEq + assert isinstance(second.left, ast.Field) + assert second.left.chain == ["min_timestamp"] + assert isinstance(second.right, ast.Call) def test_query_with_filters(self): query = ast.SelectQuery(select=[]) diff --git a/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr b/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr index ea980f1b28059..cd7310750a8de 100644 --- a/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr +++ b/posthog/hogql_queries/test/__snapshots__/test_error_tracking_query_runner.ambr @@ -325,6 +325,126 @@ max_bytes_before_external_group_by=0 ''' # --- +# name: TestErrorTrackingQueryRunner.test_status + ''' + SELECT if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) AS id, + count(DISTINCT events.uuid) AS occurrences, + count(DISTINCT nullIf(events.`$session_id`, '')) AS sessions, + count(DISTINCT events.distinct_id) AS users, + max(toTimeZone(events.timestamp, 'UTC')) AS last_seen, + min(toTimeZone(events.timestamp, 'UTC')) AS first_seen, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('hour', toStartOfHour(toTimeZone(events.timestamp, 'UTC')), toStartOfHour(now64(6, 'UTC')))), x), range(24))) AS volumeDay, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('day', toStartOfDay(toTimeZone(events.timestamp, 'UTC')), toStartOfDay(now64(6, 'UTC')))), x), range(31))) AS volumeMonth + FROM events + LEFT OUTER JOIN + (SELECT argMax(error_tracking_issue_fingerprint_overrides.issue_id, error_tracking_issue_fingerprint_overrides.version) AS issue_id, + error_tracking_issue_fingerprint_overrides.fingerprint AS fingerprint + FROM error_tracking_issue_fingerprint_overrides + WHERE equals(error_tracking_issue_fingerprint_overrides.team_id, 99999) + GROUP BY error_tracking_issue_fingerprint_overrides.fingerprint + HAVING ifNull(equals(argMax(error_tracking_issue_fingerprint_overrides.is_deleted, error_tracking_issue_fingerprint_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS events__exception_issue_override ON equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_fingerprint'), ''), 'null'), '^"|"$', ''), events__exception_issue_override.fingerprint) + WHERE and(equals(events.team_id, 99999), equals(events.event, '$exception'), isNotNull(if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID'))), 1) + GROUP BY if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) + LIMIT 101 + OFFSET 0 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- +# name: TestErrorTrackingQueryRunner.test_status.1 + ''' + SELECT if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) AS id, + count(DISTINCT events.uuid) AS occurrences, + count(DISTINCT nullIf(events.`$session_id`, '')) AS sessions, + count(DISTINCT events.distinct_id) AS users, + max(toTimeZone(events.timestamp, 'UTC')) AS last_seen, + min(toTimeZone(events.timestamp, 'UTC')) AS first_seen, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('hour', toStartOfHour(toTimeZone(events.timestamp, 'UTC')), toStartOfHour(now64(6, 'UTC')))), x), range(24))) AS volumeDay, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('day', toStartOfDay(toTimeZone(events.timestamp, 'UTC')), toStartOfDay(now64(6, 'UTC')))), x), range(31))) AS volumeMonth + FROM events + LEFT OUTER JOIN + (SELECT argMax(error_tracking_issue_fingerprint_overrides.issue_id, error_tracking_issue_fingerprint_overrides.version) AS issue_id, + error_tracking_issue_fingerprint_overrides.fingerprint AS fingerprint + FROM error_tracking_issue_fingerprint_overrides + WHERE equals(error_tracking_issue_fingerprint_overrides.team_id, 99999) + GROUP BY error_tracking_issue_fingerprint_overrides.fingerprint + HAVING ifNull(equals(argMax(error_tracking_issue_fingerprint_overrides.is_deleted, error_tracking_issue_fingerprint_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS events__exception_issue_override ON equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_fingerprint'), ''), 'null'), '^"|"$', ''), events__exception_issue_override.fingerprint) + WHERE and(equals(events.team_id, 99999), equals(events.event, '$exception'), isNotNull(if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID'))), 1) + GROUP BY if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) + LIMIT 101 + OFFSET 0 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- +# name: TestErrorTrackingQueryRunner.test_status.2 + ''' + SELECT if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) AS id, + count(DISTINCT events.uuid) AS occurrences, + count(DISTINCT nullIf(events.`$session_id`, '')) AS sessions, + count(DISTINCT events.distinct_id) AS users, + max(toTimeZone(events.timestamp, 'UTC')) AS last_seen, + min(toTimeZone(events.timestamp, 'UTC')) AS first_seen, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('hour', toStartOfHour(toTimeZone(events.timestamp, 'UTC')), toStartOfHour(now64(6, 'UTC')))), x), range(24))) AS volumeDay, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('day', toStartOfDay(toTimeZone(events.timestamp, 'UTC')), toStartOfDay(now64(6, 'UTC')))), x), range(31))) AS volumeMonth + FROM events + LEFT OUTER JOIN + (SELECT argMax(error_tracking_issue_fingerprint_overrides.issue_id, error_tracking_issue_fingerprint_overrides.version) AS issue_id, + error_tracking_issue_fingerprint_overrides.fingerprint AS fingerprint + FROM error_tracking_issue_fingerprint_overrides + WHERE equals(error_tracking_issue_fingerprint_overrides.team_id, 99999) + GROUP BY error_tracking_issue_fingerprint_overrides.fingerprint + HAVING ifNull(equals(argMax(error_tracking_issue_fingerprint_overrides.is_deleted, error_tracking_issue_fingerprint_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS events__exception_issue_override ON equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_fingerprint'), ''), 'null'), '^"|"$', ''), events__exception_issue_override.fingerprint) + WHERE and(equals(events.team_id, 99999), equals(events.event, '$exception'), isNotNull(if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID'))), 1) + GROUP BY if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) + LIMIT 101 + OFFSET 0 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- +# name: TestErrorTrackingQueryRunner.test_status.3 + ''' + SELECT if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) AS id, + count(DISTINCT events.uuid) AS occurrences, + count(DISTINCT nullIf(events.`$session_id`, '')) AS sessions, + count(DISTINCT events.distinct_id) AS users, + max(toTimeZone(events.timestamp, 'UTC')) AS last_seen, + min(toTimeZone(events.timestamp, 'UTC')) AS first_seen, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('hour', toStartOfHour(toTimeZone(events.timestamp, 'UTC')), toStartOfHour(now64(6, 'UTC')))), x), range(24))) AS volumeDay, + reverse(arrayMap(x -> countEqual(groupArray(dateDiff('day', toStartOfDay(toTimeZone(events.timestamp, 'UTC')), toStartOfDay(now64(6, 'UTC')))), x), range(31))) AS volumeMonth + FROM events + LEFT OUTER JOIN + (SELECT argMax(error_tracking_issue_fingerprint_overrides.issue_id, error_tracking_issue_fingerprint_overrides.version) AS issue_id, + error_tracking_issue_fingerprint_overrides.fingerprint AS fingerprint + FROM error_tracking_issue_fingerprint_overrides + WHERE equals(error_tracking_issue_fingerprint_overrides.team_id, 99999) + GROUP BY error_tracking_issue_fingerprint_overrides.fingerprint + HAVING ifNull(equals(argMax(error_tracking_issue_fingerprint_overrides.is_deleted, error_tracking_issue_fingerprint_overrides.version), 0), 0) SETTINGS optimize_aggregation_in_order=1) AS events__exception_issue_override ON equals(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_fingerprint'), ''), 'null'), '^"|"$', ''), events__exception_issue_override.fingerprint) + WHERE and(equals(events.team_id, 99999), equals(events.event, '$exception'), isNotNull(if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID'))), 1) + GROUP BY if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) + LIMIT 101 + OFFSET 0 SETTINGS readonly=2, + max_execution_time=60, + allow_experimental_object_type=1, + format_csv_allow_double_quotes=0, + max_ast_elements=4000000, + max_expanded_ast_elements=4000000, + max_bytes_before_external_group_by=0 + ''' +# --- # name: TestErrorTrackingQueryRunner.test_user_assignee ''' SELECT if(not(empty(events__exception_issue_override.issue_id)), events__exception_issue_override.issue_id, accurateCastOrNull(replaceRegexpAll(nullIf(nullIf(JSONExtractRaw(events.properties, '$exception_issue_id'), ''), 'null'), '^"|"$', ''), 'UUID')) AS id, diff --git a/posthog/hogql_queries/test/test_actors_query_runner.py b/posthog/hogql_queries/test/test_actors_query_runner.py index 889896ae239a5..7791341b45229 100644 --- a/posthog/hogql_queries/test/test_actors_query_runner.py +++ b/posthog/hogql_queries/test/test_actors_query_runner.py @@ -9,6 +9,10 @@ from posthog.models.utils import UUIDT from posthog.schema import ( ActorsQuery, + BaseMathType, + BreakdownFilter, + BreakdownType, + EventPropertyFilter, PersonPropertyFilter, HogQLPropertyFilter, PropertyOperator, @@ -18,6 +22,7 @@ EventsNode, IntervalType, InsightActorsQuery, + TrendsQuery, ) from posthog.test.base import ( APIBaseTest, @@ -267,3 +272,157 @@ def test_persons_query_grouping(self): response = runner.calculate() # Should show a single person despite multiple distinct_ids self.assertEqual(len(response.results), 1) + + def test_actors_query_for_first_matching_event(self): + _create_person( + team=self.team, + distinct_ids=["p1"], + properties={}, + ) + _create_person( + team=self.team, + distinct_ids=["p2"], + properties={}, + ) + _create_person( + team=self.team, + distinct_ids=["p3"], + properties={}, + ) + _create_person( + team=self.team, + distinct_ids=["p4"], + properties={}, + ) + + _create_event( + team=self.team, + event="$pageview", + distinct_id="p3", + timestamp="2020-01-02T12:00:00Z", + properties={"$browser": "Chrome", "breakdown_prop": 3}, + ) + + for i in range(1, 5): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-08T12:00:00Z", + properties={"$browser": "Chrome", "breakdown_prop": f"{i if i == 3 else 1}"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-09T12:00:00Z", + properties={"$browser": "Chrome", "breakdown_prop": f"{i if i == 3 else 1}"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-10T12:00:00Z", + properties={"$browser": "Firefox", "breakdown_prop": f"{i if i == 3 else 1}"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-11T12:00:00Z", + properties={"$browser": "Firefox", "breakdown_prop": f"{i if i == 3 else 1}"}, + ) + + flush_persons_and_events() + + source_query = TrendsQuery( + dateRange=DateRange(date_from="2020-01-08", date_to="2020-01-11"), + interval=IntervalType.DAY, + series=[ + EventsNode( + event="$pageview", + math=BaseMathType.FIRST_MATCHING_EVENT_FOR_USER, + properties=[EventPropertyFilter(key="$browser", operator=PropertyOperator.EXACT, value="Firefox")], + ) + ], + breakdownFilter=BreakdownFilter(breakdown_type=BreakdownType.EVENT, breakdown="breakdown_prop"), + ) + + runner = self._create_runner( + ActorsQuery(source=InsightActorsQuery(source=source_query, day="2020-01-10T00:00:00Z", breakdown="3")) + ) + + response = runner.calculate() + + assert len(response.results) == 1 + + def test_actors_query_url_normalization(self): + _create_person( + team=self.team, + distinct_ids=["p1"], + properties={}, + ) + _create_person( + team=self.team, + distinct_ids=["p2"], + properties={}, + ) + + for i in range(1, 4): + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-08T12:00:00Z", + properties={"$browser": "Chrome", "current_url": "https://example.com/"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-09T12:00:00Z", + properties={"$browser": "Chrome", "current_url": "https://example.com/"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-10T12:00:00Z", + properties={"$browser": "Firefox", "current_url": "https://example.com/"}, + ) + _create_event( + team=self.team, + event="$pageview", + distinct_id=f"p{i}", + timestamp="2020-01-11T12:00:00Z", + properties={"$browser": "Firefox", "current_url": "https://example.com/"}, + ) + + flush_persons_and_events() + + source_query = TrendsQuery( + dateRange=DateRange(date_from="2020-01-08", date_to="2020-01-11"), + interval=IntervalType.DAY, + series=[ + EventsNode( + event="$pageview", + math=BaseMathType.FIRST_MATCHING_EVENT_FOR_USER, + properties=[EventPropertyFilter(key="$browser", operator=PropertyOperator.EXACT, value="Firefox")], + ) + ], + breakdownFilter=BreakdownFilter( + breakdown_type=BreakdownType.EVENT, breakdown="current_url", breakdown_normalize_url=True + ), + ) + + runner = self._create_runner( + ActorsQuery( + source=InsightActorsQuery( + source=source_query, day="2020-01-10T00:00:00Z", breakdown="https://example.com" + ) + ) + ) + + response = runner.calculate() + + assert len(response.results) == 3 diff --git a/posthog/hogql_queries/test/test_error_tracking_query_runner.py b/posthog/hogql_queries/test/test_error_tracking_query_runner.py index 1762db668fe10..a3775e2e2a550 100644 --- a/posthog/hogql_queries/test/test_error_tracking_query_runner.py +++ b/posthog/hogql_queries/test/test_error_tracking_query_runner.py @@ -206,8 +206,8 @@ def override_fingerprint(self, fingerprint, issue_id, version=1): team_id=self.team.pk, fingerprint=fingerprint, issue_id=issue_id, version=version ) - def create_issue(self, issue_id, fingerprint): - issue = ErrorTrackingIssue.objects.create(id=issue_id, team=self.team) + def create_issue(self, issue_id, fingerprint, status=ErrorTrackingIssue.Status.ACTIVE): + issue = ErrorTrackingIssue.objects.create(id=issue_id, team=self.team, status=status) ErrorTrackingIssueFingerprintV2.objects.create(team=self.team, issue=issue, fingerprint=fingerprint) return issue @@ -246,7 +246,7 @@ def setUp(self): is_identified=True, ) - self.issue_one = self.create_events_and_issue( + self.create_events_and_issue( issue_id=self.issue_id_one, fingerprint="issue_one_fingerprint", distinct_ids=[self.distinct_id_one, self.distinct_id_two], @@ -276,6 +276,7 @@ def _calculate( searchQuery=None, filterGroup=None, orderBy=None, + status=None, ): return ( ErrorTrackingQueryRunner( @@ -289,6 +290,7 @@ def _calculate( searchQuery=searchQuery, filterGroup=filterGroup, orderBy=orderBy, + status=status, ), ) .calculate() @@ -484,6 +486,24 @@ def test_ordering(self): results = self._calculate(orderBy="first_seen")["results"] self.assertEqual([r["id"] for r in results], [self.issue_id_one, self.issue_id_two, self.issue_id_three]) + @snapshot_clickhouse_queries + def test_status(self): + resolved_issue = ErrorTrackingIssue.objects.get(id=self.issue_id_one) + resolved_issue.status = ErrorTrackingIssue.Status.RESOLVED + resolved_issue.save() + + results = self._calculate(status="active")["results"] + self.assertEqual([r["id"] for r in results], [self.issue_id_three, self.issue_id_two]) + + results = self._calculate(status="resolved")["results"] + self.assertEqual([r["id"] for r in results], [self.issue_id_one]) + + results = self._calculate()["results"] + self.assertEqual([r["id"] for r in results], [self.issue_id_three, self.issue_id_one, self.issue_id_two]) + + results = self._calculate(status="all")["results"] + self.assertEqual([r["id"] for r in results], [self.issue_id_three, self.issue_id_one, self.issue_id_two]) + def test_overrides_aggregation(self): self.override_fingerprint(self.issue_three_fingerprint, self.issue_id_one) results = self._calculate(orderBy="occurrences")["results"] diff --git a/posthog/queries/base.py b/posthog/queries/base.py index 907b741eafc76..6822cf07ab757 100644 --- a/posthog/queries/base.py +++ b/posthog/queries/base.py @@ -131,9 +131,12 @@ def compute_exact_match(value: ValueT, override_value: Any) -> bool: return str(value).lower() not in str(override_value).lower() if operator in ("regex", "not_regex"): + pattern = sanitize_regex_pattern(str(value)) try: - pattern = re.compile(str(value)) - match = pattern.search(str(override_value)) + # Make the pattern more flexible by using DOTALL flag to allow . to match newlines + # Added IGNORECASE for more flexibility + compiled_pattern = re.compile(pattern, re.DOTALL | re.IGNORECASE) + match = compiled_pattern.search(str(override_value)) if operator == "regex": return match is not None @@ -513,3 +516,51 @@ def sanitize_property_key(key: Any) -> str: # This is because we don't want to overwrite the value of key1 when we're trying to read key2 hash_value = hashlib.sha1(string_key.encode("utf-8")).hexdigest()[:15] return f"{substitute}_{hash_value}" + + +def sanitize_regex_pattern(pattern: str) -> str: + # If it doesn't look like a property match pattern, return it as-is + if not ('"' in pattern or "'" in pattern or ":" in pattern): + return pattern + + # First, temporarily replace escaped quotes with markers + pattern = pattern.replace(r"\"", "__ESCAPED_DOUBLE_QUOTE__") + pattern = pattern.replace(r"\'", "__ESCAPED_SINGLE_QUOTE__") + + # Replace unescaped quotes with a pattern that matches either quote type + pattern = pattern.replace('"', "['\"]") + + # Add optional whitespace around colons to handle Python dict format + pattern = pattern.replace(":", r"\s*:\s*") + + # Now restore the escaped quotes, but convert them to also match either quote type + pattern = pattern.replace("__ESCAPED_DOUBLE_QUOTE__", "['\"]") + pattern = pattern.replace("__ESCAPED_SINGLE_QUOTE__", "['\"]") + + # If the pattern looks like a property match (key:value), convert it to use lookaheads + if "['\"]" in pattern: + # Split the pattern if it's trying to match multiple properties + parts = pattern.split("[^}]*") + converted_parts = [] + for part in parts: + if "['\"]" in part: + # Extract the key and value from patterns like ['"]key['"]\s*:\s*['"]value['"] + try: + # Use a non-capturing group for quotes and match the exact key name + # This ensures we don't match partial keys or keys that are substrings of others + key = re.search(r'\[\'"\]((?:[^\'"\s:}]+))\[\'"\]\\s\*:\\s\*\[\'"\](.*?)\[\'"\]', part) + if key: + key_name, value = key.groups() + # Escape special regex characters in the key name + escaped_key_name = re.escape(key_name) + # Convert to a positive lookahead that matches the exact key-value pair + converted = f"(?=.*['\"]?{escaped_key_name}['\"]?\\s*:\\s*['\"]?{value}['\"]?)" + converted_parts.append(converted) + except Exception: + # If we can't parse it, use the original pattern + converted_parts.append(part) + else: + converted_parts.append(part) + pattern = "".join(converted_parts) + + return pattern diff --git a/posthog/queries/test/test_base.py b/posthog/queries/test/test_base.py index a363b26ce19bc..50cf47d8b229e 100644 --- a/posthog/queries/test/test_base.py +++ b/posthog/queries/test/test_base.py @@ -11,7 +11,12 @@ from posthog.models.filters.path_filter import PathFilter from posthog.models.property.property import Property -from posthog.queries.base import match_property, relative_date_parse_for_feature_flag_matching, sanitize_property_key +from posthog.queries.base import ( + match_property, + relative_date_parse_for_feature_flag_matching, + sanitize_property_key, + sanitize_regex_pattern, +) from posthog.test.base import APIBaseTest @@ -148,7 +153,7 @@ def test_match_properties_regex(self): mock_compile.return_value = pattern self.assertTrue(match_property(property_e, {"key": "5"})) - mock_compile.assert_called_once_with("5") + mock_compile.assert_called_once_with("5", re.IGNORECASE | re.DOTALL) def test_match_properties_math_operators(self): property_a = Property(key="key", value=1, operator="gt") @@ -361,7 +366,10 @@ def test_none_property_value_with_all_operators(self): self.assertTrue(match_property(property_d, {"key": None})) property_d_lower_case = Property(key="key", value="no", operator="regex") - self.assertFalse(match_property(property_d_lower_case, {"key": None})) + self.assertTrue(match_property(property_d_lower_case, {"key": None})) + + property_d_non_matching = Property(key="key", value="xyz", operator="regex") + self.assertFalse(match_property(property_d_non_matching, {"key": None})) property_e = Property(key="key", value=1, operator="gt") self.assertTrue(match_property(property_e, {"key": None})) @@ -555,3 +563,163 @@ def test_year_parsing(self): assert relative_date_parse_for_feature_flag_matching("8y") == datetime.datetime( 2012, 1, 1, 12, 1, 20, 134000, tzinfo=tz.gettz("UTC") ) + + +class TestSanitizeRegexPattern(TestCase): + def test_basic_property_matching(self): + test_cases = [ + # Simple key-value matches + ('"key":"value"', "{'key': 'value'}", True), + # Quote style variations + ("'key':'value'", "{'key': 'value'}", True), + (r"\"key\":\"value\"", "{'key': 'value'}", True), + # Whitespace handling + ('"key" : "value"', "{'key':'value'}", True), + ('"key":"value"', "{'key': 'value'}", True), + # Case sensitivity + ('"LANG":"EN"', "{'lang': 'en'}", True), + ('"lang":"en"', "{'LANG': 'EN'}", True), + # Basic non-matching cases + ('"key":"value"', "{'key': 'different'}", False), + ('"key":"value"', "{'different': 'value'}", False), + ] + + for pattern, test_string, should_match in test_cases: + sanitized = sanitize_regex_pattern(pattern) + match = re.search(sanitized, test_string, re.DOTALL | re.IGNORECASE) + self.assertEqual( + bool(match), + should_match, + f"Failed for pattern: {pattern}\nSanitized to: {sanitized}\nTesting against: {test_string}\nExpected match: {should_match}", + ) + + def test_property_name_patterns(self): + test_cases = [ + # Special characters in property names + ('"user-id":"123"', "{'user-id': '123'}", True), + ('"@timestamp":"2023-01-01"', "{'@timestamp': '2023-01-01'}", True), + ('"$ref":"#/definitions/user"', "{'$ref': '#/definitions/user'}", True), + ('"special.key":"value"', "{'special.key': 'value'}", True), + ('"snake_case_key":"value"', "{'snake_case_key': 'value'}", True), + # Unicode and emoji + ('"über":"value"', "{'über': 'value'}", True), + ('"emoji🔑":"value"', "{'emoji🔑': 'value'}", True), + # Empty or special-only names + ('"-":"value"', "{'-': 'value'}", True), + ('"_":"value"', "{'_': 'value'}", True), + # Multiple special characters + ('"$special@key.name-here":"value"', "{'$special@key.name-here': 'value'}", True), + # Partial matches should not work + ('"user-id":"123"', "{'different-id': '123'}", False), + ('"lang":"en"', "{'language': 'en'}", False), + ] + + for pattern, test_string, should_match in test_cases: + sanitized = sanitize_regex_pattern(pattern) + match = re.search(sanitized, test_string, re.DOTALL | re.IGNORECASE) + self.assertEqual( + bool(match), + should_match, + f"Failed for pattern: {pattern}\nSanitized to: {sanitized}\nTesting against: {test_string}\nExpected match: {should_match}", + ) + + def test_nested_structures(self): + test_cases = [ + # Simple nested dictionary + ('"key":"value"', "{'parent': {'key': 'value'}}", True), + # Multiple levels of nesting + ('"a":{"b":{"c":"value"}}', '{"a":{"b":{"c":"value"}}}', True), + # Array values + ('"array":["value1","value2"]', '{"array":["value1","value2"]}', True), + # Mixed nesting with arrays and objects + ('"data":{"items":[{"id":"123"}]}', '{"data":{"items":[{"id":"123"}]}}', True), + # Multiple properties with nesting + ( + '"lang":"en"[^}]*"slug":"web"', + "{'id': 123, 'data': {'lang': 'en', 'meta': {'slug': 'web'}, 'active': true}}", + True, + ), + # Incorrect nesting level should not match + ( + '"lang":"en"[^}]*"slug":"web"', + "{'data': {'lang': 'en'}, 'slug': 'mobile'}", + False, + ), + # Nested structures with whitespace and newlines + ( + '"lang":"en"[^}]*"slug":"web"', + """ + { + 'data': { + 'lang': 'en', + 'slug': 'web' + } + } + """, + True, + ), + ] + + for pattern, test_string, should_match in test_cases: + sanitized = sanitize_regex_pattern(pattern) + match = re.search(sanitized, test_string, re.DOTALL | re.IGNORECASE) + self.assertEqual( + bool(match), + should_match, + f"Failed for pattern: {pattern}\nSanitized to: {sanitized}\nTesting against: {test_string}\nExpected match: {should_match}", + ) + + def test_multiple_properties(self): + test_cases = [ + # Multiple properties in any order + ('"lang":"en"[^}]*"slug":"web"', "{'slug': 'web', 'lang': 'en'}", True), + ('"lang":"en"[^}]*"slug":"web"', "{'lang': 'en', 'slug': 'web'}", True), + # Properties with other data in between + ('"lang":"en"[^}]*"slug":"web"', "{'name': 'test', 'lang': 'en', 'other': 123, 'slug': 'web'}", True), + # Missing required property + ('"lang":"en"[^}]*"slug":"web"', "{'lang': 'en', 'url': 'web'}", False), + # Wrong property values + ('"lang":"en"[^}]*"slug":"web"', "{'lang': 'es', 'slug': 'web'}", False), + ('"lang":"en"[^}]*"slug":"web"', "{'lang': 'en', 'slug': 'mobile'}", False), + ] + + for pattern, test_string, should_match in test_cases: + sanitized = sanitize_regex_pattern(pattern) + match = re.search(sanitized, test_string, re.DOTALL | re.IGNORECASE) + self.assertEqual( + bool(match), + should_match, + f"Failed for pattern: {pattern}\nSanitized to: {sanitized}\nTesting against: {test_string}\nExpected match: {should_match}", + ) + + def test_simple_regex_patterns(self): + test_cases = [ + # Basic regex patterns + (r"^test$", "test", True), + (r"^test$", "test123", False), + (r"test\d+", "test123", True), + (r"test\d+", "test", False), + # Wildcards and character classes + (r".*\.com$", "example.com", True), + (r".*\.com$", "example.org", False), + (r"[a-z]+", "abc", True), + (r"[a-z]+", "123", False), + # Groups and alternation + (r"(foo|bar)", "foo", True), + (r"(foo|bar)", "bar", True), + (r"(foo|bar)", "baz", False), + # Common patterns (email, URL) + (r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$", "test@example.com", True), + (r"^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$", "invalid-email", False), + (r"^https?://[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$", "https://example.com", True), + (r"^https?://[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$", "not-a-url", False), + ] + + for pattern, test_string, should_match in test_cases: + sanitized = sanitize_regex_pattern(pattern) + match = re.search(sanitized, test_string, re.DOTALL | re.IGNORECASE) + self.assertEqual( + bool(match), + should_match, + f"Failed for pattern: {pattern}\nSanitized to: {sanitized}\nTesting against: {test_string}\nExpected match: {should_match}", + ) diff --git a/posthog/schema.py b/posthog/schema.py index c6aa92d4bac1a..0df08f9e9ff9b 100644 --- a/posthog/schema.py +++ b/posthog/schema.py @@ -657,6 +657,21 @@ class OrderBy(StrEnum): SESSIONS = "sessions" +class Status1(StrEnum): + ARCHIVED = "archived" + ACTIVE = "active" + RESOLVED = "resolved" + PENDING_RELEASE = "pending_release" + ALL = "all" + + +class Status2(StrEnum): + ARCHIVED = "archived" + ACTIVE = "active" + RESOLVED = "resolved" + PENDING_RELEASE = "pending_release" + + class Interval(StrEnum): MINUTE = "minute" HOUR = "hour" @@ -1352,6 +1367,7 @@ class StickinessFilterLegacy(BaseModel): display: Optional[ChartDisplayType] = None hidden_legend_keys: Optional[dict[str, Union[bool, Any]]] = None show_legend: Optional[bool] = None + show_multiple_y_axes: Optional[bool] = None show_values_on_series: Optional[bool] = None @@ -1996,6 +2012,7 @@ class CohortPropertyFilter(BaseModel): model_config = ConfigDict( extra="forbid", ) + cohort_name: Optional[str] = None key: Literal["id"] = "id" label: Optional[str] = None operator: Optional[PropertyOperator] = PropertyOperator.IN_ @@ -2111,7 +2128,7 @@ class ErrorTrackingRelationalIssue(BaseModel): first_seen: AwareDatetime id: str name: Optional[str] = None - status: Status + status: Status2 class ErrorTrackingSparklineConfig(BaseModel): @@ -2631,6 +2648,7 @@ class StickinessFilter(BaseModel): display: Optional[ChartDisplayType] = None hiddenLegendIndexes: Optional[list[int]] = None showLegend: Optional[bool] = None + showMultipleYAxes: Optional[bool] = None showValuesOnSeries: Optional[bool] = None stickinessCriteria: Optional[StickinessCriteria] = None @@ -6670,6 +6688,7 @@ class ErrorTrackingQuery(BaseModel): orderBy: Optional[OrderBy] = None response: Optional[ErrorTrackingQueryResponse] = None searchQuery: Optional[str] = None + status: Optional[Status1] = None class EventsQuery(BaseModel): diff --git a/posthog/temporal/data_imports/pipelines/sql_database/__init__.py b/posthog/temporal/data_imports/pipelines/sql_database/__init__.py index e7faa8de79583..53991e760ee2d 100644 --- a/posthog/temporal/data_imports/pipelines/sql_database/__init__.py +++ b/posthog/temporal/data_imports/pipelines/sql_database/__init__.py @@ -370,7 +370,7 @@ def sql_table( metadata: Optional[MetaData] = None, incremental: Optional[dlt.sources.incremental[Any]] = None, chunk_size: int = DEFAULT_CHUNK_SIZE, - backend: TableBackend = "sqlalchemy", + backend: TableBackend = "pyarrow", detect_precision_hints: Optional[bool] = None, reflection_level: Optional[ReflectionLevel] = "full", defer_table_reflect: Optional[bool] = None, diff --git a/posthog/temporal/data_imports/pipelines/sql_database/arrow_helpers.py b/posthog/temporal/data_imports/pipelines/sql_database/arrow_helpers.py index 8fa110b3a56f9..1feb778955271 100644 --- a/posthog/temporal/data_imports/pipelines/sql_database/arrow_helpers.py +++ b/posthog/temporal/data_imports/pipelines/sql_database/arrow_helpers.py @@ -121,6 +121,15 @@ def row_tuples_to_arrow(rows: Sequence[RowAny], columns: TTableSchemaColumns, tz [None if x is not None and math.isnan(x) else x for x in columnar_known_types[field.name]] ) + if issubclass(py_type, bytes) or issubclass(py_type, str): + # For bytes/str columns, ensure any dict values are serialized to JSON strings + # Convert to numpy array after processing + processed_values = [ + None if x is None else json_dumps(x) if isinstance(x, dict | list) else x + for x in columnar_known_types[field.name] + ] + columnar_known_types[field.name] = np.array(processed_values, dtype=object) + # If there are unknown type columns, first create a table to infer their types if columnar_unknown_types: new_schema_fields = [] diff --git a/posthog/temporal/data_imports/pipelines/sql_database/test/test_arrow_helpers.py b/posthog/temporal/data_imports/pipelines/sql_database/test/test_arrow_helpers.py index 978fb8770fd65..5e5264eb3b692 100644 --- a/posthog/temporal/data_imports/pipelines/sql_database/test/test_arrow_helpers.py +++ b/posthog/temporal/data_imports/pipelines/sql_database/test/test_arrow_helpers.py @@ -1,6 +1,6 @@ import pytest import pyarrow as pa -from posthog.temporal.data_imports.pipelines.sql_database.arrow_helpers import json_dumps +from posthog.temporal.data_imports.pipelines.sql_database.arrow_helpers import json_dumps, row_tuples_to_arrow from dlt.common.json import json @@ -20,3 +20,18 @@ def test_handle_large_integers(): json_str_array = pa.array([None if s is None else json_dumps(s) for s in [{"a": -(2**64)}]]) loaded = json.loads(json_str_array[0].as_py()) assert loaded["a"] == float(-(2**64)) + + +def test_row_tuples_to_arrow_string_column_with_dict(): + # Test that row_tuples_to_arrow properly serializes dictionaries in string columns + test_dict = {"key": "value"} + rows = [("",), (test_dict,)] + columns = {"string_col": {"name": "string_col", "data_type": "text", "nullable": True}} + + # This should now succeed and serialize the dictionary to JSON + table = row_tuples_to_arrow(rows, columns, "UTC") # type: ignore + + # Verify the results + assert table.column("string_col")[0].as_py() == "" + # The dictionary should be serialized to a JSON string + assert json.loads(table.column("string_col")[1].as_py()) == test_dict diff --git a/posthog/temporal/tests/batch_exports/test_bigquery_batch_export_workflow.py b/posthog/temporal/tests/batch_exports/test_bigquery_batch_export_workflow.py index 23f6a1e663fbb..0f8860ce971eb 100644 --- a/posthog/temporal/tests/batch_exports/test_bigquery_batch_export_workflow.py +++ b/posthog/temporal/tests/batch_exports/test_bigquery_batch_export_workflow.py @@ -59,7 +59,7 @@ pytestmark = [SKIP_IF_MISSING_GOOGLE_APPLICATION_CREDENTIALS, pytest.mark.asyncio, pytest.mark.django_db] -TEST_TIME = dt.datetime.now(dt.UTC) +TEST_TIME = dt.datetime.now(dt.UTC).replace(hour=0, minute=0, second=0, microsecond=0) EXPECTED_PERSONS_BATCH_EXPORT_FIELDS = [ "team_id", @@ -1239,9 +1239,9 @@ async def test_bigquery_export_workflow_without_events( @pytest.mark.parametrize( "data_interval_start", - # This is hardcoded relative to the `data_interval_end` used in all or most tests, since that's also - # passed to `generate_test_data` to determine the timestamp for the generated data. - [dt.datetime(2023, 4, 24, 15, 0, 0, tzinfo=dt.UTC)], + # This is set to 24 hours before the `data_interval_end` to ensure that the data created is outside the batch + # interval. + [TEST_TIME - dt.timedelta(hours=24)], indirect=True, ) @pytest.mark.parametrize("interval", ["hour"], indirect=True) diff --git a/posthog/temporal/tests/batch_exports/test_postgres_batch_export_workflow.py b/posthog/temporal/tests/batch_exports/test_postgres_batch_export_workflow.py index 30d184462a221..0007d43ba55a1 100644 --- a/posthog/temporal/tests/batch_exports/test_postgres_batch_export_workflow.py +++ b/posthog/temporal/tests/batch_exports/test_postgres_batch_export_workflow.py @@ -661,8 +661,8 @@ async def test_postgres_export_workflow_without_events( @pytest.mark.parametrize( "data_interval_start", - # This is hardcoded relative to the `data_interval_end` used in all or most tests, since that's also - # passed to `generate_test_data` to determine the timestamp for the generated data. + # This is set to 24 hours before the `data_interval_end` to ensure that the data created is outside the batch + # interval. [dt.datetime.now(tz=dt.UTC).replace(hour=0, minute=0, second=0, microsecond=0) - dt.timedelta(hours=24)], indirect=True, ) diff --git a/posthog/temporal/tests/batch_exports/test_s3_batch_export_workflow.py b/posthog/temporal/tests/batch_exports/test_s3_batch_export_workflow.py index 3c589bbe0272a..1e0b08856edb4 100644 --- a/posthog/temporal/tests/batch_exports/test_s3_batch_export_workflow.py +++ b/posthog/temporal/tests/batch_exports/test_s3_batch_export_workflow.py @@ -978,8 +978,8 @@ async def test_s3_export_workflow_with_minio_bucket( @pytest.mark.parametrize( "data_interval_start", - # This is hardcoded relative to the `data_interval_end` used in all or most tests, since that's also - # passed to `generate_test_data` to determine the timestamp for the generated data. + # This is set to 24 hours before the `data_interval_end` to ensure that the data created is outside the batch + # interval. [dt.datetime.now(tz=dt.UTC).replace(hour=0, minute=0, second=0, microsecond=0) - dt.timedelta(hours=24)], indirect=True, ) diff --git a/posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py b/posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py index d5921078a4b5e..2991b3480eb1c 100644 --- a/posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py +++ b/posthog/temporal/tests/batch_exports/test_snowflake_batch_export_workflow.py @@ -72,6 +72,9 @@ ] +TEST_TIME = dt.datetime.now(dt.UTC).replace(hour=0, minute=0, second=0, microsecond=0) + + class FakeSnowflakeCursor: """A fake Snowflake cursor that can fail on PUT and COPY queries.""" @@ -1459,9 +1462,9 @@ async def test_snowflake_export_workflow_with_many_files( @SKIP_IF_MISSING_REQUIRED_ENV_VARS @pytest.mark.parametrize( "data_interval_start", - # This is hardcoded relative to the `data_interval_end` used in all or most tests, since that's also - # passed to `generate_test_data` to determine the timestamp for the generated data. - [dt.datetime(2023, 4, 24, 15, 0, 0, tzinfo=dt.UTC)], + # This is set to 24 hours before the `data_interval_end` to ensure that the data created is outside the batch + # interval. + [TEST_TIME - dt.timedelta(hours=24)], indirect=True, ) @pytest.mark.parametrize("interval", ["hour"], indirect=True)