Repository navigation
Account-level sitelinks and per-image field types on draft_image_assets - #123
Open
illia-sapryga wants to merge 1 commit into
Open
illia-sapryga wants to merge 1 commit into
illia-sapryga wants to merge 1 commit into
Conversation
draft_sitelinks takes scope="account" and links through CustomerAsset. Account level is explicit, following resolve_scope; an empty campaign_id stays an error. draft_image_assets takes field_types, one per image: AD_IMAGE (campaign links, the default) or BUSINESS_LOGO (campaign or account links, as Google's linkage table lists them), so a logo can be created without first linking it as an ad image. scope="account" takes BUSINESS_LOGO only. Both now apply through _create_and_link; _apply_campaign_assets had no other callers and is gone.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
draft_sitelinksanddraft_image_assetscan only create and link to a campaign. To get an account-wide sitelink you have to attach it to some campaign first and then promote it withlink_asset_to_customer. A business logo is stuck the same way, and worse:draft_image_assetsalways links as AD_IMAGE, so the only way to create one is to attach it to a campaign as an ad image first.draft_sitelinksnow takesscope="account". It followsresolve_scope, so account level is explicit and an emptycampaign_idstays an error.draft_image_assetstakes the same scope plusfield_types, one entry per image, in the order uploads, paths, URLs. The values and where they're allowed come from the linkage table in Google's assets overview: AD_IMAGE on a campaign (still the default) and BUSINESS_LOGO on a campaign or the account. Account scope has no default, so it has to be named. Anything else is refused at draft time. The preview lists the resolvedfield_types.Both now apply through
_create_and_link, which takes either one field type or a function of the asset's payload._apply_campaign_assetshad no other callers, so it's gone. Plans stored without a scope or a field type still link to their campaign as AD_IMAGE.What it does not do:
CUSTOMER_ASSET_FIELD_TYPESor allow it at account level. Google's table lists AD_IMAGE for AdGroupAsset and CampaignAsset only. We have used it live on campaign links, not as a CustomerAsset, so I left it out.link_asset_to_customer.draft_image_assetsanddraft_sitelinksno longer havecampaign_idas a required schema parameter, becausescope="account"takes none.campaign_idis also now checked as numeric, which it wasn't before.tests/test_image_fetch.pyasserted the required parameter andtests/test_uploads.pycalled_validate_image_assetswith the old signature, so both changed by a line.Tested with the full suite plus
tests/test_account_level_assets.py. Plans run throughconfirm_and_apply(dry_run=True)with real protos and the real validate-only wrapper, and fake services that only record the request. I have not run this code against a live account, so Google's answer to an account-level BUSINESS_LOGO or sitelink link here comes from its documentation only.