fix(webshop-orders): stop syncing failed WooCommerce orders, remove stale unpaid rows (#2119)

* fix(webshop-orders): stop importing failed WooCommerce orders, remove stale rows on failed transition

Failed checkouts carry no money event but imported as permanently
unbookable 'Ej betald' rows (user report). orderImports() now excludes
'failed' alongside 'trash', and a re-polled order that transitioned to
failed deletes its existing row via removeWebshopOrders(), which enforces
the freeze boundary app-side: frozen rows are never deleted, and a parent
with a frozen refund child is spared because parent_order_id cascades.
Removal failures hold the sync cursor like upsert failures do.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014hQZLCdT56j8nAyoQtAs2C

* fix(webshop-orders): make failed-order removal race-safe per skeptic findings

Repeat every guard on the DELETE statement itself, not only the candidate
select: a row booked/marked/invoiced between the two round trips must
survive (TOCTOU refutation). Never remove paid rows (orderRemoves gated on
!orderIsPaid plus is_paid=false on both statements): money moved at some
point, and paid parents are the only rows that can carry refund children,
which also closes the cascade race without a DB trigger. Spare
cross-marked rows (legacy_transaction_id): the order may be booked via the
retired transactions feed without any freeze column set.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014hQZLCdT56j8nAyoQtAs2C

* fix(webshop-orders): audit-log successful failed-order removals

Compliance swarm finding (ISO A.8.10): the hard delete logged only its
failure path. Every successful removal batch now logs companyId, deleted
row ids and the requested external_ids, the only deletion record for
pre-bokforing rows that carry no behandlingshistorik.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014hQZLCdT56j8nAyoQtAs2C

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Mattsson
2026-09-01 15:55:36 +02:00
committed by GitHub
co-authored by Claude Fable 5
parent fa69174aa0
commit a57a8d968b
7 changed files with 351 additions and 12 deletions
@@ -279,6 +279,7 @@ describe('woocommerce extension routes', () => {
inserted: 4,
updated: 0,
unchanged: 0,
removed: 0,
frozenFlagged: 0,
crossMarked: 0,
errors: 0,
@@ -14,9 +14,10 @@ vi.mock('../lib/api-client', () => ({
vi.mock('@/lib/webshop-orders/ingest', () => ({
upsertWebshopOrders: vi.fn(),
removeWebshopOrders: vi.fn(),
}))
import { upsertWebshopOrders } from '@/lib/webshop-orders/ingest'
import { removeWebshopOrders, upsertWebshopOrders } from '@/lib/webshop-orders/ingest'
import type { WebshopOrderUpsert } from '@/lib/webshop-orders/types'
import { encryptCredential } from '../lib/credentials'
import {
@@ -28,6 +29,7 @@ import {
mapRefundToWebshopRow,
orderImports,
orderIsPaid,
orderRemoves,
syncWooCommerceOrders,
wooOrderExternalId,
wooRefundExternalId,
@@ -146,6 +148,7 @@ beforeEach(() => {
listOrdersPage.mockResolvedValue([])
listOrderRefunds.mockResolvedValue([])
vi.mocked(upsertWebshopOrders).mockResolvedValue({ ...emptyUpsertResult })
vi.mocked(removeWebshopOrders).mockResolvedValue({ removed: 0, errors: 0 })
})
describe('frozen external_id formats', () => {
@@ -176,12 +179,22 @@ describe('frozen external_id formats', () => {
})
})
describe('orderImports / orderIsPaid', () => {
it('every non-trash status imports, paid or not', () => {
describe('orderImports / orderRemoves / orderIsPaid', () => {
it('every status except trash and failed imports, paid or not', () => {
expect(orderImports(makeOrder())).toBe(true)
expect(orderImports(makeOrder({ status: 'pending', date_paid_gmt: null }))).toBe(true)
expect(orderImports(makeOrder({ status: 'refunded' }))).toBe(true)
expect(orderImports(makeOrder({ status: 'trash' }))).toBe(false)
expect(orderImports(makeOrder({ status: 'failed', date_paid_gmt: null }))).toBe(false)
})
it('only unpaid failed orders trigger removal of an existing row', () => {
expect(orderRemoves(makeOrder({ status: 'failed', date_paid_gmt: null }))).toBe(true)
// A failed order that was at some point paid keeps its row: money moved.
expect(orderRemoves(makeOrder({ status: 'failed' }))).toBe(false)
expect(orderRemoves(makeOrder({ status: 'trash', date_paid_gmt: null }))).toBe(false)
expect(orderRemoves(makeOrder())).toBe(false)
expect(orderRemoves(makeOrder({ status: 'cancelled' }))).toBe(false)
})
it('is_paid follows date_paid', () => {
@@ -563,6 +576,57 @@ describe('syncWooCommerceOrders', () => {
expect((rows as WebshopOrderUpsert[])[0]).toMatchObject({ is_paid: false })
})
it('removes the existing row of a failed order instead of importing it', async () => {
const { client, updates } = makeSupabaseMock()
listOrdersPage.mockResolvedValueOnce([
makeOrder({
status: 'failed',
date_paid_gmt: null,
// A stray refunds stub must not trigger a refund fetch for a
// non-importing order.
refunds: [{ id: 9, reason: '', total: '-100.00' }],
}),
])
vi.mocked(removeWebshopOrders).mockResolvedValueOnce({ removed: 1, errors: 0 })
const summary = await syncWooCommerceOrders(client, makeConnection())
expect(summary).toMatchObject({ fetched: 1, inserted: 0, removed: 1, errors: 0 })
expect(upsertWebshopOrders).not.toHaveBeenCalled()
expect(listOrderRefunds).not.toHaveBeenCalled()
expect(removeWebshopOrders).toHaveBeenCalledWith(client, 'company-1', [
'woo_shop.example.se_order_1042',
])
// The removal is complete work: the cursor advances normally.
const cursors = cursorUpdates(updates)
expect(cursors).toHaveLength(1)
expect(cursors[0].values.last_order_synced_at).toBe('2026-08-01T09:05:00.000Z')
})
it('neither imports nor removes a paid order the store reports as failed', async () => {
const { client } = makeSupabaseMock()
listOrdersPage.mockResolvedValueOnce([makeOrder({ status: 'failed' })])
const summary = await syncWooCommerceOrders(client, makeConnection())
expect(summary).toMatchObject({ fetched: 1, inserted: 0, removed: 0, errors: 0 })
expect(upsertWebshopOrders).not.toHaveBeenCalled()
expect(removeWebshopOrders).not.toHaveBeenCalled()
})
it('holds the cursor below a page whose removal reported errors', async () => {
const { client, updates } = makeSupabaseMock()
listOrdersPage.mockResolvedValueOnce([makeOrder({ status: 'failed', date_paid_gmt: null })])
vi.mocked(removeWebshopOrders).mockResolvedValueOnce({ removed: 0, errors: 1 })
const summary = await syncWooCommerceOrders(client, makeConnection())
expect(summary.errors).toBe(1)
const cursors = cursorUpdates(updates)
expect(cursors).toHaveLength(1)
expect(cursors[0].values.last_order_synced_at).toBe('2026-08-01T09:04:59.000Z')
})
it('holds the cursor below an order whose refund fetch failed', async () => {
const { client, updates } = makeSupabaseMock()
const order = makeOrder({ refunds: [{ id: 77, reason: '', total: '-250.00' }] })
@@ -1,5 +1,5 @@
import type { SupabaseClient } from '@supabase/supabase-js'
import { upsertWebshopOrders } from '@/lib/webshop-orders/ingest'
import { removeWebshopOrders, upsertWebshopOrders } from '@/lib/webshop-orders/ingest'
import type { WebshopOrderUpsert } from '@/lib/webshop-orders/types'
import { createLogger, type Logger } from '@/lib/logger'
import { roundOre as round } from '@/lib/money'
@@ -21,8 +21,9 @@ const defaultLog = createLogger('woocommerce/order-sync')
* public.webshop_orders (the Orders page), replacing the earlier
* transactions-inbox feed.
*
* Every non-trash order imports (including unpaid ones: the Orders page shows
* order state and the invoice flow needs pre-payment orders), carrying the
* Every order imports except trash and failed (including unpaid ones: the
* Orders page shows order state and the invoice flow needs pre-payment
* orders), carrying the
* full booking underlag the wc/v3 payload already contains: customer billing
* snapshot, payment method, per-rate VAT breakdown and line items. Refunds
* are separate negative rows parented to their order. Nothing here books
@@ -113,6 +114,8 @@ export interface WooCommerceSyncSummary {
updated: number
/** Re-polled rows with nothing new. */
unchanged: number
/** Rows deleted because the store now reports the order as failed. */
removed: number
/** Booked rows whose financials drifted remotely (flagged, not touched). */
frozenFlagged: number
/** Rows linked to a row the retired transactions feed already imported. */
@@ -157,9 +160,26 @@ export function orderIsPaid(order: Pick<WooOrder, 'date_paid_gmt'>): boolean {
return Boolean(order.date_paid_gmt)
}
/** Every non-trash order imports; trash is the store's recycle bin. */
/**
* Which orders exist in the feed. Trash is the store's recycle bin; failed
* is a checkout whose payment never went through, so it carries no money
* event (user report: failed attempts flooded the Orders page as permanently
* unbookable "Ej betald" rows).
*/
export function orderImports(order: Pick<WooOrder, 'status'>): boolean {
return order.status !== 'trash'
return order.status !== 'trash' && order.status !== 'failed'
}
/**
* Orders whose already-imported rows should be REMOVED on re-poll: a pending
* order that transitions to failed would otherwise sit stale forever. A
* failed order that has a date_paid is NOT removed (skeptic finding): money
* moved at some point (gateway void, admin bulk-edit mistake), and deleting
* the row would hide a possibly real money event, the same reason trash
* keeps its long-standing skip-only semantics.
*/
export function orderRemoves(order: Pick<WooOrder, 'status' | 'date_paid_gmt'>): boolean {
return order.status === 'failed' && !orderIsPaid(order)
}
/**
@@ -467,6 +487,8 @@ export function mapRefundToWebshopRow(
interface PageRowsOutcome {
rows: WebshopOrderUpsert[]
/** external_ids of orders whose existing rows should be removed (failed). */
removalExternalIds: string[]
/**
* date_modified (ms) of every order whose refund rows are incomplete this
* run (fetch failed or skipped on deadline). The cursor must not advance
@@ -486,14 +508,28 @@ async function buildPageRows(
log: Logger,
deadlineMs?: number,
): Promise<PageRowsOutcome> {
const outcome: PageRowsOutcome = { rows: [], incompleteModifiedMs: [], hitDeadline: false }
const outcome: PageRowsOutcome = {
rows: [],
removalExternalIds: [],
incompleteModifiedMs: [],
hitDeadline: false,
}
for (const order of orders) {
if (orderRemoves(order)) {
outcome.removalExternalIds.push(wooOrderExternalId(storeScope, order.id))
}
// Non-importing orders contribute nothing else; skipping here also keeps
// their refunds out of the feed. Unreachable for trash in practice (the
// list call asks for status=any, which excludes trash), and a failed
// order's refund would parent to a row being removed: an unexplainable
// negative either way.
if (!orderImports(order)) continue
// A corrupt total is counted and logged, never silently identical to a
// zero-total order. Deliberately NOT held via the cursor: a permanently
// corrupt total would stall the whole feed forever, where a skipped row
// plus a loud error can be followed up.
if (orderImports(order) && orderAmountUnparseable(order)) {
if (orderAmountUnparseable(order)) {
summary.errors += 1
log.warn('unparseable order total; row skipped', {
orderId: order.id,
@@ -572,6 +608,7 @@ export async function syncWooCommerceOrders(
inserted: 0,
updated: 0,
unchanged: 0,
removed: 0,
frozenFlagged: 0,
crossMarked: 0,
errors: 0,
@@ -648,6 +685,20 @@ export async function syncWooCommerceOrders(
failureFloorMs = Math.min(failureFloorMs, firstMs - 1000)
}
}
if (page.removalExternalIds.length > 0) {
const removal = await removeWebshopOrders(
supabase,
connection.company_id,
page.removalExternalIds,
)
summary.removed += removal.removed
summary.errors += removal.errors
if (removal.errors > 0) {
// Same retry contract as failed upserts: hold the cursor so the
// next run re-lists this page and retries the removal.
failureFloorMs = Math.min(failureFloorMs, firstMs - 1000)
}
}
for (const ms of page.incompleteModifiedMs) {
failureFloorMs = Math.min(failureFloorMs, ms - 1000)
}