From f143e07b1f87e734efd7216bd441fd64858b2ba3 Mon Sep 17 00:00:00 2001 From: Raquel Smith Date: Mon, 30 Dec 2024 07:50:18 -0800 Subject: [PATCH] feat(periodic-digest): exclude generated dashboards and nameless playlists (#27177) --- posthog/tasks/periodic_digest.py | 16 +++++-- posthog/tasks/test/test_periodic_digest.py | 50 +++++++++++++++++++++- 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/posthog/tasks/periodic_digest.py b/posthog/tasks/periodic_digest.py index 0c8161c1be9b2..7fe4eb440bbe2 100644 --- a/posthog/tasks/periodic_digest.py +++ b/posthog/tasks/periodic_digest.py @@ -54,7 +54,11 @@ def get_teams_for_digest() -> list[Team]: def get_teams_with_new_dashboards(end: datetime, begin: datetime) -> QuerySet: - return Dashboard.objects.filter(created_at__gt=begin, created_at__lte=end).values("team_id", "name", "id") + return ( + Dashboard.objects.filter(created_at__gt=begin, created_at__lte=end) + .exclude(name__contains="Generated Dashboard") + .values("team_id", "name", "id") + ) def get_teams_with_new_event_definitions(end: datetime, begin: datetime) -> QuerySet: @@ -62,9 +66,12 @@ 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).values( - "team_id", "name", "short_id" - ) + 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") def get_teams_with_new_experiments_launched(end: datetime, begin: datetime) -> QuerySet: @@ -146,6 +153,7 @@ def get_periodic_digest_report(all_digest_data: dict[str, Any], team: Team) -> p new_playlists=[ {"name": playlist.get("name"), "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=[ { diff --git a/posthog/tasks/test/test_periodic_digest.py b/posthog/tasks/test/test_periodic_digest.py index a4fed64b0b8f6..6a4afdac8d155 100644 --- a/posthog/tasks/test/test_periodic_digest.py +++ b/posthog/tasks/test/test_periodic_digest.py @@ -32,17 +32,32 @@ def test_periodic_digest_report(self, mock_capture: MagicMock) -> None: name="Test Dashboard", ) + # create a dashboard that is generated for a feature flag, should be excluded from the digest + Dashboard.objects.create( + team=self.team, + name="Generated Dashboard: test-flag Usage", + ) + # Create an event definition event_definition = EventDefinition.objects.create( team=self.team, name="Test Event", ) - # Create a playlist + # Create playlists - one with name, one without name, one with empty string name playlist = SessionRecordingPlaylist.objects.create( team=self.team, name="Test Playlist", ) + # These should be excluded from the digest + SessionRecordingPlaylist.objects.create( + team=self.team, + name=None, + ) + SessionRecordingPlaylist.objects.create( + team=self.team, + name="", + ) # Create experiments # this flag should not be included in the digest @@ -351,3 +366,36 @@ 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)