From f7f5964e328c8bea9c9e3ed455bd52eff1ac3c4d Mon Sep 17 00:00:00 2001
From: Raquel Smith <raquelmsmith@users.noreply.github.com>
Date: Sat, 28 Dec 2024 09:01:00 -0800
Subject: [PATCH] also exclude empty playlist names

---
 posthog/tasks/periodic_digest.py           | 10 +++--
 posthog/tasks/test/test_periodic_digest.py | 44 +++++++++++++++++++++-
 2 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/posthog/tasks/periodic_digest.py b/posthog/tasks/periodic_digest.py
index 2ee109ec31488..7fe4eb440bbe2 100644
--- a/posthog/tasks/periodic_digest.py
+++ b/posthog/tasks/periodic_digest.py
@@ -66,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:
@@ -150,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 c2a48556c75c5..6a4afdac8d155 100644
--- a/posthog/tasks/test/test_periodic_digest.py
+++ b/posthog/tasks/test/test_periodic_digest.py
@@ -44,11 +44,20 @@ def test_periodic_digest_report(self, mock_capture: MagicMock) -> None:
                 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
@@ -357,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)