facebook / facebook-nodejs-business-sdk

Node.js SDK for Meta Marketing APIs
https://developers.facebook.com/docs/business-sdk
Other
489 stars 226 forks source link

Update return value for advertiser_tracking_enabled in server-event.js #254

Closed yoongyj closed 1 year ago

yoongyj commented 1 year ago

Currently advertiser_tracking_enabled() is returning _data_processing_options_country, it should be returning _advertiser_tracking_enabled instead.

jcurlier commented 1 year ago

Related to https://github.com/facebook/facebook-nodejs-business-sdk/issues/251

jcurlier commented 1 year ago

Should we allow false in the normalize method as the parameter 'advertiser_tracking_enabled' is required for the event with the action source 'app'

Existing

        if (this.advertiser_tracking_enabled) {
            serverEvent.advertiser_tracking_enabled = this.advertiser_tracking_enabled;
        }

proposed:

        if (this.advertiser_tracking_enabled === true || this.advertiser_tracking_enabled === false) {
            serverEvent.advertiser_tracking_enabled = this.advertiser_tracking_enabled;
        }

Similar to https://github.com/facebook/facebook-nodejs-business-sdk/pull/214

facebook-github-bot commented 1 year ago

@stcheng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

stcheng commented 1 year ago

@yoongyj thank you for creating this diff to address this bug @jcurlier this seems a legit change, i'll create a diff to address this as well as to merge the #214 thank you

facebook-github-bot commented 1 year ago

@stcheng merged this pull request in facebook/facebook-nodejs-business-sdk@e59913f5862f5c1cd065050c2177e7c1d75de6eb.

yoongyj commented 1 year ago

thanks @stcheng for merging this pull request!