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

Don't null IPs for teams who toggled an ancient field #3462

Merged
merged 2 commits into from Feb 25, 2021

Conversation

mariusandra
Copy link
Collaborator

Changes

It's normally not good practice to change old migrations, but since we're working with releases and this hasn't gone out yet, might as well.

I think this line is wrong (passed my review, sorry). team.anonymize_ips means we just don't store IP information with events. team.opt_out_capture was an old flag to disable tracking on self-hosted instances. They don't mean the same thing.

The impact is probably zero users so far, but if we release with this line, then after upgrading to 1.22, some really old and still running installations that had flipped this switch will suddenly stop seeing IP addresses with their event properties. Best to revert I think.

Checklist

  • All querysets/queries filter by Organization, by Team, and by User
  • Django backend tests
  • Jest frontend tests
  • Cypress end-to-end tests

@mariusandra mariusandra changed the title Don't null ips for teams who toggled an ancient field Don't null IPs for teams who toggled an ancient field Feb 24, 2021
@timgl timgl temporarily deployed to posthog-remove-bad-migr-qci4ue February 24, 2021 21:55 Inactive
@Twixes Twixes temporarily deployed to posthog-remove-bad-migr-qci4ue February 24, 2021 22:05 Inactive
Copy link
Collaborator

@Twixes Twixes left a comment

Choose a reason for hiding this comment

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

Oh, definitely! I meant User.anonymize_data. Updated the function accordingly. Safe change overall.

@Twixes Twixes temporarily deployed to posthog-remove-bad-migr-qci4ue February 24, 2021 22:10 Inactive
@mariusandra mariusandra merged commit 74ed8bb into master Feb 25, 2021
@mariusandra mariusandra deleted the remove-bad-migration-line branch February 25, 2021 13:11
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.

None yet

3 participants