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.

Sign up for GitHub

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

fix(periodic-digest): use derived names for playlists without real names #27188

Merged
merged 2 commits into from
Jan 3, 2025
Merged

fix(periodic-digest): use derived names for playlists without real names #27188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3fb0911
use derived names for playlists without real names
raquelmsmith Dec 30, 2024
6c8a3d4
exclude playlists with no name and no derived name
raquelmsmith Dec 30, 2024
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
24 changes: 16 additions & 8 deletions posthog/tasks/periodic_digest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,21 @@ def get_teams_with_new_event_definitions(end: datetime, begin: datetime) -> Quer


def get_teams_with_new_playlists(end: datetime, begin: datetime) -> QuerySet:
return SessionRecordingPlaylist.objects.filter(
created_at__gt=begin,
created_at__lte=end,
name__isnull=False,
name__gt="", # Excludes empty strings
).values("team_id", "name", "short_id")
return (
SessionRecordingPlaylist.objects.filter(
created_at__gt=begin,
created_at__lte=end,
)
.exclude(
name__isnull=True,
derived_name__isnull=True,
)
.exclude(
name="",
derived_name="",
)
.values("team_id", "name", "short_id", "derived_name")
)


def get_teams_with_new_experiments_launched(end: datetime, begin: datetime) -> QuerySet:
Expand Down Expand Up @@ -151,9 +160,8 @@ def get_periodic_digest_report(all_digest_data: dict[str, Any], team: Team) -> p
for event_definition in all_digest_data["teams_with_new_event_definitions"].get(team.id, [])
],
new_playlists=[
{"name": playlist.get("name"), "id": playlist.get("short_id")}
{"name": playlist.get("name") or playlist.get("derived_name", "Untitled"), "id": playlist.get("short_id")}
for playlist in all_digest_data["teams_with_new_playlists"].get(team.id, [])
if playlist.get("name") # Extra safety check to exclude any playlists without names
],
new_experiments_launched=[
{
Expand Down
46 changes: 10 additions & 36 deletions posthog/tasks/test/test_periodic_digest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@ def test_periodic_digest_report(self, mock_capture: MagicMock) -> None:
team=self.team,
name="Test Playlist",
)
# These should be excluded from the digest
# This should be excluded from the digest because it has no name and no derived name
SessionRecordingPlaylist.objects.create(
team=self.team,
name=None,
derived_name=None,
)
SessionRecordingPlaylist.objects.create(
# This should be included in the digest but use the derived name
derived_playlist = SessionRecordingPlaylist.objects.create(
team=self.team,
name="",
derived_name="Derived Playlist",
)

# Create experiments
Expand Down Expand Up @@ -163,7 +166,11 @@ def test_periodic_digest_report(self, mock_capture: MagicMock) -> None:
{
"name": "Test Playlist",
"id": playlist.short_id,
}
},
{
"name": "Derived Playlist",
"id": derived_playlist.short_id,
},
],
"new_experiments_launched": [
{
Expand Down Expand Up @@ -366,36 +373,3 @@ def test_periodic_digest_dry_run_no_record(self, mock_capture: MagicMock) -> Non
# Verify no capture call and no messaging record
mock_capture.delay.assert_not_called()
self.assertEqual(MessagingRecord.objects.count(), 0)

@freeze_time("2024-01-20T00:01:00Z")
@patch("posthog.tasks.periodic_digest.capture_report")
def test_periodic_digest_excludes_playlists_without_names(self, mock_capture: MagicMock) -> None:
# Create test data from "last week"
with freeze_time("2024-01-15T00:01:00Z"):
# Create playlists with various name states
valid_playlist = SessionRecordingPlaylist.objects.create(
team=self.team,
name="Valid Playlist",
)
SessionRecordingPlaylist.objects.create(
team=self.team,
name=None, # Null name should be excluded
)
SessionRecordingPlaylist.objects.create(
team=self.team,
name="", # Empty string name should be excluded
)

# Run the periodic digest report task
send_all_periodic_digest_reports()

# Extract the playlists from the capture call
call_args = mock_capture.delay.call_args
self.assertIsNotNone(call_args)
full_report_dict = call_args[1]["full_report_dict"]
playlists = full_report_dict["new_playlists"]

# Verify only the valid playlist is included
self.assertEqual(len(playlists), 1)
self.assertEqual(playlists[0]["name"], "Valid Playlist")
self.assertEqual(playlists[0]["id"], valid_playlist.short_id)
Loading