chore: Person ID overrides by default #22811

Twixes · 2024-06-07T14:16:35Z

Changes

Person-on-events cleanup:

person_id_override_properties_joined becomes the default in code (we can get rid of the persons-on-events-person-id-override-properties-joined flag after this)
Team.person_on_events_mode is now consistent with the project setting (which is based on Team.modifiers) – previously Team.person_on_events_mode didn't know about the project setting (although this only applies to projects where the setting was explicitly changed)
Got rid of the ancient person-on-events-enabled flag

posthog-bot · 2024-06-07T14:27:38Z

📸 UI snapshots have been updated

3 snapshot changes in total. 0 added, 3 modified, 0 deleted:

chromium: 0 added, 3 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

tiina303

lgtm, but I'm not the best reviewer for this & I didn't really check query .ambr changes

posthog/models/team/team.py

posthog-bot · 2024-06-11T17:17:39Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-06-11T17:28:18Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-06-11T17:38:41Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-06-11T17:49:32Z

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

chromium: 0 added, 2 modified, 0 deleted (diff for shard 1, diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-06-11T18:00:54Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

mariusandra

Changes look good, but seem incompatible with some tests, e.g:

posthog-bot · 2024-06-13T13:25:59Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 2)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-06-21T07:31:21Z

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week.

posthog-bot · 2024-07-01T07:31:01Z

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

posthog-bot · 2024-07-29T07:30:57Z

This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the stale label – otherwise this will be closed in another week. If you want to permanentely keep it open, use the waiting label.

posthog-bot · 2024-08-05T07:31:08Z

This PR was closed due to lack of activity. Feel free to reopen if it's still relevant.

Twixes · 2024-09-11T12:03:43Z

posthog/session_recordings/queries/test/session_replay_sql.py

+    if ensure_analytics_event_in_session:
+        # Only importing from posthog.test.base if needed
+        from posthog.test.base import _create_event, flush_persons_and_events
+
+        # It's best to also create a random analytics event, since sessions querying does a JOIN with events in
+        # any person-ID-on-events mode - sessions without an analytics event are excluded
+        _create_event(
+            distinct_id=data["distinct_id"],
+            event="foobarino",
+            properties={"$session_id": data["session_id"]},
+            team_id=team_id,
+        )
+        flush_persons_and_events()


This turned out to be necessary, as the session recordings query works quite differently with PersonsOnEventsMode.DISABLED vs. with PersonsOnEventsMode.PERSON_ID_OVERRIDE_PROPERTIES_JOINED. CC @PostHog/team-replay for a sanity check

yep... we've got the laziest implementation possible right now.

i want to figure out how we can avoid having our own events query so we can just delegate completely

i guess we need to always select where session_id in (select $session_id from events where...) or something but am letting future me deal with that 🤣

posthog-bot · 2024-09-13T10:04:47Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog-bot · 2024-09-13T10:23:44Z

📸 UI snapshots have been updated

1 snapshot changes in total. 0 added, 1 modified, 0 deleted:

chromium: 0 added, 1 modified, 0 deleted (diff for shard 1)
webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

posthog/hogql/hogql.py

anirudhpillai

Someone from replay should cross check impact

Co-authored-by: Anirudh Pillai <[email protected]>

Twixes · 2024-09-13T16:18:12Z

posthog/hogql_queries/insights/test/test_retention_query_runner.py

@@ -831,6 +831,7 @@ def test_rolling_retention(self):
        _create_person(team_id=self.team.pk, distinct_ids=["person2"])
        _create_person(team_id=self.team.pk, distinct_ids=["person3"])
        _create_person(team_id=self.team.pk, distinct_ids=["person4"])
+        _create_person(team_id=self.team.pk, distinct_ids=["person5"])


Just a note that this test was off @aspicer

Twixes requested review from mariusandra, tiina303 and timgl June 7, 2024 14:16

This comment was marked as outdated.

Sign in to view

tiina303 requested a review from tkaemming June 11, 2024 12:10

tiina303 approved these changes Jun 11, 2024

View reviewed changes

posthog/models/team/team.py Show resolved Hide resolved

Twixes force-pushed the poe-rollout-2024 branch 2 times, most recently from dad46a7 to 12d4f97 Compare June 11, 2024 17:06

mariusandra reviewed Jun 13, 2024

View reviewed changes

posthog-bot added the stale label Jun 21, 2024

posthog-bot closed this Jul 1, 2024

Twixes reopened this Jul 18, 2024

posthog-bot removed the stale label Jul 19, 2024

posthog-bot added the stale label Jul 29, 2024

posthog-bot closed this Aug 5, 2024

Twixes commented Sep 11, 2024

View reviewed changes

Twixes changed the title ~~chore: Person IDs on events by default~~ chore: Person ID overrides on events by default Sep 11, 2024

Twixes changed the title ~~chore: Person ID overrides on events by default~~ chore: Person ID overrides by default Sep 11, 2024

Twixes and others added 12 commits September 11, 2024 14:32

Merge branch 'master' into poe-rollout-2024

448f550

Update test_session_replay_events.py

657b8f1

Update query snapshots

95b481b

Update query snapshots

5ec6535

Update query snapshots

8d1c9bc

Merge branch 'master' into poe-rollout-2024

0fe68ab

Update query snapshots

7399d32

Update query snapshots

4bcbf92

Merge branch 'master' into poe-rollout-2024

dd41130

Update query snapshots

afe4300

Update query snapshots

f2f94e9

Update UI snapshots for chromium (1)

6d20265

Update UI snapshots for chromium (1)

a6a1842

Twixes and others added 3 commits September 13, 2024 12:26

Fix test_rolling_retention

47069e2

Update test_session_recording_list_from_filters.py

dfaebe1

Update query snapshots

8125985

anirudhpillai reviewed Sep 13, 2024

View reviewed changes

posthog/hogql/hogql.py Outdated Show resolved Hide resolved

Update query snapshots

1577205

anirudhpillai approved these changes Sep 13, 2024

View reviewed changes

skoob13 approved these changes Sep 13, 2024

View reviewed changes

Fix typo

fbf8bbe

Co-authored-by: Anirudh Pillai <[email protected]>

Twixes commented Sep 13, 2024

View reviewed changes

Twixes enabled auto-merge (squash) September 13, 2024 16:18

Twixes merged commit de3d95a into master Sep 13, 2024
93 checks passed

Twixes deleted the poe-rollout-2024 branch September 13, 2024 16:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: Person ID overrides by default #22811

chore: Person ID overrides by default #22811

Twixes commented Jun 7, 2024 •

edited

Loading

posthog-bot commented Jun 7, 2024

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

tiina303 left a comment

posthog-bot commented Jun 11, 2024

posthog-bot commented Jun 11, 2024

posthog-bot commented Jun 11, 2024

posthog-bot commented Jun 11, 2024

posthog-bot commented Jun 11, 2024

mariusandra left a comment •

edited

Loading

posthog-bot commented Jun 13, 2024

posthog-bot commented Jun 21, 2024

posthog-bot commented Jul 1, 2024

posthog-bot commented Jul 29, 2024

posthog-bot commented Aug 5, 2024

Twixes Sep 11, 2024

pauldambra Sep 13, 2024

posthog-bot commented Sep 13, 2024

posthog-bot commented Sep 13, 2024

anirudhpillai left a comment

Twixes Sep 13, 2024

chore: Person ID overrides by default #22811

chore: Person ID overrides by default #22811

Conversation

Twixes commented Jun 7, 2024 • edited Loading

Changes

posthog-bot commented Jun 7, 2024

📸 UI snapshots have been updated

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

tiina303 left a comment

Choose a reason for hiding this comment

posthog-bot commented Jun 11, 2024

📸 UI snapshots have been updated

posthog-bot commented Jun 11, 2024

📸 UI snapshots have been updated

posthog-bot commented Jun 11, 2024

📸 UI snapshots have been updated

posthog-bot commented Jun 11, 2024

📸 UI snapshots have been updated

posthog-bot commented Jun 11, 2024

📸 UI snapshots have been updated

mariusandra left a comment • edited Loading

Choose a reason for hiding this comment

posthog-bot commented Jun 13, 2024

📸 UI snapshots have been updated

posthog-bot commented Jun 21, 2024

posthog-bot commented Jul 1, 2024

posthog-bot commented Jul 29, 2024

posthog-bot commented Aug 5, 2024

Twixes Sep 11, 2024

Choose a reason for hiding this comment

pauldambra Sep 13, 2024

Choose a reason for hiding this comment

posthog-bot commented Sep 13, 2024

📸 UI snapshots have been updated

posthog-bot commented Sep 13, 2024

📸 UI snapshots have been updated

anirudhpillai left a comment

Choose a reason for hiding this comment

Twixes Sep 13, 2024

Choose a reason for hiding this comment

Twixes commented Jun 7, 2024 •

edited

Loading

mariusandra left a comment •

edited

Loading