Tracking Settings jsonSerialize returns empty array

[schneiders11]

When I attempt to disable Click, Open, or Google Analytics tracking via the API it seems like the jsonSerialize/json_encode functions do not return the expected output. The php objects appear correct, but when encoding them, the values are lost or not returned properly.

PHP Versions: 5.6.23 & 7.0.12
SendGrid Version: 5.1.1

CODE SETUP:
$mail = new SendGrid\Mail();
...
$tracking_settings = new SendGrid\TrackingSettings();
$click_tracking = new SendGrid\ClickTracking();
$click_tracking->setEnable(false);
$click_tracking->setEnableText(false);
$tracking_settings->setClickTracking($click_tracking);
$mail->setTrackingSettings($tracking_settings);

RESPONSE:
{"errors":[{"message":"Invalid type. Expected: object, given: array.","field":"tracking_settings.click_tracking","help":"http://sendgrid.com/docs/API_Reference/Web_API_v3/Mail/errors.html#message.tracking_settings.click_tracking"}]}

SUBMITTED BODY (partial): "tracking_settings":{"click_tracking":[]}

CODE:
var_dump($click_tracking);
echo 'click:'.json_encode($click_tracking);

OUTPUT:
object(SendGrid\ClickTracking)#12 (2) {
["enable":"SendGrid\ClickTracking":private]=>
bool(false)
["enable_text":"SendGrid\ClickTracking":private]=>
bool(false)
}
click:[]

[thinkingserious]

Hi @schneiders11,

Thanks for letting us know. This bug is now added to our backlog for a fix.

[pizzot]

Pretty straightforward fix.

https://github.com/sendgrid/sendgrid-php/blob/master/lib/helpers/mail/Mail.php#L73

array_filter() filters out all false values as per documentation.

@thinkingserious Was it supposed to filter out null values?

[pizzot]

Looks like you were sleeping there in the US :) I have put a PR in the meantime. It fixes this particular issue but discovers more. It is the same problem for all the settings classes.

There's a major design problem in how these settings classes are created. See, you now need to fix the same issue on all the X classes. Having the serialized function abstracted into a base class would prevent this in the first place.

In fact all the functionality could be abstracted into a base class for the settings if we eliminate getters and setters and use magic methods instead.
I've put together a quick example:
https://github.com/okneloper/sendgrid-php/blob/refactoring/refactoring.php

[pizzot]

Looks like I need to submit an agreemt before the fix can be merged though...

[pizzot]

@schneiders11 you can clone this fix from my fork and see if it works now for you: https://github.com/okneloper/sendgrid-php/tree/clickTracking

[thinkingserious]

@okneloper,

Thanks for providing a temporary fix! Please take a moment to fill out this form so we can send you some swag :)

[schneiders11]

@okneloper That did the trick! I ended up working around it by reverting back to the v2 API, but no one likes that solution, I'm just glad I wasn't going crazy. Thanks for the quick patch option!

[thinkingserious]

Fixed with this release: https://github.com/sendgrid/sendgrid-php/releases/tag/v5.2.3