diff --git a/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png b/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png index f3ad16bac6088..140ffbe8d5e8e 100644 Binary files a/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png and b/frontend/__snapshots__/replay-player-success--recent-recordings--dark.png differ diff --git a/frontend/__snapshots__/replay-player-success--recent-recordings--light.png b/frontend/__snapshots__/replay-player-success--recent-recordings--light.png index fb9c9f692cb0f..0dce5640342cc 100644 Binary files a/frontend/__snapshots__/replay-player-success--recent-recordings--light.png and b/frontend/__snapshots__/replay-player-success--recent-recordings--light.png differ diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index 30dc33773c06c..0b143ad2684cf 100644 --- a/frontend/src/lib/api.ts +++ b/frontend/src/lib/api.ts @@ -847,13 +847,13 @@ class ApiRequest { public integrationGoogleAdsConversionActions( id: IntegrationType['id'], - customerId: string, + params: { customerId: string; parentId: string }, teamId?: TeamType['id'] ): ApiRequest { return this.integrations(teamId) .addPathComponent(id) .addPathComponent('google_conversion_actions') - .withQueryString({ customerId }) + .withQueryString(params) } public integrationLinkedInAdsAccounts(id: IntegrationType['id'], teamId?: TeamType['id']): ApiRequest { @@ -2701,14 +2701,14 @@ const api = { }, async googleAdsAccounts( id: IntegrationType['id'] - ): Promise<{ accessibleAccounts: { id: string; name: string }[] }> { + ): Promise<{ accessibleAccounts: { id: string; name: string; level: string; parent_id: string }[] }> { return await new ApiRequest().integrationGoogleAdsAccounts(id).get() }, async googleAdsConversionActions( id: IntegrationType['id'], - customerId: string + params: { customerId: string; parentId: string } ): Promise<{ conversionActions: GoogleAdsConversionActionType[] }> { - return await new ApiRequest().integrationGoogleAdsConversionActions(id, customerId).get() + return await new ApiRequest().integrationGoogleAdsConversionActions(id, params).get() }, async linkedInAdsAccounts(id: IntegrationType['id']): Promise<{ adAccounts: LinkedInAdsAccountType[] }> { return await new ApiRequest().integrationLinkedInAdsAccounts(id).get() diff --git a/frontend/src/lib/integrations/GoogleAdsIntegrationHelpers.tsx b/frontend/src/lib/integrations/GoogleAdsIntegrationHelpers.tsx index 411670d6cd1df..3a51fe9ac5186 100644 --- a/frontend/src/lib/integrations/GoogleAdsIntegrationHelpers.tsx +++ b/frontend/src/lib/integrations/GoogleAdsIntegrationHelpers.tsx @@ -7,11 +7,11 @@ import { GoogleAdsConversionActionType, IntegrationType } from '~/types' import { googleAdsIntegrationLogic } from './googleAdsIntegrationLogic' const getGoogleAdsAccountOptions = ( - googleAdsAccounts?: { id: string; name: string }[] | null + googleAdsAccounts?: { id: string; name: string; parent_id: string; level: string }[] | null ): LemonInputSelectOption[] | null => { return googleAdsAccounts ? googleAdsAccounts.map((customer) => ({ - key: customer.id, + key: `${customer.id}/${customer.parent_id}`, labelComponent: ( {customer.name} ({customer.id.replace(/(\d{3})(\d{3})(\d{4})/, '$1-$2-$3')}) @@ -65,7 +65,7 @@ export function GoogleAdsConversionActionPicker({ useEffect(() => { if (requiresFieldValue) { - loadGoogleAdsConversionActions(requiresFieldValue) + loadGoogleAdsConversionActions(requiresFieldValue.split('/')[0], requiresFieldValue.split('/')[1]) } }, [loadGoogleAdsConversionActions, requiresFieldValue]) @@ -78,7 +78,7 @@ export function GoogleAdsConversionActionPicker({ !googleAdsConversionActions && !googleAdsConversionActionsLoading && requiresFieldValue && - loadGoogleAdsConversionActions(requiresFieldValue) + loadGoogleAdsConversionActions(requiresFieldValue.split('/')[0], requiresFieldValue.split('/')[1]) } disabled={disabled} mode="single" diff --git a/frontend/src/lib/integrations/googleAdsIntegrationLogic.ts b/frontend/src/lib/integrations/googleAdsIntegrationLogic.ts index 88dd3f9d4636b..32c2d329cbcda 100644 --- a/frontend/src/lib/integrations/googleAdsIntegrationLogic.ts +++ b/frontend/src/lib/integrations/googleAdsIntegrationLogic.ts @@ -11,21 +11,30 @@ export const googleAdsIntegrationLogic = kea([ key((props) => props.id), path((key) => ['lib', 'integrations', 'googleAdsIntegrationLogic', key]), actions({ - loadGoogleAdsConversionActions: (customerId: string) => customerId, + loadGoogleAdsConversionActions: (customerId: string, parentId: string) => ({ customerId, parentId }), loadGoogleAdsAccessibleAccounts: true, }), loaders(({ props }) => ({ googleAdsConversionActions: [ null as GoogleAdsConversionActionType[] | null, { - loadGoogleAdsConversionActions: async (customerId: string) => { - const res = await api.integrations.googleAdsConversionActions(props.id, customerId) + loadGoogleAdsConversionActions: async ({ + customerId, + parentId, + }: { + customerId: string + parentId: string + }) => { + const res = await api.integrations.googleAdsConversionActions(props.id, { + customerId, + parentId, + }) return res.conversionActions }, }, ], googleAdsAccessibleAccounts: [ - null as { id: string; name: string }[] | null, + null as { id: string; level: string; parent_id: string; name: string }[] | null, { loadGoogleAdsAccessibleAccounts: async () => { const res = await api.integrations.googleAdsAccounts(props.id) diff --git a/posthog/api/integration.py b/posthog/api/integration.py index d350eac5b0556..8eabf9ca2f5c7 100644 --- a/posthog/api/integration.py +++ b/posthog/api/integration.py @@ -114,8 +114,9 @@ def conversion_actions(self, request: Request, *args: Any, **kwargs: Any) -> Res instance = self.get_object() google_ads = GoogleAdsIntegration(instance) customer_id = request.query_params.get("customerId") + parent_id = request.query_params.get("parentId") - conversion_actions = google_ads.list_google_ads_conversion_actions(customer_id) + conversion_actions = google_ads.list_google_ads_conversion_actions(customer_id, parent_id) if len(conversion_actions) == 0: return Response({"conversionActions": []}) @@ -126,7 +127,7 @@ def conversion_actions(self, request: Request, *args: Any, **kwargs: Any) -> Res "name": conversionAction["conversionAction"]["name"], "resourceName": conversionAction["conversionAction"]["resourceName"], } - for conversionAction in google_ads.list_google_ads_conversion_actions(customer_id)[0]["results"] + for conversionAction in google_ads.list_google_ads_conversion_actions(customer_id, parent_id)[0]["results"] ] return Response({"conversionActions": conversion_actions}) diff --git a/posthog/cdp/templates/google_ads/template_google_ads.py b/posthog/cdp/templates/google_ads/template_google_ads.py index 5003e93158d8a..763d40ccdae89 100644 --- a/posthog/cdp/templates/google_ads/template_google_ads.py +++ b/posthog/cdp/templates/google_ads/template_google_ads.py @@ -21,7 +21,7 @@ 'conversions': [ { 'gclid': inputs.gclid, - 'conversion_action': f'customers/{replaceAll(inputs.customerId, '-', '')}/conversionActions/{inputs.conversionActionId}', + 'conversion_action': f'customers/{splitByString('/', inputs.customerId)[1]}/conversionActions/{inputs.conversionActionId}', 'conversion_date_time': inputs.conversionDateTime } ], @@ -38,11 +38,12 @@ body.conversions[1].order_id := inputs.orderId } -let res := fetch(f'https://googleads.googleapis.com/v18/customers/{replaceAll(inputs.customerId, '-', '')}:uploadClickConversions', { +let res := fetch(f'https://googleads.googleapis.com/v18/customers/{splitByString('/', inputs.customerId)[1]}:uploadClickConversions', { 'method': 'POST', 'headers': { 'Authorization': f'Bearer {inputs.oauth.access_token}', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + 'login-customer-id': splitByString('/', inputs.customerId)[2] }, 'body': body }) diff --git a/posthog/cdp/templates/google_ads/test_template_google_ads.py b/posthog/cdp/templates/google_ads/test_template_google_ads.py index b3923d3cebbe7..e38c0708e9b50 100644 --- a/posthog/cdp/templates/google_ads/test_template_google_ads.py +++ b/posthog/cdp/templates/google_ads/test_template_google_ads.py @@ -13,7 +13,7 @@ def _inputs(self, **kwargs): "oauth": { "access_token": "oauth-1234", }, - "customerId": "123-123-1234", + "customerId": "1231231234/5675675678", "conversionActionId": "123456789", "gclid": "89y4thuergnjkd34oihroh3uhg39uwhgt9", "conversionDateTime": "2024-10-10 16:32:45+02:00", @@ -32,6 +32,7 @@ def test_function_works(self): "headers": { "Authorization": "Bearer oauth-1234", "Content-Type": "application/json", + "login-customer-id": "5675675678", }, "body": { "conversions": [ diff --git a/posthog/models/integration.py b/posthog/models/integration.py index c4b95991c5681..33e35aa2c7e2b 100644 --- a/posthog/models/integration.py +++ b/posthog/models/integration.py @@ -481,7 +481,7 @@ def __init__(self, integration: Integration) -> None: def client(self) -> WebClient: return WebClient(self.integration.sensitive_config["access_token"]) - def list_google_ads_conversion_actions(self, customer_id) -> list[dict]: + def list_google_ads_conversion_actions(self, customer_id, parent_id=None) -> list[dict]: response = requests.request( "POST", f"https://googleads.googleapis.com/v18/customers/{customer_id}/googleAds:searchStream", @@ -490,6 +490,7 @@ def list_google_ads_conversion_actions(self, customer_id) -> list[dict]: "Content-Type": "application/json", "Authorization": f"Bearer {self.integration.sensitive_config['access_token']}", "developer-token": settings.GOOGLE_ADS_DEVELOPER_TOKEN, + **({"login-customer-id": parent_id} if parent_id else {}), }, ) @@ -501,6 +502,8 @@ def list_google_ads_conversion_actions(self, customer_id) -> list[dict]: return response.json() + # Google Ads manager accounts can have access to other accounts (including other manager accounts). + # Filter out duplicates where a user has direct access and access through a manager account, while prioritizing direct access. def list_google_ads_accessible_accounts(self) -> list[dict[str, str]]: response = requests.request( "GET", @@ -516,35 +519,65 @@ def list_google_ads_accessible_accounts(self) -> list[dict[str, str]]: capture_exception(Exception(f"GoogleAdsIntegration: Failed to list accessible accounts: {response.text}")) raise Exception(f"There was an internal error") - accounts = response.json() - accounts_with_name = [] + accessible_accounts = response.json() + all_accounts: list[dict[str, str]] = [] - for account in accounts["resourceNames"]: + def dfs(account_id, accounts=None, parent_id=None) -> list[dict]: + if accounts is None: + accounts = [] response = requests.request( "POST", - f"https://googleads.googleapis.com/v18/customers/{account.split('/')[1]}/googleAds:searchStream", + f"https://googleads.googleapis.com/v18/customers/{account_id}/googleAds:searchStream", json={ - "query": "SELECT customer_client.descriptive_name, customer_client.client_customer FROM customer_client WHERE customer_client.level <= 1" + "query": "SELECT customer_client.descriptive_name, customer_client.client_customer, customer_client.level, customer_client.manager, customer_client.status FROM customer_client WHERE customer_client.level <= 5" }, headers={ "Content-Type": "application/json", "Authorization": f"Bearer {self.integration.sensitive_config['access_token']}", "developer-token": settings.GOOGLE_ADS_DEVELOPER_TOKEN, + **({"login-customer-id": parent_id} if parent_id else {}), }, ) if response.status_code != 200: - continue + return [] data = response.json() - accounts_with_name.append( - { - "id": account.split("/")[1], - "name": data[0]["results"][0]["customerClient"].get("descriptiveName", "Google Ads account"), - } - ) - return accounts_with_name + for nested_account in data[0]["results"]: + if any( + account["id"] == nested_account["customerClient"]["clientCustomer"].split("/")[1] + and account["level"] > nested_account["customerClient"]["level"] + for account in accounts + ): + accounts = [ + account + for account in accounts + if account["id"] != nested_account["customerClient"]["clientCustomer"].split("/")[1] + ] + elif any( + account["id"] == nested_account["customerClient"]["clientCustomer"].split("/")[1] + and account["level"] < nested_account["customerClient"]["level"] + for account in accounts + ): + continue + if nested_account["customerClient"].get("status") != "ENABLED": + continue + accounts.append( + { + "parent_id": parent_id, + "id": nested_account["customerClient"].get("clientCustomer").split("/")[1], + "level": nested_account["customerClient"].get("level"), + "name": nested_account["customerClient"].get("descriptiveName", "Google Ads account"), + } + ) + + return accounts + + for account in accessible_accounts["resourceNames"]: + all_accounts = dfs(account.split("/")[1], all_accounts, account.split("/")[1]) + + return all_accounts class GoogleCloudIntegration: