Skip to content
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 support for batched templated emails #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Arttii
Copy link

@Arttii Arttii commented Sep 29, 2022

Hi,

I need to be able to send batches of templates as well, so I added another function to take care of it.

Feedback is welcome

Thank!

@Arttii Arttii changed the title Add uspport for batched templated emails Add support for batched templated emails Sep 29, 2022
@mattevans
Copy link
Owner

Hi Arttii,

Thanks so much - and apologies for my slow response - I don't know how I missed this.

I've left a couple of comments, but the biggest issue would be around the error handling for this endpoint.

From Postmark docs:

Please note that the /batchWithTemplates endpoint will return a 200-level http status, even when validation for individual messages may fail. Users of these endpoints should check the success and error code for each message in the response from our API (the results are ordered the same as the original messages).

The pkg currently isn't setup to handle errors that return a 200-level response, so some adjustments will be needed so errors can be returned correctly from your new method.

Thanks!

@@ -100,3 +101,27 @@ func (s *EmailService) SendBatch(emailRequests []*Email) ([]*EmailResponse, *Res

return email, response, nil
}

// SendBatchTemplated will build and execute request to send batch templated emails via the API.
func (s *EmailService) SendBatchTemplated(emailRequests []*Email) ([]*EmailResponse, *Response, error) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we perhaps rename to something more in alignment with postmark?

SendBatchWithTemplates perhaps?

Comment on lines +111 to +113
var formatEmails map[string]interface{} = map[string]interface{}{
"Messages": emailRequests,
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
var formatEmails map[string]interface{} = map[string]interface{}{
"Messages": emailRequests,
}
formatEmails := map[string]interface{}{
"Messages": emailRequests,
}

}

email := []*EmailResponse{}
response, err := s.client.Do(request, email)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment left on PR. But the tldr; is we will never get an error back here due to this endpoint always returning 200.

@josephjoeljo
Copy link

Any updates on this?

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

Successfully merging this pull request may close these issues.

3 participants