8000 chore(persons): Refactor person properties update code so it can be reused by jose-sequeira · Pull Request #33368 · PostHog/posthog · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

chore(persons): Refactor person properties update code so it can be reused #33368

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

Merged
merged 4 commits into from
Jun 11, 2025

Conversation

jose-sequeira
Copy link
Contributor

Important

👉 Stay up-to-date with PostHog coding conventions for a smoother review.

Problem

This refactor moves the person-state.ts file into persons as well as moving all of the update logic into its own separate file so we can reuse this code when making the changes for batch writing.

Changes

Did you write or update any docs for this change?

How did you test this code?

@jose-sequeira jose-sequeira requested a review from a team June 9, 2025 10:06
Copy link
Contributor
@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Refactored person properties update logic by moving it into a dedicated directory structure and extracting update functionality for reusability in batch processing.

  • Moved person-state.ts to plugin-server/src/worker/ingestion/persons/ and split property update logic into new person-update.ts file
  • Added personPropertyKeyUpdateCounter in metrics.ts to track property updates with proper Prometheus instrumentation
  • Updated all import paths from ../person-state to ../persons/person-state across multiple files
  • Maintained thorough test coverage by extending person-state.test.ts with JSONB optimization flag compatibility tests
  • Renamed parameter distinctIdBatchStore to personStoreBatch for better clarity in function signatures

6 file(s) reviewed, 1 comment(s)
Edit PR Review Bot Settings | Greptile

@jose-sequeira jose-sequeira changed the title Refactor person properties update code so it can be reused chore(persons): Refactor person properties update code so it can be reused Jun 9, 2025
Comment on lines 112 to 113
})
})
Copy link
Contributor

Choose a reason for hiding this comment

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

There appears to be an extra closing bracket }) on line 113 that doesn't correspond to any opening bracket. This will cause a syntax error when the code is executed. Please remove this redundant bracket to ensure the code compiles correctly.

Suggested change
})
})
})

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

Copy link
Contributor
@eli-r-ph eli-r-ph left a comment

Choose a reason for hiding this comment

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

This LGTM

Base automatically changed from person-store-reuse-fetch to master June 11, 2025 08:16
@jose-sequeira jose-sequeira force-pushed the person-refactor-update-code branch from 8624b1f to 7ebb096 Compare June 11, 2025 12:08
@jose-sequeira jose-sequeira merged commit f282111 into master Jun 11, 2025
95 checks passed
@jose-sequeira jose-sequeira deleted the person-refactor-update-code branch June 11, 2025 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0