-
Notifications
You must be signed in to change notification settings - Fork 408
Alway serialize click-tracking parameters #220
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
Conversation
Othwerwise setting them to a false value produces an invalid request, because the fields are required. Fixes sendgrid#181
|
LGTM |
| ClickTrackingSetting clickTrackingSetting = new ClickTrackingSetting(); | ||
| clickTrackingSetting.setEnable(true); | ||
| clickTrackingSetting.setEnableText(true); | ||
| clickTrackingSetting.setEnableText(false); |
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.
@thinkingserious This seems like more of a problem with the setEnable function? Why are we doing
clickTrackingSetting.setEnable(true);
and then
clickTrackingSetting.setEnableText(true);
Isn't that redundant?
In Setting we only return enable - I didn't see a place where we return enableText - is that for the "pretty" print? If so, can we modify the class to return enable instead of enableText in that case?
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.
The real change was removing @JsonInclude(Include.NON_DEFAULT) from ClickTrackingSetting.java to solve issue #181. I think the test was changed to demonstrate that the change worked.
|
I merged in all recent changes in the hopes of getting a passing build! |
Othwerwise setting them to a false value produces an invalid request, because
the fields are required.
Fixes #181