diff --git a/app/api/invoices/recurring/[id]/__tests__/route.test.ts b/app/api/invoices/recurring/[id]/__tests__/route.test.ts index 32a3c490..660c3c26 100644 --- a/app/api/invoices/recurring/[id]/__tests__/route.test.ts +++ b/app/api/invoices/recurring/[id]/__tests__/route.test.ts @@ -25,7 +25,9 @@ const chain: any = { const mockSupabase = { auth: { getUser: vi.fn() }, - from: vi.fn(() => chain), + // The table name is unused by the shared chain, but declared so a test can + // install a table-aware implementation of its own. + from: vi.fn((_table?: string) => chain), } vi.mock('@/lib/supabase/server', () => ({ @@ -147,3 +149,160 @@ describe('PATCH /api/invoices/recurring/[id] reactivation', () => { expect(updatePayloads[0]).toEqual({ auto_send: true }) }) }) + +/** + * A combined edit (header fields + items) writes two tables, which PostgREST + * cannot do atomically. Issue #1275: an item-insert failure used to leave the + * header update committed. These tests pin the compensating rollback and the + * partial-state error when a compensation itself fails. + */ +describe('PATCH /api/invoices/recurring/[id] combined edit rollback', () => { + const SCHEDULES = 'recurring_invoice_schedules' + const STAMP = '2026-07-30T09:00:00.000Z' + const headerRow: Record = { + id: 's-1', + name: 'Månadsavgift', + day_of_month: 25, + next_run_date: '2026-08-25', + updated_at: '2026-07-01T00:00:00Z', + } + + const storedItem = { + id: 'i-1', + created_at: '2026-07-01T00:00:00Z', + schedule_id: 's-1', + sort_order: 0, + description: 'Gammal rad', + quantity: 1, + unit: 'st', + unit_price: 500, + vat_rate: 25, + dimensions: {}, + } + + let itemsInsertErrors: (Record | null)[] = [] + let itemsSnapshot: Record[] = [] + let headerExists = true + const itemsInserts: Record[][] = [] + + beforeEach(() => { + vi.clearAllMocks() + updatePayloads.length = 0 + itemsInserts.length = 0 + itemsInsertErrors = [] + itemsSnapshot = [] + headerExists = true + mockSupabase.auth.getUser.mockResolvedValue({ data: { user: mockUser } }) + + let headerUpdateCall = 0 + let itemsInsertCall = 0 + mockSupabase.from.mockImplementation((table?: string) => { + if (table === SCHEDULES) { + const selectChain: Record = { + eq: () => selectChain, + single: () => Promise.resolve({ data: headerExists ? headerRow : null, error: null }), + maybeSingle: () => + Promise.resolve({ data: headerExists ? headerRow : null, error: null }), + } + return { + select: () => selectChain, + update: (payload: Record) => { + updatePayloads.push(payload) + const call = headerUpdateCall + headerUpdateCall += 1 + const chain: Record = { + eq: () => chain, + // The first update reads back updated_at; the rollback update + // reads back the matched ids. + select: () => + call === 0 + ? { maybeSingle: () => Promise.resolve({ data: { updated_at: STAMP }, error: null }) } + : Promise.resolve({ data: [{ id: 's-1' }], error: null }), + } + return chain + }, + } + } + const itemsChain: Record = { + // The snapshot is read through fetchAllRows, hence .order().range(). + select: () => ({ + eq: () => ({ + order: () => ({ + range: () => Promise.resolve({ data: itemsSnapshot, error: null }), + }), + }), + }), + delete: () => ({ eq: () => Promise.resolve({ error: null }) }), + insert: (rows: Record[]) => { + itemsInserts.push(rows) + const error = itemsInsertErrors[itemsInsertCall] ?? null + itemsInsertCall += 1 + return Promise.resolve({ error }) + }, + } + return itemsChain + }) + }) + + afterEach(() => { + // clearAllMocks does not reset implementations, so hand the shared chain + // back or the first describe breaks when the file order changes. + mockSupabase.from.mockImplementation(() => chain) + }) + + const items = [{ description: 'Rad A', quantity: 1, unit: 'st', unit_price: 1000 }] + + it('restores the prior header fields when the items insert fails', async () => { + itemsSnapshot = [storedItem] + itemsInsertErrors = [{ message: 'check violation', code: '23514' }, null] + + const { status, body } = await parseJsonResponse<{ error: { code: string } }>( + await PATCH(patchReq({ name: 'Nytt namn', items }), params), + ) + + expect(status).toBe(400) + // Clean rollback keeps the PG-mapped error, not the partial-state one. + expect(body.error.code).toBe('VALIDATION_ERROR') + expect(updatePayloads).toHaveLength(2) + expect(updatePayloads[1]).toEqual({ name: 'Månadsavgift' }) + // Items back too: the failed replace, then the snapshot restore. + expect(itemsInserts).toHaveLength(2) + expect(itemsInserts[1][0]).toMatchObject({ description: 'Gammal rad', schedule_id: 's-1' }) + }) + + it('returns the partial-state error when the items restore also fails', async () => { + itemsSnapshot = [storedItem] + itemsInsertErrors = [ + { message: 'check violation', code: '23514' }, + { message: 'restore boom' }, + ] + + const { status, body } = await parseJsonResponse<{ + error: { code: string; message: string } + }>(await PATCH(patchReq({ name: 'Nytt namn', items }), params)) + + expect(status).toBe(500) + expect(body.error.code).toBe('INVOICE_RECURRING_UPDATE_PARTIAL') + expect(body.error.message).toMatch(/halvsparat/) + }) + + it('writes no header row for an item-only edit', async () => { + const { status } = await parseJsonResponse(await PATCH(patchReq({ items }), params)) + + expect(status).toBe(200) + expect(updatePayloads).toHaveLength(0) + expect(itemsInserts).toHaveLength(1) + }) + + it('404s before writing the header when the schedule does not exist', async () => { + headerExists = false + + const { status, body } = await parseJsonResponse<{ type: string }>( + await PATCH(patchReq({ name: 'Nytt namn', items }), params), + ) + + expect(status).toBe(404) + expect(body.type).toBe('not_found') + expect(updatePayloads).toHaveLength(0) + }) +}) diff --git a/app/api/invoices/recurring/[id]/route.ts b/app/api/invoices/recurring/[id]/route.ts index 06c434e9..ca3c6659 100644 --- a/app/api/invoices/recurring/[id]/route.ts +++ b/app/api/invoices/recurring/[id]/route.ts @@ -1,8 +1,9 @@ import { NextResponse } from 'next/server' import { ensureInitialized } from '@/lib/init' import { withRouteContext } from '@/lib/api/with-route-context' -import { errorResponse } from '@/lib/errors/get-structured-error' +import { errorResponse, errorResponseFromCode } from '@/lib/errors/get-structured-error' import { UpdateRecurringScheduleSchema } from '@/lib/api/schemas' +import { applyRecurringScheduleUpdate } from '@/lib/invoices/apply-recurring-schedule-update' import { computeInitialRunDate, computeNextRunDate, @@ -162,22 +163,9 @@ export const PATCH = withRouteContext( } } - if (Object.keys(updateRow).length > 0) { - const { error: updateError } = await supabase - .from('recurring_invoice_schedules') - .update(updateRow) - .eq('id', id) - .eq('company_id', companyId) - - if (updateError) { - log.error('failed to update recurring schedule', updateError) - return errorResponse(updateError, log, { requestId }) - } - } - + // Existence check BEFORE any write: with items in the payload the request + // writes two tables, so a 404 must not leave a header update behind. if (items) { - // Replace items wholesale. Cheaper than diffing for a small list and - // matches how the UI form sends the full list back on every save. const { data: existing } = await supabase .from('recurring_invoice_schedules') .select('id') @@ -191,61 +179,51 @@ export const PATCH = withRouteContext( { status: 404 }, ) } + } - // Snapshot existing rows so we can restore them if the insert fails. - // Without this, a failed replace would leave the schedule with zero - // items and every subsequent cron run would throw "schedule has no - // items", silently skipping billing dates. - const { data: previousItems } = await supabase - .from('recurring_invoice_schedule_items') - .select('sort_order, description, quantity, unit, unit_price, vat_rate, dimensions') - .eq('schedule_id', id) + // Items are replaced wholesale. Cheaper than diffing for a small list and + // matches how the UI form sends the full list back on every save. Both + // writes are compensated on failure inside the shared helper. + const result = await applyRecurringScheduleUpdate(supabase, { + scheduleId: id, + companyId, + fields: updateRow, + items, + log, + }) - await supabase - .from('recurring_invoice_schedule_items') - .delete() - .eq('schedule_id', id) - - const itemRows = items.map((item, idx) => ({ - schedule_id: id, - sort_order: idx, - description: item.description, - quantity: item.quantity, - unit: item.unit, - unit_price: item.unit_price, - vat_rate: item.vat_rate ?? null, - dimensions: item.dimensions ?? {}, - })) - const { error: itemsError } = await supabase - .from('recurring_invoice_schedule_items') - .insert(itemRows) - if (itemsError) { - log.error('failed to replace schedule items', itemsError) - // Restore the snapshot so the schedule stays valid for the cron. - if (previousItems && previousItems.length > 0) { - const restoreRows = previousItems.map((row) => ({ - schedule_id: id, - sort_order: row.sort_order, - description: row.description, - quantity: row.quantity, - unit: row.unit, - unit_price: row.unit_price, - vat_rate: row.vat_rate, - dimensions: row.dimensions ?? {}, - })) - const { error: restoreError } = await supabase - .from('recurring_invoice_schedule_items') - .insert(restoreRows) - if (restoreError) { - log.error( - 'failed to restore schedule items after failed replace: schedule may be left empty', - restoreError, - { scheduleId: id }, - ) - } - } - return errorResponse(itemsError, log, { requestId }) + if (!result.ok) { + if (result.stage === 'header') { + log.error('failed to update recurring schedule', result.error) + return errorResponse(result.error, log, { requestId }) } + if (!result.itemsRestored || !result.headerRestored) { + // A compensation did not apply, so the schedule may be half-saved: say + // so instead of reporting a clean failure. Logged here with the repair + // context (errorResponseFromCode only records the code itself), which + // the clean-rollback path below does not need. + log.error('recurring schedule update left a partial state', result.error, { + scheduleId: id, + stage: result.stage, + itemsRestored: result.itemsRestored, + headerRestored: result.headerRestored, + }) + return errorResponseFromCode('INVOICE_RECURRING_UPDATE_PARTIAL', log, { + requestId, + // camelCase throughout, matching the pgCode key errorResponse itself + // merges into details for Postgres failures. + details: { + pgCode: result.error.code, + stage: result.stage, + itemsRestored: result.itemsRestored, + headerRestored: result.headerRestored, + }, + }) + } + // Clean rollback: keep the PG-mapped error so a CHECK violation still + // surfaces its specific Swedish message. errorResponse logs it, so no + // second log line here. + return errorResponse(result.error, log, { requestId }) } const { data: complete } = await supabase diff --git a/components/invoices/NewRecurringScheduleDialog.tsx b/components/invoices/NewRecurringScheduleDialog.tsx index a51219cb..d826e2f1 100644 --- a/components/invoices/NewRecurringScheduleDialog.tsx +++ b/components/invoices/NewRecurringScheduleDialog.tsx @@ -195,8 +195,13 @@ function NewRecurringScheduleForm({ }, ) if (!res.ok) { - const body = await res.json().catch(() => ({})) - throw new Error(body.error || t('create_failed_fallback')) + const body = (await res.json().catch(() => ({}))) as { + error?: string | { message?: string } + } + // errorResponse() returns the nested envelope { error: { code, message } }, + // so reading body.error directly would stringify to "[object Object]". + const message = typeof body.error === 'string' ? body.error : body.error?.message + throw new Error(message || t('create_failed_fallback')) } toast({ title: schedule ? t('updated_title') : t('created_title') }) onSaved() diff --git a/lib/errors/structured-errors.ts b/lib/errors/structured-errors.ts index 49a10331..d1c680c6 100644 --- a/lib/errors/structured-errors.ts +++ b/lib/errors/structured-errors.ts @@ -1125,6 +1125,13 @@ const INVOICE: Record = { message_sv: 'Fakturanumret tilldelades men fakturan kunde inte läsas tillbaka. Ladda om sidan och kontrollera fakturan.', message_en: 'The invoice number was assigned but the invoice could not be re-read. Reload the page and verify the invoice.', }, + INVOICE_RECURRING_UPDATE_PARTIAL: { + httpStatus: 500, + message_sv: + 'Ändringen av det återkommande schemat kunde inte slutföras och schemat kan ha hamnat i ett halvsparat läge. Öppna schemat och kontrollera både fält och rader innan du sparar igen.', + message_en: + 'The recurring schedule update failed and the compensating rollback did not fully apply: the schedule may be left in a partial state (header fields and items out of sync). Inspect the schedule fields and items before retrying.', + }, // Quotes / Offerter QUOTE_NOT_FOUND: { httpStatus: 404, diff --git a/lib/invoices/__tests__/apply-recurring-schedule-update.test.ts b/lib/invoices/__tests__/apply-recurring-schedule-update.test.ts new file mode 100644 index 00000000..22b3a54c --- /dev/null +++ b/lib/invoices/__tests__/apply-recurring-schedule-update.test.ts @@ -0,0 +1,502 @@ +import { describe, it, expect, vi } from 'vitest' +import type { SupabaseClient } from '@supabase/supabase-js' +import { + applyRecurringScheduleUpdate, + type RecurringScheduleItemInput, +} from '../apply-recurring-schedule-update' + +/** + * A combined edit writes two tables over PostgREST, which is not atomic: the + * header update commits before the item replace runs. These tests pin that BOTH + * writes are compensated, that an unchecked delete can no longer double-insert, + * and that a failed compensation is reported instead of swallowed (issue #1275). + */ + +const SCHEDULE_ID = 's-1' +const COMPANY_ID = 'company-1' +const STAMP = '2026-07-30T09:00:00.000Z' + +function makeItem( + overrides: Partial = {}, +): RecurringScheduleItemInput { + return { + description: 'Support', + quantity: 1, + unit: 'st', + unit_price: 5000, + ...overrides, + } +} + +/** A stored item row as SELECT * returns it (server columns included). */ +function storedItem(overrides: Record = {}): Record { + return { + id: 'item-old-1', + schedule_id: SCHEDULE_ID, + created_at: '2026-07-01T00:00:00Z', + sort_order: 0, + description: 'Gammal rad', + quantity: 2, + unit: 'st', + unit_price: 500, + vat_rate: 25, + dimensions: {}, + ...overrides, + } +} + +const headerRow = { + id: SCHEDULE_ID, + company_id: COMPANY_ID, + name: 'Månadsavgift', + day_of_month: 25, + default_dimensions: { '1': 'A' }, + next_run_date: '2026-08-25', + updated_at: '2026-07-01T00:00:00Z', +} + +const SCHEDULES = 'recurring_invoice_schedules' +const ITEMS = 'recurring_invoice_schedule_items' + +type Fail = { message: string; code?: string } | null + +/** + * Table-aware harness. Records every update payload and insert rows array per + * table, plus the `.eq()` filters used on the restore update, and allows + * injecting an error on the delete, the n:th items insert, or the header + * writes. + */ +function createHarness(opts: { + headerSnapshot?: Record | null + headerSnapshotError?: Fail + headerUpdateError?: Fail + /** Value of updated_at returned by the header update representation. */ + headerStamp?: string | null + /** Rows matched by the restore update's .select('id'). Default: one row. */ + headerRestoreMatched?: boolean + headerRestoreError?: Fail + itemsSnapshot?: Record[] | null + itemsSnapshotError?: Fail + itemsDeleteError?: Fail + /** Error for the n:th insert on the items table (index 0 = the replace). */ + itemsInsertErrors?: Fail[] +}) { + const updates: Record[]> = {} + const inserts: Record[][]> = {} + const eqFilters: Record = {} + const deletes: string[] = [] + let headerUpdateCall = 0 + let itemsInsertCall = 0 + + const tables = vi.fn((table: string) => { + if (table === SCHEDULES) { + return { + select: () => ({ + eq: () => ({ + eq: () => ({ + maybeSingle: () => + Promise.resolve({ + data: opts.headerSnapshotError + ? null + : opts.headerSnapshot === undefined + ? headerRow + : opts.headerSnapshot, + error: opts.headerSnapshotError ?? null, + }), + }), + }), + }), + update: (payload: Record) => { + ;(updates[table] ??= []).push(payload) + const call = headerUpdateCall + headerUpdateCall += 1 + const filters: unknown[] = [] + ;(eqFilters[`${table}:${call}`] ??= []).push(filters) + const chain: Record = { + eq: (...args: unknown[]) => { + filters.push(args) + return chain + }, + select: () => { + if (call === 0) { + return { + maybeSingle: () => + Promise.resolve({ + data: opts.headerUpdateError + ? null + : { + updated_at: + opts.headerStamp === undefined ? STAMP : opts.headerStamp, + }, + error: opts.headerUpdateError ?? null, + }), + } + } + // The restore update reads back the matched ids. + const matched = opts.headerRestoreMatched === false ? [] : [{ id: SCHEDULE_ID }] + return Promise.resolve({ + data: opts.headerRestoreError ? null : matched, + error: opts.headerRestoreError ?? null, + }) + }, + } + return chain + }, + } + } + return { + // The snapshot is read through fetchAllRows, so the chain ends in + // .order().range() rather than resolving straight after .eq(). + select: () => ({ + eq: () => ({ + order: () => ({ + range: () => + Promise.resolve({ + data: opts.itemsSnapshot === undefined ? [storedItem()] : opts.itemsSnapshot, + error: opts.itemsSnapshotError ?? null, + }), + }), + }), + }), + delete: () => ({ + eq: () => { + deletes.push(table) + return Promise.resolve({ error: opts.itemsDeleteError ?? null }) + }, + }), + insert: (rows: Record[]) => { + ;(inserts[table] ??= []).push(rows) + const error = opts.itemsInsertErrors?.[itemsInsertCall] ?? null + itemsInsertCall += 1 + return Promise.resolve({ error }) + }, + } + }) + + const log = { error: vi.fn() } + return { + supabase: { from: tables } as unknown as SupabaseClient, + from: tables, + updates, + inserts, + eqFilters, + deletes, + log, + } +} + +const insertBoom = { message: 'insert boom', code: '23514' } + +describe('applyRecurringScheduleUpdate', () => { + it('writes only the header for a header-only edit', async () => { + const h = createHarness({}) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { name: 'Nytt namn' }, + log: h.log, + }) + + expect(result).toEqual({ ok: true }) + expect(h.updates[SCHEDULES]).toEqual([{ name: 'Nytt namn' }]) + // No snapshot read: exactly one call against the schedules table, and the + // items table is never touched. + const tablesTouched = h.from.mock.calls.map((c) => c[0]) + expect(tablesTouched).toEqual([SCHEDULES]) + }) + + it('writes only the items for an item-only edit', async () => { + const h = createHarness({}) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: {}, + items: [makeItem({ description: 'Rad A' }), makeItem({ description: 'Rad B', vat_rate: 25 })], + log: h.log, + }) + + expect(result).toEqual({ ok: true }) + // The schedules table is READ (the ownership proof for the schedule_id-only + // item writes) but never written on an item-only edit. + expect(h.from.mock.calls.map((c) => c[0])).toContain(SCHEDULES) + expect(h.updates[SCHEDULES]).toBeUndefined() + expect(h.inserts[ITEMS]).toHaveLength(1) + expect(h.inserts[ITEMS][0]).toEqual([ + { + schedule_id: SCHEDULE_ID, + sort_order: 0, + description: 'Rad A', + quantity: 1, + unit: 'st', + unit_price: 5000, + vat_rate: null, + dimensions: {}, + }, + { + schedule_id: SCHEDULE_ID, + sort_order: 1, + description: 'Rad B', + quantity: 1, + unit: 'st', + unit_price: 5000, + vat_rate: 25, + dimensions: {}, + }, + ]) + }) + + it('rolls the header fields back when the items insert fails on a combined edit', async () => { + const h = createHarness({ + itemsSnapshot: [storedItem(), storedItem({ id: 'item-old-2', sort_order: 1, description: 'Rad 2' })], + itemsInsertErrors: [insertBoom, null], + }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5, default_dimensions: {} }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toEqual({ + ok: false, + stage: 'items_insert', + error: insertBoom, + itemsRestored: true, + headerRestored: true, + }) + // Second header update restores exactly the written keys, nothing else. + expect(h.updates[SCHEDULES]).toHaveLength(2) + expect(h.updates[SCHEDULES][1]).toEqual({ + day_of_month: 25, + default_dimensions: { '1': 'A' }, + }) + // Guarded on the stamp our own update produced, so a concurrent writer wins. + expect(h.eqFilters[`${SCHEDULES}:1`][0]).toEqual([ + ['id', SCHEDULE_ID], + ['company_id', COMPANY_ID], + ['updated_at', STAMP], + ]) + // Items restored from the snapshot, server columns stripped. + expect(h.inserts[ITEMS]).toHaveLength(2) + const restore = h.inserts[ITEMS][1] + expect(restore.map((r) => r.description)).toEqual(['Gammal rad', 'Rad 2']) + for (const row of restore) { + expect(row.id).toBeUndefined() + expect(row.created_at).toBeUndefined() + expect(row.schedule_id).toBe(SCHEDULE_ID) + } + }) + + it('reports itemsRestored: false and logs the lost rows when the items restore fails', async () => { + const h = createHarness({ + itemsSnapshot: [storedItem()], + itemsInsertErrors: [insertBoom, { message: 'restore boom' }], + }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { name: 'Nytt namn' }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ + ok: false, + stage: 'items_insert', + itemsRestored: false, + headerRestored: true, + }) + const logged = h.log.error.mock.calls.find((c) => /may be left with no items/.test(String(c[0]))) + expect(logged).toBeDefined() + expect(logged?.[2]).toMatchObject({ scheduleId: SCHEDULE_ID }) + expect((logged?.[2] as { previousItems: unknown[] }).previousItems).toHaveLength(1) + }) + + it('reports headerRestored: false when the rollback update errors', async () => { + const h = createHarness({ + itemsInsertErrors: [insertBoom, null], + headerRestoreError: { message: 'restore update boom' }, + }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ ok: false, headerRestored: false, itemsRestored: true }) + const logged = h.log.error.mock.calls.find((c) => /half-saved/.test(String(c[0]))) + expect(logged?.[2]).toMatchObject({ restoreRow: { day_of_month: 25 }, headerStamp: STAMP }) + }) + + it('reports headerRestored: false when a concurrent writer moved the row on', async () => { + // 0 rows matched: updated_at changed between our write and the rollback, so + // the other writer's value must stand rather than be clobbered. + const h = createHarness({ + itemsInsertErrors: [insertBoom, null], + headerRestoreMatched: false, + }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ ok: false, headerRestored: false }) + expect(h.log.error.mock.calls.some((c) => /half-saved/.test(String(c[0])))).toBe(true) + }) + + it('writes nothing at all when the header row is missing', async () => { + // No snapshot means no rollback source and no proof the schedule belongs to + // this company, so the header must NOT be updated: a write here would be + // committed in the full knowledge that it could never be undone. + const h = createHarness({ headerSnapshot: null, itemsInsertErrors: [insertBoom, null] }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ ok: false, stage: 'header' }) + expect(h.log.error.mock.calls.some((c) => /nothing was written/.test(String(c[0])))).toBe(true) + expect(h.updates[SCHEDULES]).toBeUndefined() + expect(h.inserts[ITEMS]).toBeUndefined() + expect(h.deletes).toEqual([]) + }) + + it('writes nothing at all when the header snapshot read errors', async () => { + const snapshotBoom = { message: 'snapshot boom', code: '57014' } + const h = createHarness({ headerSnapshotError: snapshotBoom }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toEqual({ ok: false, stage: 'header', error: snapshotBoom }) + expect(h.updates[SCHEDULES]).toBeUndefined() + expect(h.from.mock.calls.map((c) => c[0])).not.toContain(ITEMS) + }) + + it('leaves the items untouched and rolls the header back when the item snapshot errors', async () => { + // The single branch that can end with a schedule holding zero items: an + // unreadable snapshot must abort BEFORE the delete, not delete first and + // report itemsRestored: false afterwards. + const snapshotBoom = { message: 'items snapshot boom', code: '57014' } + const h = createHarness({ itemsSnapshotError: snapshotBoom }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ + ok: false, + stage: 'items_delete', + itemsRestored: true, + headerRestored: true, + }) + expect((result as { error: { message: string } }).error.message).toContain( + 'items snapshot boom', + ) + // Nothing was deleted and nothing was inserted: the schedule keeps its + // lines, so the cron's "schedule has no items" invariant still holds. + expect(h.deletes).toEqual([]) + expect(h.inserts[ITEMS]).toBeUndefined() + // The header edit is undone, so the failure is clean rather than partial. + expect(h.updates[SCHEDULES]).toHaveLength(2) + expect(h.updates[SCHEDULES][1]).toEqual({ day_of_month: 25 }) + expect(h.log.error.mock.calls.some((c) => /snapshot unreadable/.test(String(c[0])))).toBe(true) + }) + + it('stops at the delete stage without inserting anything', async () => { + const deleteBoom = { message: 'delete boom' } + const h = createHarness({ itemsDeleteError: deleteBoom }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toEqual({ + ok: false, + stage: 'items_delete', + error: deleteBoom, + itemsRestored: true, + headerRestored: true, + }) + // Nothing was removed, so nothing may be inserted: a blind insert after a + // failed delete would duplicate every line. + expect(h.inserts[ITEMS]).toBeUndefined() + // The header edit is still rolled back. + expect(h.updates[SCHEDULES]).toHaveLength(2) + expect(h.updates[SCHEDULES][1]).toEqual({ day_of_month: 25 }) + }) + + it('stops at the header stage without touching the items table', async () => { + const headerBoom = { message: 'header boom', code: '23514' } + const h = createHarness({ headerUpdateError: headerBoom }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: { day_of_month: 5 }, + items: [makeItem()], + log: h.log, + }) + + expect(result).toEqual({ ok: false, stage: 'header', error: headerBoom }) + expect(h.from.mock.calls.map((c) => c[0])).not.toContain(ITEMS) + }) + + it('is a no-op when there is nothing to write', async () => { + const h = createHarness({}) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: {}, + log: h.log, + }) + + expect(result).toEqual({ ok: true }) + expect(h.from).not.toHaveBeenCalled() + }) + + it('restores an empty item set without a write when there were no items before', async () => { + const h = createHarness({ itemsSnapshot: [], itemsInsertErrors: [insertBoom] }) + + const result = await applyRecurringScheduleUpdate(h.supabase, { + scheduleId: SCHEDULE_ID, + companyId: COMPANY_ID, + fields: {}, + items: [makeItem()], + log: h.log, + }) + + expect(result).toMatchObject({ ok: false, itemsRestored: true, headerRestored: true }) + expect(h.inserts[ITEMS]).toHaveLength(1) + }) +}) diff --git a/lib/invoices/apply-recurring-schedule-update.ts b/lib/invoices/apply-recurring-schedule-update.ts new file mode 100644 index 00000000..b1855d41 --- /dev/null +++ b/lib/invoices/apply-recurring-schedule-update.ts @@ -0,0 +1,368 @@ +import type { PostgrestError, SupabaseClient } from '@supabase/supabase-js' +import type { z } from 'zod' +import type { RecurringScheduleItemSchema } from '@/lib/api/schemas' +import { fetchAllRows } from '@/lib/supabase/fetch-all' +import { createLogger } from '@/lib/logger' + +/** + * Apply an edit to a recurring invoice schedule: the header fields first, then + * a full replace of its items (delete everything, reinsert the new set with + * `schedule_id` stamped on). + * + * Neither write is atomic over PostgREST, and a combined edit touches two + * tables, so BOTH writes are compensated on failure: + * - the items are snapshotted before the delete and best-effort reinserted + * when the insert fails (without that, a rejected insert leaves the + * schedule with ZERO items and every cron run throws "schedule has no + * items", silently skipping billing dates), and + * - the header fields are snapshotted before the update and rolled back to + * their previous values, so an item failure cannot leave a half-applied + * edit (a new day_of_month kept while the line edit was undone). + * + * Same snapshot/restore idiom as replaceInvoiceItems in + * lib/invoices/replace-invoice-items.ts. `itemsRestored` / `headerRestored` on + * the failure shapes say whether the schedule is back in its prior state; when + * either is false the caller must tell the user the schedule may be partially + * saved instead of reporting a clean failure. + * + * No destructive write happens once its compensating snapshot is known to be + * unavailable: an unreadable header snapshot fails before the header UPDATE, + * and an unreadable item snapshot fails before the DELETE (rolling the header + * back). That is what keeps the cron invariant "a schedule always has items" + * true on EVERY failure path, not just the ones where the restore succeeded. + * + * Tenancy: `recurring_invoice_schedule_items` has no company_id, so the item + * DELETE/INSERT can only be scoped by `schedule_id`, and the commit executor + * runs this on a service-role client with RLS off. The helper therefore proves + * ownership itself: whenever `items` is provided it first reads the header row + * filtered by BOTH id and company_id, and a miss aborts before anything is + * written. Callers should still 404 on their own beforehand (they own the + * user-facing not-found shape), but a caller that forgets cannot reach another + * tenant's rows through here. + * + * Shared by the cookie PATCH route (app/api/invoices/recurring/[id]) and the + * update_recurring_schedule commit executor so the two surfaces cannot drift. + */ + +const moduleLog = createLogger('invoices/recurring-schedule-update') + +export type RecurringScheduleItemInput = z.infer + +/** Server-generated recurring_invoice_schedule_items columns, never re-inserted. */ +const SERVER_GENERATED_ITEM_COLUMNS = ['id', 'created_at'] as const + +/** Minimal logger surface, so callers can pass their own module logger. */ +type Log = { error: (message: string, ...args: unknown[]) => void } + +export type ApplyRecurringScheduleUpdateResult = + | { ok: true } + /** + * Nothing was written at all: either the pre-write header read (ownership + * proof + rollback snapshot) failed, or the header update itself did. + */ + | { ok: false; stage: 'header'; error: PostgrestError } + | { + ok: false + stage: 'items_delete' | 'items_insert' + error: PostgrestError + /** Whether the pre-delete item rows are back (or never went away). */ + itemsRestored: boolean + /** Whether the header fields are back at their pre-edit values. */ + headerRestored: boolean + } + +export async function applyRecurringScheduleUpdate( + supabase: SupabaseClient, + opts: { + scheduleId: string + companyId: string + /** Header columns to write. Empty object = no header write at all. */ + fields: Record + /** Provided = replace all items; omitted = keep the existing ones. */ + items?: RecurringScheduleItemInput[] + log?: Log + }, +): Promise { + const { scheduleId, companyId, fields, items, log = moduleLog } = opts + const hasFields = Object.keys(fields).length > 0 + if (!hasFields && !items) return { ok: true } + + // Read the header row up front whenever the items are replaced. It does two + // jobs at once: + // - it is the only proof that the schedule belongs to `companyId` before + // the schedule_id-scoped item DELETE/INSERT below, and + // - it is the snapshot a later item failure rolls the header fields back + // to on a combined edit. + // A header-only edit needs neither (its single UPDATE is company-scoped and + // nothing after it can fail), so that path keeps its one round trip. + let headerSnapshot: Record | null = null + if (items) { + // select('*') on purpose: the restore must be able to put back every + // column named in `fields`, including ones added to the update schema after + // this function was written, so an explicit list here would silently fail + // to restore new fields. + const { data, error } = await supabase + .from('recurring_invoice_schedules') + .select('*') + .eq('id', scheduleId) + .eq('company_id', companyId) + .maybeSingle() + + // No row read means no ownership proof and no rollback source. Writing the + // header anyway would be committing a change we already know we could + // never undo, which is exactly the half-applied state this function + // exists to prevent: stop before touching anything. + if (error || !data) { + log.error( + 'recurring schedule header snapshot unavailable: nothing was written', + error ?? undefined, + { scheduleId, companyId, fields: Object.keys(fields) }, + ) + return { + ok: false, + stage: 'header', + error: error ?? asPostgrestError('Recurring schedule not found for this company', 'PGRST116'), + } + } + headerSnapshot = data as Record + } + + let headerStamp: string | null = null + if (hasFields) { + const { data: updated, error: updateError } = await supabase + .from('recurring_invoice_schedules') + .update(fields) + .eq('id', scheduleId) + .eq('company_id', companyId) + .select('updated_at') + .maybeSingle() + + if (updateError) return { ok: false, stage: 'header', error: updateError } + // Used to guard the rollback below. Null (no representation returned, or a + // mock) deliberately falls back to an unguarded restore: putting the user's + // own prior values back matters more than the race window. + headerStamp = (updated as { updated_at?: string } | null)?.updated_at ?? null + } + + if (!items) return { ok: true } + + // Snapshot the items before deleting them. Paginated on the id PK: a plain + // select is silently capped at 1000 rows by PostgREST, and a truncated + // snapshot would restore only part of the schedule while still reporting + // itemsRestored: true. + let snapshotRows: Record[] + try { + snapshotRows = await fetchAllRows>(({ from, to }) => + supabase + .from('recurring_invoice_schedule_items') + .select('*') + .eq('schedule_id', scheduleId) + .order('id') + .range(from, to), + ) + } catch (err) { + // Without a snapshot the delete below could never be compensated, so the + // items are left alone (the schedule keeps its lines and the cron keeps + // working) and only the header write is undone. + const snapshotError = asPostgrestError( + err instanceof Error ? err.message : String(err), + 'RECURRING_ITEMS_SNAPSHOT_FAILED', + ) + log.error( + 'recurring schedule items snapshot unreadable: items left untouched', + snapshotError, + { scheduleId, companyId }, + ) + const headerRestored = await restoreHeaderFields(supabase, { + scheduleId, + companyId, + fields, + snapshot: headerSnapshot, + headerStamp, + log, + }) + return { + ok: false, + stage: 'items_delete', + error: snapshotError, + itemsRestored: true, + headerRestored, + } + } + + const { error: deleteError } = await supabase + .from('recurring_invoice_schedule_items') + .delete() + .eq('schedule_id', scheduleId) + + if (deleteError) { + // Nothing was removed, so the items are trivially intact; the header write + // still has to be undone. + const headerRestored = await restoreHeaderFields(supabase, { + scheduleId, + companyId, + fields, + snapshot: headerSnapshot, + headerStamp, + log, + }) + return { + ok: false, + stage: 'items_delete', + error: deleteError, + itemsRestored: true, + headerRestored, + } + } + + const itemRows = items.map((item, idx) => ({ + schedule_id: scheduleId, + sort_order: idx, + description: item.description, + quantity: item.quantity, + unit: item.unit, + unit_price: item.unit_price, + vat_rate: item.vat_rate ?? null, + dimensions: item.dimensions ?? {}, + })) + + const { error: insertError } = await supabase + .from('recurring_invoice_schedule_items') + .insert(itemRows) + + if (insertError) { + // Best-effort restore of the previous lines so the schedule stays valid for + // the cron. A failed restore is reported, never swallowed. (An unreadable + // snapshot cannot get here: it aborts before the delete above.) + let itemsRestored = false + if (snapshotRows.length === 0) { + // Nothing existed before, so the prior (empty) state already holds. + itemsRestored = true + } else { + const restoreRows = snapshotRows.map((row) => { + const copy: Record = { ...row } + for (const column of SERVER_GENERATED_ITEM_COLUMNS) delete copy[column] + copy.schedule_id = scheduleId + return copy + }) + const { error: restoreError } = await supabase + .from('recurring_invoice_schedule_items') + .insert(restoreRows) + itemsRestored = !restoreError + if (restoreError) { + // The rows are gone from the table and the reinsert was rejected, so + // this log line is the only remaining copy: it is logged deliberately + // so support can rebuild the schedule by hand. It is customer content + // (line descriptions and prices), which is why it appears on this + // branch only, never on a successful edit. lib/logger.ts redacts known + // PII keys before the record leaves the process. + log.error( + 'recurring schedule items restore failed: schedule may be left with no items', + restoreError, + { scheduleId, companyId, previousItems: restoreRows }, + ) + } + } + + const headerRestored = await restoreHeaderFields(supabase, { + scheduleId, + companyId, + fields, + snapshot: headerSnapshot, + headerStamp, + log, + }) + + return { + ok: false, + stage: 'items_insert', + error: insertError, + itemsRestored, + headerRestored, + } + } + + return { ok: true } +} + +/** + * Build the PostgrestError shape callers already handle for a failure that did + * not come from PostgREST itself (a missing row, or fetchAllRows rethrowing a + * paged read error as a plain Error). + */ +function asPostgrestError(message: string, code: string): PostgrestError { + return { name: 'PostgrestError', message, details: '', hint: '', code } as PostgrestError +} + +/** + * Put the header columns named in `fields` back at their snapshot values. + * Returns whether the schedule's header is known to be in its pre-edit state. + */ +async function restoreHeaderFields( + supabase: SupabaseClient, + opts: { + scheduleId: string + companyId: string + fields: Record + snapshot: Record | null + headerStamp: string | null + log: Log + }, +): Promise { + const { scheduleId, companyId, fields, snapshot, headerStamp, log } = opts + const keys = Object.keys(fields) + if (keys.length === 0) return true // No header write happened. + + // Defensive only: a restore is reached exclusively after the pre-write read + // succeeded, which aborts the whole update when the row is missing. The + // branch stays so a future path that skips that read fails loudly instead of + // silently reporting a rollback it never performed. + if (!snapshot) { + log.error( + 'recurring schedule header snapshot missing: the field update cannot be rolled back', + undefined, + { scheduleId, companyId, fields }, + ) + return false + } + + const restoreRow: Record = {} + for (const key of keys) { + if (key in snapshot) restoreRow[key] = snapshot[key] + } + if (Object.keys(restoreRow).length !== keys.length) { + // A partial restore could null out a column that was never read back, so + // report the half-saved state instead of writing a guess. + log.error( + 'recurring schedule header snapshot incomplete: fields may be left half-saved', + undefined, + { scheduleId, companyId, fields, snapshotKeys: Object.keys(snapshot) }, + ) + return false + } + + // Guard the rollback on the stamp our own update produced. The table has an + // updated_at trigger (migration 20260518150000, + // recurring_invoice_schedules_updated_at), so a non-matching stamp means a + // concurrent writer (the hourly cron, a second edit) landed in between; its + // value must win rather than be clobbered from a stale snapshot. That is the + // exact failure documented as audit finding C2 in + // lib/invoices/voucher-matching.ts. + let query = supabase + .from('recurring_invoice_schedules') + .update(restoreRow) + .eq('id', scheduleId) + .eq('company_id', companyId) + if (headerStamp) query = query.eq('updated_at', headerStamp) + const { data, error } = await query.select('id') + + if (error || !data || (data as unknown[]).length === 0) { + log.error( + 'recurring schedule header rollback did not apply: fields may be left half-saved', + error ?? undefined, + { scheduleId, companyId, restoreRow, headerStamp }, + ) + return false + } + return true +} diff --git a/lib/pending-operations/__tests__/recurring-schedule-executors.test.ts b/lib/pending-operations/__tests__/recurring-schedule-executors.test.ts index 5ee9c978..f5bbe7e8 100644 --- a/lib/pending-operations/__tests__/recurring-schedule-executors.test.ts +++ b/lib/pending-operations/__tests__/recurring-schedule-executors.test.ts @@ -206,11 +206,16 @@ describe('commitPendingOperation: update_recurring_schedule', () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: { id: 'op-recurring-1' } }) // claim enqueue({ data: existingRow }) // existing schedule + // The helper re-reads the header row scoped by company_id before touching + // the items: the items table has no company_id of its own, so this read is + // what keeps the schedule_id-scoped delete/insert inside the tenant on the + // service-role (RLS-off) executor path. + enqueue({ data: existingRow }) // header ownership read enqueue({ data: [ { sort_order: 0, description: 'Old', quantity: 1, unit: 'st', unit_price: 100, vat_rate: null }, ], - }) // snapshot + }) // items snapshot enqueue({ data: null }) // delete old items enqueue({ data: null }) // insert new items enqueue({ data: null }) // finalize @@ -236,9 +241,10 @@ describe('commitPendingOperation: update_recurring_schedule', () => { items_replaced: true, item_count: 2, }) - expect(supabase.from).toHaveBeenNthCalledWith(3, 'recurring_invoice_schedule_items') + expect(supabase.from).toHaveBeenNthCalledWith(3, 'recurring_invoice_schedules') expect(supabase.from).toHaveBeenNthCalledWith(4, 'recurring_invoice_schedule_items') expect(supabase.from).toHaveBeenNthCalledWith(5, 'recurring_invoice_schedule_items') + expect(supabase.from).toHaveBeenNthCalledWith(6, 'recurring_invoice_schedule_items') }) it('keeps existing items when items are omitted', async () => { @@ -384,6 +390,93 @@ describe('commitPendingOperation: update_recurring_schedule', () => { expect(result.http_status).toBe(404) }) + /** + * A combined edit writes the header and then replaces the items, which + * PostgREST cannot do atomically. Issue #1275: an item-insert failure used to + * leave the header update committed, so the edit half-applied. + */ + const fullExistingRow = { + ...existingRow, + name: 'Månadsavgift', + updated_at: '2026-07-01T00:00:00Z', + } + const snapshotItem = { + id: 'item-old-1', + created_at: '2026-07-01T00:00:00Z', + schedule_id: SCHEDULE_ID, + sort_order: 0, + description: 'Old', + quantity: 1, + unit: 'st', + unit_price: 100, + vat_rate: null, + dimensions: {}, + } + const combinedChanges = { + schedule_id: SCHEDULE_ID, + changes: { + name: 'Nytt namn', + items: [{ description: 'Ny rad', quantity: 2, unit: 'tim', unit_price: 1200 }], + }, + } + + it('restores the prior header field when the items insert fails on a combined edit', async () => { + const { supabase, enqueue, findCalls } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-recurring-1' } }) // claim + enqueue({ data: fullExistingRow }) // existing schedule + enqueue({ data: fullExistingRow }) // header snapshot + enqueue({ data: { updated_at: '2026-07-30T09:00:00.000Z' } }) // header update + enqueue({ data: [snapshotItem] }) // items snapshot + enqueue({ data: null }) // delete old items + enqueue({ error: { message: 'items insert failed', code: '23514' } }) // items insert + enqueue({ data: null }) // items restore insert + enqueue({ data: [{ id: SCHEDULE_ID }] }) // header restore update + enqueue({ data: null }) // finalize + + const result = await commitPendingOperation( + supabase as never, + 'user-1', + 'company-1', + makePendingOp('update_recurring_schedule', combinedChanges), + ) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(500) + const updates = findCalls('recurring_invoice_schedules', 'update') + expect(updates).toHaveLength(2) + // The rollback puts the prior name back rather than leaving it half-saved. + expect(updates[1][0]).toEqual({ name: 'Månadsavgift' }) + }) + + it('reports the partial-state message when the items restore also fails', async () => { + const { supabase, enqueue } = createQueuedMockSupabase() + enqueue({ data: { id: 'op-recurring-1' } }) // claim + enqueue({ data: fullExistingRow }) // existing schedule + enqueue({ data: fullExistingRow }) // header snapshot + enqueue({ data: { updated_at: '2026-07-30T09:00:00.000Z' } }) // header update + enqueue({ data: [snapshotItem] }) // items snapshot + enqueue({ data: null }) // delete old items + enqueue({ error: { message: 'items insert failed', code: '23514' } }) // items insert + enqueue({ error: { message: 'restore boom' } }) // items restore insert fails + enqueue({ data: [{ id: SCHEDULE_ID }] }) // header restore update + enqueue({ data: null }) // finalize + + const result = await commitPendingOperation( + supabase as never, + 'user-1', + 'company-1', + makePendingOp('update_recurring_schedule', combinedChanges), + ) + + expect(result.status).toBe('failed') + expect(result.http_status).toBe(500) + // Same registry sentence the PATCH route returns. + expect(result.error).toMatch(/halvsparat/) + // And the same machine-readable code, so an MCP caller can detect the + // partial state without substring-matching the Swedish prose. + expect(result.code).toBe('INVOICE_RECURRING_UPDATE_PARTIAL') + }) + it('rejects tampered change fields at the commit boundary', async () => { const { supabase, enqueue } = createQueuedMockSupabase() enqueue({ data: { id: 'op-recurring-1' } }) // claim diff --git a/lib/pending-operations/commit.ts b/lib/pending-operations/commit.ts index 48081bdb..b6049858 100644 --- a/lib/pending-operations/commit.ts +++ b/lib/pending-operations/commit.ts @@ -125,6 +125,7 @@ import { } from '@/lib/invoices/build-invoice-write' import { isEditableInvoiceDraft } from '@/lib/invoices/is-editable-draft' import { replaceInvoiceItems } from '@/lib/invoices/replace-invoice-items' +import { applyRecurringScheduleUpdate } from '@/lib/invoices/apply-recurring-schedule-update' import { BulkBookInboxSchema } from '@/lib/api/schemas' import { ensureArticleNumber } from '@/lib/articles/ensure-article-number' import { isValidRevenueAccount } from '@/lib/articles/validate-revenue-account' @@ -159,10 +160,13 @@ export interface CommitResult { error?: string http_status?: number auto_rejected?: boolean - // Set when the commit failed because the booking posts to BAS accounts not - // active in the company chart. Recoverable: the op is left 'pending' so the - // caller can activate the accounts and retry. Lets the route rebuild the - // structured ACCOUNTS_NOT_IN_CHART envelope (code + account_numbers). + // Structured-error registry code for the failure, when one is known, so a + // caller can branch on the failure mode instead of parsing `error` text. + // ACCOUNTS_NOT_IN_CHART is the recoverable case: the booking posts to BAS + // accounts not active in the company chart, the op is left 'pending', and + // the route rebuilds the structured envelope (code + account_numbers). + // Other codes (e.g. INVOICE_RECURRING_UPDATE_PARTIAL) are informational: + // callers that do not recognize the code fall back to `error`. code?: string account_numbers?: string[] } @@ -238,6 +242,10 @@ async function recordSkippedInvoiceJournalEntry( type ExecutorResult = { data?: Record error?: string + // Structured-error registry code for `error`, when the executor has one. + // Surfaced as CommitResult.code and persisted in result_data.error_code so a + // caller can branch on the failure mode instead of parsing the message text. + errorCode?: string status?: number // Set when the executor already performed an irreversible side-effect // (posted voucher, persisted credit note) before the failure in `error`: @@ -691,75 +699,40 @@ async function commitUpdateRecurringSchedule( } } - if (Object.keys(updateRow).length > 0) { - const { error: updateError } = await supabase - .from('recurring_invoice_schedules') - .update(updateRow) - .eq('id', scheduleId) - .eq('company_id', companyId) - - if (updateError) return { error: updateError.message, status: 500 } - } - - let itemsReplaced = false - if (items) { - // Provided = replace all; omitted = keep existing (the schema contract). - // Snapshot first so a failed insert can restore the previous lines: an - // item-less schedule makes every cron run throw "schedule has no items" - // and silently skip billing dates. - const { data: previousItems } = await supabase - .from('recurring_invoice_schedule_items') - .select('sort_order, description, quantity, unit, unit_price, vat_rate, dimensions') - .eq('schedule_id', scheduleId) - - await supabase - .from('recurring_invoice_schedule_items') - .delete() - .eq('schedule_id', scheduleId) - - const itemRows = items.map((item, idx) => ({ - schedule_id: scheduleId, - sort_order: idx, - description: item.description, - quantity: item.quantity, - unit: item.unit, - unit_price: item.unit_price, - vat_rate: item.vat_rate ?? null, - dimensions: item.dimensions ?? {}, - })) - - const { error: itemsError } = await supabase - .from('recurring_invoice_schedule_items') - .insert(itemRows) - - if (itemsError) { - // Restore the snapshot so the schedule stays valid for the cron. - if (previousItems && previousItems.length > 0) { - const restoreRows = previousItems.map((row) => ({ - schedule_id: scheduleId, - sort_order: row.sort_order, - description: row.description, - quantity: row.quantity, - unit: row.unit, - unit_price: row.unit_price, - vat_rate: row.vat_rate, - dimensions: row.dimensions ?? {}, - })) - const { error: restoreError } = await supabase - .from('recurring_invoice_schedule_items') - .insert(restoreRows) - if (restoreError) { - log.error( - 'failed to restore schedule items after failed replace: schedule may be left empty', - restoreError, - { scheduleId }, - ) - } + // Items provided = replace all; omitted = keep existing (the schema + // contract). The shared helper compensates BOTH writes on failure, so an + // item failure cannot leave the header fields half-saved. + const result = await applyRecurringScheduleUpdate(supabase, { + scheduleId, + companyId, + fields: updateRow, + items, + log, + }) + if (!result.ok) { + if (result.stage !== 'header' && (!result.itemsRestored || !result.headerRestored)) { + // Same registry sentence the PATCH route returns, so the two surfaces + // cannot drift on what the user is told. + const partial = getErrorEntry('INVOICE_RECURRING_UPDATE_PARTIAL') + log.error('recurring schedule update left a partial state', result.error, { + scheduleId, + companyId, + stage: result.stage, + itemsRestored: result.itemsRestored, + headerRestored: result.headerRestored, + }) + return { + error: `${partial?.message_sv ?? 'Ändringen kunde inte slutföras.'} (${result.error.message})`, + // Machine-readable twin of the PATCH route's envelope code, so an + // MCP/staged-op caller can detect the partial state without + // substring-matching the Swedish sentence. + errorCode: 'INVOICE_RECURRING_UPDATE_PARTIAL', + status: 500, } - return { error: itemsError.message, status: 500 } } - itemsReplaced = true + return { error: result.error.message, status: 500 } } + const itemsReplaced = Boolean(items) return { data: { @@ -5652,7 +5625,11 @@ async function commitPendingOperationInner( resolved_at: new Date().toISOString(), result_data: isAutoReject ? { auto_rejected: true, reason: result.error } - : { error: result.error, http_status: result.status }, + : { + error: result.error, + http_status: result.status, + ...(result.errorCode ? { error_code: result.errorCode } : {}), + }, }) .eq('id', pendingOp.id) if (isAutoReject) { @@ -5667,6 +5644,7 @@ async function commitPendingOperationInner( status: 'failed', error: result.error, http_status: result.status ?? 500, + ...(result.errorCode ? { code: result.errorCode } : {}), } }