-
Notifications
You must be signed in to change notification settings - Fork 585
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
fix: Ensures the serialization of the SendGridMessage is untainted by defaults set by applications on the JsonSerializer #938
fix: Ensures the serialization of the SendGridMessage is untainted by defaults set by applications on the JsonSerializer #938
Conversation
If anyone can tell me what failed the Travis build, I'm happy to fix. I believe the error responsible is:
I don't know how to re-run the build (I'm guessing a networking issue caused the check to fail?). |
This fix broke my application. It doesn't camelcase anymore, please change back or fix. Should be at least backwards compatible. I'll investigate why this is happening more. |
I've just tested (on my original branch) that default serialization is unaffected. This passes.
I can't get this to run in the latest just yet (.net core framework conflicts). Will get a pull request to get this merged when I get it to run (to ensure defaults aren't broken). I'm curious how we can reproduce your issue. |
This breaks all of our email templates, which were expecting camel-case JSON. Variables are now coming across in Pascal case. Shouldn't this, at least, be configurable? Or am I missing something? |
@mfarnz I don't think there's one solution to fix the serialization of both email headers and email template parameters when using the So, here's my fix so that you can choose what you want to do - backwards compatible for your work. Let me file a bug, and suggest this as a fix - see if it gets approved. |
@duncanchung thanks for the update. The change looks good to me. |
Issue raised: #1063 |
Fixes
Fixes #937
Checklist
Short description of what this PR does: