feat(adgroups): expose eight nested *FieldNames on adgroups get by axisrow · Pull Request #420 · axisrow/direct-cli

axisrow · 2026-05-28T04:11:06Z

Summary

Add eight --*-field-names flags to direct adgroups get for the nested WSDL *FieldNames parameters declared by AdGroupsGetRequest:
- --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
Mirrors the Wave 1 sitelinks pattern (PR feat(api): expose separate *FieldNames params for sitelinks and keywordbids #403). One selector loop drives all eight options.
The two AutotargetingSettings.* enums resurface on direct keywords get per Wave 2 (#402): expose keywords get nested *FieldNames as CLI options #413; sample values and help wording stay consistent.
Remove the eight corresponding rows from NESTED_FIELDNAMES_EXCLUSIONS.

Closes #405. Sixth PR in Wave 2 milestone 0.3.14.

Test plan

pytest tests/test_api_coverage.py::test_every_nested_fieldnames_param_has_cli_option — passes after row removal.
pytest tests/test_dry_run.py -k adgroups_get — 11 new tests pass.
pytest tests/test_api_coverage.py tests/test_cli.py tests/test_comprehensive.py — 247 tests pass.
direct adgroups get --help | grep field-names — 8 new flags + --fields visible.

🤖 Generated with Claude Code

Add eight `--*-field-names` flags to `direct adgroups get` for the nested WSDL `*FieldNames` request parameters declared by `AdGroupsGetRequest`: - `--autotargeting-settings-brand-options-field-names` (AutotargetingBrandOptionsFieldEnum: WithoutBrands, WithAdvertiserBrand, WithCompetitorsBrand) - `--autotargeting-settings-categories-field-names` (AutotargetingCategoriesFieldEnum: Exact, Narrow, Alternative, Accessory, Broader) - `--dynamic-text-ad-group-field-names` (DynamicTextAdGroupFieldEnum) - `--dynamic-text-feed-ad-group-field-names` (DynamicTextFeedAdGroupFieldEnum) - `--mobile-app-ad-group-field-names` (MobileAppAdGroupFieldEnum) - `--smart-ad-group-field-names` (SmartAdGroupFieldEnum) - `--text-ad-group-feed-params-field-names` (TextAdGroupFeedParamsFieldEnum) - `--unified-ad-group-field-names` (UnifiedAdGroupFieldEnum) Mirrors the Wave 1 sitelinks pattern (#403). Each flag parses CSV via `parse_csv_strings`, rejects empty input with `click.UsageError`, and only injects the WSDL key when non-empty. The eight options share one selector loop. The two AutotargetingSettings.* enums are reused by keywords.get per #413; sample values and help wording stay consistent. Removes the corresponding rows from `NESTED_FIELDNAMES_EXCLUSIONS` so `test_every_nested_fieldnames_param_has_cli_option` enforces all eight flags. Adds parametrized dry-run tests for payload, default-omission, `--help` visibility, and empty-CSV rejection. Closes #405. Co-Authored-By: Claude Opus 4.7 <[email protected]>

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

axisrow · 2026-05-28T05:00:37Z

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

claude · 2026-05-28T05:00:51Z

Claude finished @axisrow's task in 2m 2s —— View job

Claude PR Review

Read changed files
Analyze code for critical issues
Post review feedback

Verdict: No critical issues found ✅

Reviewed against the requested focus areas (bugs, security, logical errors, data-loss, performance). This PR is a clean, mechanical extension of the existing sitelinks.get pattern from PR #403 and the implementation is sound.

What I verified

1. WSDL parity is exact — All eight WSDL element names referenced in code match tests/wsdl_cache/adgroups.xml exactly:

AutotargetingSettingsBrandOptionsFieldNames  → AutotargetingBrandOptionsFieldEnum (general:)
AutotargetingSettingsCategoriesFieldNames    → AutotargetingCategoriesFieldEnum  (general:)
DynamicTextAdGroupFieldNames                 → DynamicTextAdGroupFieldEnum
DynamicTextFeedAdGroupFieldNames             → DynamicTextFeedAdGroupFieldEnum
MobileAppAdGroupFieldNames                   → MobileAppAdGroupFieldEnum
SmartAdGroupFieldNames                       → SmartAdGroupFieldEnum
TextAdGroupFeedParamsFieldNames              → TextAdGroupFeedParamsFieldEnum
UnifiedAdGroupFieldNames                     → UnifiedAdGroupFieldEnum

2. Empty-CSV handling is correct (adgroups.py:614-622)

parse_csv_strings(None) returns None → skipped, key not injected. ✅
parse_csv_strings(",") or parse_csv_strings("") returns None, but raw_value is not None → UsageError is raised. ✅
This avoids silently sending an empty array, which the live API would reject with 400.

3. UsageError propagation is correct (adgroups.py:668-672)
The explicit except click.UsageError: raise before the broad except Exception ensures the WSDL-validation UsageError from the empty-CSV check is not swallowed and converted into click.Abort() (which would hide the actionable error message). Consistent with sitelinks.get at sitelinks.py:180.

4. No data-loss risk — get is read-only; this only adds top-level request parameters per WSDL. The eight new keys are unique and cannot collide with existing params keys (SelectionCriteria, FieldNames, Page).

5. Default behavior preserved — When no new flag is supplied, parsed_nested is empty and params.update({}) is a no-op. The test_adgroups_get_omits_nested_field_names_by_default test asserts this directly.

6. Backwards-compatible signature change — The eight new keyword args on get(...) are all Click-injected; no external caller of this function exists outside Click dispatch.

Minor (non-blocking) observation

The eight options share one selector loop driven by raw_nested, which keeps the diff tight. The flag-to-WSDL mapping is duplicated between the production raw_nested tuple (adgroups.py:589-613) and the test fixture _ADGROUPS_GET_NESTED_FIELD_FLAGS (test_dry_run.py:22019-22057). That's intentional duplication for testability — fine as-is.

Tests

The PR description shows the 11 new tests pass and the NESTED_FIELDNAMES_EXCLUSIONS coverage guard is now enforcing all eight flags. The test matrix covers: payload shape, default omission, --help visibility, and empty-CSV rejection (parametrized across all 8 flags). Comprehensive.

Approving in spirit — no production-affecting issues.
• Branch: feat/405-adgroups-nested-field-names

Resolve conflicts with #415 #416 #417 #418 #419: keep all six CHANGELOG entries ordered by merge time and append adgroups tests after the feeds/keywords/creatives/clients/agencyclients blocks in test_dry_run.py.

Resolve conflicts with #415 #416 #417 #418 #419 #420: keep all six previously-merged CHANGELOG entries (feeds/keywords/creatives/clients/ agencyclients/adgroups) and place the campaigns BREAKING CHANGES block at the end of Unreleased. Drop the already-removed NESTED_FIELDNAMES_EXCLUSIONS rows. Update the surviving legacy `-fields` test fixtures to the new `-field-names` form, while keeping one explicit rejection test for `--text-campaign-fields` (the old name).

Resolve conflicts with #415 #416 #417 #418 #419 #420 #421: keep all six previously-merged CHANGELOG entries (feeds/keywords/creatives/clients/ agencyclients/adgroups), the campaigns BREAKING CHANGES block, and add the bidmodifiers entry as a separate 'Additional features' block. Drop already-removed NESTED_FIELDNAMES_EXCLUSIONS rows. Append bidmodifiers tests after the canonical campaigns block in test_dry_run.py.

Copilot AI review requested due to automatic review settings May 28, 2026 04:11

Copilot AI reviewed May 28, 2026

Merge main into feat/405-adgroups-nested-field-names

2140153

Resolve conflicts with #415 #416 #417 #418 #419: keep all six CHANGELOG entries ordered by merge time and append adgroups tests after the feeds/keywords/creatives/clients/agencyclients blocks in test_dry_run.py.

axisrow merged commit bc4d08e into main May 28, 2026
6 checks passed

axisrow deleted the feat/405-adgroups-nested-field-names branch May 28, 2026 05:07

This was referenced May 28, 2026

Wave 2: expose remaining nested *FieldNames request parameters as CLI options #402

Closed

audit: снимок покрытия Yandex Direct API после волны #415–#425 (v5 ✅, v4 23/33) #428

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(adgroups): expose eight nested *FieldNames on adgroups get#420

feat(adgroups): expose eight nested *FieldNames on adgroups get#420
axisrow merged 2 commits into
mainfrom
feat/405-adgroups-nested-field-names

axisrow commented May 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

axisrow commented May 28, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Claude PR Review

Verdict: No critical issues found ✅

What I verified

Minor (non-blocking) observation

Tests

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented May 28, 2026 •

edited

Loading