Skip to content

Comments

feat(nimbus): Send emails to feature subscribers#13988

Merged
yashikakhurana merged 7 commits intomainfrom
13937
Nov 19, 2025
Merged

feat(nimbus): Send emails to feature subscribers#13988
yashikakhurana merged 7 commits intomainfrom
13937

Conversation

@yashikakhurana
Copy link
Contributor

Because

  • We want to send emails to the feature subscribers of the linked deliveries in which that feature is used

This commit

  • Add feature subscribers to the list to send the email

Fixes #13937

@yashikakhurana yashikakhurana marked this pull request as ready for review November 18, 2025 21:21
Copy link
Collaborator

@jaredlockhart jaredlockhart left a comment

Choose a reason for hiding this comment

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

Yep perfect, great use of set, great test coverage, 💯 A+ 10/10 thnx @yashikakhurana

Comment on lines +145 to +149
self.assertIn(experiment.owner.email, sent_email.recipients())
self.assertIn(experiment_subscriber.email, sent_email.cc)
self.assertIn(feature_subscriber1.email, sent_email.cc)
self.assertIn(feature_subscriber2.email, sent_email.cc)
self.assertEqual(len(sent_email.cc), 3)
Copy link
Collaborator

Choose a reason for hiding this comment

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

🙏 🎉

self.assertIn(feature_subscriber2.email, sent_email.cc)
self.assertEqual(len(sent_email.cc), 3)

def test_send_experiment_ending_email_deduplicates_subscribers(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah good idea 👍

Comment on lines +39 to 51
emails = chain(
experiment.feature_configs.values_list("subscribers__email", flat=True),
experiment.subscribers.values_list("email", flat=True),
)
cc_emails = {email for email in emails if email}

email = EmailMessage(
subject.format(name=experiment.name),
content,
settings.EMAIL_SENDER,
[experiment.owner.email],
cc=experiment.subscribers.all().values_list("email", flat=True),
cc=list(cc_emails),
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Yep definitely exactly 👍

@yashikakhurana yashikakhurana added this pull request to the merge queue Nov 19, 2025
Merged via the queue into main with commit f783304 Nov 19, 2025
17 checks passed
@yashikakhurana yashikakhurana deleted the 13937 branch November 19, 2025 20:58
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.

Update celery tasks to send emails to feature subscribers

2 participants