Skip to content

Commit

Permalink
feat: add settings to allow opt in to AI processing (#20516)
Browse files Browse the repository at this point in the history
* fangl settings nicerly

* team logic safety

* wire up in the backend team api

* separate flag for the settings

* make the text a bit more generic

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update query snapshots

* Update query snapshots

* Update query snapshots

* Update query snapshots

* Update UI snapshots for `chromium` (2)

* Update UI snapshots for `chromium` (2)

* Update query snapshots

* test the patching

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
  • Loading branch information
pauldambra and github-actions[bot] authored Feb 26, 2024
1 parent c940fa5 commit cec3a7d
Show file tree
Hide file tree
Showing 13 changed files with 553 additions and 177 deletions.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions frontend/src/lib/constants.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,6 +196,7 @@ export const FEATURE_FLAGS = {
SIDEPANEL_STATUS: 'sidepanel-status', // owner: @benjackwhite
NEW_FEATURE_FLAG_OPERATORS: 'new-feature-flag-operators', // owner: @neilkakkar
AI_SESSION_SUMMARY: 'ai-session-summary', // owner: #team-replay
AI_SESSION_PERMISSIONS: 'ai-session-permissions', // owner: #team-replay
PRODUCT_INTRO_PAGES: 'product-intro-pages', // owner: @raquelmsmith
DATANODE_CONCURRENCY_LIMIT: 'datanode-concurrency-limit', // owner: @robbie-c
SESSION_REPLAY_DOCTOR: 'session-replay-doctor', // owner: #team-replay
Expand Down
13 changes: 12 additions & 1 deletion frontend/src/scenes/settings/SettingsMap.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,12 @@ import {
ProjectVariables,
WebSnippet,
} from './project/ProjectSettings'
import { ReplayAuthorizedDomains, ReplayCostControl, ReplayGeneral } from './project/SessionRecordingSettings'
import {
ReplayAuthorizedDomains,
ReplayCostControl,
ReplayGeneral,
ReplaySummarySettings,
} from './project/SessionRecordingSettings'
import { SettingPersonsOnEvents } from './project/SettingPersonsOnEvents'
import { SlackIntegration } from './project/SlackIntegration'
import { SurveySettings } from './project/SurveySettings'
Expand Down Expand Up @@ -167,6 +172,12 @@ export const SettingsMap: SettingSection[] = [
AvailableFeature.REPLAY_RECORDING_DURATION_MINIMUM,
],
},
{
id: 'replay-ai-config',
title: 'AI Recording Summary',
component: <ReplaySummarySettings />,
flag: 'AI_SESSION_PERMISSIONS',
},
],
},
{
Expand Down
469 changes: 316 additions & 153 deletions frontend/src/scenes/settings/project/SessionRecordingSettings.tsx

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions frontend/src/scenes/settings/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export type SettingId =
| 'notifications'
| 'optout'
| 'theme'
| 'replay-ai-config'

export type Setting = {
id: SettingId
Expand Down
8 changes: 8 additions & 0 deletions frontend/src/scenes/teamLogic.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,14 @@ export const teamLogic = kea<teamLogicType>([
throw new Error('Current team has not been loaded yet, so it cannot be updated!')
}

// session replay config is nested, so we need to make sure we don't overwrite config
if (payload.session_replay_config) {
payload.session_replay_config = {
...values.currentTeam.session_replay_config,
...payload.session_replay_config,
}
}

const patchedTeam = (await api.update(`api/projects/${values.currentTeam.id}`, payload)) as TeamType
breakpoint()

Expand Down
10 changes: 9 additions & 1 deletion frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,14 @@ export interface CorrelationConfigType {
excluded_event_names?: string[]
}

export interface SessionRecordingAIConfig {
opt_in: boolean
preferred_events: string[]
excluded_events: string[]
included_event_properties: string[]
important_user_properties: string[]
}

export interface TeamType extends TeamBasicType {
created_at: string
updated_at: string
Expand All @@ -418,7 +426,7 @@ export interface TeamType extends TeamBasicType {
| { recordHeaders?: boolean; recordBody?: boolean }
| undefined
| null
session_replay_config: { record_canvas?: boolean } | undefined | null
session_replay_config: { record_canvas?: boolean; ai_config?: SessionRecordingAIConfig } | undefined | null
autocapture_exceptions_opt_in: boolean
surveys_opt_in?: boolean
autocapture_exceptions_errors_to_ignore: string[]
Expand Down
56 changes: 54 additions & 2 deletions posthog/api/team.py
Original file line number Diff line number Diff line change
Expand Up @@ -227,8 +227,33 @@ def validate_session_replay_config(self, value) -> Dict | None:
if not isinstance(value, Dict):
raise exceptions.ValidationError("Must provide a dictionary or None.")

if not all(key in ["record_canvas"] for key in value.keys()):
raise exceptions.ValidationError("Must provide a dictionary with only 'record_canvas' key.")
known_keys = ["record_canvas", "ai_config"]
if not all(key in known_keys for key in value.keys()):
raise exceptions.ValidationError(
f"Must provide a dictionary with only known keys. One or more of {', '.join(known_keys)}."
)

if "ai_config" in value:
self.validate_session_replay_ai_summary_config(value["ai_config"])

return value

def validate_session_replay_ai_summary_config(self, value: Dict | None) -> Dict | None:
if value is not None:
if not isinstance(value, Dict):
raise exceptions.ValidationError("Must provide a dictionary or None.")

allowed_keys = [
"included_event_properties",
"opt_in",
"preferred_events",
"excluded_events",
"important_user_properties",
]
if not all(key in allowed_keys for key in value.keys()):
raise exceptions.ValidationError(
f"Must provide a dictionary with only allowed keys: {', '.join(allowed_keys)}."
)

return value

Expand Down Expand Up @@ -316,6 +341,33 @@ def update(self, instance: Team, validated_data: Dict[str, Any]) -> Team:
if "timezone" in validated_data and validated_data["timezone"] != instance.timezone:
self._handle_timezone_update(instance)

if (
"session_replay_config" in validated_data
and validated_data["session_replay_config"] is not None
and instance.session_replay_config is not None
):
# for session_replay_config and its top level keys we merge existing settings with new settings
# this way we don't always have to receive the entire settings object to change one setting
# so for each key in validated_data["session_replay_config"] we merge it with the existing settings
# and then merge any top level keys that weren't provided

for key, value in validated_data["session_replay_config"].items():
if key in instance.session_replay_config:
# if they're both dicts then we merge them, otherwise, the new value overwrites the old
if isinstance(instance.session_replay_config[key], dict) and isinstance(
validated_data["session_replay_config"][key], dict
):
validated_data["session_replay_config"][key] = {
**instance.session_replay_config[key], # existing values
**value, # and new values on top
}

# then also add back in any keys that exist but are not in the provided data
validated_data["session_replay_config"] = {
**instance.session_replay_config,
**validated_data["session_replay_config"],
}

updated_team = super().update(instance, validated_data)
changes = dict_changes_between("Team", before_update, updated_team.__dict__, use_field_exclusions=True)

Expand Down
16 changes: 16 additions & 0 deletions posthog/api/test/__snapshots__/test_decide.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,22 @@
LIMIT 21 /*controller='team-detail',route='api/projects/%28%3FP%3Cid%3E%5B%5E/.%5D%2B%29/%3F%24'*/
'''
# ---
# name: TestDecide.test_decide_doesnt_error_out_when_database_is_down.10
'''
SELECT "posthog_pluginconfig"."id",
"posthog_pluginconfig"."web_token",
"posthog_pluginsourcefile"."updated_at",
"posthog_plugin"."updated_at",
"posthog_pluginconfig"."updated_at"
FROM "posthog_pluginconfig"
INNER JOIN "posthog_plugin" ON ("posthog_pluginconfig"."plugin_id" = "posthog_plugin"."id")
INNER JOIN "posthog_pluginsourcefile" ON ("posthog_plugin"."id" = "posthog_pluginsourcefile"."plugin_id")
WHERE ("posthog_pluginconfig"."enabled"
AND "posthog_pluginsourcefile"."filename" = 'site.ts'
AND "posthog_pluginsourcefile"."status" = 'TRANSPILED'
AND "posthog_pluginconfig"."team_id" = 2)
'''
# ---
# name: TestDecide.test_decide_doesnt_error_out_when_database_is_down.2
'''
SELECT "posthog_organizationmembership"."id",
Expand Down
11 changes: 11 additions & 0 deletions posthog/api/test/dashboards/__snapshots__/test_dashboard.ambr
Original file line number Diff line number Diff line change
Expand Up @@ -8350,6 +8350,17 @@
LIMIT 1 /*controller='project_dashboards-detail',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/dashboards/%28%3FP%3Cpk%3E%5B%5E/.%5D%2B%29/%3F%24'*/
'''
# ---
# name: TestDashboard.test_loading_individual_dashboard_does_not_prefetch_all_possible_tiles.315
'''
SELECT "posthog_instancesetting"."id",
"posthog_instancesetting"."key",
"posthog_instancesetting"."raw_value"
FROM "posthog_instancesetting"
WHERE "posthog_instancesetting"."key" = 'constance:posthog:PERSON_ON_EVENTS_ENABLED'
ORDER BY "posthog_instancesetting"."id" ASC
LIMIT 1 /*controller='project_dashboards-detail',route='api/projects/%28%3FP%3Cparent_lookup_team_id%3E%5B%5E/.%5D%2B%29/dashboards/%28%3FP%3Cpk%3E%5B%5E/.%5D%2B%29/%3F%24'*/
'''
# ---
# name: TestDashboard.test_loading_individual_dashboard_does_not_prefetch_all_possible_tiles.32
'''
SELECT "posthog_team"."id",
Expand Down
127 changes: 116 additions & 11 deletions posthog/api/test/test_team.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import json
import uuid
from typing import List, cast, Dict, Optional
from typing import List, cast, Dict, Optional, Any
from unittest import mock
from unittest.mock import MagicMock, call, patch, ANY

from asgiref.sync import sync_to_async
from django.core.cache import cache
from django.http import HttpResponse
from freezegun import freeze_time
from parameterized import parameterized
from rest_framework import status
Expand Down Expand Up @@ -914,19 +915,123 @@ def test_can_set_and_unset_session_recording_network_payload_capture_config(self

def test_can_set_and_unset_session_replay_config(self) -> None:
# can set
first_patch_response = self.client.patch(
"/api/projects/@current/",
{"session_replay_config": {"record_canvas": True}},
self._patch_session_replay_config({"record_canvas": True})
self._assert_replay_config_is({"record_canvas": True})

# can unset
self._patch_session_replay_config(None)
self._assert_replay_config_is(None)

@parameterized.expand(
[
[
"string",
"Marple bridge",
"invalid_input",
"Must provide a dictionary or None.",
],
["numeric", "-1", "invalid_input", "Must provide a dictionary or None."],
[
"unexpected json - no record",
{"key": "something"},
"invalid_input",
"Must provide a dictionary with only allowed keys: included_event_properties, opt_in, preferred_events, excluded_events, important_user_properties.",
],
]
)
def test_invalid_session_replay_config_ai_config(
self, _name: str, provided_value: str, expected_code: str, expected_error: str
) -> None:
response = self._patch_session_replay_config(
{"ai_config": provided_value}, expected_status=status.HTTP_400_BAD_REQUEST
)
assert first_patch_response.status_code == status.HTTP_200_OK
assert response.json() == {
"attr": "session_replay_config",
"code": expected_code,
"detail": expected_error,
"type": "validation_error",
}

def test_can_set_and_unset_session_replay_config_ai_config(self) -> None:
# can set just the opt-in
self._patch_session_replay_config({"ai_config": {"opt_in": True}})
self._assert_replay_config_is({"ai_config": {"opt_in": True}})

# can set some preferences
self._patch_session_replay_config({"ai_config": {"opt_in": False, "included_event_properties": ["something"]}})
self._assert_replay_config_is({"ai_config": {"opt_in": False, "included_event_properties": ["something"]}})

self._patch_session_replay_config({"ai_config": None})
self._assert_replay_config_is({"ai_config": None})

def test_can_set_replay_configs_without_providing_them_all(self) -> None:
# can set just the opt-in
self._patch_session_replay_config({"ai_config": {"opt_in": True}})
self._assert_replay_config_is({"ai_config": {"opt_in": True}})

self._patch_session_replay_config({"record_canvas": True})
self._assert_replay_config_is({"record_canvas": True, "ai_config": {"opt_in": True}})

def test_can_set_replay_configs_without_providing_them_all_even_when_either_side_is_none(self) -> None:
# because we do some dictionary copying we need a regression test to ensure we can always set and unset keys
self._patch_session_replay_config({"record_canvas": True, "ai_config": {"opt_in": True}})
self._assert_replay_config_is({"record_canvas": True, "ai_config": {"opt_in": True}})

self._patch_session_replay_config({"record_canvas": None})
self._assert_replay_config_is({"record_canvas": None, "ai_config": {"opt_in": True}})

# top-level from having a value to None
self._patch_session_replay_config(None)
self._assert_replay_config_is(None)

# top-level from None to having a value
self._patch_session_replay_config({"ai_config": None})
self._assert_replay_config_is({"ai_config": None})

# next-level from None to having a value
self._patch_session_replay_config({"ai_config": {"opt_in": True}})
self._assert_replay_config_is({"ai_config": {"opt_in": True}})

# next-level from having a value to None
self._patch_session_replay_config({"ai_config": None})
self._assert_replay_config_is({"ai_config": None})

def test_can_set_replay_configs_patch_session_replay_config_one_level_deep(self) -> None:
# can set just the opt-in
self._patch_session_replay_config({"ai_config": {"opt_in": True}})
self._assert_replay_config_is({"ai_config": {"opt_in": True}})

self._patch_session_replay_config({"ai_config": {"included_event_properties": ["something"]}})
# even though opt_in was not provided in the patch it should be preserved
self._assert_replay_config_is({"ai_config": {"opt_in": True, "included_event_properties": ["something"]}})

self._patch_session_replay_config({"ai_config": {"opt_in": None, "included_event_properties": ["something"]}})
# even though opt_in was not provided in the patch it should be preserved
self._assert_replay_config_is({"ai_config": {"opt_in": None, "included_event_properties": ["something"]}})

# but we don't go into the next nested level and patch that data
# sending a new value without the original
self._patch_session_replay_config({"ai_config": {"included_event_properties": ["and another"]}})
# and the existing second level nesting is not preserved
self._assert_replay_config_is({"ai_config": {"opt_in": None, "included_event_properties": ["and another"]}})

def _assert_replay_config_is(self, expected: Dict[str, Any] | None) -> HttpResponse:
get_response = self.client.get("/api/projects/@current/")
assert get_response.json()["session_replay_config"] == {"record_canvas": True}
assert get_response.status_code == status.HTTP_200_OK, get_response.json()
assert get_response.json()["session_replay_config"] == expected

# can unset
response = self.client.patch("/api/projects/@current/", {"session_replay_config": None})
assert response.status_code == status.HTTP_200_OK
second_get_response = self.client.get("/api/projects/@current/")
assert second_get_response.json()["session_replay_config"] is None
return get_response

def _patch_session_replay_config(
self, config: Dict[str, Any] | None, expected_status: int = status.HTTP_200_OK
) -> HttpResponse:
patch_response = self.client.patch(
"/api/projects/@current/",
{"session_replay_config": config},
)
assert patch_response.status_code == expected_status, patch_response.json()

return patch_response


def create_team(organization: Organization, name: str = "Test team") -> Team:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@
WHERE and(equals(events.team_id, 2), greaterOrEquals(toTimeZone(events.timestamp, 'UTC'), minus(toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-12 00:00:00', 6, 'UTC'))), toIntervalDay(1))), less(toTimeZone(events.timestamp, 'UTC'), plus(toStartOfDay(assumeNotNull(parseDateTime64BestEffortOrNull('2020-01-19 23:59:59', 6, 'UTC'))), toIntervalDay(1))), ifNull(in(person_id,
(SELECT cohortpeople.person_id AS person_id
FROM cohortpeople
WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 6))
WHERE and(equals(cohortpeople.team_id, 2), equals(cohortpeople.cohort_id, 8))
GROUP BY cohortpeople.person_id, cohortpeople.cohort_id, cohortpeople.version
HAVING ifNull(greater(sum(cohortpeople.sign), 0), 0))), 0), equals(events.event, '$pageview'))
GROUP BY person_id)
Expand Down
Loading

0 comments on commit cec3a7d

Please sign in to comment.