feat(v4)!: drop --currency from transfer-money to match docs 1:1 (closes #432) by axisrow · Pull Request #441 · axisrow/direct-cli

axisrow · 2026-05-28T12:04:59Z

Summary

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

The prior CLI implementation sent Currency on every FromCampaigns/ToCampaigns item, which is not part of the docs-defined wire-shape. To match the docs 1:1, this PR removes --currency from the command and drops Currency from the wire-body items.

Changes

Remove --currency option from direct v4finance transfer-money.
Drop Currency from FromCampaigns/ToCampaigns items.
Update direct_cli/v4_contracts.py:TransferMoney example_param and notes (Docs-verified 2026-05-28).
Update tests/test_v4finance_money.py:test_transfer_money_dry_run_*.
README example no longer passes --currency.
Bump version to 0.3.15; CHANGELOG entry under BREAKING CHANGES.

Migration

Before:

direct v4finance transfer-money --from-campaign-id 123 --to-campaign-id 456 --amount 100.50 --currency RUB --finance-token TOKEN --operation-num 42 --dry-run

After:

direct v4finance transfer-money --from-campaign-id 123 --to-campaign-id 456 --amount 100.50 --finance-token TOKEN --operation-num 42 --dry-run

The API takes Sum in conventional units; the currency is implied by the campaign, not by the CLI option.

Test plan

pytest tests/test_v4finance_money.py -k transfer_money — 3 passing.
pytest tests/test_v4finance_money.py tests/test_v4finance_read.py tests/test_comprehensive.py tests/test_cli.py — 129 passing.
direct v4finance transfer-money --help — --currency absent.

Verification

Live API was not re-verified due to absence of finance credentials in this session. If the API turns out to also accept the extra Currency field silently, no regression — but the docs-canonical shape is now what we send.

Closes #432. Part of milestone 22.

🤖 Generated with Claude Code

Closes #432. BREAKING CHANGE. Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/TransferMoney and confirmed the PayCampElement object carries only CampaignID and Sum; the Sum is in conventional units ("условные единицы") with no Currency field anywhere in the request. The prior CLI sent Currency on every FromCampaigns/ToCampaigns item, which is not part of the docs-defined wire-shape. Changes: - Remove --currency option from direct v4finance transfer-money. - Drop Currency from the FromCampaigns/ToCampaigns items. - Update v4_contracts.py:TransferMoney example_param and notes (Docs-verified 2026-05-28). - Update tests/test_v4finance_money.py:test_transfer_money_dry_run_*. - README example no longer passes --currency. - Bump version to 0.3.15; CHANGELOG entry under BREAKING CHANGES. 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.

Three sentences in TransferMoney.notes all restated "Currency is absent from the wire". Consolidate to a single docs-verified statement. Also update tests/test_v4_contracts.py fixture to drop the stale Currency field that no longer appears in example_param.

axisrow · 2026-05-28T16:50:33Z

@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-28T16:50:52Z

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

Claude Code Review

Read changed files
Check for breaking-change implications (call sites, smoke scripts, sandbox/dangerous matrix)
Verify CHANGELOG + version bump consistency
Verify test fixtures match new wire-shape
Post review

No critical/blocking issues. Reviewed under the "bugs, security, data-loss, performance only" lens you asked for.

Why the production-harm risk is essentially zero

direct v4finance transfer-money is dry-run-only: _require_dry_run(dry_run) runs before any client construction (direct_cli/commands/v4finance.py:483), and test_v4finance_money_commands_require_dry_run_before_api_call (tests/test_v4finance_money.py:313) pins that behavior. The command literally cannot move money in this release — so dropping Currency from the wire body can't trigger a failed live transfer, even in the worst case where the docs were wrong and the API actually required it.
Docs verification path is sound: PayCampElement per dg-v4/reference/TransferMoney is CampaignID + Sum. Removing Currency makes the wire shape strictly match the published contract.
BREAKING semantics are correctly signaled: major-version-respecting BREAKING CHANGES heading in CHANGELOG.md, 0.3.14 → 0.3.15 bump, README example updated, migration shown in the PR body.
V4_FINANCE_CURRENCIES is still imported & used by create-invoice and pay-campaigns (direct_cli/commands/v4finance.py:340, :527), so no dead-code/import leftover.
Test fixtures updated consistently in both tests/test_v4_contracts.py:198-204 and tests/test_v4finance_money.py:89-97 — both now omit Currency, matching the new wire shape.

Non-blocking follow-ups (doc drift from this PR)

These are documentation-only and don't break runtime, but they're direct fallout from this PR and worth a quick cleanup before merge or in an immediate follow-up:

tests/API_COVERAGE.md:62-63 still asserts:

"The docs-backed PayCampaigns and TransferMoney contracts require Currency on each payment/transfer item."

Now inaccurate for TransferMoney. Suggest splitting the sentence so only PayCampaigns keeps the Currency requirement.

Fix this →
scripts/test_dangerous_commands.sh:24 checklist still prints:

direct v4finance transfer-money --from-campaign-id ... --to-campaign-id ... --amount ... --currency ... --dry-run

The --currency ... token would now error with No such option: --currency if anyone hand-copies the line. Script exits 1 by design (per CLAUDE.md) so it's not in CI, but the example is misleading.

Fix this →

Things I checked and found clean

No remaining "Currency" literal in any TransferMoney-related fixture / contract (grep confirmed).
No test asserts the stale API_COVERAGE.md sentence, so docs are advisory-only.
direct_cli/v4_contracts.py:78-99 notes are coherent and dated (2026-05-28); the error_code=353 invariant is auto-satisfied by the single-from/single-to CLI shape.
direct_cli/commands/v4account.py (separate account-management TransferMoney action) still keeps Currency as the docs/CLI require it for the v4account variant — this PR correctly does not touch that path.
README example updated; integration / sandbox matrices unchanged (the command is DANGEROUS in direct_cli/smoke_matrix.py:172 and was never auto-tested, per tests/test_v4_safety.py:6).

Verdict

Ship-safe. Two doc lines above are worth fixing — neither is a blocker, but both are stale specifically because of this PR.
· feat/milestone-22-close-432-transfer-money

…dangerous-commands example Direct fallout of dropping --currency from transfer-money: - tests/API_COVERAGE.md still claimed TransferMoney requires Currency - scripts/test_dangerous_commands.sh checklist still passed --currency, which now errors with "No such option: --currency"

axisrow · 2026-05-28T16:55: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-28T16:55:44Z

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

Claude Code Review (round 2)

Read changed files (diff vs origin/main)
Check for breaking-change fallout (call sites, smoke scripts, dangerous matrix)
Verify wire-shape change is consistent across contracts + tests
Check CHANGELOG + version bump
Post review

No critical / blocking issues. Reviewed under the "bugs, security, data-loss, performance only" lens you asked for, and the two stale-docs follow-ups from my prior review (tests/API_COVERAGE.md, scripts/test_dangerous_commands.sh) have both been cleaned up in commits 29171af and bd78aac. Diff is consistent end-to-end.

Why the production-harm risk is essentially zero

direct v4finance transfer-money is dry-run-only: _require_dry_run(dry_run) at direct_cli/commands/v4finance.py:483 runs before any client construction, and test_v4finance_money_commands_require_dry_run_before_api_call pins that behavior. Even in the worst case where docs are wrong and the API actually required Currency, this command literally cannot move money in 0.3.15 — so there is no live-transfer failure path to regress.
Docs verification path is sound: PayCampElement per dg-v4/reference/TransferMoney is CampaignID + Sum. Removing Currency makes the wire shape strictly match the published contract.
V4_FINANCE_CURRENCIES is still imported & used by create-invoice and pay-campaigns (direct_cli/commands/v4finance.py:340, :527) — no dead-import leftover.

Cross-file consistency (the thing this PR could most easily get wrong)

The TransferMoney wire-shape is now {CampaignID, Sum} (no Currency) in every place it appears, including the docs:

File	Status
`direct_cli/commands/v4finance.py:493-505`	param dict drops `Currency` ✅
`direct_cli/v4_contracts.py:88-91`	`example_param` updated ✅
`tests/test_v4_contracts.py:198-204`	assertion matches ✅
`tests/test_v4finance_money.py:89-97`	dry-run fixture matches ✅
`tests/API_COVERAGE.md:62-64`	text now distinguishes `PayCampaigns` (keeps Currency) from `TransferMoney` (no Currency) ✅
`scripts/test_dangerous_commands.sh:24`	`--currency ...` token removed ✅
`README.md:194`	example updated ✅
`CHANGELOG.md:3-11`	BREAKING CHANGES entry + 0.3.14 → 0.3.15 bump ✅

The remaining Currency literals in tests/test_v4finance_money.py (lines 125, 155, 183, 522, 639) belong to pay-campaigns and create-invoice fixtures, which are correctly out of scope.

Things I checked and found clean

No remaining "Currency" literal in any TransferMoney-related fixture / contract.
direct_cli/commands/v4account.py (separate account-management TransferMoney action) still keeps --currency as the docs/CLI require it for the v4account variant — this PR correctly does not touch that path.
direct_cli/v4_contracts.py:78-99 notes are coherent and dated (Docs-verified 2026-05-28); the error_code=353 invariant note about total-from-sum = total-to-sum is auto-satisfied by the single-from/single-to CLI shape.
BREAKING semantics are correctly signaled: BREAKING CHANGES heading in CHANGELOG.md, 0.3.14 → 0.3.15 bump, README example updated, migration shown in the PR body.

Note on migration semantics

Worth noting for users hand-copying the migration example: removing --currency is a drop, not a rename — the value previously passed (e.g. --currency RUB) is now silently encoded by the campaign's own currency setting, not by an alias on the CLI. If a user was scripting cross-currency transfers via this flag (which the prior implementation actually allowed but the docs never sanctioned), that script silently changes behavior. The Yandex API is the only thing that can refuse cross-currency moves now, which is the docs-canonical state — so this is the right direction, just a real semantic shift to flag in support if anyone reports "my script stopped passing --currency and now everything's RUB."

Not a blocker — just a future-support breadcrumb. CHANGELOG language is accurate.

Verdict

Ship-safe. No issues to fix.
· feat/milestone-22-close-432-transfer-money

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 12:05

Copilot AI reviewed May 28, 2026

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

axisrow deleted the feat/milestone-22-close-432-transfer-money branch May 28, 2026 16:58

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

feat(v4)!: drop --currency from transfer-money to match docs 1:1 (closes #432)#441

feat(v4)!: drop --currency from transfer-money to match docs 1:1 (closes #432)#441
axisrow merged 3 commits into
mainfrom
feat/milestone-22-close-432-transfer-money

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!

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!

3 participants

Conversation

axisrow commented May 28, 2026

Summary

Changes

Migration

Test plan

Verification

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 Code Review

Why the production-harm risk is essentially zero

Non-blocking follow-ups (doc drift from this PR)

Things I checked and found clean

Verdict

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 Code Review (round 2)

Why the production-harm risk is essentially zero

Cross-file consistency (the thing this PR could most easily get wrong)

Things I checked and found clean

Note on migration semantics

Verdict

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

claude Bot commented May 28, 2026 •

edited

Loading

claude Bot commented May 28, 2026 •

edited

Loading