type fix

Author: nickbest-ph

posthog/models/person/person.py

@@ -572,6 +572,9 @@ def get_distinct_ids_for_subquery(person: Person | None, team: Team) -> list[str
     last_ids_limit = MAX_LIMIT_DISTINCT_IDS - first_ids_limit
 
     if person is not None:
+        # When a Person comes from personhog (via proto_person_to_model), distinct IDs
+        # are already populated on _distinct_ids — use them directly to avoid hitting
+        # the Django ORM below, which would defeat the purpose of the personhog path.
         if hasattr(person, "_distinct_ids") and person._distinct_ids is not None:
             ids = person._distinct_ids
             if len(ids) <= MAX_LIMIT_DISTINCT_IDS:

posthog/personhog_client/test_helpers.py

@@ -41,6 +41,7 @@ class PersonhogTestMixin:
     """
 
     personhog: bool = False
+    _personhog_cm: Any = None
     _fake_client: FakePersonHogClient | None = None
 
     def setUp(self) -> None:

products/conversations/backend/api/tests/test_tickets.py

@@ -6,11 +6,12 @@
 from django.db import transaction
 from django.utils import timezone
 
-from parameterized import parameterized
+from parameterized import parameterized, parameterized_class
 from rest_framework import status
 
 from posthog.models import ActivityLog, Comment, Organization, User
 from posthog.models.person import Person
+from posthog.personhog_client.test_helpers import PersonhogTestMixin
 
 from products.conversations.backend.models import Ticket, TicketAssignment
 from products.conversations.backend.models.constants import Channel, ChannelDetail, Priority, Status
@@ -99,61 +100,6 @@ def test_retrieve_ticket_marks_as_read(self, mock_on_commit):
         self.ticket.refresh_from_db()
         self.assertEqual(self.ticket.unread_team_count, 0)
 
-    def test_retrieve_ticket_includes_person_data(self, mock_on_commit):
-        """Test that retrieve includes person data when person exists."""
-        person = Person.objects.create(
-            team=self.team,
-            distinct_ids=["user-123", "user@example.com", "another-id"],
-            properties={"email": "test@example.com", "name": "Test User"},
-        )
-
-        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertIn("person", response.json())
-        self.assertIsNotNone(response.json()["person"])
-        self.assertEqual(response.json()["person"]["id"], str(person.uuid))
-        self.assertEqual(response.json()["person"]["properties"]["email"], "test@example.com")
-        # Verify all distinct_ids are returned, not just the ticket's
-        self.assertCountEqual(
-            response.json()["person"]["distinct_ids"],
-            ["user-123", "user@example.com", "another-id"],
-        )
-
-    def test_retrieve_ticket_person_null_when_no_person(self, mock_on_commit):
-        """Test that person is null when no person exists for distinct_id."""
-        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertIn("person", response.json())
-        self.assertIsNone(response.json()["person"])
-
-    def test_list_tickets_includes_person_data(self, mock_on_commit):
-        """Test that list includes person data for tickets with persons."""
-        Person.objects.create(
-            team=self.team,
-            distinct_ids=["user-123", "user@example.com"],
-            properties={"email": "test@example.com"},
-        )
-
-        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/")
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.json()["count"], 1)
-        self.assertIn("person", response.json()["results"][0])
-        self.assertIsNotNone(response.json()["results"][0]["person"])
-        self.assertEqual(response.json()["results"][0]["person"]["properties"]["email"], "test@example.com")
-        # Verify all distinct_ids are returned, not just the ticket's
-        self.assertCountEqual(
-            response.json()["results"][0]["person"]["distinct_ids"],
-            ["user-123", "user@example.com"],
-        )
-
-    def test_list_tickets_person_null_when_no_person(self, mock_on_commit):
-        """Test that person is null in list when no person exists."""
-        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/")
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.json()["count"], 1)
-        self.assertIn("person", response.json()["results"][0])
-        self.assertIsNone(response.json()["results"][0]["person"])
-
     def test_retrieve_ticket_includes_anonymous_traits(self, mock_on_commit):
         """Test that retrieve includes anonymous_traits."""
         self.ticket.anonymous_traits = {"name": "John Doe", "email": "john@example.com"}
@@ -177,21 +123,6 @@ def test_list_tickets_includes_anonymous_traits(self, mock_on_commit):
         self.assertEqual(response.json()["results"][0]["anonymous_traits"]["name"], "Jane Doe")
         self.assertEqual(response.json()["results"][0]["anonymous_traits"]["company"], "ACME")
 
-    def test_person_data_scoped_to_team(self, mock_on_commit):
-        """Test that person lookup is scoped to team (no cross-team leakage)."""
-        # Create person in different team
-        other_team = self.organization.teams.create(name="Other Team")
-        Person.objects.create(
-            team=other_team,
-            distinct_ids=["user-123"],  # Same distinct_id as ticket
-            properties={"email": "other@example.com"},
-        )
-
-        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        # Should be null because person is in different team
-        self.assertIsNone(response.json()["person"])
-
     def test_update_sla_due_at(self, mock_on_commit):
         sla_time = timezone.now() + timedelta(hours=5)
         response = self.client.patch(
@@ -1100,3 +1031,79 @@ def test_auto_increments_ticket_number(self):
 
         self.assertEqual(ticket1.ticket_number, 1)
         self.assertEqual(ticket2.ticket_number, 2)
+
+
+@parameterized_class(("personhog",), [(False,), (True,)])
+@patch.object(transaction, "on_commit", side_effect=immediate_on_commit)
+class TestTicketPersonData(PersonhogTestMixin, APIBaseTest):
+    """Tests that ticket person enrichment produces identical results
+    via the ORM and personhog paths."""
+
+    def setUp(self):
+        super().setUp()
+        self.ticket = Ticket.objects.create_with_number(
+            team=self.team,
+            channel_source=Channel.WIDGET,
+            widget_session_id="test-session-123",
+            distinct_id="user-123",
+            status=Status.NEW,
+        )
+
+    def test_retrieve_ticket_includes_person_data(self, mock_on_commit):
+        person = self._seed_person(
+            team=self.team,
+            distinct_ids=["user-123", "user@example.com", "another-id"],
+            properties={"email": "test@example.com", "name": "Test User"},
+        )
+
+        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
+
+        assert response.status_code == status.HTTP_200_OK
+        person_data = response.json()["person"]
+        assert person_data is not None
+        assert person_data["id"] == str(person.uuid)
+        assert person_data["properties"]["email"] == "test@example.com"
+        assert set(person_data["distinct_ids"]) == {"user-123", "user@example.com", "another-id"}
+
+    def test_retrieve_ticket_person_null_when_no_person(self, mock_on_commit):
+        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["person"] is None
+
+    def test_list_tickets_includes_person_data(self, mock_on_commit):
+        self._seed_person(
+            team=self.team,
+            distinct_ids=["user-123", "user@example.com"],
+            properties={"email": "test@example.com"},
+        )
+
+        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/")
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["count"] == 1
+        person_data = response.json()["results"][0]["person"]
+        assert person_data is not None
+        assert person_data["properties"]["email"] == "test@example.com"
+        assert set(person_data["distinct_ids"]) == {"user-123", "user@example.com"}
+        self._assert_personhog_called("get_persons_by_distinct_ids_in_team")
+
+    def test_list_tickets_person_null_when_no_person(self, mock_on_commit):
+        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/")
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["count"] == 1
+        assert response.json()["results"][0]["person"] is None
+
+    def test_person_data_scoped_to_team(self, mock_on_commit):
+        other_team = self.organization.teams.create(name="Other Team")
+        self._seed_person(
+            team=other_team,
+            distinct_ids=["user-123"],
+            properties={"email": "other@example.com"},
+        )
+
+        response = self.client.get(f"/api/projects/{self.team.id}/conversations/tickets/{self.ticket.id}/")
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["person"] is None