Send a Single Email to a Single Recipient

[thinkingserious]

Acceptance Criteria:

• A email object that represents the response body payload
• A mail object that handles sending email objects, data validation and error handling

Reference:

• https://github.com/sendgrid/sendgrid-csharp/tree/master/SendGrid/SendGrid/Helpers/Mail
• https://github.com/sendgrid/sendgrid-csharp/blob/master/CONTRIBUTING.md
• https://sendgrid.com/docs/API_Reference/Web_API_v3/Mail/index.html

[M3LiNdRu]

If you don't mind, I'd say I can help here. I'll do a PR.

[thinkingserious]

That's awesome @M3LiNdRu!

Could you please take a moment and sign our CLA so we will be able to merge your work? https://github.com/sendgrid/sendgrid-csharp/blob/master/CONTRIBUTING.md#cla

A bit more detail on the project can be found here: https://github.com/sendgrid/sendgrid-csharp/projects

[jpitchardu]

Is this issue taken?

[M3LiNdRu]

Yep, I´m working on it. I'll do a PR once it´s finished

[jpitchardu]

Ok thanks

[M3LiNdRu]

@thinkingserious About previous points:

• A email object that represents the response body payload.

What are you meaning? Create an object to handle http response body payloads of the request? Assuming you're using with dynamic objects right now...

• A mail object that handles sending email objects, data validation and error handling

Is it already created? As I can see there is an object (https://github.com/sendgrid/sendgrid-csharp/blob/master/SendGrid/SendGrid/Helpers/Mail/Mail.cs) for that purpose.

Many Thanks,
Roger

[thinkingserious]

Yes, the objects are already, this is to be a refactor based on the feedback from the community. I suggest you also take a look at the other v9beta projects here: https://github.com/sendgrid/sendgrid-csharp/projects and also the links included int the description. I'm currently working on "Remove Dynamic Dependency".

For this task, I would start by suggesting a new Interface that implements this use case.

For example we have this now:

string apiKey = Environment.GetEnvironmentVariable("NAME_OF_THE_ENVIRONMENT_VARIABLE_FOR_YOUR_SENDGRID_KEY", EnvironmentVariableTarget.User);
dynamic sg = new SendGridAPIClient(apiKey);

Email from = new Email("[email protected]");
string subject = "Hello World from the SendGrid CSharp Library!";
Email to = new Email("[email protected]");
Content content = new Content("text/plain", "Hello, Email!");
Mail mail = new Mail(from, subject, to, content);

dynamic response = await sg.client.mail.send.post(requestBody: mail.Get());

On solution might be:

string apiKey = Environment.GetEnvironmentVariable("NAME_OF_THE_ENVIRONMENT_VARIABLE_FOR_YOUR_SENDGRID_KEY", EnvironmentVariableTarget.User);
sg = new SendGridAPIClient(apiKey);

// Build the Mail object
Email from = new Email("[email protected]", "Example User");
Subject subject = new Subject("Hello World from the SendGrid CSharp Library!");
Email to = new Email("[email protected]", "Example User");
Content content = new Content("text/plain", "Hello, Email!");
Mail mail = new Mail(from, subject, to, content);

Response response = await sg.Client.SendAsync(mail);

or

string apiKey = Environment.GetEnvironmentVariable("NAME_OF_THE_ENVIRONMENT_VARIABLE_FOR_YOUR_SENDGRID_KEY", EnvironmentVariableTarget.User);
sg = new SendGridAPIClient(apiKey);

Mail mail = new Mail(sg);
mail.AddToEmail("[email protected]", "Example User");
mail.SetFromEmail("[email protected]", "Example User");
mail.SetSubject("Hello World from the SendGrid CSharp Library!");
mail.AddContent("text/plain", "Hello, Email!");
mail.AddContent("text/html", "Hello, Email!");
Response response = await mail.SendAsync()

I hope that helps. Please reach back out with further questions and feedback.

[thinkingserious]

Also, please be sure to fork off this branch: https://github.com/sendgrid/sendgrid-csharp/tree/v9beta Thanks!

[M3LiNdRu]

Perfect, I'll work on this.

Thanks for the details

[thinkingserious]

@M3LiNdRu,

I'm now actively working on this issue: #303

My work is on the remove-dynamic branch: https://github.com/sendgrid/sendgrid-csharp/tree/remove-dynamic

I've already removed the dependency to SendGrid.CSharp.HTTP.Client and I'm in the process of removing the dynamic functionality.

[thinkingserious]

@M3LiNdRu something to consider: #352

[M3LiNdRu]

@thinkingserious You should close this issue and merge the PR before start fixing #352.

Meanwhile I will have a look at it.

Cheers

[thinkingserious]

@M3LiNdRu,

I'm not finished yet, but I've merged this to the v9beta branch so you can start thinking about integration: https://github.com/sendgrid/sendgrid-csharp/pull/356/files

Basically, I've removed the dynamic dependency and brought the client functionality into the library.

[thinkingserious]

Also, here are my next steps:

• combine SendGridAPIClient and Client
• update the README
• update all tests

Then, I'm moving over to help you with the Mail Helper refactor.

[jkewley]

I've got to concur with @chadwackerman on the naming conventions he objected to over here. This thread is titled 'Send a Single Email to a Single Recipient' but if you applied those names to the V3 API, you'd end up sending an unknown entity someone's email address. I hate to be pedantic, but

Email is a term that the one generally uses to describe the complete message (send a single email)
Mail, when not a verb, is not commonly used in place of Email (can I mail you a quote? I'd prefer email)
Content by itself is a collection of information, not a type of information
Client is a suffix for a fully named type that explains what it is a client of

These are quick reactions to the limited part of the API that I see in your sample code over here.

I feel like you are trying to abbreviate widely-used terms to make the verbiage more succinct, but so many devs use intellisense-powered coding tools that the names of variables and types don't really matter to productivity. And instead of making something easier through brevity, (in my opinion) you've made it harder through ambiguity and redefinition.

Since you're targeting C# devs with this library, why not use the type names (or something very close) used by System.Net.Mail? If you do that, people will just know how to use your library without any trouble.

[Bartmax]

100% Agree with @jkewley
The entity names are misleading, confusing and very very obscure.

Few more issues I want to note

Content looks like have "body" and "header" (or mailtype).
if I'm not mistaken the most common mail-types are only 2.
Is it really needed to specify text/plain as a magic string? If it's possible i would avoid that, by having different objects (EmailBodyHtml & EmailBodyText [and a custom if needed]) or as part of the method signature.

---

The order of the parameters of the Mail constructor looks weird.
Main(from, subject, to, content);
I think from,to,subject,content will be more familiar, i suggest that pick the ordering from the System.Net.Mail or another popular mail library.

---

Why the client isn't an abstraction?

Response response = await client.RequestAsync(method: Client.Methods.POST,
                                                          requestBody: mail.Get(),
                                                          urlPath: "mail/send");

I would imagine more into this lines:

SendEmailResponse response = await client.SendEmailAsync(mail, cancellationToken);

---

I think it's also important to address that normally people send 2 versions of same email text/plain and html. So i think it would be great to have this consideration in mind. Maybe it is supported just not used in the sample.

Those were my 2 cents! by only reading the sample code

[Bartmax]

One more question:
This is an implementation of SMTP client or just a wrapper around HttpClient and REST API ?

[thinkingserious]

Thanks for the solid feedback @Bartmax! We will definitely be absrtacting the client in the helper and you should see your suggestions reflected shortly on this thread when I propose the updated interface.

Regarding your question, this is a wrapper around our v3 Web API.

[Jericho]

@Bartmax totally agree with your statement

Is it really needed to specify text/plain as a magic string? If it's possible i would avoid that, by having different objects (EmailBodyHtml & EmailBodyText [and a custom if needed]) or as part of the method signature.

In fact, I implemented something very similar to what you are suggesting in the StrongGrid library. I added two convenient methods called SendToSingleRecipientAsync and SendToMultipleRecipientsAsync that accept, among other things, a htmlContent and textContent parameters and the library takes care of adding the appropriate "magic strings" before making the request to SendGrid's API.

[thinkingserious]

Following is a proposal for the interface rewrite for the Mail Helper and interface for the use case of sending a single email.

I would like to begin implementation within days, if possible. Any feedback is appreciated as always.

using SendGrid;
using SendGrid.Helpers.Mail;

//// GENERIC HELLO WORLD SEND
// Wraps https://sendgrid.com/docs/API_Reference/Web_API_v3/Mail/index.html 

var sg = new SendGridClient("APIKEY");
var msg = new SendGridMessage();
var from = new MailAddress("[email protected]", "Name");
msg.From(from);
var to = new MailAddress("[email protected]", "Name");
msg.AddTo(to);
msg.Subject("Subject String");
msg.Content("HTML content string", sg.MimeType.HTML);
// or msg.ContentHTML("HTML content string");
msg.Content("Plain text content string", sg.MimeType.TEXT);
// or msg.ContentText("HTML content string");
var response = await sg.SendEmailAsync(msg);

//// SEND A SINGLE EMAIL TO A SINGLE RECIPIENT

var sg = new SendGridClient("APIKEY");
var from = new MailAddress("[email protected]", "Name");
var to = new MailAddress("[email protected]", "Name");
var subject = "Subject String";
var contentText = "Text content";
var contentHTML = "<bold>HTML content</bold>";
// Optionally, you can pass a fifth parameter of type
// SendGridMessage, to include things like attachments
// or turn on/off settings
var response = await sg.Mail.SendSingleEmailAsync(from, to, subject, contentText, contentHTML); 

The goal will be to keep this interface through the lifetime of the v3 API, adding functionality as needed.

With regards to the new version of SendGridMessage class, the plan is to stay close to the previous SendGridMessage class, but we will not include System.Net.Mail as it seems unstable at best in .NET Core.

I deviated from using Body instead of Content, as System.Net.Mail does, to be consistent with the v3 API conventions.

Thanks for taking the time to review!

[thinkingserious]

The WIP is in the single-email branch, and here are the preliminary changes.

The next step is that I will be uploading a beta to Nuget v9.0.0-beta and requesting additional feedback before we push the official v9.0.0 release.

Thanks again for the patience and continued support!

[darrelmiller]

Using functions to assign values is fairly uncommon thing to do in C#.  Using properties is much more common as it allows you to take advantage of property initialization. Off the top of my head, this is how I would do it.

var sg = new SendGridClient("APIKEY");
var msg = new SendGridMessage() {
  From = new MailAddress("[email protected]", "Name"),
  To = new MailAddress[] { new MailAddress("[email protected]", "Name")},
  Subject = "Subject String",
  TextContent = new PlainTextContent("Plain text content string"),
  HtmlContent = new HtmlContent("HTML content string") 
};
var response = await sg.SendEmailAsync(msg);

Using this initialization syntax is completely option and devs could assign values to properties in separate lines.  
I also changed the capitalization of HTML. .Net framework guidelines say any acronym over 3 chars should be Pascal cased.
The use of String/HTML Content classes mirror the way HttpClient handles content. It allows keeping the serialization details of the particular format with the class and the content-type header can be predefined in the content class, so there is no need for a user to specify it.

If you are looking for a way to make it simpler for easy cases, I would consider creating a helper method. e.g.

var sg = new SendGridClient("APIKEY");
var from = new MailAddress("[email protected]", "Name");
var to = new MailAddress("[email protected]", "Name");
var subject = "Subject String";
var contentText = "Text content";
var contentHTML = "<bold>HTML content</bold>";

var message = MessageHelper.CreateSingleEmailMessage(from, to, subject, contentText, contentHTML);
var response = await sg.SendEmailAsync(message); 

Although this approach introduces an extra step, it clarifies to a user that the sending process is identical regardless of whether they use CreateSingleEmailMessage or they construct the message themselves. This makes moving from the simple scenario to the advanced scenario easier.

[jkewley]

@thinkingserious - I don't mean this as an attack or dismissal in any way, but the code sample you put forth feels like an approach by someone who is coming to C# as a second or third programming language (your GitHub activity suggests that you dev across multiple languages). I'd strongly suggest that you reach out to someone who works with C# on a regular basis as a consumer of APIs and platforms. Given your preferred status within Azure at Microsoft, I'd expect that you should be able to engage someone over there for a little help/feedback.

I've got to say that @darrelmiller's suggested approach seems so obvious that I can't imagine the implementation being much different. It makes appropriate use of C# language features, library names, and programming patterns and is exactly the style of code that you'll find on any current blog, article, or open source repository across the .Net ecosystem. (I personally would use EmailAddress over MailAddress, but that's fairly insignificant relative to everything else).

He made several good arguments for his approach, including the use of properties over methods. I don't think I've seen the use of a method to set plain properties on a class since the 1.x versions of .Net. Maybe have a look at the Framework Design Guidelines.

As for .Net Core, it looks like they've decided which APIs they'll be re-implementing and have apparently made an initial commit to the System.Net.Mail namespace.

[thinkingserious]

@darrelmiller,

Thanks for feedback along with the quick turn around time :)

I'm definitely implementing your changes and they will be reflected on the single-email branch shortly.

@jkewley,

Indeed, I dev across 7 languages currently and C# is not my primary language. But I'm very much enjoying it and I've always loved Visual Studio since my C++ days at the University. I especially enjoy the passionate community here, within the C# community in general and at Microsoft. I've learned so much from all of you in such a short period. Thanks again!

Also, thanks for the confirmation on Darrel's suggestions and the heads up on the System.Net.Mail namespace, I will revisit this. We will continue to lean on this community and our friends at Microsoft as we continue to iterate forward. If you are interested, you can follow the branch I mentioned above for progress.

@chadwackerman,

Thanks for the upvote on Darrel's comments!

[thinkingserious]

@darrelmiller,

I have rewritten the two code samples as follows and have verified that the code is functional:

• Generic Hello World Send using the Mail Helper
• Send a Single Email using the Mail Helper

Your feedback would be greatly appreciated.

@jkewley, @chadwackerman,

Your feedback on the changes would also be greatly appreciated.

I will now clean up the code, update the examples, add comments and update the tests before moving on to the case of "Send a Single Email to Multiple Recipients".

[jkewley]

Looks much better. Couple of suggestions:

• consider renaming Client to something more specialized. SendGrid is probably going to be used in a project that has several types of clients which perform different interactions with other APIs
• the Personalization hierarchy feels like an API leak. As a consumer I would prefer to not have to know about that entity because it's not something that is typically part of an email api. Flatten everything out, use methods (AddRecipient) to add to internal collections using flat objects (EmailAddress) as parameters
• Same feedback on Contents. Plain text and html content have a 1 to 1 relationship with an email message, right? So let them be properties of the SendGridMessage class

[chadwackerman]

@jkewley Given the skills mismatch, in the absence of you providing actual code may I present the likely outcome here. https://www.flickr.com/photos/sluggerotoole/153603564

[jkewley]

Ok, I wrote some throw away code and pushed it out here.

Note that I implemented 2 approaches for adding a recipient.
The first is just a public List of EmailAddress types hanging off of the SendGridMessage. This makes for nice object initialization, but classes shouldn't expose underlying collections, so
The second is the AddRecipient approach which uses methods to add recipients

I'd welcome any feedback (except for my variable naming approach, which I acknowledge is non-standard) considering I whipped it up in about 10 mins.

[thinkingserious]

Thanks for the quick turnaround on the feedback @jkewley!

I will rename the Client to SendGridClient.

Personalizations is a key concept for advanced usage of the API, so I will hide that usage under a section called "Advanced or Custom", as there are some users who want to directly manipulate the entire request body for custom solutions. The helpers (e.g. Send a Single Email using the Mail Helper will serve to hide the implementation details, and we will highlight these use cases in README. These are the helpers I will implement for v9.0.0.

I agree with your comment on Contents and I will make that adjustment.

[thinkingserious]

@jkewley,

Just saw your comment, I will review shortly. Thanks for taking the time to do that!

[thinkingserious]

@jkewley,

Thanks again for taking the time to write up a useful code sample!

In addition the updates I mentioned above, I will add some convenience methods to the SendGridMessage object. I'll push the result up to the single-email branch.

[jkewley]

@chadwackerman, @darrelmiller please have a look at my scratch solution to provide feedback before @thinkingserious heads down the path of integrating the suggestions

[thinkingserious]

@jkewley,

Using the convenience methods added to SendGridMessage

[chadwackerman]

Looks good. I think you're missing a ".com" on this line: ... new EmailAddress("dx@sendgrid", "DX Team")

On the non-helper version you're creating a string, converting it to Object via JsonConvert, then converting it back to a string. I suppose this gives you some crude form of validation but better to create wrapper classes:

public class SendMailOptions
{
// (other fields needed)
public EmailAddress From { get; set; }
public System.Net.Http.HttpContent Content { get; set; }
}

The user fills that out, passes it to you, and you call JsonConvert.SerializeObject() to convert it to string for sending to your web service.

Gradually build up these little wrapper objects and you wind up with a clean library. You can often reuse types from the .NET libraries which will make things instantly familiar to .NET devs.

You may also want to check out Swagger.

[thinkingserious]

Thanks for taking the time to check out the changes @chadwackerman! And for the useful feedback, once again.

I'll fix the .com typo. For the final release, those will be fake email addresses.

That non-helper version was for those who prefer to cut and paste the actual JSON object. There was at least one person who specifically asked for that. It sounds like is probably a rare use case so we should probably tuck that example away in a dark corner.

What you describe with the SendMailOptions class, I believe, is achieved with the SendGridMessage class. True?

I will go through the code and look for places where we can use .NET types. Thanks!

We use Swagger to auto-generate interactive docs, usage docs, integration tests and examples.

[chadwackerman]

Was the Swagger autogenerated C# client a total disaster? (Is that how we got here in the first place?)

https://github.com/swagger-api/swagger-codegen

[thinkingserious]

Hello everyone,

BIG thanks to all of those that have contributed on this thread, we greatly appreciate your patience and support!

The v9beta branch has been pushed to Nuget as a prerelease for testing. Please give it a spin and let us know what you think :)

The main updates are:

• Removed dynamic dependencies
• Added support for .NET Standard 1.3
• Updated the Mail Helper for better UX/DX

Here is what we think needs to be completed before we exit beta:

• A few weeks of iterative improvements based on your feedback and internal testing
• Updated docs (USAGE, USE_CASES, TROUBLESHOOTING, CONTRIBUTING, /examples and the Helper Example)
• Update the auto-generated portions of the tests to use CamelCase function names
• Move from Travis to Appveyor
• Implement SendSingleEmailToMultipleRecipientsAsync
• Implement SendMultipleEmailsToMultipleRecipientsAsync
• Adopt StyleCop

The remainder of our wants for this version will be added to GitHub as issues.

With Best Regards,

Elmer

[thinkingserious]

v9 is launched