Skip to content

Conversation

polypixeldev
Copy link
Member

@polypixeldev polypixeldev commented Jul 24, 2025

Summary of the problem

A notice should be sent out to managers of organizations that have had monthly announcements automatically enabled to let them know about the feature.

Describe your changes

Adds the notice email to the announcement mailer, and a one time job to send out the email.

image

@polypixeldev polypixeldev marked this pull request as ready for review August 4, 2025 23:34
@polypixeldev polypixeldev requested review from a team as code owners August 4, 2025 23:34
Event.includes(:config).where(config: { generate_monthly_announcement: true }).find_each do |event|
monthly_announcement = event.announcements.monthly.last

if monthly_announcement.present?
Copy link
Member

Choose a reason for hiding this comment

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

concern: does this mean we'll keep sending these until they post an announcement?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, this was just meant to be a one time email - the check for monthly announcement is present is in case an event enabled generating monthly announcements after this month's job ran

Copy link
Member

@sampoder sampoder left a comment

Choose a reason for hiding this comment

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

lgtm! are we confident these won't double send?

@polypixeldev
Copy link
Member Author

Not 100% - it's unlikely that they will ratelimit since when we run this job it will be the only mass email send running - however I can't be 100% sure that something else won't error and cause the job to re-run.

I'd like to get this sent out soon but it's better to wait until we have better error handling and ratelimiting for emails before running this, I'll be working on that next with the goal of getting it done to send this email out early next week

@sampoder sampoder merged commit e995ef4 into main Sep 4, 2025
13 checks passed
@sampoder sampoder deleted the polypixeldev/announcements-notice branch September 4, 2025 21:49
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.

3 participants