fix(v4): align pay-campaigns wire-body with docs; drop Currency (closes #433) by axisrow · Pull Request #442 · axisrow/direct-cli

axisrow · 2026-05-28T15:58:12Z

Summary

Non-breaking change. Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/PayCampaigns and confirmed that PayCampElement carries only CampaignID and Sum (in conventional units / условные единицы). No Currency field exists in the docs-defined request body.

This PR aligns the wire-body with the docs without breaking the CLI surface:

--currency option is kept for backward compatibility.
The option value is no longer forwarded to the API wire-body.
Help text on --currency now explains the behaviour.

Changes

direct_cli/commands/v4finance.py:pay_campaigns — drop Currency from Payments[] items; update help text on --currency.
direct_cli/v4_contracts.py:PayCampaigns — example_param updated; notes record the docs verification (Docs-verified 2026-05-28) and the Overdraft retention rationale (docs list only Bank; CLI keeps Overdraft as historical undocumented value).
tests/test_v4finance_money.py — assertions updated to expect wire-body without Currency.

Verification

Live API was not re-verified due to absence of finance credentials. CLI users keeping --currency RUB in their scripts continue to work; the silent value drop only changes the JSON sent over the wire.

Test plan

pytest tests/test_v4finance_money.py tests/test_v4finance_read.py tests/test_comprehensive.py tests/test_cli.py — 129 passing.

Closes #433. Part of milestone 22. Supersedes branch feat/milestone-22-close-433-pay-campaigns (closed without merge).

🤖 Generated with Claude Code

Closes #433. Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/PayCampaigns and confirmed the PayCampElement object carries only CampaignID and Sum; Sum is in conventional units ("условные единицы") with no Currency field anywhere in the request body. Non-breaking change. The --currency CLI option is kept for backward compatibility (its value is no longer forwarded to the API wire-body). Help text updated to explain. Tests assert the wire-body has no Currency on Payments[] items. v4_contracts.py:PayCampaigns example_param updated to match; notes record the docs verification and the Overdraft retention rationale (docs list only Bank; Overdraft is kept as a historical undocumented value used by sandbox flow). 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.

The v4 docs (dg-v4/reference/PayCampaigns) define PayCampElement with only CampaignID and Sum. Currency is not part of the wire-body and was never forwarded to the API. The previous non-breaking shim accepted --currency for backward compatibility; per user direction, the CLI must mirror docs strictly, so the option is removed entirely. - Remove --currency Click option from pay-campaigns - Bump version 0.3.14 -> 0.3.15, add CHANGELOG BREAKING CHANGES entry - Update README example, v4_contracts.py notes, and stale test assertions Closes #433. Co-Authored-By: Claude Opus 4.7 <[email protected]>

axisrow · 2026-05-28T16:20:30Z

Upgraded to BREAKING per user direction: «надо делать как в документации 1 в 1, удалять все, чего нет».

Latest commit:

Removes --currency Click option entirely from pay-campaigns (was previously kept as no-op for backward compat).
Bumps version 0.3.14 → 0.3.15, adds CHANGELOG entry under BREAKING CHANGES.
Updates README example and stale test assertion in test_v4_contracts.py.

Docs re-verified 2026-05-28 — PayCampElement = {CampaignID, Sum}, no Currency. Source: https://yandex.ru/dev/direct/doc/dg-v4/reference/PayCampaigns

Tests: 1909 passed, 50 skipped. Title still says fix(v4) but the commit is refactor(v4)!: — squash-merge should use the breaking title.

Strict 1:1 docs alignment per user direction: «удалять все, чего нет». The v4 docs (dg-v4/reference/PayCampaigns#PayMethod) list only `Bank` as a supported PayMethod value. `Overdraft` was retained in the CLI choices as a historical undocumented value for sandbox flow, but nothing in the public docs guarantees it. - Remove `Overdraft` from V4_PAY_METHODS - Replace test_pay_campaigns_allows_overdraft_without_contract with test_pay_campaigns_rejects_undocumented_pay_method (Click Choice rejects the value before reaching the API) - Update test_pay_campaigns_rejects_non_positive_campaign_ids to use Bank + --contract-id (Overdraft is no longer accepted) - Add second BREAKING line to CHANGELOG 0.3.15 - Update v4_contracts.py notes Closes #433. Co-Authored-By: Claude Opus 4.7 <[email protected]>

axisrow · 2026-05-28T16:27:31Z

Second BREAKING commit added — full audit per «удалять все, чего нет»:

Drops --pay-method Overdraft from pay-campaigns. Docs (dg-v4/reference/PayCampaigns#PayMethod) list only "Bank". Overdraft was a CLI-internal historical value used by sandbox flow; nothing in public docs guarantees it.
Updates tests: test_pay_campaigns_allows_overdraft_without_contract removed; replaced with test_pay_campaigns_rejects_undocumented_pay_method which asserts Click Choice rejects the value.
Updates test_pay_campaigns_rejects_non_positive_campaign_ids to use Bank + --contract-id (Overdraft no longer in enum).
Adds second BREAKING line to CHANGELOG 0.3.15.

Tests: 1909 passed, 50 skipped.

This PR now contains two BREAKING changes for pay-campaigns:

--currency removed.
--pay-method Overdraft removed.

If sandbox still supports Overdraft and that flow needs to be preserved, follow-up issue should re-introduce it with an explicit --sandbox-only gate or a separate undocumented-flag opt-in.

# Conflicts: # CHANGELOG.md # README.md

Two sentences in PayCampaigns.notes restated "no Currency in wire-body"; consolidate to a single docs-verified statement matching the TransferMoney style. Also update tests/API_COVERAGE.md to reflect that PayCampaigns now mirrors TransferMoney (PayCampElement = CampaignID + Sum, no Currency) and that PayMethod is restricted to 'Bank'.

axisrow · 2026-05-28T17:03:28Z

@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-28T17:03:41Z

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

Review

Read PR diff against origin/main
Inspect direct_cli/commands/v4finance.py (pay_campaigns)
Inspect direct_cli/v4_contracts.py (PayCampaigns contract)
Inspect updated tests
Check for critical issues
Post final review

Scope of review: critical issues only (bugs, security, logic, data loss, performance). Style/naming/formatting intentionally skipped per request.

Findings

1. Live-API risk from dropping Currency — worth a second thought, not a blocker. ⚠️

The PR drops Currency from PayCampElement based on docs verification, but explicitly states "Live API was not re-verified due to absence of finance credentials." The previous CLI was forwarding Currency: "RUB" (default) to the live API; Yandex was presumably accepting and either using or ignoring it. Two scenarios:

If Yandex ignored it (most likely, given docs spec only CampaignID+Sum in conventional units tied to the contract): no behavioral change, this PR is correct.
If Yandex was actually using Currency to disambiguate for multi-currency contracts: removing it could change billing behavior for users on non-RUB contracts.

Per the «1 в 1 с документацией» rule the team has adopted, this is the right call — but I'd flag in the release notes / a follow-up that anyone on a non-RUB agency contract should sanity-check the next invoice. The notes field in v4_contracts.py already says "PayMethod currently accepts 'Bank' only" — consider adding "non-RUB contract behavior unverified live" there too.

2. Overdraft removal — same docs-1:1 rationale, lower risk. ✓

The author already calls out the mitigation path in PR comment 2 (re-introduce with --sandbox gate if needed). No new bug; just confirming the test refactor (test_pay_campaigns_rejects_non_positive_campaign_ids switching to Bank + --contract-id) is correct — the order of validation in pay_campaigns puts _campaign_ids_param(...) before the contract check, so 0,-1 still trips first as intended.

3. Dead branch (cosmetic, not a bug). ℹ️

direct_cli/commands/v4finance.py:595 — if contract_id: is now always true on the success path, because V4_PAY_METHODS = ["Bank"] and the preceding check (pay_method == "Bank" and not contract_id → UsageError) means any code that reaches line 595 already has a non-empty contract_id. Not a bug, but the conditional is now redundant. If/when Overdraft (or other methods that allow empty ContractID) is re-added, leaving the guard in place is correct, so I'd leave it.

Similarly _non_empty_option(pay_method, "--pay-method") on line 584 is unreachable as a no-op trigger since Click Choice with the only value "Bank" already rejects empty/invalid input. Harmless.

4. PR title vs commit message mismatch — already flagged by author. ℹ️

PR title is fix(v4): ... but it contains two refactor(v4)!: BREAKING commits and bumps to 0.3.15. Author notes squash-merge should use the breaking title. Worth being explicit at merge time so the changelog generator / release tooling picks up the !.

5. No security or perf issues found. ✓

Finance token handling, masking, and dry-run gating are unchanged.

Verdict

No blocking bugs. The change is internally consistent: option removed, wire-body, contract example, docs example, README, and tests are all updated coherently. The main thing worth tracking is item (1) — the inability to live-verify on a non-RUB contract — which justifies a brief mention in the changelog / a smoke-test follow-up if/when finance credentials are available.

· branch: feat/milestone-22-close-433-pay-campaigns-v2

axisrow · 2026-05-28T17:07:33Z

Triage of the review:

Finding 1 (non-RUB contract behavior unverified live) — SKIP for the merge but acknowledged as a release-notes followup; v4finance pay-campaigns remains dry-run-only in 0.3.15, so no live billing path runs in this release.
Finding 2 (Overdraft removal) — accepted.
Finding 3 (dead branches) — SKIP, intentionally kept as guard for future PayMethod values.
Finding 4 (squash-merge title) — will squash with a BREAKING commit message to keep changelog tooling happy.
Finding 5 — no action.

After PRs #441/#442/#443 dropped --currency from transfer-money, pay-campaigns, and create-invoice, this module-level constant has no remaining callers in direct_cli/ or tests/.

…#441/#442/#443) (#450) PRs #441/#442/#443 dropped the obligatory Live 4 input parameter Currency from PayCampElement on transfer-money, pay-campaigns, and create-invoice; #442 additionally dropped the valid Overdraft PayMethod. The commits cited dg-v4/reference/<Method> as the verification source. That URL is legacy v4 (pre-Live), where PayCampElement is {CampaignID, Sum} only. The CLI ships these commands in the v4finance Live group and must mirror dg-v4/live/<Method>. Verified 2026-05-29 by curling the live pages: - dg-v4/live/TransferMoney → PayCampElement = {CampaignID, Sum, Currency}; changelog "Новое в версии Live 4: Входной параметр Currency стал обязательным". - dg-v4/live/PayCampaigns → PayCampElement = {CampaignID, Sum, Currency}; same changelog plus "Добавлен метод оплаты Overdraft". - dg-v4/live/CreateInvoice → PayCampElement = {CampaignID, Sum, Currency}. Restored: - --currency (required, Choice: RUB/CHF/EUR/KZT/TRY/UAH/USD/BYN) on transfer-money, pay-campaigns, create-invoice. - Currency emitted on every Payments[] / FromCampaigns[] / ToCampaigns[] item. - Overdraft re-enabled in V4_PAY_METHODS; --contract-id remains required only when --pay-method Bank. Updated v4_contracts.py example_param and notes (now pointing at dg-v4/live/*); updated tests/test_v4finance_money.py and tests/test_v4_contracts.py expectations; updated README, CHANGELOG (0.3.16 BREAKING CHANGES), scripts/test_dangerous_commands.sh checklist, tests/API_COVERAGE.md, and the opt-in live-write test for CreateInvoice. Bumped version to 0.3.16. Audit trail: #125 (comment) Co-authored-by: hapi <[email protected]> Co-authored-by: Claude Opus 4.7 <[email protected]>

Copilot AI review requested due to automatic review settings May 28, 2026 15:58

Copilot AI reviewed May 28, 2026

axisrow mentioned this pull request May 28, 2026

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

Closed

3 tasks

axisrow mentioned this pull request May 28, 2026

fix(v4): align create-invoice wire-body with docs; drop Currency (closes #434) #443

Merged

1 task

axisrow mentioned this pull request May 28, 2026

chore(v4finance): remove dead V4_FINANCE_CURRENCIES constant after #441/#442/#443 merge #448

Closed

3 tasks

axisrow added 2 commits May 29, 2026 00:00

Merge branch 'main' into feat/milestone-22-close-433-pay-campaigns-v2

b2d1cca

# Conflicts: # CHANGELOG.md # README.md

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

axisrow deleted the feat/milestone-22-close-433-pay-campaigns-v2 branch May 28, 2026 17:08

This was referenced May 28, 2026

Add typed V4 Live finance and shared-account commands #125

Closed

fix(v4)!: restore --currency for live finance methods (regression from #441/#442/#443) #450

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(v4): align pay-campaigns wire-body with docs; drop Currency (closes #433)#442

fix(v4): align pay-campaigns wire-body with docs; drop Currency (closes #433)#442
axisrow merged 5 commits into
mainfrom
feat/milestone-22-close-433-pay-campaigns-v2

axisrow commented May 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

axisrow commented May 28, 2026

Uh oh!

axisrow commented May 28, 2026

Uh oh!

axisrow commented May 28, 2026

Uh oh!

claude Bot commented May 28, 2026 •

edited

Loading

Uh oh!

axisrow commented May 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

axisrow commented May 28, 2026

Summary

Changes

Verification

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!

axisrow commented May 28, 2026

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!

Review

Findings

Verdict

Uh oh!

axisrow commented May 28, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude Bot commented May 28, 2026 •

edited

Loading