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: customer io email support #28578

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

zlwaterfield
Copy link
Contributor

Changes

We're consolidating email sending into customer.io so moving off Mailgun. Billing emails are already using this and it's working well. This is adding the basic setup. Future PRs will enable this for a few emails.

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

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

Built this as an alternative to the existing STMP

How did you test this code?

Added a few unit tests

@zlwaterfield zlwaterfield self-assigned this Feb 11, 2025
@zlwaterfield zlwaterfield marked this pull request as ready for review February 12, 2025 00:09
@zlwaterfield zlwaterfield requested a review from a team February 12, 2025 00:09
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

This PR introduces Customer.io as a new email service provider, replacing Mailgun for transactional emails, with implementation focused on API integration and backward compatibility with existing SMTP functionality.

  • Added Customer.io API integration in /posthog/email.py with new _send_via_http function and service availability checks
  • Implemented Decimal to float conversion in /posthog/email.py for proper JSON serialization in Customer.io payload
  • Added CUSTOMER_IO_API_KEY configuration in /ee/settings.py for enterprise deployment support
  • Added comprehensive HTTP-based email sending tests in /posthog/test/test_email.py covering API authentication and data handling
  • Maintained backward compatibility by preserving SMTP functionality with a use_http toggle flag

3 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile

@@ -44,6 +44,8 @@
# This is because in the demo env social signups get is_staff=True to facilitate instance management
SOCIAL_AUTH_GOOGLE_OAUTH2_WHITELISTED_DOMAINS = ["posthog.com"]

CUSTOMER_IO_API_KEY = get_from_env("CUSTOMER_IO_API_KEY", "", type_cast=str)
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider adding documentation comments above this setting to explain its purpose and whether it's required for cloud vs self-hosted setups

Comment on lines +93 to +94
if not customerio_api_key:
raise Exception(f"Missing Customer.io API key: {customerio_api_key}")
Copy link
Contributor

Choose a reason for hiding this comment

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

logic: Logging the API key in the error message could expose sensitive information

Suggested change
if not customerio_api_key:
raise Exception(f"Missing Customer.io API key: {customerio_api_key}")
if not customerio_api_key:
raise Exception("Missing Customer.io API key")


response = requests.post("https://api.customer.io/v1/send/email", headers=api_headers, json=payload)
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using settings.CUSTOMER_IO_BASE_URL instead of hardcoding the API endpoint

Comment on lines +101 to +136
try:
for dest in to:
with transaction.atomic():
record, _ = MessagingRecord.objects.get_or_create(
raw_email=dest["raw_email"], campaign_key=campaign_key
)

record = MessagingRecord.objects.select_for_update().get(pk=record.pk)
if record.sent_at:
continue

properties = {
"subject": subject,
"body": html_body,
"reply_to": reply_to or get_instance_setting("EMAIL_REPLY_TO"),
**headers,
}

# Convert any Decimal values to float for JSON serialization
properties = {k: float(v) if isinstance(v, Decimal) else v for k, v in properties.items()}

payload = {
"transactional_message_id": campaign_key,
"to": dest["raw_email"],
"identifiers": {"email": dest["raw_email"]},
"message_data": properties,
}

response = requests.post("https://api.customer.io/v1/send/email", headers=api_headers, json=payload)

if response.status_code != 200:
raise Exception(f"Customer.io API error: {response.status_code} - {response.text}")

record.sent_at = timezone.now()
record.save()

Copy link
Contributor

Choose a reason for hiding this comment

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

style: Transaction block is unnecessarily large - move the API call outside

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.

1 participant