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

Fix async events UserData key mapping. #32

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Invader444
Copy link

Summary:
The keys in the normalized data were incorrectly associated to the single value constructor variants; since the UserData normalization encodes arrays, the multiple value constructor variants should be used.

@Invader444 Invader444 mentioned this pull request Mar 10, 2022
Invader444 referenced this pull request in digitalflowsys/Facebook-Pixel-for-Wordpress Mar 13, 2022
@Invader444
Copy link
Author

Invader444 commented Mar 13, 2022

@HeyMultiverse @josejia if possible could someone please review and merge this?

Historically it looks like no one ever gets around to PR reviews in a timely fashion for this repo and having to reapply patches every time a WordPress plugin updates is a headache.

@burtonrhodes
Copy link

+1 for accepting this PR. As WP users upgrade to PHP 8+ this will become an ever increasing issue.

@chuckmo
Copy link

chuckmo commented May 19, 2022

+1 bump etc etc

Please maintain your official plugins

Summary:
The keys in the normalized data were incorrectly associated to the single value constructor variants; since the UserData normalization encodes arrays, the multiple value constructor variants should be used.
@Invader444
Copy link
Author

Bump. Getting to be a little ridiculous now at almost 3 months... @fbisaso, maybe you could take a look at this?

@walid-few
Copy link

This is ridiculous. How can a multi billion company fail in such simple tasks. Like what are you even doing. Everybody is getting error messages for which there seems to be a seemingly easy fix, but you guys do not even care as it seems. Merge that commit already!!!

@burtonrhodes
Copy link

What is most infuriating is that @Invader444 has already done the work for you on this simple fix. Just accept the PR already. Geez.

@Invader444
Copy link
Author

The sad part is, none of these comments get their attention. I also tried to comment on all the related bug reports on the wordpress.org support forum for this plugin with a link to this PR... and still silence from Facebook.

@facebook-github-bot
Copy link
Contributor

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

@JohanRonstrom
Copy link

Bump? Hello? Anything?

@MACscr
Copy link

MACscr commented Oct 12, 2022

The issue is that the array values should be plural? that seems odd.

@danieliser
Copy link

Why isn't this merged & released yet @vinothsa @josejia. Can we get some extra committers or get someone assigned to get these things cleaned up? Lots of errors in logs from this plugin alone.

@pedronastasi
Copy link

pedronastasi commented Dec 2, 2022

Hello @facebook-github-bot we all need a fix

@danieliser
Copy link

Guessing this plugin is no longer active.

@pedronastasi
Copy link

I applied this change locally in the plugin and didn't fix the bug... Are you sure this will fix it?

@pedronastasi
Copy link

still throws same error
tipo E_ERROR en la línea 42 del archivo /var/www/html/wp-content/plugins/official-facebook-pixel/vendor/facebook/php-business-sdk/src/FacebookAds/Object/ServerSide/Normalizer.php. Mensaje de error: Uncaught TypeError: strlen(): Argument #1 ($string) must be of type string, array given in /var/www/html/wp-content/plugins/official-facebook-pixel/vendor/facebook/php-business-sdk/src/FacebookAds/Object/ServerSide/Normalizer.php:42

@Invader444
Copy link
Author

Invader444 commented Dec 3, 2022 via email

@facebook-github-bot
Copy link
Contributor

@Invader444 has updated the pull request. You must reimport the pull request before landing.

@BoyetDgte
Copy link

Wow, until now it wasn't updated :(

@fbisaso
Copy link
Contributor

fbisaso commented Mar 28, 2023

I wanted to apologize for the delay in reviewing and accepting this pull request. We appreciate the time and effort you put into this contribution, and we're sorry it took us so long to get back to you.

That being said, I'm happy to inform you that your changes have been accepted and will soon be merged into the main branch of our plugin.

facebook-github-bot pushed a commit that referenced this pull request Mar 28, 2023
Summary: #32

Reviewed By: vahidkay-meta

Differential Revision: D44457641

fbshipit-source-id: 5008689e616b09d7633ddb40e90b31651cee4a33
facebook-github-bot pushed a commit that referenced this pull request Apr 3, 2023
Summary:
Release V3.0.9
* Removed hard coded OpenBridge Javascript
* Fix async events UserData key mapping. #32
* Delay pixel events firing, to track engaged visitors
* Bug Fixes

Reviewed By: vahidkay-meta

Differential Revision: D44620289

fbshipit-source-id: 25581b15056a8b1d68474650bd7f3e8eb70c1388
@burtonrhodes
Copy link

Since this PR has been implemented into version 3.0.9, you might consider closing this PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet