-
Notifications
You must be signed in to change notification settings - Fork 293
STRATCONN-6214 - [Google Enhanced Conversions] - Enhancements #3376
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (76.66%) is below the target coverage (80.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #3376 +/- ##
==========================================
- Coverage 80.00% 79.95% -0.06%
==========================================
Files 1211 1223 +12
Lines 22353 22721 +368
Branches 4411 4518 +107
==========================================
+ Hits 17884 18166 +282
- Misses 3689 3720 +31
- Partials 780 835 +55 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds enhancements to the Google Enhanced Conversions destination to improve conversion tracking capabilities. The changes add three new fields to capture additional session and user data.
Key changes:
- Added
user_ip_addressfield to capture the IP address of users who initiated conversions - Added
session_attributes_encodedfield with a default mapping - Added
session_attributes_key_value_pairsfield as an alternative to the encoded field for offline conversions
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
uploadClickConversion2/index.ts |
Added three new input fields and updated request object construction to conditionally include the new fields |
uploadClickConversion/index.ts |
Applied the same field additions and logic as uploadClickConversion2 for consistency |
uploadClickConversion2/generated-types.ts |
Updated TypeScript types to include the new fields in the Payload interface |
uploadClickConversion/generated-types.ts |
Updated TypeScript types to match uploadClickConversion2 |
types.ts |
Added KeyValuePairList and KeyValueItem types and updated ClickConversionRequestObjectInterface |
functions.ts |
Added timestampToEpochMicroseconds and getSessionAttributesKeyValuePairs helper functions |
__tests__/uploadClickConversion2.test.ts |
Updated test snapshots and added new test for sessionAttributesKeyValuePairs |
__tests__/uploadClickConversion.test.ts |
Updated test snapshots and added new test for sessionAttributesKeyValuePairs |
__tests__/functions.test.ts |
Added tests for the new timestampToEpochMicroseconds function |
| } | ||
|
|
||
| export function timestampToEpochMicroseconds(timestamp: string): string | undefined { | ||
| if(!timestamp){ |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing space after 'if' keyword. According to JavaScript/TypeScript style conventions, there should be a space between 'if' and the opening parenthesis. Change if(!timestamp) to if (!timestamp).
| if(!timestamp){ | |
| if (!timestamp){ |
| sessionAttributeKey: | ||
| 'gad_source' | ||
| | 'gad_campaignid' | ||
| | 'landing_page_url' | ||
| | 'session_start_time_usec' | ||
| | 'landing_page_referrer' | ||
| | 'landing_page_user_agent' |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Inconsistent formatting of union type. The first line sessionAttributeKey: has trailing whitespace after the colon. For consistency and cleaner code, the union type definition should start on the same line as the property name or have consistent indentation. Consider reformatting to either sessionAttributeKey: 'gad_source' | 'gad_campaignid' | ... on one line or ensure consistent alignment.
| sessionAttributeKey: | |
| 'gad_source' | |
| | 'gad_campaignid' | |
| | 'landing_page_url' | |
| | 'session_start_time_usec' | |
| | 'landing_page_referrer' | |
| | 'landing_page_user_agent' | |
| sessionAttributeKey: 'gad_source' | 'gad_campaignid' | 'landing_page_url' | 'session_start_time_usec' | 'landing_page_referrer' | 'landing_page_user_agent' |
| } | ||
|
|
||
| const { session_attributes_encoded, user_ip_address } = payload | ||
|
|
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Line 370 contains only trailing whitespace. Remove this empty line or ensure it doesn't contain any whitespace characters for cleaner code.
| const { session_attributes_encoded, user_ip_address } = payload | ||
|
|
||
| const request_object: ClickConversionRequestObjectInterface = { |
Copilot
AI
Nov 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] For consistency with the codebase, consider whether the destructuring of session_attributes_encoded and user_ip_address should occur inline within the spread operators where they're used (lines 369-371), rather than as separate variable declarations. This would avoid creating intermediate variables that are only used once and make the intent clearer that these are conditional includes.
Enhancements to Google Enhanced Conversions.
Testing
Unit tests updated.
Tested in staging - though without credentials
Confirmed payload is correct with team at Google.