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

pc/consent: Fides.meta integration function #2217

Merged
merged 3 commits into from
Jan 24, 2023

Conversation

ssangervasi
Copy link
Contributor

@ssangervasi ssangervasi commented Jan 12, 2023

Closes #2160
Doc: https://ethyca.atlassian.net/wiki/spaces/EN/pages/2583789680/Facebook+fides-consent.js

Code Changes

  • Reverse engineered the fbq shimming process that FB just wants you to paste into pages. Somehow worse than GTM.
  • Added some basic global variable tests to the cypress demo page.

Steps to Confirm

  • Read all the the documentation links that I reference in the code/readme. Use this info to compile JS+TS in your head to see if my implementation of their API is reasonable.
  • If you're a wizard, or have a facebook business account, try actually putting this on a page and seeing analytics changes. I can't.

Pre-Merge Checklist

Description Of Changes

It's been tough to integrate with Meta because:

  • I'm unable to create a test account/business to see the API track/not-track end-to-end
  • fbq doesn't have an API for reading what its state is, so you just have to hope a lack of error messages means something happened
  • Whack-a-mole documentation

That said, the options I've provided for this function cover the cases that were documented, leaning towards privacy-by-default instead of give-away-everything by default.

@codeclimate
Copy link

codeclimate bot commented Jan 12, 2023

Code Climate has analyzed commit b07547c and detected 6 issues on this pull request.

Here's the issue category breakdown:

Category Count
Duplication 6

View more on Code Climate.

@ssangervasi ssangervasi force-pushed the ssangervasi/fides-2160/facebook-fides-consent-js branch from 947aeee to ea29343 Compare January 12, 2023 22:50
Copy link

@mfbrown mfbrown left a comment

Choose a reason for hiding this comment

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

@ssangervasi This makes sense to me in general. Since we are just shipping the defaults, do you think it makes sense to create cookie keys that are even more explicit? Something like meta_tracking and shopify_tracking since we can have multiple keys on a data use?

I know it's a bit repetitive, but the very obvious naming might make it easier for someone unfamiliar with the project to parse.

clients/privacy-center/packages/fides-consent/README.md Outdated Show resolved Hide resolved
@ssangervasi ssangervasi force-pushed the ssangervasi/fides-2160/facebook-fides-consent-js branch from b07547c to 38509b5 Compare January 23, 2023 22:36
Copy link
Contributor

@Kelsey-Ethyca Kelsey-Ethyca left a comment

Choose a reason for hiding this comment

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

LGTM! Blessings on your whack-a-mole hunt

@ssangervasi ssangervasi merged commit 02a6af3 into main Jan 24, 2023
@ssangervasi ssangervasi deleted the ssangervasi/fides-2160/facebook-fides-consent-js branch January 24, 2023 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support third party integrations with Fides consent script - Facebook
3 participants