Skip to content

GYR1-977-allow-returning-file-myself-users-to-navigate-to-tax-slayer-without-error#6239

Open
DrewProebstel wants to merge 8 commits intomainfrom
GYR1-977-allow-returning-file-myself-users-to-navigate-to-tax-slayer-without-error
Open

GYR1-977-allow-returning-file-myself-users-to-navigate-to-tax-slayer-without-error#6239
DrewProebstel wants to merge 8 commits intomainfrom
GYR1-977-allow-returning-file-myself-users-to-navigate-to-tax-slayer-without-error

Conversation

@DrewProebstel
Copy link
Copy Markdown
Contributor

@DrewProebstel DrewProebstel commented Mar 20, 2026

Link to pivotal/JIRA issue

Is PM acceptance required? (delete one)

  • Yes - don't merge until JIRA issue is accepted!

Reminder: merge main into this branch and get green tests before merging to main

What was done?

  • Remove the unique constraint on campaign email and allows at most 2 to be created

How to test?

  • make sure "send_diy_survey" flipper tag is on

@DrewProebstel DrewProebstel changed the title allow 2 campaign emails to be created GYR1-977-allow-returning-file-myself-users-to-navigate-to-tax-slayer-without-error Mar 20, 2026
@github-actions
Copy link
Copy Markdown

Heroku app: https://gyr-review-app-6239-0a43e1abc58e.herokuapp.com/
View logs: heroku logs --app gyr-review-app-6239 (optionally add --tail)

@embarnard
Copy link
Copy Markdown
Contributor

@DrewProebstel i guess I'm worried about the case where we are batching emails and the batches that are creating the emails overlap. So you have the some of the same contacts in both batches. Before when we had this uniqueness constraint it would catch these and never create duplicate emails by message name for the same client. Now it could theoretically? I wonder if we could add something similar to what we have in the automated messages controller like self.send_only_once? so in CampaignMessage::DiyFollowupSurvey send_only_once? would return true but in the other CampaignMessages its false and we check for this before actually sending the message in Campaign::SendCampaignEmailJob.perform?


return if existing_send_count >= 2

CampaignEmail.create!(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

check for exisitng message and see if scheduled_send_at has already passed before trying to send one

DrewProebstelCfa and others added 3 commits March 24, 2026 11:13
Only create a second campaign email once the first has left the
in-progress state (delivered/failed), preventing back-to-back sends.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@DrewProebstel
Copy link
Copy Markdown
Contributor Author

@DrewProebstel i guess I'm worried about the case where we are batching emails and the batches that are creating the emails overlap. So you have the some of the same contacts in both batches. Before when we had this uniqueness constraint it would catch these and never create duplicate emails by message name for the same client. Now it could theoretically? I wonder if we could add something similar to what we have in the automated messages controller like self.send_only_once? so in CampaignMessage::DiyFollowupSurvey send_only_once? would return true but in the other CampaignMessages its false and we check for this before actually sending the message in Campaign::SendCampaignEmailJob.perform?

https://github.com/codeforamerica/vita-min/pull/6239/changes#diff-ec4b175698c9c3507597e2ae87048fd7522facfcfcebd9cce4d897ed43128e53R40

I think this like would keep duplicate emails from being batched at the same time rather than checking them in the job since if they are in different batches we may not pick up on the fact that they are duplicated. I added the uniqueness restraint back to the start_of_season_outreach messages. And the above lines should prevent having two 'diy_surveys' batched.

sent_count = CampaignEmail.where(
campaign_contact_id: email.campaign_contact_id,
message_name: email.message_name
).where.not(sent_at: nil).count
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

maybe down the line we would also check for a successful delivery but we don't have to worry about that now probably

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