digitalwithyou / craft-facebook-conversion

Craft CMS plugin to send web events directly to Facebook
https://digitalwithyou.com/en/plugins/facebook-conversion/getting-started
Other
0 stars 3 forks source link

pixexId through settings doesn't work on some systems #12

Closed jolappi closed 1 year ago

jolappi commented 2 years ago

Problem is that getpixelId function try's to get id as integer and in 32bit systems that integer overflows and id will be wrong. And another problem is that settings returns id as string so it will end in php error. But fix is simple and it is done to Settings.php just change pixelid to be string instead of int public function rules(): array { return [ [['pixelId', 'accessToken'], 'required'], ['pixelId', 'string'], ]; }

public function getPixelId(): ?**string**
{
    return Craft::parseEnv($this->pixelId);
}
Diewy commented 1 year ago

Hi @jolappi ,

Sorry for the late reply, you're totaly right. Even more, the Facebook library expects the Pixel id as string as well. It is fixed in the latest release.

Thank you for reporting!

thijskaspers commented 1 year ago

@Diewy Has this been reverted?

I'm getting the error in Craft 4: dwy\FacebookConversion\models\Settings::getPixelId(): Return value must be of type ?int, string returned

In file: dwy/facebook-conversion/src/models/Settings.php the return type of getPixelId() is set to ?int. We use environment variables to set the Pixel ID, and these are always set as strings.

thijskaspers commented 1 year ago

Ah, the variable is set to type string, PHPdoc too, but the return type of the method still says ?int, which gives a TypeError in newer PHP versions.

Diewy commented 1 year ago

You are absolutely right. Should be fixed now.

thijskaspers commented 1 year ago

Thanks for the quick fix!