-
Notifications
You must be signed in to change notification settings - Fork 2
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’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add custom args to SGMailV3 #7
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for putting this together, while i love that you're adding this feature i think we should take a different approach.
sendgrid_sender.go
Outdated
@@ -23,7 +23,7 @@ type SendgridSender struct { | |||
|
|||
//Send sends an email to Sendgrid for delivery, it assumes | |||
//bodies[0] is HTML body and bodies[1] is text. | |||
func (ps SendgridSender) Send(m mail.Message) error { | |||
func (ps SendgridSender) Send(m mail.Message, customArgs ...map[string]string) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tatang26 this will not work since it implies our Sender is no longer a [buffalo] mail.Sender. My recommendation would be to put these into Message.Data, we could then use a particular key to know that that's newrelic custom-args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paganotoni You are right, I updated the buildMail function to add custom args from Message.Data
sendgrid_sender.go
Outdated
@@ -73,6 +73,10 @@ func buildMail(m mail.Message) (*smail.SGMailV3, error) { | |||
p.AddTos(smail.NewEmail(to.Name, to.Address)) | |||
} | |||
|
|||
for k, v := range m.Data { | |||
p.SetCustomArg(k, v.(string)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very close! we now need to use just one of the keys in that m.Data for this purpose. Also, we need to add documentation in our README that explains how to add custom args.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it!
sendgrid_sender.go
Outdated
@@ -73,6 +73,10 @@ func buildMail(m mail.Message) (*smail.SGMailV3, error) { | |||
p.AddTos(smail.NewEmail(to.Name, to.Address)) | |||
} | |||
|
|||
for k, v := range m.Data { | |||
p.SetCustomArg(k, fmt.Sprintf("%v", v)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it should be something like:
if customArgs, ok := m.Data[CustomArgsKey].(CustomArgs); ok {
[iterate through CustomArgs]
}
CustomArgsKey
should be a constant in this package.
CustomArgs
should be a type map[string]string
No description provided.