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.

Sign up for GitHub

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

feat: move google ads to new ui #1055

Closed
wants to merge 28 commits into from
Closed

feat: move google ads to new ui #1055

Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
2c8ad03
feat: move google ads to new ui
mihir-4116 Nov 7, 2023
0afc474
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 7, 2023
f15210b
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 8, 2023
a8dcf89
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 10, 2023
5dcdfca
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 13, 2023
c97b7b8
chore: develop pull
mihir-4116 Nov 14, 2023
86971b8
chore: new ui improvements
mihir-4116 Nov 14, 2023
0a042ba
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 14, 2023
c406d5e
chore: reverted deprecated flags
mihir-4116 Nov 21, 2023
37e13e3
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 21, 2023
d812c4c
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 28, 2023
d190f01
fix: improved field notes and added defaultConnectionModes
mihir-4116 Nov 28, 2023
891ab77
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Nov 29, 2023
c90461e
chore: code review changes
mihir-4116 Nov 30, 2023
b940178
chore: code review changes
mihir-4116 Nov 30, 2023
4410827
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Dec 1, 2023
7980f27
chore: pr conflicts resolved
mihir-4116 Dec 4, 2023
05055d6
chore: removed duplicate keys
mihir-4116 Dec 4, 2023
d9c7343
chore: pr conflicts resolved
mihir-4116 Dec 7, 2023
218776f
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Dec 7, 2023
e666b1a
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Dec 9, 2023
2991a11
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Dec 11, 2023
8cd139c
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Dec 12, 2023
0aadf6e
chore: pr conflicts resolved
mihir-4116 Dec 20, 2023
918b1e0
chore: pr conflicts resolved
mihir-4116 Dec 27, 2023
de03995
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Jan 2, 2024
64d9783
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Jan 4, 2024
9a4638a
Merge branch 'develop' into feat.google_ads_new_ui
mihir-4116 Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions src/configurations/destinations/googleads/db-config.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,10 @@
"enableConversionLabel"
],
"excludeKeys": [],
"supportedSourceTypes": ["web"],
"supportedConnectionModes": {
"web": ["device"]
},
"supportedSourceTypes": ["web"],
"supportedMessageTypes": {
"device": {
"web": ["identify", "track", "page"]
Expand All @@ -42,12 +42,10 @@
"defaultConfig": [
"conversionID",
"eventMappingFromConfig",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there no functional impact by removal of pageLoadConversions & dynamicRemarketing ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, these fields are not used anywhere in SDK. it's redundant to keep it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you check if any destination is using by querying DB?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added this flags back as older SDK versions might be using these flags

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are present in destination config, they continue to get sent via sourceConfig API & based on SDK, they get used. But they can't be updated. Right ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can remove it but for that we need migration script.

"pageLoadConversions",
"clickEventConversions",
"defaultPageConversion",
"sendPageView",
"conversionLinker",
"dynamicRemarketing",
"trackConversions",
"enableConversionEventsFiltering",
"eventsToTrackConversions",
Expand All @@ -62,7 +60,7 @@
"oneTrustCookieCategories",
"enableConversionLabel"
],
"web": ["useNativeSDK", "dynamicRemarketing"]
"web": ["useNativeSDK", "connectionMode"]
},
"secretKeys": []
}
Expand Down
109 changes: 76 additions & 33 deletions src/configurations/destinations/googleads/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,38 @@
"Checkout",
"Search",
"AddToCart",
"purchase",
"Purchase",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the case-sensitivity handled in the backend ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, as google ads expect it as Purchase instead of purchase

""
]
}
}
}
},
"trackConversions": { "type": "boolean", "default": true },
"trackDynamicRemarketing": { "type": "boolean", "default": false },
"useNativeSDK": { "type": "object", "properties": { "web": { "type": "boolean" } } },
"trackConversions": {
"type": "boolean",
"default": true
},
"trackDynamicRemarketing": {
"type": "boolean",
"default": false
},
"useNativeSDK": {
"type": "object",
"properties": {
"web": {
"type": "boolean"
}
}
},
"connectionMode": {
"type": "object",
"properties": {
"web": {
"type": "string",
"enum": ["device"]
}
}
},
"eventFilteringOption": {
"type": "string",
"enum": ["disable", "whitelistedEvents", "blacklistedEvents"],
Expand Down Expand Up @@ -67,22 +89,6 @@
}
}
},
"pageLoadConversions": {
"type": "array",
"items": {
"type": "object",
"properties": {
"conversionLabel": {
"type": "string",
"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"
},
"name": {
"type": "string",
"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"
}
}
}
},
"defaultPageConversion": {
"type": "string",
"pattern": "(^\\{\\{.*\\|\\|(.*)\\}\\}$)|(^env[.].+)|^(.{0,100})$"
Expand All @@ -103,11 +109,26 @@
}
}
},
"sendPageView": { "type": "boolean", "default": true },
"conversionLinker": { "type": "boolean", "default": true },
"disableAdPersonalization": { "type": "boolean", "default": false },
"enableConversionLabel": { "type": "boolean", "default": false },
"allowEnhancedConversions": { "type": "boolean", "default": false },
"sendPageView": {
"type": "boolean",
"default": true
},
"conversionLinker": {
"type": "boolean",
"default": true
},
"disableAdPersonalization": {
"type": "boolean",
"default": false
},
"enableConversionLabel": {
"type": "boolean",
"default": false
},
"allowEnhancedConversions": {
"type": "boolean",
"default": false
},
"oneTrustCookieCategories": {
"type": "array",
"items": {
Expand All @@ -124,22 +145,33 @@
"allOf": [
{
"if": {
"properties": { "trackConversions": { "const": true } },
"properties": {
"trackConversions": {
"const": true
}
},
"required": ["trackConversions"]
},
"then": {
"properties": {
"enableConversionEventsFiltering": { "type": "boolean", "default": false }
"enableConversionEventsFiltering": {
"type": "boolean",
"default": false
}
},
"required": []
}
},
{
"if": {
"properties": {
"trackConversions": { "const": true },
"enableConversionEventsFiltering": { "const": true }
"trackConversions": {
"const": true
},
"enableConversionEventsFiltering": {
"const": true
}
},
"required": ["trackConversions", "enableConversionEventsFiltering"]
},
"then": {
Expand All @@ -162,22 +194,33 @@
},
{
"if": {
"properties": { "trackDynamicRemarketing": { "const": true } },
"properties": {
"trackDynamicRemarketing": {
"const": true
}
},
"required": ["trackDynamicRemarketing"]
},
"then": {
"properties": {
"enableDynamicRemarketingEventsFiltering": { "type": "boolean", "default": false }
"enableDynamicRemarketingEventsFiltering": {
"type": "boolean",
"default": false
}
},
"required": []
}
},
{
"if": {
"properties": {
"trackDynamicRemarketing": { "const": true },
"enableDynamicRemarketingEventsFiltering": { "const": true }
"trackDynamicRemarketing": {
"const": true
},
"enableDynamicRemarketingEventsFiltering": {
"const": true
}
},
"required": ["trackDynamicRemarketing", "enableDynamicRemarketingEventsFiltering"]
},
"then": {
Expand Down
Loading