From 6c756fc44926a4384edbd557576ec536e1214b90 Mon Sep 17 00:00:00 2001 From: Codex Date: Sat, 20 Jun 2026 13:42:06 +0000 Subject: [PATCH] refactor: migrate Group-4 get commands to make_get_command (#588) Move the five migratable "complex" get commands onto the shared make_get_command factory, reusing only its existing hooks (no new factory parameter): - adgroups get, keywords get: criteria-limit + 8/2 nested *FieldNames; the --status/--statuses mutual-exclusion guard lives in the per-module criteria_builder. - creatives get: require-filter + 4 nested. - strategies get: require-filter + 16 nested (the largest set so far). - audiencetargets get: criteria-limit + long require-filter message, no nested. Factory ordering fix: nested *FieldNames are now parsed (and their provided-but-empty-CSV UsageError raised) before both criteria-limit enforcement and the empty-criteria require guard, matching the order every hand-rolled command used. A "-- ''" combined with no filter or an over-limit array reports the nested error, not the require/limit one. The parsed dict is still merged after the common params, so payload key order is unchanged; this also restores the pre-factory nested-before-limits order for the migrated bidmodifiers get. A new test_dry_run regression test pins the precedence. --help, --dry-run payloads and the error-precedence edge cases are byte-identical, verified against 33 pre-migration baselines captured via CliRunner from the pre-migration tree. As with every prior factory migration, --dry-run is now a token-free test seam for these commands. ads get and campaigns get stay hand-rolled as documented carve-outs (see _get.py): ads always emits TextAdFieldNames with a per-field default (like keywordbids); campaigns' --fields rejects an explicitly empty CSV rather than falling back to the default FieldNames. Net -278 lines. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 33 +++ direct_cli/commands/_get.py | 33 ++- direct_cli/commands/adgroups.py | 296 ++++++++------------- direct_cli/commands/audiencetargets.py | 118 +++------ direct_cli/commands/creatives.py | 155 ++++------- direct_cli/commands/keywords.py | 173 ++++--------- direct_cli/commands/strategies.py | 345 ++++++++----------------- tests/test_dry_run.py | 29 +++ 8 files changed, 452 insertions(+), 730 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f119954..c3b4990 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,39 @@ ## Unreleased +**Internal — `make_get_command` covers the complex Group-4 `get`s (#588):** + +- `adgroups get`, `keywords get`, `creatives get`, `strategies get` and + `audiencetargets get` now register through the shared `make_get_command` + factory (net −345 lines). They reuse the existing `extra_options` / + `criteria_builder` / `criteria_limits` / `require_criteria_message` / + `nested_field_options` hooks — the `--status`/`--statuses` mutual-exclusion + guard (adgroups/keywords) lives in the per-module `criteria_builder`, not a + new factory flag. adgroups (8) and strategies (16) exercise the factory's + largest nested-`*FieldNames` sets to date. +- Factory ordering fix: nested `*FieldNames` are now parsed (and their + provided-but-empty-CSV `UsageError` raised) *before* both the criteria-limit + enforcement and the empty-criteria `require_criteria_message` guard, matching + the order every hand-rolled command used. So a `-- ""` combined with + either no filter at all or an over-limit array reports the nested error, not + the require/limit one (pinned by a new `test_dry_run` regression test). The + parsed dict is still merged after the common params, so payload key order is + unchanged, and this also restores the pre-factory nested-before-limits order + for the already-migrated `bidmodifiers get`. No module lacking both a nested + option and a `criteria_limits`/`require_criteria_message` is affected. +- No CLI surface change: `--help` (option order, all nested-field options, the + `--is-archived` choice/default), `--dry-run` payloads, and the error-precedence + edge cases (empty-nested vs. no-filter and vs. over-limit) are byte-identical, + verified against 33 pre-migration baselines captured via CliRunner from the + pre-migration tree. As with every prior `make_get_command` migration, the + factory resolves the API client only on the live path, so these five `get`s + now honor `--dry-run` as a token-free test seam (no behavior change vs. the + other factory-backed commands). +- Two Group-4 `get`s stay hand-rolled as documented carve-outs (see `_get.py`): + `ads get` (its `TextAdFieldNames` is always emitted with a per-field default, + like the carved-out `keywordbids`) and `campaigns get` (its `--fields` rejects + an explicitly empty CSV instead of falling back to the default `FieldNames`). + **Internal — `make_get_command` covers the criteria-limit `get`s `bids` / `bidmodifiers` (completes #587):** - `bids get` (criteria-limit + require-at-least-one-filter, no `--ids`) and diff --git a/direct_cli/commands/_get.py b/direct_cli/commands/_get.py index afd79a6..2061000 100644 --- a/direct_cli/commands/_get.py +++ b/direct_cli/commands/_get.py @@ -14,15 +14,19 @@ and the live path runs the module's real ``create_client`` (the VCR cassette read-tests replay it at the ``requests`` transport, so they are unaffected). -Three ``get`` commands deliberately stay hand-rolled (issue #587), each for a -structural reason this factory does not encode: +Five ``get`` commands deliberately stay hand-rolled (issues #587, #588), each +for a structural reason this factory does not encode: * ``leads get`` — ``--datetime-from`` / ``--datetime-to`` sit between ``--limit`` and ``--fetch-all``, a bespoke option order the shared ``get_options`` stack cannot reproduce (it always renders ``--limit`` / ``--fetch-all`` adjacent). -* ``keywordbids get`` — its nested ``SearchFieldNames`` / ``NetworkFieldNames`` - carry per-field defaults and are *always* emitted, unlike the provided-only - nested projections ``nested_field_options`` builds. +* ``keywordbids get`` and ``ads get`` — they emit a nested ``*FieldNames`` + (``Search``/``NetworkFieldNames``; ``TextAdFieldNames``) with a per-field + default that is *always* present, unlike the provided-only nested projections + ``nested_field_options`` builds. +* ``campaigns get`` — its ``--fields`` rejects an explicitly empty CSV + (``_parse_csv_option``) instead of the factory's silent fall-back to the + default ``FieldNames``. * ``reports get`` — a custom non-RPC TSV stream, not a JSON-RPC ``get``. """ @@ -184,6 +188,18 @@ def get( default_fields_key ) criteria = build_criteria(ids, **kwargs) + # Validate provided-but-empty nested *FieldNames CSVs (parse_nested_field_names + # rejects them) before the criteria-limit enforcement and the empty-criteria + # guard, matching the hand-rolled order of every migrated command: with both + # an over-limit array and an empty "-- ''" (or a "-- ''" and + # no filter at all), the nested error is the one reported. The parsed dict is + # merged after the common params below, so the payload key order is unchanged. + parsed_nested = {} + if nested_specs: + raw_nested = tuple( + (wsdl_key, kwargs[kwarg]) for wsdl_key, kwarg in nested_specs + ) + parsed_nested = parse_nested_field_names(raw_nested) if criteria_limits: enforce_criteria_array_limits( criteria, criteria_limits, command_name=f"{svc} get" @@ -199,11 +215,8 @@ def get( params = build_common_params( criteria=criteria, field_names=field_names, limit=limit ) - if nested_specs: - raw_nested = tuple( - (wsdl_key, kwargs[kwarg]) for wsdl_key, kwarg in nested_specs - ) - params.update(parse_nested_field_names(raw_nested)) + if parsed_nested: + params.update(parsed_nested) if adextensions_wire_layout and limit: # Page after the nested *FieldNames, matching adextensions' layout. params["Page"] = {"Limit": limit} diff --git a/direct_cli/commands/adgroups.py b/direct_cli/commands/adgroups.py index 6e12e7d..e242bc8 100644 --- a/direct_cli/commands/adgroups.py +++ b/direct_cli/commands/adgroups.py @@ -10,15 +10,12 @@ from ..i18n import t from ..output import format_output, handle_api_errors from . import _batch +from ._get import make_get_command from ._lifecycle import make_lifecycle_command from ..utils import ( add_criteria_csv, - build_common_params, - enforce_criteria_array_limits, - get_default_fields, parse_csv_strings, parse_ids, - parse_nested_field_names, ) from .._autotargeting import ( @@ -410,162 +407,26 @@ def _post_adgroups(client: Any, body: dict[str, Any]) -> Any: return client.adgroups().post(data=body) -@adgroups.command() -@click.option("--ids", help="Comma-separated ad group IDs") -@click.option("--campaign-ids", help="Comma-separated campaign IDs") -@click.option("--status", help="Filter by status") -@click.option("--statuses", help="Comma-separated statuses") -@click.option("--types", help="Filter by types") -@click.option("--tag-ids", help="Comma-separated tag IDs") -@click.option("--tags", help="Comma-separated tag names") -@click.option("--app-icon-statuses", help="Comma-separated app icon statuses") -@click.option("--serving-statuses", help="Comma-separated serving statuses") -@click.option( - "--negative-keyword-shared-set-ids", - help="Comma-separated negative keyword shared set IDs", -) -@click.option("--limit", type=int, help="Limit number of results") -@click.option("--fetch-all", is_flag=True, help="Fetch all pages") -@click.option("--format", "output_format", default="json", help="Output format") -@click.option("--output", help="Output file") -@click.option("--fields", help="Comma-separated field names") -@click.option( - "--autotargeting-settings-brand-options-field-names", - help=( - "Comma-separated AutotargetingSettingsBrandOptionsFieldNames " - "(e.g. WithoutBrands,WithAdvertiserBrand,WithCompetitorsBrand). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--autotargeting-settings-categories-field-names", - help=( - "Comma-separated AutotargetingSettingsCategoriesFieldNames " - "(e.g. Exact,Narrow,Alternative,Accessory,Broader). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--dynamic-text-ad-group-field-names", - help=( - "Comma-separated DynamicTextAdGroupFieldNames " - "(e.g. AutotargetingSettings,DomainUrl). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--dynamic-text-feed-ad-group-field-names", - help=( - "Comma-separated DynamicTextFeedAdGroupFieldNames " - "(e.g. Source,FeedId,SourceType). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--mobile-app-ad-group-field-names", - help=( - "Comma-separated MobileAppAdGroupFieldNames " - "(e.g. StoreUrl,TargetDeviceType,AppOperatingSystemType). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--smart-ad-group-field-names", - help=( - "Comma-separated SmartAdGroupFieldNames " - "(e.g. FeedId,AdTitleSource,AdBodySource). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--text-ad-group-feed-params-field-names", - help=( - "Comma-separated TextAdGroupFeedParamsFieldNames " - "(e.g. FeedId,FeedCategoryIds). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option( - "--unified-ad-group-field-names", - help=( - "Comma-separated UnifiedAdGroupFieldNames (e.g. OfferRetargeting). " - "Sent as separate top-level request parameter per the " - "AdGroupsGetRequest WSDL." - ), -) -@click.option("--dry-run", is_flag=True, help="Show request without sending") -@click.pass_context -@handle_api_errors -def get( - ctx, - ids, - campaign_ids, - status, - statuses, - types, - tag_ids, - tags, - app_icon_statuses, - serving_statuses, - negative_keyword_shared_set_ids, - limit, - fetch_all, - output_format, - output, - fields, - autotargeting_settings_brand_options_field_names, - autotargeting_settings_categories_field_names, - dynamic_text_ad_group_field_names, - dynamic_text_feed_ad_group_field_names, - mobile_app_ad_group_field_names, - smart_ad_group_field_names, - text_ad_group_feed_params_field_names, - unified_ad_group_field_names, - dry_run, +def _adgroups_get_criteria( + ids=None, + campaign_ids=None, + status=None, + statuses=None, + types=None, + tag_ids=None, + tags=None, + app_icon_statuses=None, + serving_statuses=None, + negative_keyword_shared_set_ids=None, + **_, ): - """Get ad groups""" + """SelectionCriteria for ``adgroups get``: optional Ids/CampaignIds, a + singular ``--status`` or upper-cased ``--statuses`` (mutually exclusive), + upper-cased Types/AppIconStatuses/ServingStatuses, plain Tags, and integer + TagIds/NegativeKeywordSharedSetIds.""" if status and statuses: raise click.UsageError(t("--status and --statuses are mutually exclusive")) - - client = client_from_ctx(ctx, create_client) - - field_names = parse_csv_strings(fields) or get_default_fields("adgroups") - - raw_nested = ( - ( - "AutotargetingSettingsBrandOptionsFieldNames", - autotargeting_settings_brand_options_field_names, - ), - ( - "AutotargetingSettingsCategoriesFieldNames", - autotargeting_settings_categories_field_names, - ), - ( - "DynamicTextAdGroupFieldNames", - dynamic_text_ad_group_field_names, - ), - ( - "DynamicTextFeedAdGroupFieldNames", - dynamic_text_feed_ad_group_field_names, - ), - ("MobileAppAdGroupFieldNames", mobile_app_ad_group_field_names), - ("SmartAdGroupFieldNames", smart_ad_group_field_names), - ( - "TextAdGroupFeedParamsFieldNames", - text_ad_group_feed_params_field_names, - ), - ("UnifiedAdGroupFieldNames", unified_ad_group_field_names), - ) - parsed_nested = parse_nested_field_names(raw_nested) - - criteria: dict[str, Any] = {} + criteria = {} if ids: criteria["Ids"] = parse_ids(ids) if campaign_ids: @@ -584,35 +445,98 @@ def get( negative_keyword_shared_set_ids, integers=True, ) - - enforce_criteria_array_limits( - criteria, ADGROUPS_GET_CRITERIA_LIMITS, command_name="adgroups get" - ) - - if not criteria: - raise click.UsageError(t("Provide at least one typed filter")) - - params = build_common_params( - criteria=criteria, field_names=field_names, limit=limit - ) - params.update(parsed_nested) - - body = {"method": "get", "params": params} - - if dry_run: - format_output(body, "json", None) - return - - result = client.adgroups().post(data=body) - - if fetch_all: - items = [] - for item in result().iter_items(): - items.append(item) - format_output(items, output_format, output) - else: - data = result().extract() - format_output(data, output_format, output) + return criteria + + +get = make_get_command( + adgroups, + create_client, + default_fields_key="adgroups", + help_text="Get ad groups", + ids_help="Comma-separated ad group IDs", + extra_options=( + click.option("--campaign-ids", help="Comma-separated campaign IDs"), + click.option("--status", help="Filter by status"), + click.option("--statuses", help="Comma-separated statuses"), + click.option("--types", help="Filter by types"), + click.option("--tag-ids", help="Comma-separated tag IDs"), + click.option("--tags", help="Comma-separated tag names"), + click.option("--app-icon-statuses", help="Comma-separated app icon statuses"), + click.option("--serving-statuses", help="Comma-separated serving statuses"), + click.option( + "--negative-keyword-shared-set-ids", + help="Comma-separated negative keyword shared set IDs", + ), + ), + criteria_builder=_adgroups_get_criteria, + criteria_limits=ADGROUPS_GET_CRITERIA_LIMITS, + require_criteria_message="Provide at least one typed filter", + nested_field_options=( + ( + "--autotargeting-settings-brand-options-field-names", + "AutotargetingSettingsBrandOptionsFieldNames", + "Comma-separated AutotargetingSettingsBrandOptionsFieldNames " + "(e.g. WithoutBrands,WithAdvertiserBrand,WithCompetitorsBrand). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--autotargeting-settings-categories-field-names", + "AutotargetingSettingsCategoriesFieldNames", + "Comma-separated AutotargetingSettingsCategoriesFieldNames " + "(e.g. Exact,Narrow,Alternative,Accessory,Broader). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--dynamic-text-ad-group-field-names", + "DynamicTextAdGroupFieldNames", + "Comma-separated DynamicTextAdGroupFieldNames " + "(e.g. AutotargetingSettings,DomainUrl). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--dynamic-text-feed-ad-group-field-names", + "DynamicTextFeedAdGroupFieldNames", + "Comma-separated DynamicTextFeedAdGroupFieldNames " + "(e.g. Source,FeedId,SourceType). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--mobile-app-ad-group-field-names", + "MobileAppAdGroupFieldNames", + "Comma-separated MobileAppAdGroupFieldNames " + "(e.g. StoreUrl,TargetDeviceType,AppOperatingSystemType). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--smart-ad-group-field-names", + "SmartAdGroupFieldNames", + "Comma-separated SmartAdGroupFieldNames " + "(e.g. FeedId,AdTitleSource,AdBodySource). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--text-ad-group-feed-params-field-names", + "TextAdGroupFeedParamsFieldNames", + "Comma-separated TextAdGroupFeedParamsFieldNames " + "(e.g. FeedId,FeedCategoryIds). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ( + "--unified-ad-group-field-names", + "UnifiedAdGroupFieldNames", + "Comma-separated UnifiedAdGroupFieldNames (e.g. OfferRetargeting). " + "Sent as separate top-level request parameter per the " + "AdGroupsGetRequest WSDL.", + ), + ), +) # dest -> "--flag" map for the `adgroups add` flag set. Hoisted to module level diff --git a/direct_cli/commands/audiencetargets.py b/direct_cli/commands/audiencetargets.py index fd781b2..ade379c 100644 --- a/direct_cli/commands/audiencetargets.py +++ b/direct_cli/commands/audiencetargets.py @@ -2,23 +2,17 @@ AudienceTargets commands """ -from typing import Any - import click -from ..api import client_from_ctx, create_client +from ..api import create_client from ..i18n import t -from ..output import format_output, handle_api_errors +from ..output import handle_api_errors from ._execute import execute_request +from ._get import make_get_command from ._lifecycle import register_lifecycle_commands from ..utils import ( MICRO_RUBLES, add_criteria_csv, - build_common_params, - enforce_criteria_array_limits, - get_default_fields, - get_options, - parse_csv_strings, parse_ids, ) @@ -41,35 +35,18 @@ def audiencetargets(): """Manage audience targets""" -@audiencetargets.command() -@click.option("--ids", help="Comma-separated target IDs") -@click.option("--adgroup-ids", help="Comma-separated ad group IDs") -@click.option("--campaign-ids", help="Comma-separated campaign IDs") -@click.option("--retargeting-list-ids", help="Comma-separated retargeting list IDs") -@click.option("--interest-ids", help="Comma-separated interest IDs") -@click.option("--states", help="Comma-separated states") -@get_options -@click.pass_context -@handle_api_errors -def get( - ctx, - ids, - adgroup_ids, - campaign_ids, - retargeting_list_ids, - interest_ids, - states, - limit, - fetch_all, - output_format, - output, - fields, - dry_run, +def _audiencetargets_get_criteria( + ids=None, + adgroup_ids=None, + campaign_ids=None, + retargeting_list_ids=None, + interest_ids=None, + states=None, + **_, ): - """Get audience targets""" - client = client_from_ctx(ctx, create_client) - - criteria: dict[str, Any] = {} + """SelectionCriteria for ``audiencetargets get``: optional Ids/AdGroupIds/ + CampaignIds, integer RetargetingListIds/InterestIds and upper-cased States.""" + criteria = {} if ids: criteria["Ids"] = parse_ids(ids) if adgroup_ids: @@ -81,47 +58,36 @@ def get( ) add_criteria_csv(criteria, "InterestIds", interest_ids, integers=True) add_criteria_csv(criteria, "States", states, upper=True) + return criteria - enforce_criteria_array_limits( - criteria, - AUDIENCETARGETS_GET_CRITERIA_LIMITS, - command_name="audiencetargets get", - ) - if not criteria: - raise click.UsageError( - t( - "audiencetargets get requires at least one filter " - "(--ids, --adgroup-ids, --campaign-ids, --retargeting-list-ids, " - "--interest-ids, or --states). The Yandex Direct API rejects an " - "empty SelectionCriteria (error 8000/4001), so whole-account " - "paging is not available. To sweep the account, first run " - "`campaigns get`, then page `audiencetargets get` in batches of " - "campaign ids." - ) - ) - - field_names = parse_csv_strings(fields) or get_default_fields("audiencetargets") - params = build_common_params( - criteria=criteria, field_names=field_names, limit=limit - ) - - body = {"method": "get", "params": params} - - if dry_run: - format_output(body, "json", None) - return - - result = client.audiencetargets().post(data=body) - - if fetch_all: - items = [] - for item in result().iter_items(): - items.append(item) - format_output(items, output_format, output) - else: - data = result().extract() - format_output(data, output_format, output) +get = make_get_command( + audiencetargets, + create_client, + default_fields_key="audiencetargets", + help_text="Get audience targets", + ids_help="Comma-separated target IDs", + extra_options=( + click.option("--adgroup-ids", help="Comma-separated ad group IDs"), + click.option("--campaign-ids", help="Comma-separated campaign IDs"), + click.option( + "--retargeting-list-ids", help="Comma-separated retargeting list IDs" + ), + click.option("--interest-ids", help="Comma-separated interest IDs"), + click.option("--states", help="Comma-separated states"), + ), + criteria_builder=_audiencetargets_get_criteria, + criteria_limits=AUDIENCETARGETS_GET_CRITERIA_LIMITS, + require_criteria_message=( + "audiencetargets get requires at least one filter " + "(--ids, --adgroup-ids, --campaign-ids, --retargeting-list-ids, " + "--interest-ids, or --states). The Yandex Direct API rejects an " + "empty SelectionCriteria (error 8000/4001), so whole-account " + "paging is not available. To sweep the account, first run " + "`campaigns get`, then page `audiencetargets get` in batches of " + "campaign ids." + ), +) @audiencetargets.command() diff --git a/direct_cli/commands/creatives.py b/direct_cli/commands/creatives.py index c0d5b0f..3e7a0e6 100644 --- a/direct_cli/commands/creatives.py +++ b/direct_cli/commands/creatives.py @@ -4,17 +4,13 @@ import click -from ..api import client_from_ctx, create_client +from ..api import create_client from ._execute import execute_request -from ..i18n import t -from ..output import format_output, handle_api_errors +from ._get import make_get_command +from ..output import handle_api_errors from ..utils import ( - build_common_params, - get_default_fields, - parse_csv_strings, parse_csv_upper, parse_ids, - parse_nested_field_names, ) @@ -23,111 +19,60 @@ def creatives(): """Manage creatives""" -@creatives.command() -@click.option("--ids", help="Comma-separated creative IDs") -@click.option("--types", help="Comma-separated creative types") -@click.option("--limit", type=int, help="Limit number of results") -@click.option("--fetch-all", is_flag=True, help="Fetch all pages") -@click.option("--format", "output_format", default="json", help="Output format") -@click.option("--output", help="Output file") -@click.option("--fields", help="Comma-separated field names") -@click.option( - "--cpc-video-creative-field-names", - help=( - "Comma-separated CpcVideoCreativeFieldNames (e.g. Duration). " - "Sent as separate top-level request parameter per the " - "CreativesGetRequest WSDL." - ), -) -@click.option( - "--cpm-video-creative-field-names", - help=( - "Comma-separated CpmVideoCreativeFieldNames (e.g. Duration). " - "Sent as separate top-level request parameter per the " - "CreativesGetRequest WSDL." - ), -) -@click.option( - "--smart-creative-field-names", - help=( - "Comma-separated SmartCreativeFieldNames " - "(e.g. CreativeGroupId,CreativeGroupName,BusinessType). " - "Sent as separate top-level request parameter per the " - "CreativesGetRequest WSDL." - ), -) -@click.option( - "--video-extension-creative-field-names", - help=( - "Comma-separated VideoExtensionCreativeFieldNames (e.g. Duration). " - "Sent as separate top-level request parameter per the " - "CreativesGetRequest WSDL." - ), -) -@click.option("--dry-run", is_flag=True, help="Show request without sending") -@click.pass_context -@handle_api_errors -def get( - ctx, - ids, - types, - limit, - fetch_all, - output_format, - output, - fields, - cpc_video_creative_field_names, - cpm_video_creative_field_names, - smart_creative_field_names, - video_extension_creative_field_names, - dry_run, -): - """Get creatives""" - client = client_from_ctx(ctx, create_client) - - field_names = parse_csv_strings(fields) or get_default_fields("creatives") - - nested_field_name_options = ( - ("CpcVideoCreativeFieldNames", cpc_video_creative_field_names), - ("CpmVideoCreativeFieldNames", cpm_video_creative_field_names), - ("SmartCreativeFieldNames", smart_creative_field_names), - ( - "VideoExtensionCreativeFieldNames", - video_extension_creative_field_names, - ), - ) - parsed_nested_field_names = parse_nested_field_names(nested_field_name_options) - +def _creatives_get_criteria(ids=None, types=None, **_): + """SelectionCriteria for ``creatives get``: optional ``Ids`` plus an + upper-cased ``Types`` list (an empty ``--types`` CSV maps to ``[]``).""" criteria = {} if ids: criteria["Ids"] = parse_ids(ids) if types: criteria["Types"] = parse_csv_upper(types) or [] + return criteria - if not criteria: - raise click.UsageError(t("Provide at least one typed filter")) - - params = build_common_params( - criteria=criteria, field_names=field_names, limit=limit - ) - params.update(parsed_nested_field_names) - - body = {"method": "get", "params": params} - - if dry_run: - format_output(body, "json", None) - return - - result = client.creatives().post(data=body) - if fetch_all: - items = [] - for item in result().iter_items(): - items.append(item) - format_output(items, output_format, output) - else: - data = result().extract() - format_output(data, output_format, output) +get = make_get_command( + creatives, + create_client, + default_fields_key="creatives", + help_text="Get creatives", + ids_help="Comma-separated creative IDs", + extra_options=( + click.option("--types", help="Comma-separated creative types"), + ), + criteria_builder=_creatives_get_criteria, + require_criteria_message="Provide at least one typed filter", + nested_field_options=( + ( + "--cpc-video-creative-field-names", + "CpcVideoCreativeFieldNames", + "Comma-separated CpcVideoCreativeFieldNames (e.g. Duration). " + "Sent as separate top-level request parameter per the " + "CreativesGetRequest WSDL.", + ), + ( + "--cpm-video-creative-field-names", + "CpmVideoCreativeFieldNames", + "Comma-separated CpmVideoCreativeFieldNames (e.g. Duration). " + "Sent as separate top-level request parameter per the " + "CreativesGetRequest WSDL.", + ), + ( + "--smart-creative-field-names", + "SmartCreativeFieldNames", + "Comma-separated SmartCreativeFieldNames " + "(e.g. CreativeGroupId,CreativeGroupName,BusinessType). " + "Sent as separate top-level request parameter per the " + "CreativesGetRequest WSDL.", + ), + ( + "--video-extension-creative-field-names", + "VideoExtensionCreativeFieldNames", + "Comma-separated VideoExtensionCreativeFieldNames (e.g. Duration). " + "Sent as separate top-level request parameter per the " + "CreativesGetRequest WSDL.", + ), + ), +) @creatives.command() diff --git a/direct_cli/commands/keywords.py b/direct_cli/commands/keywords.py index 302619a..83b9a8f 100644 --- a/direct_cli/commands/keywords.py +++ b/direct_cli/commands/keywords.py @@ -7,6 +7,7 @@ import click from ..api import client_from_ctx, create_client +from ._get import make_get_command from ..i18n import t from ..output import ( format_output, @@ -15,10 +16,6 @@ from ..utils import ( MICRO_RUBLES, add_criteria_csv, - build_common_params, - enforce_criteria_array_limits, - get_default_fields, - parse_csv_strings, parse_ids, ) @@ -214,95 +211,23 @@ def keywords(): """Manage keywords""" -@keywords.command() -@click.option("--ids", help="Comma-separated keyword IDs") -@click.option("--adgroup-ids", help="Comma-separated ad group IDs") -@click.option("--campaign-ids", help="Comma-separated campaign IDs") -@click.option("--status", help="Filter by status") -@click.option("--statuses", help="Comma-separated statuses") -@click.option("--states", help="Comma-separated states") -@click.option("--modified-since", help="ModifiedSince datetime") -@click.option("--serving-statuses", help="Comma-separated serving statuses") -@click.option("--limit", type=int, help="Limit number of results") -@click.option("--fetch-all", is_flag=True, help="Fetch all pages") -@click.option("--format", "output_format", default="json", help="Output format") -@click.option("--output", help="Output file") -@click.option("--fields", help="Comma-separated field names") -@click.option( - "--autotargeting-settings-brand-options-field-names", - help=( - "Comma-separated AutotargetingSettingsBrandOptionsFieldNames " - "(e.g. WithoutBrands,WithAdvertiserBrand,WithCompetitorsBrand). " - "Sent as separate top-level request parameter per the " - "KeywordsGetRequest WSDL." - ), -) -@click.option( - "--autotargeting-settings-categories-field-names", - help=( - "Comma-separated AutotargetingSettingsCategoriesFieldNames " - "(e.g. Exact,Narrow,Alternative,Accessory,Broader). " - "Sent as separate top-level request parameter per the " - "KeywordsGetRequest WSDL." - ), -) -@click.option("--dry-run", is_flag=True, help="Show request without sending") -@click.pass_context -@handle_api_errors -def get( - ctx, - ids, - adgroup_ids, - campaign_ids, - status, - statuses, - states, - modified_since, - serving_statuses, - limit, - fetch_all, - output_format, - output, - fields, - autotargeting_settings_brand_options_field_names, - autotargeting_settings_categories_field_names, - dry_run, +def _keywords_get_criteria( + ids=None, + adgroup_ids=None, + campaign_ids=None, + status=None, + statuses=None, + states=None, + modified_since=None, + serving_statuses=None, + **_, ): - """Get keywords""" + """SelectionCriteria for ``keywords get``: optional Ids/AdGroupIds/CampaignIds, + a singular ``--status`` or upper-cased ``--statuses`` (mutually exclusive), + upper-cased States/ServingStatuses and a ModifiedSince scalar.""" if status and statuses: raise click.UsageError(t("--status and --statuses are mutually exclusive")) - - client = client_from_ctx(ctx, create_client) - - field_names = parse_csv_strings(fields) or get_default_fields("keywords") - - parsed_brand_options = parse_csv_strings( - autotargeting_settings_brand_options_field_names - ) - if ( - autotargeting_settings_brand_options_field_names is not None - and not parsed_brand_options - ): - raise click.UsageError( - t( - "Provide a non-empty comma-separated " - "AutotargetingSettingsBrandOptionsFieldNames list." - ) - ) - - parsed_categories = parse_csv_strings(autotargeting_settings_categories_field_names) - if ( - autotargeting_settings_categories_field_names is not None - and not parsed_categories - ): - raise click.UsageError( - t( - "Provide a non-empty comma-separated " - "AutotargetingSettingsCategoriesFieldNames list." - ) - ) - - criteria: dict[str, Any] = {} + criteria = {} if ids: criteria["Ids"] = parse_ids(ids) if adgroup_ids: @@ -316,38 +241,46 @@ def get( if modified_since: criteria["ModifiedSince"] = modified_since add_criteria_csv(criteria, "ServingStatuses", serving_statuses, upper=True) + return criteria - enforce_criteria_array_limits( - criteria, KEYWORDS_GET_CRITERIA_LIMITS, command_name="keywords get" - ) - - if not criteria: - raise click.UsageError(t("Provide at least one typed filter")) - - params = build_common_params( - criteria=criteria, field_names=field_names, limit=limit - ) - if parsed_brand_options: - params["AutotargetingSettingsBrandOptionsFieldNames"] = parsed_brand_options - if parsed_categories: - params["AutotargetingSettingsCategoriesFieldNames"] = parsed_categories - - body = {"method": "get", "params": params} - if dry_run: - format_output(body, "json", None) - return - - result = client.keywords().post(data=body) - - if fetch_all: - items = [] - for item in result().iter_items(): - items.append(item) - format_output(items, output_format, output) - else: - data = result().extract() - format_output(data, output_format, output) +get = make_get_command( + keywords, + create_client, + default_fields_key="keywords", + help_text="Get keywords", + ids_help="Comma-separated keyword IDs", + extra_options=( + click.option("--adgroup-ids", help="Comma-separated ad group IDs"), + click.option("--campaign-ids", help="Comma-separated campaign IDs"), + click.option("--status", help="Filter by status"), + click.option("--statuses", help="Comma-separated statuses"), + click.option("--states", help="Comma-separated states"), + click.option("--modified-since", help="ModifiedSince datetime"), + click.option("--serving-statuses", help="Comma-separated serving statuses"), + ), + criteria_builder=_keywords_get_criteria, + criteria_limits=KEYWORDS_GET_CRITERIA_LIMITS, + require_criteria_message="Provide at least one typed filter", + nested_field_options=( + ( + "--autotargeting-settings-brand-options-field-names", + "AutotargetingSettingsBrandOptionsFieldNames", + "Comma-separated AutotargetingSettingsBrandOptionsFieldNames " + "(e.g. WithoutBrands,WithAdvertiserBrand,WithCompetitorsBrand). " + "Sent as separate top-level request parameter per the " + "KeywordsGetRequest WSDL.", + ), + ( + "--autotargeting-settings-categories-field-names", + "AutotargetingSettingsCategoriesFieldNames", + "Comma-separated AutotargetingSettingsCategoriesFieldNames " + "(e.g. Exact,Narrow,Alternative,Accessory,Broader). " + "Sent as separate top-level request parameter per the " + "KeywordsGetRequest WSDL.", + ), + ), +) @keywords.command() diff --git a/direct_cli/commands/strategies.py b/direct_cli/commands/strategies.py index 43bf576..45daf3f 100644 --- a/direct_cli/commands/strategies.py +++ b/direct_cli/commands/strategies.py @@ -4,19 +4,16 @@ import click -from ..api import client_from_ctx, create_client +from ..api import create_client from ..i18n import t -from ..output import format_output, handle_api_errors +from ..output import handle_api_errors from ._execute import execute_request +from ._get import make_get_command from ._lifecycle import register_lifecycle_commands from ..utils import ( MICRO_RUBLES, add_criteria_csv, - build_common_params, - get_default_fields, - parse_csv_strings, parse_ids, - parse_nested_field_names, validate_priority_goal_value, ) @@ -425,267 +422,149 @@ def strategies(): """Manage strategies""" -@strategies.command() -@click.option("--ids", help="Comma-separated strategy IDs") -@click.option( - "--types", - help="Comma-separated strategy types", -) -@click.option( - "--is-archived", - type=click.Choice(["YES", "NO"], case_sensitive=False), - help="Filter by archived status", -) -@click.option("--limit", type=int, help="Limit number of results") -@click.option("--fetch-all", is_flag=True, help="Fetch all pages") -@click.option("--format", "output_format", default="json", help="Output format") -@click.option("--output", help="Output file") -@click.option("--fields", help="Comma-separated field names") -@click.option( - "--strategy-average-cpa-field-names", - help=( - "Comma-separated StrategyAverageCpaFieldNames " - "(e.g. AverageCpa,GoalId). Sent as separate top-level request " - "parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpa-multiple-goals-field-names", - help=( - "Comma-separated StrategyAverageCpaMultipleGoalsFieldNames " - "(e.g. WeeklySpendLimit,BidCeiling,PriorityGoals). Sent as separate " - "top-level request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpa-per-campaign-field-names", - help=( - "Comma-separated StrategyAverageCpaPerCampaignFieldNames " - "(e.g. AverageCpa,GoalId). Sent as separate top-level request " - "parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpa-per-filter-field-names", - help=( - "Comma-separated StrategyAverageCpaPerFilterFieldNames " - "(e.g. AverageCpa,GoalId). Sent as separate top-level request " - "parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpc-field-names", - help=( - "Comma-separated StrategyAverageCpcFieldNames " - "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpc-per-campaign-field-names", - help=( - "Comma-separated StrategyAverageCpcPerCampaignFieldNames " - "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-cpc-per-filter-field-names", - help=( - "Comma-separated StrategyAverageCpcPerFilterFieldNames " - "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-average-crr-field-names", - help=( - "Comma-separated StrategyAverageCrrFieldNames " - "(e.g. AverageCrr,GoalId). Sent as separate top-level request " - "parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-max-profit-field-names", - help=( - "Comma-separated StrategyMaxProfitFieldNames " - "(e.g. WeeklySpendLimit,BidCeiling). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-maximum-clicks-field-names", - help=( - "Comma-separated StrategyMaximumClicksFieldNames " - "(e.g. WeeklySpendLimit,BidCeiling). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-maximum-conversion-rate-field-names", - help=( - "Comma-separated StrategyMaximumConversionRateFieldNames " - "(e.g. WeeklySpendLimit,GoalId). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-pay-for-conversion-crr-field-names", - help=( - "Comma-separated StrategyPayForConversionCrrFieldNames " - "(e.g. Crr,GoalId). Sent as separate top-level request parameter " - "per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-pay-for-conversion-field-names", - help=( - "Comma-separated StrategyPayForConversionFieldNames " - "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " - "per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-pay-for-conversion-multiple-goals-field-names", - help=( - "Comma-separated StrategyPayForConversionMultipleGoalsFieldNames " - "(e.g. WeeklySpendLimit,PriorityGoals). Sent as separate top-level " - "request parameter per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-pay-for-conversion-per-campaign-field-names", - help=( - "Comma-separated StrategyPayForConversionPerCampaignFieldNames " - "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " - "per the StrategiesGetRequest WSDL." - ), -) -@click.option( - "--strategy-pay-for-conversion-per-filter-field-names", - help=( - "Comma-separated StrategyPayForConversionPerFilterFieldNames " - "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " - "per the StrategiesGetRequest WSDL." - ), -) -@click.option("--dry-run", is_flag=True, help="Show request without sending") -@click.pass_context -@handle_api_errors -def get( - ctx, - ids, - types, - is_archived, - limit, - fetch_all, - output_format, - output, - fields, - strategy_average_cpa_field_names, - strategy_average_cpa_multiple_goals_field_names, - strategy_average_cpa_per_campaign_field_names, - strategy_average_cpa_per_filter_field_names, - strategy_average_cpc_field_names, - strategy_average_cpc_per_campaign_field_names, - strategy_average_cpc_per_filter_field_names, - strategy_average_crr_field_names, - strategy_max_profit_field_names, - strategy_maximum_clicks_field_names, - strategy_maximum_conversion_rate_field_names, - strategy_pay_for_conversion_crr_field_names, - strategy_pay_for_conversion_field_names, - strategy_pay_for_conversion_multiple_goals_field_names, - strategy_pay_for_conversion_per_campaign_field_names, - strategy_pay_for_conversion_per_filter_field_names, - dry_run, -): - """Get strategies""" - field_names = parse_csv_strings(fields) or get_default_fields("strategies") +def _strategies_get_criteria(ids=None, types=None, is_archived=None, **_): + """SelectionCriteria for ``strategies get``: optional ``Ids``, a plain + (non-upper-cased) ``Types`` list, and an upper-cased ``IsArchived`` scalar.""" + criteria = {} + if ids: + criteria["Ids"] = parse_ids(ids) + add_criteria_csv(criteria, "Types", types) + if is_archived: + criteria["IsArchived"] = is_archived.upper() + return criteria + - raw_nested = ( - ("StrategyAverageCpaFieldNames", strategy_average_cpa_field_names), +get = make_get_command( + strategies, + create_client, + default_fields_key="strategies", + help_text="Get strategies", + ids_help="Comma-separated strategy IDs", + extra_options=( + click.option("--types", help="Comma-separated strategy types"), + click.option( + "--is-archived", + type=click.Choice(["YES", "NO"], case_sensitive=False), + help="Filter by archived status", + ), + ), + criteria_builder=_strategies_get_criteria, + require_criteria_message="Provide at least one typed filter", + nested_field_options=( ( + "--strategy-average-cpa-field-names", + "StrategyAverageCpaFieldNames", + "Comma-separated StrategyAverageCpaFieldNames " + "(e.g. AverageCpa,GoalId). Sent as separate top-level request " + "parameter per the StrategiesGetRequest WSDL.", + ), + ( + "--strategy-average-cpa-multiple-goals-field-names", "StrategyAverageCpaMultipleGoalsFieldNames", - strategy_average_cpa_multiple_goals_field_names, + "Comma-separated StrategyAverageCpaMultipleGoalsFieldNames " + "(e.g. WeeklySpendLimit,BidCeiling,PriorityGoals). Sent as separate " + "top-level request parameter per the StrategiesGetRequest WSDL.", ), ( + "--strategy-average-cpa-per-campaign-field-names", "StrategyAverageCpaPerCampaignFieldNames", - strategy_average_cpa_per_campaign_field_names, + "Comma-separated StrategyAverageCpaPerCampaignFieldNames " + "(e.g. AverageCpa,GoalId). Sent as separate top-level request " + "parameter per the StrategiesGetRequest WSDL.", ), ( + "--strategy-average-cpa-per-filter-field-names", "StrategyAverageCpaPerFilterFieldNames", - strategy_average_cpa_per_filter_field_names, + "Comma-separated StrategyAverageCpaPerFilterFieldNames " + "(e.g. AverageCpa,GoalId). Sent as separate top-level request " + "parameter per the StrategiesGetRequest WSDL.", + ), + ( + "--strategy-average-cpc-field-names", + "StrategyAverageCpcFieldNames", + "Comma-separated StrategyAverageCpcFieldNames " + "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", ), - ("StrategyAverageCpcFieldNames", strategy_average_cpc_field_names), ( + "--strategy-average-cpc-per-campaign-field-names", "StrategyAverageCpcPerCampaignFieldNames", - strategy_average_cpc_per_campaign_field_names, + "Comma-separated StrategyAverageCpcPerCampaignFieldNames " + "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", ), ( + "--strategy-average-cpc-per-filter-field-names", "StrategyAverageCpcPerFilterFieldNames", - strategy_average_cpc_per_filter_field_names, + "Comma-separated StrategyAverageCpcPerFilterFieldNames " + "(e.g. AverageCpc,WeeklySpendLimit). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", + ), + ( + "--strategy-average-crr-field-names", + "StrategyAverageCrrFieldNames", + "Comma-separated StrategyAverageCrrFieldNames " + "(e.g. AverageCrr,GoalId). Sent as separate top-level request " + "parameter per the StrategiesGetRequest WSDL.", + ), + ( + "--strategy-max-profit-field-names", + "StrategyMaxProfitFieldNames", + "Comma-separated StrategyMaxProfitFieldNames " + "(e.g. WeeklySpendLimit,BidCeiling). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", + ), + ( + "--strategy-maximum-clicks-field-names", + "StrategyMaximumClicksFieldNames", + "Comma-separated StrategyMaximumClicksFieldNames " + "(e.g. WeeklySpendLimit,BidCeiling). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", ), - ("StrategyAverageCrrFieldNames", strategy_average_crr_field_names), - ("StrategyMaxProfitFieldNames", strategy_max_profit_field_names), - ("StrategyMaximumClicksFieldNames", strategy_maximum_clicks_field_names), ( + "--strategy-maximum-conversion-rate-field-names", "StrategyMaximumConversionRateFieldNames", - strategy_maximum_conversion_rate_field_names, + "Comma-separated StrategyMaximumConversionRateFieldNames " + "(e.g. WeeklySpendLimit,GoalId). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", ), ( + "--strategy-pay-for-conversion-crr-field-names", "StrategyPayForConversionCrrFieldNames", - strategy_pay_for_conversion_crr_field_names, + "Comma-separated StrategyPayForConversionCrrFieldNames " + "(e.g. Crr,GoalId). Sent as separate top-level request parameter " + "per the StrategiesGetRequest WSDL.", ), ( + "--strategy-pay-for-conversion-field-names", "StrategyPayForConversionFieldNames", - strategy_pay_for_conversion_field_names, + "Comma-separated StrategyPayForConversionFieldNames " + "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " + "per the StrategiesGetRequest WSDL.", ), ( + "--strategy-pay-for-conversion-multiple-goals-field-names", "StrategyPayForConversionMultipleGoalsFieldNames", - strategy_pay_for_conversion_multiple_goals_field_names, + "Comma-separated StrategyPayForConversionMultipleGoalsFieldNames " + "(e.g. WeeklySpendLimit,PriorityGoals). Sent as separate top-level " + "request parameter per the StrategiesGetRequest WSDL.", ), ( + "--strategy-pay-for-conversion-per-campaign-field-names", "StrategyPayForConversionPerCampaignFieldNames", - strategy_pay_for_conversion_per_campaign_field_names, + "Comma-separated StrategyPayForConversionPerCampaignFieldNames " + "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " + "per the StrategiesGetRequest WSDL.", ), ( + "--strategy-pay-for-conversion-per-filter-field-names", "StrategyPayForConversionPerFilterFieldNames", - strategy_pay_for_conversion_per_filter_field_names, + "Comma-separated StrategyPayForConversionPerFilterFieldNames " + "(e.g. Cpa,GoalId). Sent as separate top-level request parameter " + "per the StrategiesGetRequest WSDL.", ), - ) - parsed_nested = parse_nested_field_names(raw_nested) - - criteria = {} - if ids: - criteria["Ids"] = parse_ids(ids) - add_criteria_csv(criteria, "Types", types) - if is_archived: - criteria["IsArchived"] = is_archived.upper() - - if not criteria: - raise click.UsageError(t("Provide at least one typed filter")) - - params = build_common_params( - criteria=criteria, field_names=field_names, limit=limit - ) - params.update(parsed_nested) - - body = {"method": "get", "params": params} - if dry_run: - format_output(body, "json", None) - return - - client = client_from_ctx(ctx, create_client) - result = client.strategies().post(data=body) - - if fetch_all: - items = [] - for item in result().iter_items(): - items.append(item) - format_output(items, output_format, output) - else: - format_output(result().extract(), output_format, output) + ), +) @strategies.command() diff --git a/tests/test_dry_run.py b/tests/test_dry_run.py index 049290a..9812977 100644 --- a/tests/test_dry_run.py +++ b/tests/test_dry_run.py @@ -18341,6 +18341,35 @@ def test_strategies_get_rejects_empty_nested_field_names(flag, wsdl_key): assert f"Provide a non-empty comma-separated {wsdl_key} list." in result.output +@pytest.mark.parametrize( + "resource,flag,wsdl_key", + [ + ( + "keywords", + "--autotargeting-settings-brand-options-field-names", + "AutotargetingSettingsBrandOptionsFieldNames", + ), + ("adgroups", "--smart-ad-group-field-names", "SmartAdGroupFieldNames"), + ], +) +def test_get_empty_nested_field_names_precedes_criteria_limit(resource, flag, wsdl_key): + """For the two ``get`` commands with BOTH criteria-limits and nested + ``*FieldNames`` (keywords, adgroups), an over-limit array AND an empty nested + CSV at once reports the nested error, not the array-limit error — pinning the + make_get_command check order against the pre-factory hand-rolled order (#588). + """ + over_limit = ",".join(str(i) for i in range(1, 12)) # 11 CampaignIds > limit 10 + result = CliRunner().invoke( + cli, + [resource, "get", "--campaign-ids", over_limit, flag, ",", "--dry-run"], + ) + + assert result.exit_code != 0 + assert ( + f"Provide a non-empty comma-separated {wsdl_key} list." in result.output + ), f"{resource}: expected the nested error to win, got: {result.output}" + + def test_strategies_add_payload(): body = _dry_run( "strategies",