Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions app/(dashboard)/sales/[id]/so-detail-client.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1007,6 +1007,12 @@ export function SoDetailClient({ order: so, warehouses, currencies, externalOrde
</button>
</span>
)}
{/* audit-M-o2c: the paid→unpaid mismatch from deleting a payment on an
advanced-status order is recorded as a payment_status_mismatch WARNING
activity log (the durable, accurate signal). A read-time chip on
`!paidAt` alone can't tell "shipped on credit, never paid" from
"was paid then unpaid", so it isn't shown here; the existing
Paid / Part. Paid indicators cover the payment state. */}

{canRefund && (
<Button type="button" variant="outline" size="sm" onClick={() => setShowRefund(true)} disabled={isPending}>
Expand Down
36 changes: 34 additions & 2 deletions app/actions/sales.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ import {
} from '@/lib/domain/sales/sales-order-tax-validation'
import { isExternalRefundIdUniqueConflict } from '@/lib/domain/sales/refund-idempotency'
import { shouldWarnPaidWithoutInvoice } from '@/lib/domain/sales/paid-without-invoice'
import { isPaymentStatusMismatch } from '@/lib/domain/sales/o2c-guards'
import {
cancelSalesOrderFulfillmentState,
updateSalesOrderStatusUnderLock,
Expand Down Expand Up @@ -1245,12 +1246,21 @@ export async function applySalesOrderStatusTransition(
orderNumber: true,
externalOrderNumber: true,
status: true,
archived: true,
shipFromWarehouseId: true,
shoppingLinks: { where: { connector: 'woocommerce' }, select: { id: true }, take: 1 },
lines: { select: { id: true, productId: true, sku: true, qty: true } },
},
})
if (!so) return { success: false, error: 'Order not found' }
// audit-M-o2c: an archived order is filed away — block MANUAL status edits.
// Automated data-pushes still apply: the WooCommerce force-sync
// (internalBypassToken) and the sessionless delivery cron (skipPermissionCheck)
// are carrier/source-of-truth signals, so they bypass this guard — otherwise an
// archived-but-shipped order could never auto-reach DELIVERED.
if (so.archived && !bypassPermission && !options?.skipPermissionCheck) {
return { success: false, error: 'This order is archived; unarchive it before changing its status.' }
}

const transition = validateManualSalesOrderStatusTransition(so.status, targetStatus, {
bypass: bypassPermission,
Expand Down Expand Up @@ -2390,6 +2400,8 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
externalOrderNumber: true,
currency: true,
totalForeign: true,
status: true,
paidAt: true,
},
})
if (!so) return { error: 'Order not found' }
Expand All @@ -2401,6 +2413,7 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
return { error: 'Payment not found for this order' }
}
await tx.payment.delete({ where: { id: paymentId } })
let becameUnpaid = false
if (!payment.refundId) {
const remainingPayments = await tx.payment.findMany({
where: { orderId, refundId: null },
Expand All @@ -2410,12 +2423,17 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
if (p.currency !== so.currency) return sum
return sum + Number(p.amount)
}, 0)
const stillFullyPaid = totalPaid >= Number(so.totalForeign) - 0.0001
// Only a genuine paid → not-paid transition is a mismatch. An order that
// was never fully paid (e.g. shipped on credit terms) isn't flagged just
// because a partial payment was removed.
becameUnpaid = so.paidAt !== null && !stillFullyPaid
await tx.salesOrder.update({
where: { id: orderId },
data: { paidAt: totalPaid >= Number(so.totalForeign) - 0.0001 ? undefined : null },
data: { paidAt: stillFullyPaid ? undefined : null },
})
}
return { so, payment: { refundId: payment.refundId, amount: Number(payment.amount), currency: payment.currency } }
return { so, becameUnpaid, payment: { refundId: payment.refundId, amount: Number(payment.amount), currency: payment.currency } }
}, STOCK_TX_OPTIONS)
if ('error' in txResult) return { success: false, error: txResult.error }
if (!txResult.payment.refundId) {
Expand Down Expand Up @@ -2461,6 +2479,20 @@ export async function deletePayment(paymentId: string, orderId: string): Promise
description: `Deleted payment from order ${getSalesOrderReference(txResult.so)}`,
metadata: { orderNumber: getSalesOrderReference(txResult.so), paymentId },
})
// audit-M-o2c: deleting the last payment clears paidAt but does not revert
// status — flag the mismatch when the order has already advanced past
// payment (shipped/completed) so it doesn't sit silently unpaid-but-shipped.
if (isPaymentStatusMismatch(txResult.so.status, txResult.becameUnpaid)) {
await logActivity({
entityType: 'SALES_ORDER',
entityId: orderId,
action: 'payment_status_mismatch',
tag: 'sales',
level: 'WARNING',
description: `Order ${getSalesOrderReference(txResult.so)} is ${txResult.so.status} but is no longer fully paid after deleting a payment. Review whether the status should be reverted.`,
metadata: { orderNumber: getSalesOrderReference(txResult.so), status: txResult.so.status, paymentId },
})
}
return { success: true }
} catch (e) {
await logActivity({
Expand Down
16 changes: 16 additions & 0 deletions docs/workflows.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,22 @@ state (e.g. cancelled or refunded after dispatch) between the poll's SHIPPED
query and the under-lock write, the guard rejects the change and the cron logs a
`delivery_status_skipped` warning instead of forcing it.

**`COMPLETED` vs `DELIVERED`.** Both are manual, operator-set terminal-ish
statuses reached from `SHIPPED` (and `COMPLETED → DELIVERED`). `DELIVERED` means
the carrier confirmed delivery and can be set automatically by the delivery-status
cron; `COMPLETED` is a manual "this order is done from our side" marker for
businesses that don't track delivery (no automatic trigger sets it). Neither is
forced by any sync; both still allow `PARTIALLY_REFUNDED`/`REFUNDED` afterwards.
Use `DELIVERED` when delivery tracking is in play, `COMPLETED` otherwise.

Manual status edits are rejected on **archived** orders (unarchive first);
automated pushes (WooCommerce force-sync, the delivery-status cron) still apply.
Deleting a payment that takes an already-paid order in an advanced status
(`SHIPPED`/`COMPLETED`/`DELIVERED`/`PARTIALLY_REFUNDED`) back below fully-paid
does not auto-revert the status but raises a `payment_status_mismatch` warning
activity log so the operator can decide (it fires only on a genuine paid→unpaid
transition, not for orders that were never fully paid, e.g. credit terms).

### Shipments

| Status | Allowed next statuses | Notes |
Expand Down
43 changes: 43 additions & 0 deletions lib/domain/sales/o2c-guards.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
// ---------------------------------------------------------------------------
// Order-to-cash guard predicates (audit-M-o2c)
//
// Small pure helpers for the order-to-cash medium-gap fixes, extracted so the
// numeric/threshold logic is unit-testable without a database.
// ---------------------------------------------------------------------------

export type SalesOrderStatusName = string

/** Fixed rounding slack for the cumulative-refund-vs-total check (a hair over a penny). */
export const REFUND_TOTAL_EPSILON = 0.011

/**
* Statuses that have advanced past payment. An order in one of these that is no
* longer fully paid (e.g. after deleting its last payment) is a status/payment
* mismatch worth surfacing.
*/
export const PAID_EXPECTED_SALES_STATUSES: ReadonlySet<string> = new Set([
'SHIPPED',
'COMPLETED',
'DELIVERED',
'PARTIALLY_REFUNDED',
])

/**
* Whether recording `thisRefund` on top of `previouslyRefunded` would push the
* cumulative refunded amount past the order total (within a fixed rounding
* epsilon). Cumulative — not per-refund — so N partial refunds can't each creep
* over a relative slack.
*/
export function refundWouldExceedOrderTotal(
thisRefund: number,
previouslyRefunded: number,
orderTotal: number,
epsilon: number = REFUND_TOTAL_EPSILON,
): boolean {
return thisRefund + previouslyRefunded > orderTotal + epsilon
}

/** Whether an order that just became not-fully-paid is in a status that expected payment. */
export function isPaymentStatusMismatch(status: string, becameUnpaid: boolean): boolean {
return becameUnpaid && PAID_EXPECTED_SALES_STATUSES.has(status)
}
6 changes: 5 additions & 1 deletion lib/domain/sales/refund-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import { roundQuantity, toDecimal, type DecimalInput } from '@/lib/domain/math/d
import { getSalesOrderReference } from '@/lib/sales-order-display'
import { validateRefundSalesOrderStatusUpdate } from '@/lib/domain/workflows/action-guards'
import { isFullRefundAmount } from '@/lib/domain/sales/refund-thresholds'
import { refundWouldExceedOrderTotal } from '@/lib/domain/sales/o2c-guards'
import {
isStockMovementIdempotencyConflict,
refundInboundMovementKey,
Expand Down Expand Up @@ -1342,7 +1343,10 @@ export async function createSalesOrderRefund(
select: { totalBase: true },
})
const previouslyRefunded = existingRefunds.reduce((sum, refund) => sum + refundBoundaryNumber(refund.totalBase), 0)
if (totalBase + previouslyRefunded > refundBoundaryNumber(so.totalBase) * 1.001) {
// audit-M-o2c: cumulative refunded must not exceed the order total, with a
// fixed rounding epsilon (not a 0.1% relative slack, which on a large order
// is pounds of headroom) so N partial refunds can't creep over.
if (refundWouldExceedOrderTotal(totalBase, previouslyRefunded, refundBoundaryNumber(so.totalBase))) {
return { error: 'Refund total would exceed order total' } as const
}

Expand Down
43 changes: 43 additions & 0 deletions tests/domain/sales/o2c-guards.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import assert from 'node:assert/strict'
import test from 'node:test'

import {
refundWouldExceedOrderTotal,
isPaymentStatusMismatch,
REFUND_TOTAL_EPSILON,
} from '@/lib/domain/sales/o2c-guards'

test('refund within the order total is allowed', () => {
assert.equal(refundWouldExceedOrderTotal(40, 50, 100), false)
})

test('refund exactly at the order total is allowed (epsilon slack)', () => {
assert.equal(refundWouldExceedOrderTotal(50, 50, 100), false)
})

test('single-shot full refund (no prior refunds) is always allowed', () => {
assert.equal(refundWouldExceedOrderTotal(100, 0, 100), false)
assert.equal(refundWouldExceedOrderTotal(9999.99, 0, 9999.99), false)
})

test('cumulative partial refunds cannot creep over the total (fixed epsilon, not relative)', () => {
// Old 0.1% relative slack on a £10,000 order allowed ~£10 over; the fixed
// epsilon rejects anything beyond a rounding penny.
assert.equal(refundWouldExceedOrderTotal(1, 10000, 10000), true)
// A sub-penny rounding remainder is still tolerated.
assert.equal(refundWouldExceedOrderTotal(0.01, 9999.99, 10000), false)
assert.ok(REFUND_TOTAL_EPSILON < 0.02)
})

test('isPaymentStatusMismatch: advanced status + became unpaid → mismatch', () => {
assert.equal(isPaymentStatusMismatch('SHIPPED', true), true)
assert.equal(isPaymentStatusMismatch('COMPLETED', true), true)
assert.equal(isPaymentStatusMismatch('DELIVERED', true), true)
assert.equal(isPaymentStatusMismatch('PARTIALLY_REFUNDED', true), true)
})

test('isPaymentStatusMismatch: pre-payment status or still-paid → no mismatch', () => {
assert.equal(isPaymentStatusMismatch('PROCESSING', true), false)
assert.equal(isPaymentStatusMismatch('ALLOCATED', true), false)
assert.equal(isPaymentStatusMismatch('SHIPPED', false), false)
})
Loading