Skip to content
This repository has been archived by the owner on Feb 8, 2018. It is now read-only.

Persist email messages #4572

Merged
merged 3 commits into from
Aug 19, 2017
Merged

Persist email messages #4572

merged 3 commits into from
Aug 19, 2017

Conversation

chadwhitacre
Copy link
Contributor

@chadwhitacre chadwhitacre commented Aug 15, 2017

Factor out email tests (#4570) — Persist email verifications (#4573) →


Previously the email queue was ephemeral. We would delete messages from it after successfully firing them off to SES. That prevents us from doing several things:

This PR:

  • changes email_queue to email_messages, a table that we never delete from
  • changes emails to email_addresses to disambiguate

Todo

  • store MessageId

@chadwhitacre
Copy link
Contributor Author

Ready for review, @rohitpaulk! :-)

@chadwhitacre
Copy link
Contributor Author

As we start persisting messages, we should store the MessageId returned from SES for successful submissions. That'll be crucial to bounce handling (#4284), and in general is a Good Idea™ (cf. #4442).

@chadwhitacre
Copy link
Contributor Author

chadwhitacre commented Aug 15, 2017

Alright, MessageId stored in e45ba2b.

@chadwhitacre chadwhitacre force-pushed the save-emails branch 2 times, most recently from 59f3d98 to 819ec4f Compare August 15, 2017 16:00
@chadwhitacre chadwhitacre changed the title Save email messages forever Persist email messages Aug 16, 2017
This was referenced Aug 16, 2017
@rohitpaulk
Copy link
Contributor

Looks good to me. I'm a bit confused as to why log_every has to be patched for the test, but that isn't really important compared to the purpose of this patch itself.

Waiting for Travis.

@rohitpaulk rohitpaulk merged commit d90fa7c into master Aug 19, 2017
@chadwhitacre chadwhitacre deleted the save-emails branch August 19, 2017 16:45
@chadwhitacre
Copy link
Contributor Author

I'm a bit confused as to why log_every has to be patched for the test

Good guestion! That might be noise. 😶

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants