Skip to content

feat(clients): expose five nested *FieldNames on clients get#418

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

feat(clients): expose five nested *FieldNames on clients get#418
axisrow merged 2 commits into
mainfrom
feat/410-clients-nested-field-names

Conversation

@axisrow

@axisrow axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add five --*-field-names flags to direct clients get for the nested WSDL *FieldNames parameters declared by ClientsGetRequest:
    • --contract-field-names (ContractInfoFieldEnum)
    • --contragent-field-names (ContragentInfoFieldEnum)
    • --contragent-tin-info-field-names (TinInfoFieldEnum)
    • --organization-field-names (OrgInfoFieldEnum)
    • --tin-info-field-names (TinInfoFieldEnum)
  • Add --dry-run to clients get for parity with every other read-path command (and the matching rationale in DRY_RUN_PAYLOAD_EXCLUSIONS).
  • Same five enum types resurface on direct agencyclients get per Wave 2 (#402): expose agencyclients get nested *FieldNames as CLI options #407 — sample values and help wording stay consistent.
  • Remove the five corresponding rows from NESTED_FIELDNAMES_EXCLUSIONS.

Closes #410. Fourth 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 clients_get — 8 new tests pass (payload, default-omission, --help, 5 parametrized empty-CSV).
  • pytest tests/test_api_coverage.py tests/test_cli.py tests/test_comprehensive.py — 247 tests pass.
  • direct clients get --help | grep field-names — 5 new flags + existing --fields visible.

🤖 Generated with Claude Code

@axisrow @claude
feat(clients): expose five nested *FieldNames on clients get
02bc940 
Add `--contract-field-names`, `--contragent-field-names`,
`--contragent-tin-info-field-names`, `--organization-field-names`,
and `--tin-info-field-names` to `direct clients get` for the five
nested WSDL `*FieldNames` request parameters declared by
`ClientsGetRequest` (`ContractInfoFieldEnum`,
`ContragentInfoFieldEnum`, `TinInfoFieldEnum`, `OrgInfoFieldEnum`,
`TinInfoFieldEnum`).

Also add `--dry-run` to `clients get` for parity with every other
read-path command, and the corresponding "Read path omits optional
SelectionCriteria when --ids is absent." rationale in
DRY_RUN_PAYLOAD_EXCLUSIONS so the canonical dry-run command
registry stays consistent.

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 five options share one selector loop.

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

The same five enum types are reused on `direct agencyclients get`
per #407; sample values and help wording stay consistent.

Closes #410.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Copilot AI review requested due to automatic review settings May 28, 2026 04:03
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 mentioned this pull request May 28, 2026
Merged
4 tasks
@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 2m 5s —— View job

Code Review — PR #418

  • Read the changed files
  • Analyze for critical issues (bugs, security, logic, data loss, performance)
  • Post review findings

Verdict: no critical issues found. The PR is a clean read-path extension and follows existing conventions in the codebase.

What I verified

  1. WSDL field name parity — All five *FieldNames keys (ContractFieldNames, ContragentFieldNames, ContragentTinInfoFieldNames, OrganizationFieldNames, TinInfoFieldNames) match exactly against tests/wsdl_cache/clients.xml. No silent typos that would cause the server to ignore the parameter.

  2. Empty-CSV guard is correct — direct_cli/commands/clients.py:119-124:

    for wsdl_key, raw_value in raw_nested:
    parsed = parse_csv_strings(raw_value)
    if raw_value is not None and not parsed:
        raise click.UsageError(...)

    parse_csv_strings returns None for "", ",", and " , " (after strip+filter), so raw_value is not None and not parsed correctly distinguishes "flag absent" (Click default None) from "flag provided but degenerate" (empty string, comma-only, whitespace). No data-loss risk where empty input gets silently dropped.

  3. except click.UsageError: raise re-raise — Necessary because the outer except Exception would otherwise swallow it into a generic print_error/Abort, masking the user-facing message. Matches the established pattern (advideos.py:112, bidmodifiers.py:315, bids.py:173, etc.).

  4. Dry-run flow — format_output(body, "json", None) short-circuits before client.clients().post(...), so no API call leaks when --dry-run is set. The create_client() call still runs before the dry-run branch, but that's consistent with every other dry-run command in the repo and not introduced by this PR.

  5. Param-dict update order — params.update(parsed_nested) happens after FieldNames/SelectionCriteria and before Page. Since the request is a JSON object, ordering is irrelevant for the API contract. No collision risk because none of the five keys overlap with existing param keys.

  6. DRY_RUN_PAYLOAD_EXCLUSIONS entry — The rationale ("Read path omits optional SelectionCriteria when --ids is absent.") matches the convention used by businesses.get and others, and clients.get truly does omit SelectionCriteria when --ids is missing (clients.py:134-135). The specific dry-run tests added in this PR cover the payload paths, so the generic exclusion is safe.

  7. NESTED_FIELDNAMES_EXCLUSIONS row removal — Exactly the five rows being implemented are removed; the remaining #402 rows are unrelated to ClientsGetRequest.

Nits I'm flagging only because they're load-bearing later

None affecting functionality. PR is ready to merge from a correctness standpoint.

• Branch: feat/410-clients-nested-field-names

@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
axisrow merged commit 1981161 into main May 28, 2026
6 checks passed
@axisrow
axisrow deleted the feat/410-clients-nested-field-names branch May 28, 2026 04:38
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 @claude
feat(agencyclients): expose five nested *FieldNames on agencyclients …
61f8ef9 
…get (#419)

Add `--contract-field-names`, `--contragent-field-names`,
`--contragent-tin-info-field-names`, `--organization-field-names`,
and `--tin-info-field-names` to `direct agencyclients get` for the
five nested WSDL `*FieldNames` request parameters declared by
`AgencyClientsGetRequest` (`ContractInfoFieldEnum`,
`ContragentInfoFieldEnum`, `TinInfoFieldEnum`, `OrgInfoFieldEnum`,
`TinInfoFieldEnum`). Mirrors PR #418 (clients) which exposes the
exact same five enums on the sibling `clients get` request.

Also add `--dry-run` for parity with every other read-path command,
and the matching rationale in DRY_RUN_PAYLOAD_EXCLUSIONS.

Each flag parses CSV via `parse_csv_strings`, rejects empty input
with `click.UsageError`, and only injects the WSDL key when
non-empty. The five options share one selector loop.

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

Closes #407.

Co-authored-by: Claude Opus 4.7 <[email protected]>
Co-authored-by: axisrow <[email protected]>
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 clients get nested *FieldNames as CLI options

2 participants

@axisrow