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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

LG-12294: Set maximum delay for aggregated email events #10436

Merged
merged 1 commit into from
Apr 15, 2024

Conversation

aduth
Copy link
Member

@aduth aduth commented Apr 15, 2024

馃帿 Ticket

Supplemental improvement for LG-12294

馃洜 Summary of changes

Updates logic for aggregated email sign-in notification to prevent stale events from being included in the email.

Related Slack discussion: https://gsa-tts.slack.com/archives/C0NGESUN5/p1713191072377119?thread_ts=1712932922.568999&cid=C0NGESUN5

Examples:

  • As discussed in the Slack thread above, enabling in sandbox environments and then quickly disabling to address LG-12294: Fix asynchronous email delivery for aggregated sign-in email聽#10421 produced stale events when re-enabling today
  • As discussed in the Slack thread above, starting local development server today (Monday) prompted an email notification for sign-ins that occurred at the end of day Friday

馃摐 Testing Plan

Verify tests pass:

rspec spec/services/user_alerts/alert_user_about_new_device_spec.rb

Verify you don't get an email after a long delay:

  1. Go to http://localhost:3000
  2. Sign in with email and password (don't MFA)
  3. Stop the local development server process
  4. Wait 15 minutes
  5. Start the local development server
  6. Wait for the job to run
  7. Observe that you don't receive an email notification about your sign-in

changelog: Upcoming Features, Sign In, Send single aggregated email notification for new device sign-in
@aduth aduth requested a review from a team April 15, 2024 18:46
Copy link
Contributor

@jmdembe jmdembe left a comment

Choose a reason for hiding this comment

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

馃憤馃従

Copy link
Contributor

@kevinsmaster5 kevinsmaster5 left a comment

Choose a reason for hiding this comment

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

LGTM

@aduth aduth merged commit 38b21fd into main Apr 15, 2024
2 checks passed
@aduth aduth deleted the aduth-sign-in-events-max-delay branch April 15, 2024 20:00
@mitchellhenke mitchellhenke mentioned this pull request Apr 16, 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.

None yet

3 participants