Initial implementation of the invite email sending service #3735
Conversation
rdimitrov
force-pushed
the
mail
branch
2 times, most recently
from
4a048ed
to
a905fea
Compare
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
rdimitrov
force-pushed
the
mail
branch
2 times, most recently
from
cc0a318
to
b12efed
Compare
Signed-off-by: Radoslav Dimitrov <radoslav@stacklok.com>
rdimitrov
dismissed
dmjb’s stale review
The comment about renaming this was addressed.
|
@dmjb - The comment about renaming this was addressed. Dismissing this so we can unblock merging the PR. Do get back to me in case you wanted me to address something once you're back from PTO and I'll do a follow-up 👍 |
| //nolint:lll | ||
| const ( | ||
| // bodyHTML is the HTML body of the email | ||
| bodyHTML = ` |
There was a problem hiding this comment.
Files stored in cmd/server/kodata will be packaged into the application container, though I suspect we actually want to read this from a separate file that's not packaged in the container, so it can be Stacklok-branded without needing to ship the base email as Stacklok-branded.
This is fine for getting started, though.
There was a problem hiding this comment.
Yes, this is the initial approach and there'll be a follow-up where we can decide how to distribute the template.
| } | ||
|
|
||
| // getBodyHTML returns the HTML body for the email based on the message payload | ||
| func (e *MailEventPayload) getEmailBodyHTML() string { |
There was a problem hiding this comment.
It would be nice to have this expansion be consistent across different email providers; I don't know if we'd want to actually store the expanded payload in the message at some point in the future where the email sending service might be separate from Minder.
That's something we can adjust in the future, though.
If we make the expansion part of the message, it feels like we could make the different email senders simply Register directly, rather than needing to go through the MailEventHandler construct.
| // Register the email manager to handle email invitations | ||
| mailClient, err := email.NewAWSSES(cfg.Email.AWSSES.Sender, cfg.Email.AWSSES.Region) |
There was a problem hiding this comment.
This feels like we should only do this block if cfg.Email.AWSSES.Sender is non-empty, but I guess we'll do that in the future to allow local development to not need AWS credentials set up.
Summary
The following PR is the initial implementation of the email sending service for invitations.
Details:
What's left:
Moving away from using a builtin template for the mail content has a separate issue and will be done in a follow-up.
Change Type
Mark the type of change your PR introduces:
Testing
Example rendering of the template:
Review Checklist: