-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: query-ify the matching_events API #26916
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,9 @@ import { actions, connect, events, kea, key, listeners, path, props, propsChange | |
import { loaders } from 'kea-loaders' | ||
import api from 'lib/api' | ||
import { TaxonomicFilterGroupType } from 'lib/components/TaxonomicFilter/types' | ||
import { FEATURE_FLAGS } from 'lib/constants' | ||
import { Dayjs, dayjs } from 'lib/dayjs' | ||
import { featureFlagLogic } from 'lib/logic/featureFlagLogic' | ||
import { getCoreFilterDefinition } from 'lib/taxonomy' | ||
import { eventToDescription, humanizeBytes, objectsEqual, toParams } from 'lib/utils' | ||
import { eventUsageLogic } from 'lib/utils/eventUsageLogic' | ||
|
@@ -22,6 +24,7 @@ import { | |
MatchingEventsMatchType, | ||
} from 'scenes/session-recordings/playlist/sessionRecordingsPlaylistLogic' | ||
|
||
import { RecordingsQuery } from '~/queries/schema' | ||
import { | ||
FilterableInspectorListItemTypes, | ||
MatchedRecordingEvent, | ||
|
@@ -248,6 +251,8 @@ export const playerInspectorLogic = kea<playerInspectorLogicType>([ | |
['allPerformanceEvents'], | ||
sessionRecordingDataLogic(props), | ||
['trackedWindow'], | ||
featureFlagLogic, | ||
['featureFlags'], | ||
], | ||
})), | ||
actions(() => ({ | ||
|
@@ -275,7 +280,7 @@ export const playerInspectorLogic = kea<playerInspectorLogicType>([ | |
}, | ||
], | ||
})), | ||
loaders(({ props }) => ({ | ||
loaders(({ props, values }) => ({ | ||
matchingEventUUIDs: [ | ||
[] as MatchedRecordingEvent[] | null, | ||
{ | ||
|
@@ -297,17 +302,29 @@ export const playerInspectorLogic = kea<playerInspectorLogicType>([ | |
if (!filters) { | ||
throw new Error('Backend matching events type must include its filters') | ||
} | ||
const params = toParams({ | ||
// as_query is a temporary parameter as a flag | ||
// to let the backend know not to convert the query to a legacy filter when processing | ||
const params: RecordingsQuery & { as_query?: boolean } = { | ||
...convertUniversalFiltersToRecordingsQuery(filters), | ||
session_ids: [props.sessionRecordingId], | ||
}) | ||
const response = await api.recordings.getMatchingEvents(params) | ||
} | ||
if (values.listAPIAsQuery) { | ||
params.as_query = true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. switch to the new query code (or not) |
||
} | ||
const response = await api.recordings.getMatchingEvents(toParams(params)) | ||
return response.results.map((x) => ({ uuid: x } as MatchedRecordingEvent)) | ||
}, | ||
}, | ||
], | ||
})), | ||
selectors(({ props }) => ({ | ||
listAPIAsQuery: [ | ||
(s) => [s.featureFlags], | ||
(featureFlags) => { | ||
return !!featureFlags[FEATURE_FLAGS.REPLAY_LIST_RECORDINGS_AS_QUERY] | ||
}, | ||
], | ||
|
||
allowMatchingEventsFilter: [ | ||
(s) => [s.miniFilters], | ||
(miniFilters): boolean => { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,6 +21,7 @@ | |
from rest_framework.response import Response | ||
from rest_framework.utils.encoders import JSONEncoder | ||
|
||
import posthog.session_recordings.queries.session_recording_list_from_query | ||
from ee.session_recordings.session_summary.summarize_session import summarize_recording | ||
from posthog.api.person import MinimalPersonSerializer | ||
from posthog.api.routing import TeamAndOrgViewSetMixin | ||
|
@@ -360,31 +361,54 @@ def list(self, request: request.Request, *args: Any, **kwargs: Any) -> Response: | |
) | ||
@action(methods=["GET"], detail=False) | ||
def matching_events(self, request: request.Request, *args: Any, **kwargs: Any) -> JsonResponse: | ||
filter = SessionRecordingsFilter(request=request, team=self.team) | ||
use_query_type = (request.GET.get("as_query", "False")).lower() == "true" | ||
|
||
if not filter.session_ids or len(filter.session_ids) != 1: | ||
raise exceptions.ValidationError( | ||
"Must specify exactly one session_id", | ||
) | ||
if use_query_type: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a little duplication is ok since we're going to delete one branch pretty soon |
||
data_dict = query_as_params_to_dict(request.GET.dict()) | ||
query = RecordingsQuery.model_validate(data_dict) | ||
|
||
# a little duplication for now | ||
if not query.session_ids or len(query.session_ids) != 1: | ||
raise exceptions.ValidationError( | ||
"Must specify exactly one session_id", | ||
) | ||
|
||
if not filter.events and not filter.actions: | ||
raise exceptions.ValidationError( | ||
"Must specify at least one event or action filter", | ||
if not query.events and not query.actions: | ||
raise exceptions.ValidationError( | ||
"Must specify at least one event or action filter", | ||
) | ||
|
||
distinct_id = str(cast(User, request.user).distinct_id) | ||
modifiers = safely_read_modifiers_overrides(distinct_id, self.team) | ||
results, _, timings = ( | ||
posthog.session_recordings.queries.session_recording_list_from_query.ReplayFiltersEventsSubQuery( | ||
query=query, team=self.team, hogql_query_modifiers=modifiers | ||
).get_event_ids_for_session() | ||
) | ||
else: | ||
filter = SessionRecordingsFilter(request=request, team=self.team) | ||
|
||
distinct_id = str(cast(User, request.user).distinct_id) | ||
modifiers = safely_read_modifiers_overrides(distinct_id, self.team) | ||
matching_events_query_response = ReplayFiltersEventsSubQuery( | ||
filter=filter, team=self.team, hogql_query_modifiers=modifiers | ||
).get_event_ids_for_session() | ||
if not filter.session_ids or len(filter.session_ids) != 1: | ||
raise exceptions.ValidationError( | ||
"Must specify exactly one session_id", | ||
) | ||
|
||
if not filter.events and not filter.actions: | ||
raise exceptions.ValidationError( | ||
"Must specify at least one event or action filter", | ||
) | ||
|
||
distinct_id = str(cast(User, request.user).distinct_id) | ||
modifiers = safely_read_modifiers_overrides(distinct_id, self.team) | ||
results, _, timings = ReplayFiltersEventsSubQuery( | ||
filter=filter, team=self.team, hogql_query_modifiers=modifiers | ||
).get_event_ids_for_session() | ||
|
||
response = JsonResponse(data={"results": matching_events_query_response.results}) | ||
response = JsonResponse(data={"results": results}) | ||
|
||
response.headers["Server-Timing"] = ", ".join( | ||
f"{key};dur={round(duration, ndigits=2)}" | ||
for key, duration in _generate_timings( | ||
matching_events_query_response.timings, ServerTimingsGathered() | ||
).items() | ||
for key, duration in _generate_timings(timings, ServerTimingsGathered()).items() | ||
) | ||
return response | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1155,6 +1155,45 @@ def test_get_matching_events_for_unknown_session(self) -> None: | |
assert response.status_code == status.HTTP_200_OK | ||
assert response.json() == {"results": []} | ||
|
||
def test_get_matching_events_with_query(self) -> None: | ||
base_time = (now() - relativedelta(days=1)).replace(microsecond=0) | ||
|
||
# the matching session | ||
session_id = f"test_get_matching_events-1-{uuid.uuid4()}" | ||
self.produce_replay_summary("user", session_id, base_time) | ||
event_id = _create_event( | ||
event="$pageview", | ||
properties={"$session_id": session_id}, | ||
team=self.team, | ||
distinct_id=uuid.uuid4(), | ||
) | ||
|
||
# a non-matching session | ||
non_matching_session_id = f"test_get_matching_events-2-{uuid.uuid4()}" | ||
self.produce_replay_summary("user", non_matching_session_id, base_time) | ||
_create_event( | ||
event="$pageview", | ||
properties={"$session_id": non_matching_session_id}, | ||
team=self.team, | ||
distinct_id=uuid.uuid4(), | ||
) | ||
|
||
flush_persons_and_events() | ||
# data needs time to settle :'( | ||
time.sleep(1) | ||
|
||
query_params = [ | ||
f'{SESSION_RECORDINGS_FILTER_IDS}=["{session_id}"]', | ||
'events=[{"id": "$pageview", "type": "events", "order": 0, "name": "$pageview"}]', | ||
] | ||
|
||
response = self.client.get( | ||
f"/api/projects/{self.team.id}/session_recordings/matching_events?{'&'.join(query_params)}&as_query=true" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. prove it works |
||
) | ||
|
||
assert response.status_code == status.HTTP_200_OK | ||
assert response.json() == {"results": [event_id]} | ||
|
||
def test_get_matching_events(self) -> None: | ||
base_time = (now() - relativedelta(days=1)).replace(microsecond=0) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are never present here, so we can delete this