search

combindma / laravel-facebook-pixel

Meta Pixel integration for Laravel
MIT License
33 stars 9 forks source link

[Bug]: Lack of support for non-integer user_ids #3

Closed piotrknapik closed 4 months ago

piotrknapik commented 6 months ago

What happened?

The package doesn't support non-numeric user_ids like UUIDs. It cause a JS error: 'Uncaught SyntaxError: Invalid or unexpected token' because: external_id: 0b8afcd1-47...

BTW: By default laravel-facebook-picture sends the Advanced Matching data (email and Laravel's user_id) by default for logged in users. Many privacy policies doesn't support that. I think the documentation should be update on how to disable it. I'll propose a PR with the new config variable within minutes.

I tried publishing the views and modyfing resources/views/vendor/head.blade.php file: php artisan vendor:publish --provider="Combindma\FacebookPixel\FacebookPixelServiceProvider" --tag="views"

...but despite clearing caches this didn't work for me. Maybe because views are set up on packageBooted() instead of boot() in FacebookPixelServiceProvider? Not sure - I decided to propose a PR frm my fork instead ;P

How to reproduce the bug

Set user_id to a string and try running the package. You'll get the JS error.

Package Version

4.0.2

PHP Version

8.1.0

Laravel Version

10.43.0

Which operating systems does with happen with?

No response

Notes

No response

omarherri commented 4 months ago

Hi, I apologize for the delayed response.

Actually, to address the issue with the non-integer user_id, I'll be releasing a new version. In this version, you'll be able to add the tag using the new Laravel component syntax, allowing everyone to customize it according to their requirements.

Additionally, it will handle the enabling or disabling of advanced matching.