Skip to content

feat(creatives): expose per-subtype *CreativeFieldNames on creatives get#417

Merged
axisrow merged 2 commits into
mainfrom
feat/411-creatives-nested-field-names
May 28, 2026
Merged

feat(creatives): expose per-subtype *CreativeFieldNames on creatives get#417
axisrow merged 2 commits into
mainfrom
feat/411-creatives-nested-field-names

Conversation

@axisrow

@axisrow axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Closes #411. Third 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 creatives_get — 7 tests pass (payload, default-omission, --help, 4 parametrized empty-CSV rejections).
  • pytest tests/test_api_coverage.py tests/test_cli.py tests/test_comprehensive.py — 68 tests pass.
  • direct creatives get --help | grep creative-field-names — 4 flags visible.

🤖 Generated with Claude Code

@axisrow @claude
feat(creatives): expose per-subtype *CreativeFieldNames on creatives get
120dd5d 
Add `--cpc-video-creative-field-names`,
`--cpm-video-creative-field-names`, `--smart-creative-field-names`,
and `--video-extension-creative-field-names` to
`direct creatives get` for the four nested WSDL `*FieldNames`
request parameters declared by `CreativesGetRequest`
(`CpcVideoCreativeFieldEnum`: `Duration`;
`CpmVideoCreativeFieldEnum`: `Duration`;
`SmartCreativeFieldEnum`: `CreativeGroupId`, `CreativeGroupName`,
`BusinessType`; `VideoExtensionCreativeFieldEnum`: `Duration`).

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
(Yandex falls back to its server-side default otherwise). The four
options share one selector loop instead of four near-duplicate
blocks.

Removes the corresponding rows from `NESTED_FIELDNAMES_EXCLUSIONS`
so `test_every_nested_fieldnames_param_has_cli_option` enforces all
four flags. Adds parametrized dry-run tests for payload,
default-omission, `--help` visibility, and empty-CSV rejection.

Closes #411.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Copilot AI review requested due to automatic review settings May 28, 2026 03:58
Copilot AI reviewed May 28, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@axisrow

axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner Author

@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

claude Bot commented May 28, 2026 •
edited
Loading

Copy link
Copy Markdown

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

Code Review

Focused on critical issues per request — no bugs, security holes, logic errors, data-loss risks, or perf concerns found. The implementation is a faithful mirror of the Wave 1 sitelinks pattern in direct_cli/commands/sitelinks.py:130-184.

Critical checks (all pass)

  • UsageError propagation (direct_cli/commands/creatives.py:135-139) — except click.UsageError: raise is correctly ordered before except Exception, so the empty-CSV validation isn't swallowed by the generic handler. ✓
  • Empty-CSV guard logic (creatives.py:97) — parse_csv_strings returns None for ",", "", or whitespace-only input. The if raw_value is not None and not parsed check distinguishes "user passed an empty value" (→ UsageError) from "user omitted the flag" (→ skip). ✓
  • No silent data loss — only keys with if parsed: get added to parsed_nested_field_names, then params.update(...) merges them. None of the four WSDL keys collide with SelectionCriteria or FieldNames. ✓
  • Default behavior preserved — when no new flags are passed, payload is byte-identical to pre-PR (verified by test_creatives_get_omits_nested_field_names_by_default). ✓
  • Pagination/fetch-all — nested params live in the top-level params dict that iter_items() re-uses across pages, so --fetch-all keeps the projections consistent. ✓
  • Coverage gate — the four NESTED_FIELDNAMES_EXCLUSIONS rows are removed, so test_every_nested_fieldnames_param_has_cli_option is now actively enforcing all four flags rather than excused. ✓

Non-critical observations (informational, not blockers)

  • Pre-existing inconsistency — creatives.get still uses fields.split(",") (line 83) while sitelinks uses parse_csv_strings(fields). This means --fields "" and --fields "," behave differently than the new flags do (no trim/strip, no rejection). Not introduced by this PR and out of scope here.
  • Empty SelectionCriteria: {} is still always sent when neither --ids nor --types is supplied — pre-existing behavior, unchanged.

Verdict

No production-impacting issues. LGTM for merging from a critical-correctness perspective.

  • Gather context
  • Read changed files
  • Analyze for critical issues
  • Post review
    • Branch

@axisrow
Merge main into feat/411-creatives-nested-field-names
459641c 
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.
@axisrow
axisrow merged commit c3036f7 into main May 28, 2026
6 checks passed
@axisrow
axisrow deleted the feat/411-creatives-nested-field-names branch May 28, 2026 04:30
axisrow added a commit that referenced this pull request May 28, 2026
@axisrow
Merge main into feat/410-clients-nested-field-names
76dbe66 
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.
axisrow added a commit that referenced this pull request May 28, 2026
@axisrow
Merge main into feat/407-agencyclients-nested-field-names
5f8f252 
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.
axisrow added a commit that referenced this pull request May 28, 2026
@axisrow
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 added a commit that referenced this pull request May 28, 2026
@axisrow
Merge main into feat/409-campaigns-nested-field-names
164b3f9 
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).
axisrow added a commit that referenced this pull request May 28, 2026
@axisrow
Merge main into feat/408-bidmodifiers-nested-field-names
28efe50 
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.
This was referenced May 28, 2026
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Wave 2 (#402): expose creatives get nested *FieldNames as CLI options

2 participants

@axisrow