Commit c80be0f
Fix click_id API validation error (#3668)
Summary:
## Description
After WooC 3.5.10 added ParamBuilder integration for improved fbc/fbp coverage, the plugin started receiving API validation errors from Facebook's Conversions API:
**Error:** `Server Side Api Parameter Error: Unexpected key "click_id" on param "$['data'][0]['user_data']"`
This fix addresses the root causes:
**Problem 1: Broken conditional logic in `get_click_id()`**
The original code had `else if ( ! $fbc )` after checking for cookies. Since `$fbc` was initialized as an empty string `''`, this condition always evaluated to true when the cookie was empty, preventing subsequent checks from executing. The priority order was broken.
**Problem 2: Empty/null values sent to API**
When ParamBuilder or other sources returned null or empty values, these were still being sent to Facebook's API because:
- Empty strings pass `isset()` checks (returns true for `''`)
- The transformation code only conditionally removed the keys
- `array_filter()` only filters the top-level array, not nested `user_data`
**Changes Made:**
1. **Updated FBC/FBP priority order** (in `Event.php`):
- **FBC Priority:** Cookie → ParamBuilder → Request parameter (`fbclid`) → Session
- **FBP Priority:** Cookie → ParamBuilder → Session
- Uses `empty()` checks instead of if-elseif-else to allow fallthrough to each source
2. **Simplified transformation checks** (in `Request.php`):
- Use `! empty()` directly instead of `isset() && ! empty()` (redundant since `empty()` handles undefined keys)
- Only copy to `fbc`/`fbp` when non-empty values exist
3. **Unconditional cleanup:**
- Always unset `click_id` and `browser_id` keys after processing to ensure they're never sent to the API
4. **Return null for empty values:**
- Changed both `get_click_id()` and `get_browser_id()` to return `null` instead of empty string when no value found
This ensures that `click_id` and `browser_id` (internal parameter names) are never sent to Facebook's API, only their transformed equivalents (`fbc` and `fbp`) when valid values exist.
### Type of change
- [x] Fix (non-breaking change which fixes an issue)
## Checklist
- [x] I have commented my code, particularly in hard-to-understand areas, if any.
- [x] I have confirmed that my changes do not introduce any new PHPCS warnings or errors.
- [x] I have checked plugin debug logs that my changes do not introduce any new PHP warnings or FATAL errors.
- [x] I followed general Pull Request best practices. Meta employees to follow this [wiki]([url](https://fburl.com/wiki/2cgfduwc)).
- [ ] I have added tests (if necessary) and all the new and existing unit tests pass locally with my changes.
- [x] I have completed dogfooding and QA testing, or I have conducted thorough due diligence to ensure that it does not break existing functionality.
- [ ] I have updated or requested update to plugin documentations (if necessary). Meta employees to follow this [wiki]([url](https://fburl.com/wiki/nhx73tgs)).
## Changelog entry
Fixed API validation error where click_id parameter was being sent to Facebook Conversions API instead of being properly transformed to fbc.
Pull Request resolved: #3668
Test Plan:
Manually tested the following scenarios to verify the new FBC/FBP priority order:
1. **Cookie exists** - Uses cookie value (highest priority for both FBC and FBP)
2. **No cookie, ParamBuilder returns value** - Uses ParamBuilder value (2nd priority)
3. **No cookie, no ParamBuilder, fbclid in URL** - Creates fbc from query parameter (3rd priority, FBC only)
4. **Session fallback** - Uses session value when all above unavailable (lowest priority)
5. **Empty/null from all sources** - Returns null, does not send empty click_id/browser_id to API
Verified using browser dev tools and API payload inspection that `click_id` and `browser_id` are never present in the final payload sent to Facebook's Conversions API. Only their transformed equivalents (`fbc` and `fbp`) are sent when valid values exist.
Ran `arc lint` to confirm no PHPCS violations introduced.
## Screenshots
N/A - This is a backend API integration fix with no UI changes.
Reviewed By: jarretth
Differential Revision: D84942070
Pulled By: devbodaghe
fbshipit-source-id: c5171f70e46043b1877753e019d470465f9b5f691 parent b950b45 commit c80be0f
4 files changed
Lines changed: 495 additions & 18 deletions
File tree
- includes
- API/Pixel/Events
- Events
- tests/Unit
- Api/Pixel/Events
- Events
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
63 | 63 | | |
64 | 64 | | |
65 | 65 | | |
66 | | - | |
| 66 | + | |
67 | 67 | | |
68 | 68 | | |
69 | | - | |
70 | | - | |
71 | 69 | | |
72 | 70 | | |
73 | | - | |
| 71 | + | |
74 | 72 | | |
75 | 73 | | |
76 | | - | |
77 | | - | |
78 | 74 | | |
79 | 75 | | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
80 | 82 | | |
81 | 83 | | |
82 | 84 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
250 | 250 | | |
251 | 251 | | |
252 | 252 | | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
253 | 260 | | |
254 | 261 | | |
255 | 262 | | |
| |||
261 | 268 | | |
262 | 269 | | |
263 | 270 | | |
264 | | - | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
265 | 278 | | |
266 | 279 | | |
267 | | - | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
268 | 283 | | |
269 | 284 | | |
270 | 285 | | |
271 | | - | |
272 | | - | |
273 | 286 | | |
274 | | - | |
| 287 | + | |
| 288 | + | |
275 | 289 | | |
276 | 290 | | |
277 | | - | |
| 291 | + | |
| 292 | + | |
278 | 293 | | |
279 | 294 | | |
280 | 295 | | |
281 | 296 | | |
282 | 297 | | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
283 | 304 | | |
284 | 305 | | |
285 | 306 | | |
286 | 307 | | |
287 | 308 | | |
288 | 309 | | |
289 | 310 | | |
290 | | - | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
291 | 317 | | |
292 | 318 | | |
293 | 319 | | |
294 | | - | |
295 | | - | |
296 | | - | |
297 | | - | |
| 320 | + | |
| 321 | + | |
298 | 322 | | |
299 | 323 | | |
300 | | - | |
| 324 | + | |
| 325 | + | |
301 | 326 | | |
302 | 327 | | |
303 | 328 | | |
| |||
0 commit comments