Write the refunded invoice status, and keep nexus measurement intact - #3
Merged
Conversation
`InvoiceStatus::Refunded` existed in the enum and was assigned nowhere in the app. Its only other appearance was RefundInvoiceAction's approval preview, which shows the operator `status → refunded` before they approve — the refund then executed and left the invoice `paid`. The console promised a transition the code never performed, to the one person whose job was to check it. Set cumulatively rather than per-refund: several partial refunds that together cover the invoice leave it as fully reversed just as one full refund does. A partial refund still leaves it `Paid`, which is correct — it IS paid, with part reversed, and the credit notes carry the detail. NEXUS CONSEQUENCE, handled deliberately rather than inherited. The economic-nexus ledger filters on [Open, Paid]. Until now a fully-refunded invoice stayed `Paid` and was measured; writing the real status would have silently dropped it, REDUCING measured US sales as a side effect of an unrelated fix — the kind of change that surfaces years later as a missed registration. `Refunded` is therefore counted explicitly, which is also the safer reading: thresholds are generally stated on gross sales, and the errors are asymmetric — over-counting prompts a registration that may not be needed, under-counting misses one that is, which is back taxes plus penalties. Where a state permits netting returns, that is a per-state refinement against the dataset's rules, not a blanket exclusion. Both tests verified to fail with the transition removed.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
InvoiceStatus::Refundedexisted in the enum and was assigned nowhere in the app. Its only other appearance was the approval preview, which shows the operatorstatus → refundedbefore they approve — the refund then executed and left the invoicepaid. The console promised a transition the code never performed, to the one person whose job was to check it.Set cumulatively: several partial refunds that together cover the invoice leave it fully reversed just as one full refund does. A partial refund still leaves it
Paid, which is correct.The part worth reviewing
The economic-nexus ledger filters on
[Open, Paid]. Until now a fully-refunded invoice stayedPaidand was measured — so writing the real status would have silently reduced measured US sales as a side effect of an unrelated fix. That surfaces years later as a missed registration.Refundedis therefore counted explicitly, which is also the safer reading: thresholds are generally stated on gross sales, and the errors are asymmetric — over-counting prompts a registration that may not be needed; under-counting misses one that is, which is back taxes plus penalties. Where a state permits netting returns, that's a per-state refinement against the dataset's rules, not a blanket exclusion to assume here.This is the one judgement call in the PR — if you'd rather net refunds out of nexus sales, it's a one-line change plus a per-state rule.
Gate
pint · PHPStan level max (0 errors) · 1021 tests — green. Both tests verified to fail with the transition removed.