Only send complete application reminders when email present #51726
Conversation
megcrenshaw
changed the title
megcrenshaw
force-pushed
the
meg/only-send-mail-when-email-present
branch
from
6321e5e
to
5fce4f7
Compare
| where(application_year: Pd::Application::ActiveApplicationModels::APPLICATION_CURRENT_YEAR, status: 'incomplete'). | ||
| select do |app| | ||
| app.email.present? | ||
| end |
There was a problem hiding this comment.
I'm curious if we need the 'where' and 'select' clauses separated here when they're both being used to filter out, could these be combined? (not sure if this runs two separate pass throughs under the hood or not, but it may at least be simpler to read by having all conditions within the same clause). Maybe select do |app| and then check if it meets the 3 criteria (application year, incomplete, and email present)
There was a problem hiding this comment.
Good question Turner. There is an argument to "optimize later" by doing it all with select, but in this case I agree with Meg that it seems worth the extra step to get a smaller response sent back from the database.
There was a problem hiding this comment.
Thanks for the question, Turner, and thanks for the explanation, Dave! I had to read more to understand this fully. I found these articles helpful FWIW:
- Doc for select and where
- Article comparing the two queries.
- Stack overflow
| where(application_year: Pd::Application::ActiveApplicationModels::APPLICATION_CURRENT_YEAR, status: 'incomplete'). | ||
| select do |app| | ||
| app.email.present? | ||
| end |
There was a problem hiding this comment.
Good question Turner. There is an argument to "optimize later" by doing it all with select, but in this case I agree with Meg that it seems worth the extra step to get a smaller response sent back from the database.
When clearing the queue of unsent mailers, I found some incomplete emails that were trying to send but were unable to do so because no email was present. This PR changes the "complete application reminders" to only select incomplete apps that also have an email.
Links
Testing story
Added unit tests to reflect updates
Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: