-
Notifications
You must be signed in to change notification settings - Fork 1.9k
fix: person constraint in person overrides table #14319
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
tiina303
merged 4 commits into
fix/add-person-consrtaint-on-override
from
tiina/person-contstraint-wip
Feb 21, 2023
Merged
fix: person constraint in person overrides table #14319
tiina303
merged 4 commits into
fix/add-person-consrtaint-on-override
from
tiina/person-contstraint-wip
Feb 21, 2023
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hey @tiina303! 👋 |
35b6e22
to
dfeb531
Compare
dfeb531
to
d944c53
Compare
9e600f1
to
f88d3db
Compare
tomasfarias
pushed a commit
that referenced
this pull request
Feb 22, 2023
Closed
tomasfarias
pushed a commit
that referenced
this pull request
Mar 2, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 3, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 3, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 3, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 6, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 7, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 8, 2023
tomasfarias
pushed a commit
that referenced
this pull request
Mar 10, 2023
tomasfarias
added a commit
that referenced
this pull request
Mar 13, 2023
* feat(person-override): Add a helper model to indirectly reference person overrides This allows us to use an exclusion constraint on the person overrides table instead of directly using a FK on posthog_person. * test(person-overrides): Update tests to match new constraint * fix(migration): Add drop extension query to reverse migration * fix(migration): Use correct table name * refactor(person-overrides): Make team a regular bigint field without FK * refactor(person-overrides): Rename Helper model to Mapping * feat(person-override): Add a helper model to indirectly reference person overrides This allows us to use an exclusion constraint on the person overrides table instead of directly using a FK on posthog_person. * wip: add test for concurrent updates to posthog_personoverride table * refactor * Update snapshots * fix(isort): Correctly sort imports * fix(ee-cohort-test): Delete person after creating it * fix: person constraint in person overrides table (#14319) * nits * fix migration tests * chore(migration): Bump migration number to 0302 * Update snapshots * feat: person-overrides writes * test(person-overrides): Add concurrent tests to person-overrides model * feat(person-merge): Update merge to use new helper table * fix(tests): Pass poEEmbraceJoin to updatePersonState in test * fix(person-state): Format person-overrides message for ClickHouse * test(api): Add function to reload dictionary for person overrides * fix: poe final test failure * refactor(person-state): Make failed attempts a class variable to This allows us to mock it during testing as some tests require immediate failures. * fix(postgres-utils): Apply some magic changes lost to time * fix(person-state): Join with helper table to return UUIDs * fix(person-state): Use single quotes for UUID queries * test(person-overrides): Skip test that doesn't work without a merge command * test(person-state): Add a very complicated query to get UUIDs in a test * test(person-state): Try waiting longer, this is flaky * fix(migrations): Remove unused migration * fix(person): Undo unneeded person model changes * chore: Clean-up artifacts from rebase on model branch * refactor(person-state): Rename mergeAttempts and don't read from ENV * chore: Better clarify oldest_event usage in comment Co-authored-by: Tiina Turban <tiina303@gmail.com> * refactor(person-state): Update version in queries * fix(person-state): Use new mapping model instead of helper * fix(migrations): Re-add constraint deleted on field drop * revert: Re-enable test that was skipped Test was originally skipped due to missing a $merge command, but now $merge_dangerously is available, so the test should pass. * test: Attempt to refresh dictionary in test * Update query snapshots * Update query snapshots * test: Expect dictionaries to be refreshed before resuming test * fix(test): Use alias property for $merge_dangerously in test * fix(test): Expect any string like in all other tests * test: Fix order of events to determine merged persons * chore: Update comment regarding overrides mapping query * chore: Remove out of date comment * test: Set number of retries to 0 when updating persons * test: Delete comment * test: Assert clickhouse state after identify * chore: Move comment * chore: Delete TODO regarding oldest_event updates * test: Assert events are still processed if merge fails * test: Assert properties are processed if merge fails * test: Don't expect to throw when failing merges * fix: Indent RETURNING query clause Co-authored-by: Harry Waye <harry@posthog.com> * chore: Remove out of date comment Co-authored-by: Tiina Turban <tiina303@gmail.com> * test: Resume merges after test suite * test: Run processPersonStep tests for both poEEmbraceJoin modes --------- Co-authored-by: Harry Waye <harry@posthog.com> Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tiina Turban <tiina303@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Changes
👉 Stay up-to-date with PostHog coding conventions for a smoother review.
How did you test this code?