fix: drop group_identify() calls with a missing group type or key (sdk-specs group-identify)

.sampo/changesets/group-identify-validates-group-identity.md

@@ -0,0 +1,5 @@
+---
+pypi/posthog: patch
+---
+
+`group_identify()` now validates the group identity before enqueuing. Previously `group_identify("company", None)` (or an empty-string `group_type` / `group_key`) sent a `$groupidentify` event with a null/empty `$group_type` or `$group_key`, which cannot address a group profile and just adds an unusable event to the project. Missing values are now dropped with a warning instead, matching the sdk-specs `group-identify` contract. Valid values, including non-string group keys, are passed through unchanged.

posthog/__init__.py

@@ -578,8 +578,10 @@ def group_identify(
     Set properties on a group.
 
     Args:
-        group_type: Type of your group
-        group_key: Unique identifier of the group
+        group_type: Type of your group. Required - the call is dropped with a
+            warning if it is missing or empty.
+        group_key: Unique identifier of the group. Required - the call is
+            dropped with a warning if it is missing or empty.
         properties: Properties to set on the group
         timestamp: Optional timestamp for the event
         uuid: Optional UUID for the event

posthog/client.py

@@ -1652,8 +1652,10 @@ def group_identify(
         Identify a group and set its properties.
 
         Args:
-            group_type: The type of group (e.g., 'company', 'team').
-            group_key: The unique identifier for the group.
+            group_type: The type of group (e.g., 'company', 'team'). Required -
+                the call is dropped with a warning if it is missing or empty.
+            group_key: The unique identifier for the group. Required - the call
+                is dropped with a warning if it is missing or empty.
             properties: A dictionary of properties to set on the group.
             timestamp: The timestamp of the event.
             uuid: A unique identifier for the event. If provided, it must be a
@@ -1675,6 +1677,18 @@ def group_identify(
 
         Note: This method will not raise exceptions. Errors are logged.
         """
+        if not stringify_id(group_type):
+            self.log.warning(
+                "group_identify() called without a group_type, dropping the $groupidentify event"
+            )
+            return None
+
+        if not stringify_id(group_key):
+            self.log.warning(
+                "group_identify() called without a group_key, dropping the $groupidentify event"
+            )
+            return None
+
         properties = properties or {}
 
         # group_identify is purposefully always personful

posthog/test/test_client.py

@@ -1893,6 +1893,49 @@ def test_advanced_group_identify_with_distinct_id(self):
             )
             self.assertEqual(msg["timestamp"], "2014-09-03T00:00:00+00:00")
 
+    @parameterized.expand(
+        [
+            ("none", None),
+            ("empty_string", ""),
+        ]
+    )
+    def test_group_identify_without_group_type_is_dropped(self, _name, group_type):
+        with mock.patch("posthog.client.batch_post") as mock_post:
+            client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True)
+            with self.assertLogs("posthog", level="WARNING") as logs:
+                msg_uuid = client.group_identify(group_type, "id:5")
+
+            self.assertIsNone(msg_uuid)
+            mock_post.assert_not_called()
+            self.assertIn("group_type", logs.output[0])
+
+    @parameterized.expand(
+        [
+            ("none", None),
+            ("empty_string", ""),
+        ]
+    )
+    def test_group_identify_without_group_key_is_dropped(self, _name, group_key):
+        with mock.patch("posthog.client.batch_post") as mock_post:
+            client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True)
+            with self.assertLogs("posthog", level="WARNING") as logs:
+                msg_uuid = client.group_identify("organization", group_key)
+
+            self.assertIsNone(msg_uuid)
+            mock_post.assert_not_called()
+            self.assertIn("group_key", logs.output[0])
+
+    def test_group_identify_accepts_falsy_non_string_group_key(self):
+        with mock.patch("posthog.client.batch_post") as mock_post:
+            client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True)
+            msg_uuid = client.group_identify("organization", 0)
+            self.assertIsNotNone(msg_uuid)
+
+            mock_post.assert_called_once()
+            msg = mock_post.call_args[1]["batch"][0]
+            # The group key is validated, not normalized - it goes out as passed.
+            self.assertEqual(msg["properties"]["$group_key"], 0)
+
     def test_basic_alias(self):
         with mock.patch("posthog.client.batch_post") as mock_post:
             client = Client(FAKE_TEST_API_KEY, on_error=self.set_fail, sync_mode=True)