Skip to content

eops-370-fix(cli): add patch command so partial updates stop clearing fields - #21

Closed
mykhaylob-de wants to merge 1 commit into
mainfrom
eops-370-feat-patch-command
Closed

mykhaylob-de wants to merge 1 commit into
mainfrom
eops-370-feat-patch-command

Conversation

@mykhaylob-de

Copy link
Copy Markdown
Collaborator

Summary

update sends PUT, which is a full replace, so any field missing from the JSON file was silently blanked. The natural CLI workflow — write a small file, apply it — destroyed data, and on invoices it also flipped drafts to posted because the update schema defaults record_status to posted.

Twelve /public/v2 resources expose PATCH and the API already merges correctly under test (11 tests, including patch_items_absent_keeps_existing_lines), so this adds a patch command rather than having update fetch and merge client-side. Reimplementing the merge in the CLI would be both weaker and racy.

Changes

  • patch command on the twelve resources whose API exposes PATCH: invoices, bills, customer payments/credits/prepayments/prepayment-applications, customers, vendors, items, classifications, fixed assets, contracts
  • update keeps PUT semantics but reads the record first and warns which populated fields the payload would clear, confirming when a terminal is attached. Without a terminal it warns and continues, so existing scripted updates do not break on a prompt they cannot answer. --yes skips the warning and the read it needs, so batch runs do not pay an extra GET per record
  • update warns when omitting record_status would post a draft
  • the shared IJE-shaped post whitelist becomes a per-resource PostWritableFields (record fields, line fields, line key). post now only exists where a shape is declared, so enabling it on another resource cannot wipe fields outside a list written for intercompany journal entries
  • the warning skips server-assigned fields (id, number, created_at, totals…), otherwise every update would warn and the warning would stop meaning anything

Test plan

  • Unit tests pass (uv run pytest) — 180 passed, 125 skipped (integration tests need X_API_KEY)
  • Linter passes (uv run ruff check .) — clean, and ruff format --check clean
  • Manually tested with dualentry invoices patch / dualentry invoices update

New coverage:

  • tests/test_e2e_partial_update.py runs the built dualentry binary as a subprocess over a real socket against a stub v2 API. It proves a two-field patch leaves every other field byte-identical, and reproduces the original data loss: after update with only memo, customer_id and items are null, reference_number is "", and record_status has flipped draft → posted
  • tests/test_partial_updates.py covers the warning contents, the confirm/abort path, --yes, advisory-probe failures (API error, connect error, timeout), and that patch is present on exactly the resources whose API supports it

Reviewed independently with Cursor (gpt-5.3-codex-high); its two Important findings — the pre-read only caught APIError rather than transport failures, and the unconditional extra GET on every update — are fixed here.

… fields

`update` sends PUT, which is a full replace, so any field missing from the
JSON file was silently blanked. The natural CLI workflow - write a small file,
apply it - destroyed data.

Twelve /public/v2 resources expose PATCH and the API already merges correctly
under test, so add a `patch` command rather than having `update` fetch and
merge client-side; reimplementing the merge in the CLI would be both weaker
and racy.

- add a `patch` command on the twelve resources whose API exposes PATCH
- `update` keeps PUT semantics but now reads the record first and warns which
  populated fields the payload would clear, confirming when a terminal is
  attached. --yes skips the warning and the read it needs, so batch runs do
  not pay an extra GET per record
- warn when omitting record_status would post a draft: the update schema
  defaults record_status to posted, so a partial PUT silently promoted drafts
- replace the shared IJE-shaped `post` whitelist with a per-resource
  PostWritableFields, so enabling post elsewhere cannot wipe fields outside a
  list written for intercompany journal entries
- cover both paths end to end by running the built CLI against a stub v2 API
  over a real socket, which also reproduces the original PUT data loss
@mykhaylob-de
mykhaylob-de deleted the eops-370-feat-patch-command branch September 1, 2026 19:24
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.

1 participant