fix: preserve POS payment amounts during draft creation

[my-dev-jour]

Summary

• preserve cashier-entered POS payment rows across set_missing_values() in update_invoice()
• restore the original payment rows before totals/account assignment continue
• resync paid_amount and base_paid_amount after restoring payments
• add a focused regression test for the payment snapshot/restore helpers

Root Cause

ERPNext rebuilds POS payment rows during set_missing_values() from POS Profile defaults. POSNext was calling that flow during draft creation without restoring the cashier-entered payment rows, so a paid sale could be saved/submitted with zeroed or replaced payments and end up unpaid.

Validation

• validated on ERPNext 16.11.0 staging with a real POS flow
• confirmed submitted invoice preserved payment rows and showed outstanding_amount = 0
• local verification was limited to py_compile in this workspace

Notes

• this supersedes the earlier GL-focused hypothesis from PR fix: rebase POS sales invoice GL entries on ERPNext 16 #209; the staging test showed the immediate root cause was payment-row loss during draft creation

[my-dev-jour]

CI server passed. The remaining failing checks appear to be repo-level workflow/lint issues rather than regressions from this patch: the dependency audit cannot resolve ERPNext in CI, and the linter runs pre-commit with --all-files, which surfaces existing repo-wide formatting and Ruff issues outside this change.

[engahmed1190]

Code Review — RECOMMEND CLOSE (Superseded)

Summary

Fixes ERPNext's set_missing_values() wiping payment amounts by using a snapshot/restore pattern: captures payment rows before set_missing_values(), lets it run destructively, then restores the original rows.

Test Results

Both tests pass on nexus.local.

Superseded by commit fbf8e80b

This PR fixes the same bug as commit fbf8e80b on fix/payment-amount-wiped-by-erpnext, which is already merged. Here's the comparison:

Aspect	fbf8e80b (merged)	This PR (#215)
Approach	set_missing_values(for_validate=True)	Snapshot → let destructive run → restore
Lines changed	~25 (1 line change + comments)	~100+ (4 helper functions + tests)
Complexity	Prevents the problem	Lets it happen then undoes it
Risk	Relies on ERPNext's for_validate=True contract	More defensive, doesn't depend on ERPNext internals
Tests	None	2 passing unit tests

The for_validate=True approach is simpler and more maintainable. It prevents update_multi_mode_option() from running at all, rather than letting it destroy data and then restoring from a snapshot. The commit message explains why this is safe: POS Next already sets the fields that for_validate=True skips (customer, ignore_pricing_rule, tax_category).

What's Worth Keeping

The test file (test_invoices.py) has useful tests for payment snapshot/restore helpers. If you decide to keep the snapshot approach as a fallback in the future, these tests are valuable.

Code Quality (for the record)

The helper functions are well-structured:

• _clean_payment_row — strips parent metadata, handles both dict and object inputs
• _snapshot_invoice_payments / _restore_invoice_payments — clean capture/restore
• _sync_invoice_payment_amounts — correctly handles conversion_rate and pre-existing base_amount

Minor concern: _sync_invoice_payment_amounts has flt(payment.get("base_amount") or 0) or flt(...) — if base_amount is explicitly 0, it falls through to the calculation. Unlikely to matter in practice.

Verdict

Recommend closing — the bug is already fixed by a simpler approach. If there are concerns about for_validate=True skipping needed fields, those should be addressed as separate targeted fixes.

[engahmed1190]

Closing — superseded by commit fbf8e80 on fix/payment-amount-wiped-by-erpnext which fixes the same bug with a simpler approach (set_missing_values(for_validate=True)). See review comment for full comparison.