Skip to content

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

Merged
axisrow merged 4 commits into
mainfrom
feat/milestone-22-close-434-create-invoice
May 28, 2026
Merged

fix(v4): align create-invoice wire-body with docs; drop Currency (closes #434)#443
axisrow merged 4 commits into
mainfrom
feat/milestone-22-close-434-create-invoice

Conversation

@axisrow

@axisrow axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner

Summary

Non-breaking change. Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/CreateInvoice 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:create_invoice and _invoice_payments_param — drop Currency from Payments[] items; update help text on --currency.
  • direct_cli/v4_contracts.py:CreateInvoiceexample_param updated; notes record the docs verification (Docs-verified 2026-05-28).
  • tests/test_v4finance_money.py — assertions updated to expect wire-body without Currency.

Test plan

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

Closes #434. Part of milestone 22.

🤖 Generated with Claude Code

Closes #434.

Fetched https://yandex.ru/dev/direct/doc/dg-v4/reference/CreateInvoice
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:CreateInvoice example_param updated; notes record the
docs verification (Docs-verified 2026-05-28) and the rationale for
keeping --currency in CLI.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 28, 2026 16:00

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.

The v4 docs (dg-v4/reference/CreateInvoice) 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 create-invoice
- Simplify _invoice_payments_param helper (no more currency arg)
- Bump version 0.3.14 -> 0.3.15, add CHANGELOG BREAKING CHANGES entry
- Update README example, v4_contracts.py notes, and stale test assertion

Closes #434.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@axisrow

axisrow commented May 28, 2026

Copy link
Copy Markdown
Owner Author

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

Latest commit:

  • Removes --currency Click option entirely from create-invoice (was previously kept as no-op for backward compat).
  • Simplifies _invoice_payments_param helper (no more currency argument).
  • Bumps version 0.3.14 → 0.3.15, adds CHANGELOG entry under BREAKING CHANGES.
  • Updates README example, v4_contracts.py notes, 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/CreateInvoice

Tests: 1909 passed, 50 skipped.

Merge coordination: this PR, #441 (TransferMoney) and #442 (PayCampaigns) all bump version 0.3.14 → 0.3.15 and add the same ## 0.3.15 heading in CHANGELOG. Whoever merges last will need to rebase and merge the BREAKING CHANGES entries together.

@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

Copy link
Copy Markdown

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


Claude PR Review

  • Read PR diff vs origin/main
  • Inspect direct_cli/commands/v4finance.py (create-invoice, payments helper)
  • Inspect direct_cli/v4_contracts.py CreateInvoice contract
  • Inspect tests (test_v4finance_money.py, test_v4_contracts.py)
  • Check CHANGELOG/version bump consistency
  • Post final review

Verdict: LGTM — no critical issues found

Focused review per the request (bugs, security, logic errors, data loss, perf). I did not flag style, naming, or formatting.

What I checked

1. Wire-body change is correct and complete

direct_cli/commands/v4finance.py:37-69_invoice_payments_param now builds Payments[] items as {CampaignID, Sum} only. Matches the docs-verified shape stated in the PR. All other validation (positive int campaign IDs, uniqueness, --payment syntax, parse_v4_money_sum for amounts) is preserved.

2. Click option removal is internally consistent

  • --currency option is gone from create_invoice (v4finance.py:325-366).
  • The currency parameter is gone from the create_invoice function signature (v4finance.py:368-378).
  • The helper signature dropped currency: str (line 37) and the only caller passes the new arity (line 388).
  • Confirmed via grep that _invoice_payments_param has no other callers in direct_cli/ or tests/, so the signature change is safe.

3. Contract + tests + docs match

  • direct_cli/v4_contracts.py:166example_param updated; notes records the 2026-05-28 docs verification and the 0.3.15 BREAKING bump.
  • tests/test_v4_contracts.py:226 — asserted body matches new shape.
  • tests/test_v4finance_money.py:151-184, 631 — all Currency assertions removed; the renamed test still covers the multi-payment path; helper-only path (test_create_invoice_can_compute_finance_token_from_master_token) still covers token derivation.
  • tests/API_COVERAGE.md, README.md example, and CHANGELOG.md entry all line up with the change.

4. Versioning / release hygiene

5. No security or data-loss concerns

  • --currency was already not forwarded to the wire in the previous commit on this branch (dc39463), so removing it cannot lose data that was previously reaching Yandex. The CLI was already aligned with the docs at the wire level; this just removes the misleading user-facing knob.
  • finance_token masking and --dry-run semantics are unchanged.
  • No new input parsing, no new IO, no auth flow change.

Minor observation (non-blocking)

V4_FINANCE_CURRENCIES at direct_cli/commands/v4finance.py:18 is now an unused module-level constant — it was the click.Choice(...) source for the dropped --currency flag, and after #441 / #442 / this PR it has no remaining references anywhere in direct_cli/ or tests/ (grep -rn "V4_FINANCE_CURRENCIES" returns only the definition itself). Not a production risk, just a loose end from the three-PR refactor — safe to delete in a follow-up, or fold into whichever of #441/#442/#443 merges last during the rebase the author already plans to do. Fix this →
• branch: feat/milestone-22-close-434-create-invoice

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/.
@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

Copy link
Copy Markdown

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


Claude PR Re-Review (focused: critical issues only)

  • Read PR diff vs origin/main
  • Inspect direct_cli/commands/v4finance.py (create-invoice, payments helper)
  • Inspect direct_cli/v4_contracts.py CreateInvoice contract
  • Inspect updated tests
  • Verify version/CHANGELOG consistency and check for residual --currency / Currency references
  • Post final critical-issues review

Verdict: LGTM — no critical issues found

Focused on bugs, security, logic errors, data-loss, performance per the request. No style/naming nits.

What I verified

1. Wire-body change is correct and internally consistent

  • _invoice_payments_param (direct_cli/commands/v4finance.py:36-68) now emits {CampaignID, Sum} only — matches docs-verified PayCampElement.
  • The dropped --currency option is fully gone: removed from the Click decorators (v4finance.py:324-365), from the function signature (create_invoice at v4finance.py:367-377), and from the helper signature/body (no orphan currency / normalized_currency variables).
  • The single caller passes the new arity (v4finance.py:387); grep confirms no other callers of _invoice_payments_param in direct_cli/ or tests/.
  • The earlier review's loose-end (V4_FINANCE_CURRENCIES) is now removed in this PR (v4finance.py:18 is gone). No residual references anywhere in repo.

2. No data-loss risk from the BREAKING removal

  • Prior commit on this branch (dc39463) already stopped forwarding --currency to the wire-body, so removing the flag in this commit can't lose information that was reaching Yandex. The previous behaviour was just a silently-ignored knob.

3. Contract, tests, and docs are aligned

  • direct_cli/v4_contracts.py:166-176example_param and notes updated; docs verification dated 2026-05-28; 0.3.15 BREAKING note recorded.
  • tests/test_v4_contracts.py:226 and tests/test_v4finance_money.py:151-184, 631 — all Currency assertions removed, multi-payment coverage preserved under the renamed test.
  • tests/API_COVERAGE.md, README.md example, and CHANGELOG.md 0.3.15 BREAKING CHANGES section all match the change.
  • grep "create-invoice.*[Cc]urrency" over tests/, direct_cli/, scripts/ returns nothing — no stale assertion was left behind.

4. Versioning and merge coordination

5. No security or perf concerns

  • No new IO, parsing, auth, or token-handling code paths. finance_token masking and --dry-run semantics unchanged.
  • Removing an unused Click option/constant is a pure surface-area reduction; no performance implication.

Findings: none blocking

Nothing else to flag at the requested severity level.
• branch: feat/milestone-22-close-434-create-invoice

@axisrow
axisrow merged commit 2308ef3 into main May 28, 2026
6 checks passed
@axisrow
axisrow deleted the feat/milestone-22-close-434-create-invoice branch May 28, 2026 18:29
axisrow added a commit that referenced this pull request May 28, 2026
The opt-in live-write test for CreateInvoice still built its request
body with "Currency": "RUB", a leftover from before the docs-1:1
alignment (PR #443, commit 2308ef3) that removed Currency from the
CreateInvoice wire body. The test is gated by
YANDEX_DIRECT_LIVE_FINANCE_WRITE=1 so it never failed in a normal
pytest run, but a real run would have sent a field the official
CreateInvoice docs do not define. Align the test body with the docs.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
axisrow added a commit that referenced this pull request May 29, 2026
…#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 <bemyownrobot@gmail.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v4finance create-invoice: typed CLI command (dry-run only)

3 participants