feat(keywords): expose AutotargetingSettings*FieldNames on keywords get by axisrow · Pull Request #416 · axisrow/direct-cli

axisrow · 2026-05-28T03:56:01Z

Summary

Add --autotargeting-settings-brand-options-field-names (AutotargetingBrandOptionsFieldEnum: WithoutBrands, WithAdvertiserBrand, WithCompetitorsBrand) and --autotargeting-settings-categories-field-names (AutotargetingCategoriesFieldEnum: Exact, Narrow, Alternative, Accessory, Broader) to direct keywords get.
Mirrors Wave 1 sitelinks pattern (PR feat(api): expose separate *FieldNames params for sitelinks and keywordbids #403) and the Wave 2 (#402): expose feeds get nested *FieldNames as CLI options #412 feeds PR (feat(feeds): expose FileFeedFieldNames and UrlFeedFieldNames on feeds get #415): parse CSV via parse_csv_strings, reject empty input with click.UsageError, only inject the WSDL key when non-empty (Yandex falls back to server-side defaults otherwise).
Remove the two corresponding rows from NESTED_FIELDNAMES_EXCLUSIONS.

The same two enum types resurface on direct adgroups get per #405 — sample values and help wording stay consistent for follow-up parity.

Closes #413. Second 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 keywords_get — 5 new tests pass.
pytest tests/test_api_coverage.py tests/test_cli.py tests/test_comprehensive.py — 68 tests pass.
direct keywords get --help | grep autotargeting — both flags visible.

🤖 Generated with Claude Code

Add `--autotargeting-settings-brand-options-field-names` (`AutotargetingBrandOptionsFieldEnum`: `WithoutBrands`, `WithAdvertiserBrand`, `WithCompetitorsBrand`) and `--autotargeting-settings-categories-field-names` (`AutotargetingCategoriesFieldEnum`: `Exact`, `Narrow`, `Alternative`, `Accessory`, `Broader`) to `direct keywords get` for the separate WSDL `*FieldNames` request parameters declared by `KeywordsGetRequest`. Mirrors the Wave 1 sitelinks pattern (#403) and the #412 feeds PR: each flag parses CSV via `parse_csv_strings`, rejects empty input with `click.UsageError`, and only injects the WSDL key into the payload when non-empty. Removes the corresponding rows from `NESTED_FIELDNAMES_EXCLUSIONS` so `test_every_nested_fieldnames_param_has_cli_option` now enforces both flags. Adds dry-run tests for payload, default-omission, `--help` visibility, and empty-CSV rejection per flag. Same two enum types are reused on `direct adgroups get` per #405; sample values and help wording stay consistent for follow-up parity. Closes #413. 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-28T04:08:56Z

@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-28T04:09:09Z

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

Claude PR Review

Load tools
Review PR diff vs origin/main
Inspect changed files (keywords.py, tests)
Cross-check Wave 1 sitelinks pattern for parity
Post final review

Scope: critical-only review (bugs, security, logic, data loss, performance). Style/naming nits skipped per request.

Verdict

No critical issues found. Ready to merge from a correctness standpoint.

What I verified

CSV parse → validate → inject flow in direct_cli/commands/keywords.py:429-474. The pattern is correct:
- parse_csv_strings(None) → None (flag omitted, nothing happens — server-side default applies)
- parse_csv_strings("") → None, but the check uses is not None on the raw input, so an empty string still raises UsageError rather than silently falling through
- parse_csv_strings(",") → None → raises UsageError ✓ (covered by both new rejection tests)
- Valid input → list → conditionally injected into params only when non-empty ✓
parse_csv_strings (not parse_csv_upper) preserves enum case verbatim. This is important: AutotargetingBrandOptionsFieldEnum values (WithoutBrands, WithAdvertiserBrand, WithCompetitorsBrand) and AutotargetingCategoriesFieldEnum values (Exact, Narrow, Alternative, Accessory, Broader) are PascalCase and the API rejects upper-case. The Wave 1 sitelinks PR (direct_cli/commands/sitelinks.py:144) uses the same helper for the same reason.
except click.UsageError: raise at keywords.py:496-497 is required so the new UsageError isn't swallowed by the existing except Exception block (which would convert it to print_error + Abort, breaking the rejection tests). Same shape as sitelinks.py:180-181. ✓
NESTED_FIELDNAMES_EXCLUSIONS removal in tests/test_api_coverage.py matches the two new CLI options exactly. The coverage gate (test_every_nested_fieldnames_param_has_cli_option) will now require these flags to exist — good defense-in-depth.
Test coverage in tests/test_dry_run.py:
- Payload-shape test confirms both keys land at top level of params (not nested under SelectionCriteria or FieldNames) ✓
- Omission test confirms keys absent when flags not passed (so server-side defaults apply) ✓
- Help-text test ✓
- Two rejection tests for "," input ✓
No security concerns — no shell exec, no template/SQL injection surface; CSV is parsed by str.split and trimmed.
No performance concerns — O(n) string splits on short flag values.

Non-blocking observations (not critical)

The pre-existing --fields path at keywords.py:427 still uses fields.split(",") rather than parse_csv_strings, so --fields "Id,,CampaignId" would emit an empty string in the FieldNames list. Out of scope for this PR — would have been a finding regardless.
Running direct keywords get with only one of the new flags and no Ids/CampaignIds/AdGroupIds will still hit API error 4001 server-side (per CLAUDE.md selection-criteria contract). Pre-existing behavior; not introduced here.
· branch feat/413-keywords-nested-field-names

Resolve conflicts with #415 (feeds): keep both feeds.get and keywords.get *FieldNames additions in CHANGELOG, NESTED_FIELDNAMES_EXCLUSIONS, and dry-run tests.

Resolve conflicts with #415 (feeds) and #416 (keywords): preserve all three CHANGELOG entries, drop already-removed NESTED_FIELDNAMES_EXCLUSIONS rows, and append creatives tests after the feeds/keywords blocks in test_dry_run.py.

Resolve conflicts with #415 #416 #417: keep all four CHANGELOG entries ordered by merge time, drop the already-removed creatives/feeds/keywords NESTED_FIELDNAMES_EXCLUSIONS rows, and append clients tests after the feeds/keywords/creatives blocks in test_dry_run.py.

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

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 03:56

Copilot AI reviewed May 28, 2026

Merge main into feat/413-keywords-nested-field-names

b112910

Resolve conflicts with #415 (feeds): keep both feeds.get and keywords.get *FieldNames additions in CHANGELOG, NESTED_FIELDNAMES_EXCLUSIONS, and dry-run tests.

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

axisrow deleted the feat/413-keywords-nested-field-names branch May 28, 2026 04:22

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(keywords): expose AutotargetingSettings*FieldNames on keywords get#416

feat(keywords): expose AutotargetingSettings*FieldNames on keywords get#416
axisrow merged 2 commits into
mainfrom
feat/413-keywords-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

What I verified

Non-blocking observations (not critical)

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

claude Bot commented May 28, 2026 •

edited

Loading