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

Validate field types sent to mail helper API #822

Closed
wants to merge 8 commits into from
Closed

Validate field types sent to mail helper API #822

wants to merge 8 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 25, 2018

Fixes

Resolves #816

Checklist

  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the master branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

Short description of what this PR does:

Some of the fields from the /mail/send POST API supported by this library aren't typechecked before the request is sent. The change in this PR causes the library to throw an exception before sending the request if the types passed to it don't match the necessary types described in the API documentation.

If you have questions, please send an email to Sendgrid, or file a Github Issue in this repository.

@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 25, 2018
@@ -83,6 +83,7 @@ class MailService {
rule.pattern.test('');
return rule
} catch (err) {
// FIXME?
Copy link
Author

Choose a reason for hiding this comment

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

The build currently fails ESLint due to an empty code block.
I'm mystified as to why that happens, because my PR doesn't change anything else in this entire file.

@ghost
Copy link
Author

ghost commented Oct 27, 2018

Travis CI builds fail due to an unresolved dependency with the code coverage tool. Is this a known issue? It seems like others' builds are failing for the same reason.
@thinkingserious

@tony-ho
Copy link
Contributor

tony-ho commented Oct 27, 2018

@jcfi I had the same issue when I rebased #803 on master after the latest round of merges. I've raised #824 which fixes the errors.

@thinkingserious Can #824 be approved and merged so that affected PRs can rebase on a new version of master that passes the CI build?

@ghost
Copy link
Author

ghost commented Oct 30, 2018

@thinkingserious Any updates on this PR? Hacktoberfest only has 2 days left 😉

@thinkingserious thinkingserious added hacktoberfest difficulty: hard fix is hard in difficulty type: twilio enhancement feature request on Twilio's roadmap labels Oct 30, 2018
@thinkingserious
Copy link
Contributor

Hello @jcfi,

Thanks again for the PR!

It's HACKTOBERFEST! We want to show our appreciation by sending you some special Hacktoberfest swag. If you have not already, could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

@thinkingserious
Copy link
Contributor

Hello @jcfi,

Thanks again for the PR!

You have earned 7 out of the 5 points needed to receive glorious SendGrid Hacktoberfest swag.

Please take a moment to checkout this link to find more issues to get you past the required threshold or to simply continue the celebration.

Also, please be sure you have officially registered with us here by November 1, 2018 to qualify.

If you have any questions you can email us at dx+hacktoberfest2018@sendgrid.com.

Thank you and Happy Hacktobering!

Team SendGrid

@childish-sambino childish-sambino removed the type: twilio enhancement feature request on Twilio's roadmap label Jul 20, 2020
@childish-sambino
Copy link
Contributor

Changes have been merged: 96f2c7a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: hard fix is hard in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type Exception Handling for the Mail Helper
3 participants