Open ClaireValdivia opened 11 months ago
@ClaireValdivia A few things I wanted to clarify on the expected state here:
Email title: Are we talking about the email subject? The thing that shows up here on the email? And we want this to read simply {{ search name }}
, instead of right now reading New grants published for {{ search name }}
? (Fwiw my instinct would be that you still want something more than just the search name, since people might have names that end up confusing out of context in their email inbox.)
Email body text preview: Here are we talking about preview text (aka hidden preheader text)? We don't have this set up in our emails yet, but it wouldn't be too difficult to add. Can you help me get precise about the text, though? Do we want {{ number of grants }} new grants, {{ title of first grant }}...
(where the ...
is literally what you want in the text, rather than indicating that we should include more titles after the first)?
Probably out of scope here, but wanted to mention that the "from name" and "preview text" features are new. If we wanted to start using these for other emails we send as well, that would be a relatively easy follow-up once this ticket introduces the basic functionality.
@jeffsmohan thanks for these questions! I updated 1 in the description. confirming with caitlin for 2
consistency for all emails is a great point. ill create an issue for addressing this but it won't be ready immediately to work on as I think we have a number of things to clean up in other emails.
@jeffsmohan just updated the issue - thanks again for these really helpful questions!
Apologies for the delayed comment here, but I must have missed testing this one fully. Just tested now and things are looking fine overall but @jeffsmohan I noticed that the "From" name is still showing as grants-notifications rather than "USDR Federal Grant Finder" - was that just forgotten in the PR? if so I can definitely make a new issue to address that if helpful
@ClaireValdivia Sorry, emails are tough to fully test before they get on staging! This change was included in the PR that got merged, so if it's not working, that's just a bug that needs fixing. Can you forward me an example email so I can examine the email headers, for a start?
Also, I noticed in looking back at this code that I maybe followed the spec more slavishly than was helpful. I implemented the from name on the grant digest emails only, but I assume we would actually want the same from name for all emails related to the Grant Finder tool, yes? (And while we're at it, it'd be easy to set "USDR Grants Reporter" or similar for all the ARPA emails, if we wanted. Is that what we'd want?)
Just sent an email as an example of where grants-notifications is showing in "from."
and hmm if it's easy to update the from name at once, then yes, I think having all the grant finder ones read as from "USDR Federal Grant Finder," and ARPA ones reading as from "UDSR ARPA Reporter" would be great!
@ClaireValdivia Thanks for catching this! Looks like our email infra is different between dev and staging/prod. So while this was working in dev when the original PR went out, it wasn't implemented for staging/prod.
I've just put up a new PR (https://github.com/usdigitalresponse/usdr-gost/pull/2940) for review that implements the from name for staging/prod, as well as implementing the from names we discussed for all outgoing emails.
@ClaireValdivia We've landed the PR fixing "from" name on emails! Once that's on staging, we should be sure to double-check it's working across all Grants and ARPA emails -- lmk if you'd like a hand with that
@jeffsmohan
Login - looks great for grant finder, but for ARPA users, it also says from USDR Federal Grant Finder. if it can only be one, let's change the login email to be from "USDR Grants"
Welcome - (same as login) - looks great for grant finder, but for ARPA users, it also says from USDR Federal Grant Finder. if it can only be one, let's change the login email to be from "USDR Grants"
Grant finder assignment email - looks great
Grant finder digest emails - will test tomorrow when my digests come in
ARPA Treasury Report - this is on me (I had a typo above!) - it current says "UDSR ARPA Reporter" and should say "USDR ARPA Reporter"
ARPA Audit Report - same as treasury report - UDSR should be changed to USDR
@ClaireValdivia the from name fixes are now on staging! Please let me know if you find anything else
This all looks great on staging, this is good to go tomorrow :) Thank you Jeff!!
Why is this issue important?
follow-up to #1222
Current State
na
Expected State
Implementation Plan
The following functions need to be added...
Relevant Code Snippets
No response