Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add person processing mode preview #1109

Merged
merged 22 commits into from
Apr 8, 2024
Merged

Conversation

robbie-c
Copy link
Member

@robbie-c robbie-c commented Apr 2, 2024

Enable by setting __preview_process_person in config

Changes

See PostHog/posthog.com#8098

  • Add init config option
  • Pass flag as event property
  • Ensure that $set etc are not sent with personless events (no-op, they were only sent with identify afaict)
  • Ensure that identify cannot be called if person mode is never
  • Ensure that we send referrer and campaign props with all events with person processing when in identified_only mode

Manual tests

  • identified_only sets up initial_utm properties etc correctly

Checklist

  • Tests for new code (see advice on the tests we use)
  • Accounted for the impact of any changes across different browsers
  • Accounted for backwards compatibility of any changes (no breaking changes in posthog-js!)

On this last one, the default person processing mode is always which is identical to the current behaviour

Copy link

github-actions bot commented Apr 2, 2024

Size Change: +2.25 kB (0%)

Total Size: 947 kB

Filename Size Change
dist/array.full.js 225 kB +107 B (0%)
dist/array.js 126 kB +1.95 kB (+2%)
dist/es.js 127 kB +1.95 kB (+2%)
dist/module.js 127 kB +1.95 kB (+2%)
dist/recorder-v2.js 105 kB -1.84 kB (-2%)
dist/recorder.js 105 kB -1.84 kB (-2%)
ℹ️ View Unchanged
Filename Size
dist/exception-autocapture.js 12.1 kB
dist/surveys-module-previews.js 62 kB
dist/surveys.js 58.3 kB

compressed-size-action

@robbie-c robbie-c changed the title Add process_person init config option feat: WIP Add process_person init config option Apr 2, 2024
@robbie-c robbie-c changed the title feat: WIP Add process_person init config option feat: WIP Support Person-less Events Apr 3, 2024
@robbie-c robbie-c force-pushed the feat/support-personless-events branch from 5e3f2a8 to c74dab0 Compare April 5, 2024 15:54
@robbie-c robbie-c changed the title feat: WIP Support Person-less Events Support Person-less Events Apr 6, 2024
@robbie-c robbie-c changed the title Support Person-less Events feat: Support Person-less Events Apr 6, 2024
@robbie-c robbie-c changed the title feat: Support Person-less Events feat: Add person processing mode preview Apr 6, 2024
@robbie-c robbie-c marked this pull request as ready for review April 6, 2024 12:54
@PostHog PostHog deleted a comment from posthog-bot Apr 6, 2024
Copy link
Member

@pauldambra pauldambra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a three-glasses-of-prosecco, saturday evening review, treat it accordingly 🤣

I've not really been following this project so I'm not 100% but I think this does the right thing

src/posthog-core.ts Outdated Show resolved Hide resolved
src/posthog-core.ts Outdated Show resolved Hide resolved
src/posthog-core.ts Outdated Show resolved Hide resolved
src/posthog-core.ts Show resolved Hide resolved
src/posthog-persistence.ts Outdated Show resolved Hide resolved
src/__tests__/personProcessing.test.ts Outdated Show resolved Hide resolved
src/__tests__/personProcessing.test.ts Outdated Show resolved Hide resolved
src/__tests__/posthog-core.js Show resolved Hide resolved
src/posthog-core.ts Show resolved Hide resolved
@robbie-c robbie-c added the bump minor Bump minor version when this PR gets merged label Apr 8, 2024
@robbie-c robbie-c requested a review from pauldambra April 8, 2024 09:15
@pauldambra
Copy link
Member

Screenshot 2024-04-08 at 11 41 34

sometimes I wonder if the bundler is random 🤣

weird that we've apparently shifted code from recorder to core 🤯

maybe we should handle separately since this needs to ship I guess.

@robbie-c
Copy link
Member Author

robbie-c commented Apr 8, 2024

sometimes I wonder if the bundler is random @pauldambra

Yeah - working with optimizers is a pain for exactly this reason.

@robbie-c robbie-c merged commit 5f117a6 into main Apr 8, 2024
11 checks passed
@robbie-c robbie-c deleted the feat/support-personless-events branch April 8, 2024 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bump minor Bump minor version when this PR gets merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants