Skip to content
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: improve playlist counts #29381

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
f7f0526
feat: improve playlist counts
pauldambra Feb 28, 2025
5b98189
fix
pauldambra Feb 28, 2025
2584163
more useful logs
pauldambra Mar 1, 2025
10b9301
more useful logging
pauldambra Mar 1, 2025
dcd8c9a
Update UI snapshots for `chromium` (2)
github-actions[bot] Mar 1, 2025
90a059e
Update UI snapshots for `chromium` (2)
github-actions[bot] Mar 1, 2025
0755ba2
obey mypy
pauldambra Mar 1, 2025
afe1047
Add counts to mock
pauldambra Mar 1, 2025
52c44d1
Merge branch 'master' into feat/improve-counts-for-playlists
pauldambra Mar 1, 2025
711cb20
this?
pauldambra Mar 1, 2025
75d7c90
in the story too
pauldambra Mar 1, 2025
3dce8d1
Update posthog/session_recordings/session_recording_playlist_api.py
pauldambra Mar 1, 2025
60a1082
make sure they're available to reference
pauldambra Mar 1, 2025
6271ce9
Silly greptile
pauldambra Mar 1, 2025
f48e2ae
Update UI snapshots for `chromium` (2)
github-actions[bot] Mar 1, 2025
4793290
obey mypy
pauldambra Mar 1, 2025
ea1001f
coerce mypy
pauldambra Mar 1, 2025
c0d238a
make template match description
pauldambra Mar 1, 2025
8b6795a
run if empty despite cooldown
pauldambra Mar 1, 2025
e6f6c6e
display distinguishes between no result and zero result
pauldambra Mar 1, 2025
dfc713f
Merge branch 'master' into feat/improve-counts-for-playlists
pauldambra Mar 2, 2025
ed531a4
don't change this yet
pauldambra Mar 2, 2025
5611d91
Update UI snapshots for `chromium` (2)
github-actions[bot] Mar 2, 2025
d4ac6b8
Update UI snapshots for `chromium` (2)
github-actions[bot] Mar 2, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,8 @@ def convert_universal_filters_to_recordings_query(universal_filters: dict[str, A
expires=TASK_EXPIRATION_TIME,
)
def count_recordings_that_match_playlist_filters(playlist_id: int) -> None:
playlist: SessionRecordingPlaylist | None = None
query: RecordingsQuery | None = None
try:
with REPLAY_PLAYLIST_COUNT_TIMER.time():
playlist = SessionRecordingPlaylist.objects.get(id=playlist_id)
Expand All @@ -155,6 +157,7 @@ def count_recordings_that_match_playlist_filters(playlist_id: int) -> None:
else:
existing_value = {}

# if we have results from the last hour we don't need to run the query
if existing_value.get("refreshed_at"):
last_refreshed_at = datetime.fromisoformat(existing_value["refreshed_at"])
seconds_since_refresh = int((datetime.now() - last_refreshed_at).total_seconds())
Expand Down Expand Up @@ -182,13 +185,31 @@ def count_recordings_that_match_playlist_filters(playlist_id: int) -> None:

REPLAY_TEAM_PLAYLIST_COUNT_SUCCEEDED.inc()
except SessionRecordingPlaylist.DoesNotExist:
logger.info("Playlist does not exist", playlist_id=playlist_id)
logger.info(
"Playlist does not exist",
playlist_id=playlist_id,
playlist_short_id=playlist.short_id if playlist else None,
)
REPLAY_TEAM_PLAYLIST_COUNT_UNKNOWN.inc()
except Exception as e:
posthoganalytics.capture_exception(
e, properties={"playlist_id": playlist_id, "posthog_feature": "session_replay_playlist_counters"}
e,
properties={
"playlist_id": playlist_id,
"playlist_short_id": playlist.short_id if playlist else None,
"posthog_feature": "session_replay_playlist_counters",
"filters": playlist.filters if playlist else None,
"query": query,
Comment on lines +201 to +202
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

most important change here is grabbing these failing filters/queries so we can figure out the transformations the FE is doing that the BE is missing

},
)
logger.exception(
"Failed to count recordings that match playlist filters",
playlist_id=playlist_id,
playlist_short_id=playlist.short_id if playlist else None,
filters=playlist.filters if playlist else None,
query=query,
error=e,
)
logger.exception("Failed to count recordings that match playlist filters", playlist_id=playlist_id, error=e)
REPLAY_TEAM_PLAYLIST_COUNT_FAILED.inc()


Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,18 @@ const playlist = (playlistId: string): SessionRecordingPlaylistType => {
email: '[email protected]',
is_email_verified: true,
},
recordings_counts: {
saved_filters: {
count: 10,
watched_count: 4,
has_more: true,
increased: true,
},
collection: {
count: 10,
watched_count: 5,
},
},
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import { Meta } from '@storybook/react'
import { router } from 'kea-router'
import { FEATURE_FLAGS } from 'lib/constants'
import { useEffect } from 'react'
import { App } from 'scenes/App'
import recordingEventsJson from 'scenes/session-recordings/__mocks__/recording_events_query'
import { recordings } from 'scenes/session-recordings/__mocks__/recordings'
import { urls } from 'scenes/urls'

import { mswDecorator } from '~/mocks/browser'
import { mswDecorator, setFeatureFlags } from '~/mocks/browser'
import { ReplayTabs } from '~/types'

import { recordingPlaylists } from './__mocks__/recording_playlists'
Expand Down Expand Up @@ -43,6 +44,7 @@ const meta: Meta = {
export default meta

export function RecordingsPlayLists(): JSX.Element {
setFeatureFlags([FEATURE_FLAGS.SESSION_RECORDINGS_PLAYLIST_COUNT_COLUMN])
useEffect(() => {
router.actions.push(urls.replay(ReplayTabs.Playlists))
}, [])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,18 @@ export const recordingPlaylists: SavedSessionRecordingPlaylistsResult = {
first_name: 'Alex',
email: '[email protected]',
},
recordings_counts: {
saved_filters: {
count: 10,
watched_count: 4,
has_more: true,
increased: true,
},
collection: {
count: 10,
watched_count: 5,
},
},
},
{
id: 14,
Expand Down Expand Up @@ -81,6 +93,16 @@ export const recordingPlaylists: SavedSessionRecordingPlaylistsResult = {
first_name: 'Alex',
email: '[email protected]',
},
recordings_counts: {
saved_filters: {
count: 3,
watched_count: 4,
},
collection: {
count: 3,
watched_count: 4,
},
},
},
{
id: 15,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { DateFilter } from 'lib/components/DateFilter/DateFilter'
import { MemberSelect } from 'lib/components/MemberSelect'
import { TZLabel } from 'lib/components/TZLabel'
import { useFeatureFlag } from 'lib/hooks/useFeatureFlag'
import { IconArrowUp } from 'lib/lemon-ui/icons'
import { More } from 'lib/lemon-ui/LemonButton/More'
import { LemonTableColumn, LemonTableColumns } from 'lib/lemon-ui/LemonTable'
import { createdByColumn } from 'lib/lemon-ui/LemonTable/columnUtils'
Expand Down Expand Up @@ -39,7 +40,7 @@ function nameColumn(): LemonTableColumn<SessionRecordingPlaylistType, 'name'> {
}

function isPlaylistRecordingsCounts(x: unknown): x is PlaylistRecordingsCounts {
return isObject(x) && ('query_count' in x || 'pinned_count' in x)
return isObject(x) && ('collection' in x || 'saved_filters' in x)
}

export function SavedSessionRecordingPlaylists({ tab }: SavedSessionRecordingPlaylistsProps): JSX.Element {
Expand Down Expand Up @@ -74,39 +75,79 @@ export function SavedSessionRecordingPlaylists({ tab }: SavedSessionRecordingPla
return null
}

const count = (recordings_counts.pinned_count || 0) + (recordings_counts.query_count || 0)
const hasSavedFiltersCount = recordings_counts.saved_filters?.count !== null
const hasCollectionCount = recordings_counts.collection.count !== null

const totalPinnedCount =
recordings_counts.collection.count === null ? 'null' : recordings_counts.collection.count
const unwatchedPinnedCount =
(recordings_counts.collection.count || 0) - (recordings_counts.collection.watched_count || 0)
const totalSavedFiltersCount =
recordings_counts.saved_filters?.count === null
? 'null'
: recordings_counts.saved_filters?.count || 0
const unwatchedSavedFiltersCount =
(recordings_counts.saved_filters?.count || 0) -
(recordings_counts.saved_filters?.watched_count || 0)

const totalCount =
(totalPinnedCount === 'null' ? 0 : totalPinnedCount) +
(totalSavedFiltersCount === 'null' ? 0 : totalSavedFiltersCount)
const unwatchedCount = unwatchedPinnedCount + unwatchedSavedFiltersCount

const tooltip = (
<div className="flex flex-col space-y-1 items-center">
<span>Playlist counts are recalculated once a day.</span>
{recordings_counts.pinned_count ? (
<span>Collection has {recordings_counts.pinned_count} pinned recordings</span>
) : null}
{recordings_counts.query_count ? (
<span>
Saved filters match {recordings_counts.query_count}
{recordings_counts.has_more && '+'} recordings
</span>
) : null}
<table>
<thead>
<tr>
<th>Type</th>
<th>Count</th>
<th>Unwatched</th>
</tr>
</thead>
<tbody>
<tr>
<td>Pinned</td>
<td>{totalPinnedCount}</td>
<td>{unwatchedPinnedCount}</td>
</tr>
<tr>
<td>Saved filters</td>
<td>
{totalSavedFiltersCount}
<span>{recordings_counts.saved_filters?.has_more ? '+' : ''}</span>
</td>
<td>{unwatchedSavedFiltersCount}</td>
</tr>
</tbody>
</table>
</div>
)

return (
<div className="flex items-center justify-center w-full h-full">
<Tooltip title={tooltip}>
<span>
{count ? (
{hasSavedFiltersCount || hasCollectionCount ? (
<span className="flex items-center space-x-1">
<LemonBadge.Number
status={count ? 'primary' : 'muted'}
status={unwatchedCount || totalCount ? 'primary' : 'muted'}
className="text-xs cursor-pointer"
count={count}
count={unwatchedCount || totalCount}
maxDigits={3}
forcePlus={recordings_counts.has_more}
showZero={true}
forcePlus={
!!recordings_counts.saved_filters?.count &&
!!recordings_counts.saved_filters?.has_more
}
/>
) : (
{recordings_counts.saved_filters?.increased ? <IconArrowUp /> : null}
</span>
) : (
<span>
<LemonBadge status="muted" content="?" className="cursor-pointer" />
)}
</span>
</span>
)}
</Tooltip>
</div>
)
Expand Down
17 changes: 14 additions & 3 deletions frontend/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1433,10 +1433,21 @@ export interface RecordingEventType
fullyLoaded: boolean
}

export interface PlaylistRecordingsCounts {
query_count?: number
pinned_count?: number
export interface PlaylistCollectionCount {
count: number
watched_count: number
}

export interface PlaylistSavedFiltersCount {
count: number
watched_count: number
has_more?: boolean
increased?: boolean
}

export interface PlaylistRecordingsCounts {
saved_filters?: PlaylistSavedFiltersCount
collection: PlaylistCollectionCount
}

export interface SessionRecordingPlaylistType {
Expand Down
2 changes: 1 addition & 1 deletion posthog/helpers/session_recording_playlist_templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@
"filters": {
"order": "start_time",
"date_to": "null",
"duration": [{"key": "active_seconds", "type": "recording", "value": 5, "operator": "gt"}],
"duration": [{"key": "duration", "type": "recording", "value": 60, "operator": "gt"}],
"date_from": "-3d",
"filter_group": {"type": "AND", "values": [{"type": "AND", "values": []}]},
"filter_test_accounts": "true",
Expand Down
6 changes: 3 additions & 3 deletions posthog/session_recordings/session_recording_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ def retrieve(self, request: request.Request, *args: Any, **kwargs: Any) -> Respo

recording.load_person()
if not request.user.is_anonymous:
viewed = _current_user_viewed([str(recording.session_id)], cast(User, request.user), self.team)
viewed = current_user_viewed([str(recording.session_id)], cast(User, request.user), self.team)
other_viewers = _other_users_viewed([str(recording.session_id)], cast(User, request.user), self.team)

recording.viewed = str(recording.session_id) in viewed
Expand Down Expand Up @@ -1041,7 +1041,7 @@ def list_recordings_from_query(
recording_ids_in_list: list[str] = [str(r.session_id) for r in recordings]
# Update the viewed status for all loaded recordings
with timer("load_viewed_recordings"):
viewed_session_recordings = _current_user_viewed(recording_ids_in_list, user, team)
viewed_session_recordings = current_user_viewed(recording_ids_in_list, user, team)

with timer("load_other_viewers_by_recording"):
other_viewers = _other_users_viewed(recording_ids_in_list, user, team)
Expand Down Expand Up @@ -1090,7 +1090,7 @@ def _other_users_viewed(recording_ids_in_list: list[str], user: User | None, tea
return other_viewers


def _current_user_viewed(recording_ids_in_list: list[str], user: User | None, team: Team) -> set[str]:
def current_user_viewed(recording_ids_in_list: list[str], user: User | None, team: Team) -> set[str]:
if not user:
return set()

Expand Down
50 changes: 41 additions & 9 deletions posthog/session_recordings/session_recording_playlist_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
from posthog.schema import RecordingsQuery
from posthog.session_recordings.models.session_recording_playlist import SessionRecordingPlaylistViewed
from posthog.session_recordings.session_recording_api import (
current_user_viewed,
list_recordings_response,
query_as_params_to_dict,
list_recordings_from_query,
Expand Down Expand Up @@ -113,23 +114,54 @@ class Meta:
created_by = UserBasicSerializer(read_only=True)
last_modified_by = UserBasicSerializer(read_only=True)

def get_recordings_counts(self, playlist: SessionRecordingPlaylist) -> dict[str, int | bool | None]:
recordings_counts: dict[str, int | bool | None] = {
"query_count": None,
"pinned_count": None,
"has_more": None,
def get_recordings_counts(self, playlist: SessionRecordingPlaylist) -> dict[str, dict[str, int | bool | None]]:
recordings_counts: dict[str, dict[str, int | bool | None]] = {
"saved_filters": {
"count": None,
"has_more": None,
"watched_count": None,
"increased": None,
},
"collection": {
"count": None,
"watched_count": None,
},
}

try:
redis_client = get_client()
counts = redis_client.get(f"{PLAYLIST_COUNT_REDIS_PREFIX}{playlist.short_id}")

user = self.context["request"].user
team = self.context["get_team"]()

if counts:
count_data = json.loads(counts)
id_list = count_data.get("session_ids", None)
recordings_counts["query_count"] = len(id_list) if id_list else 0
recordings_counts["has_more"] = count_data.get("has_more", False)
id_list: list[str] = count_data.get("session_ids", None)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

messy, but wrapped in a try/catch and only here so we can validate behind a flag

current_count = len(id_list) if id_list else 0
previous_ids = count_data.get("previous_ids", None)
recordings_counts["saved_filters"] = {
"count": current_count,
"has_more": count_data.get("has_more", False),
"watched_count": len(current_user_viewed(id_list, user, team)) if id_list else 0,
"increased": previous_ids is not None and current_count > len(previous_ids),
}

playlist_items: QuerySet[SessionRecordingPlaylistItem] = playlist.playlist_items.filter(deleted=False)
watched_playlist_items = current_user_viewed(
# mypy can't detect that it's safe to pass queryset to list() 🤷
list(playlist.playlist_items.values_list("session_id", flat=True)), # type: ignore
user,
team,
)

item_count = playlist_items.count()
watched_count = len(watched_playlist_items)
recordings_counts["collection"] = {
"count": item_count if item_count > 0 else None,
"watched_count": watched_count if watched_count > 0 else None,
}

recordings_counts["pinned_count"] = playlist.playlist_items.count()
except Exception as e:
posthoganalytics.capture_exception(e)

Expand Down
Loading
Loading