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: update billing manager sync to use a single func #24205

Merged
merged 4 commits into from
Aug 7, 2024

Conversation

zlwaterfield
Copy link
Contributor

@zlwaterfield zlwaterfield commented Aug 5, 2024

Changes

Follow up to https://github.com/PostHog/billing/pull/797 and https://github.com/PostHog/billing/pull/817.

We want to sync more useful user data into billing to trigger the emails/events. We don't have the proper email/distinct id/role information right now.

This PR does two things:

  • pulls together billing syncs into a single function - right now multiple calls are made and it's inefficient. Doesn't change any existing sync data because billing still relies on them. They can be removed in the future once it no longer does.
  • adds a new key org_admin_user to the sync with dinstinct_id, email and role. This is only for admins and owners because billing-related notifications are needed only for admins and owners.

Given this is only syncing admins and owners it's not adding to much extra data.

The hope with this is we will be able to trigger events in billing that can then use the CDP to trigger emails whether than be in c.io or the CDP directly.

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

Does this work well for both Cloud and self-hosted?

It doesn't have an impact

How did you test this code?

Updated tests

@zlwaterfield zlwaterfield self-assigned this Aug 5, 2024
@zlwaterfield zlwaterfield changed the title update billing manager sync to use a single func feat: update billing manager sync to use a single func Aug 5, 2024
@zlwaterfield zlwaterfield merged commit 4bdd564 into master Aug 7, 2024
86 checks passed
@zlwaterfield zlwaterfield deleted the zach/sync-users-to-billing branch August 7, 2024 18:40
Copy link

sentry-io bot commented Aug 8, 2024

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

  • ‼️ AttributeError: 'NoneType' object has no attribute 'update_billing_organization_users' posthog.management.commands.sync_to_billing in ... View Issue
  • ‼️ Exception: ('Billing service returned bad status code: 400', 'body:', {'type': 'validation_error', 'code': '... posthog.tasks.sync_to_billing.sync_to_billing View Issue

Did you find this useful? React with a 👍 or 👎

silentninja pushed a commit to silentninja/posthog that referenced this pull request Aug 8, 2024
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