fix(reminders): settings UI discloses that automatic sending is disabled (#2033)
* fix(reminders): settings UI discloses that automatic sending is disabled The invoice reminder cron has answered 503 since May 2026 (PR #583), so no automatic reminders are sent, but the settings UI still let users configure reminder day levels as if sending worked. Introduce REMINDERS_SENDING_ENABLED (lib/invoices/reminders-enabled.ts) as the single shared flag read by both sides: the cron route uses it as its 503 gate (with the original sending pipeline restored behind it, so re-enabling later is one flag flip), and the invoice settings form shows an attn notice while the flag is off. Schedule settings stay editable; notice strings added to both sv and en locales. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 * docs(reminders): correct the re-enable contract after skeptic review The flag docblock, route docblock, and test header claimed flipping REMINDERS_SENDING_ENABLED alone resumes sending. False on hosted: the route has had no vercel.json cron entry since PR #559 and the crontab ratchet pins it in INTENTIONALLY_UNSCHEDULED, and POST requires the cron secret so no dashboard can trigger it. Rewrite the claims into the real re-enable checklist and record the pre-flip prerequisites surfaced by review: invoice_reminders lacks a unique (invoice_id, reminder_level) constraint and the fee entry is booked before the reminder row, so a run dying mid-batch double-books the fee; the backlog would get highest-level reminders first. Comments and a test name only; no runtime change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NtvffGr6uVk2J2Skuz6L98 --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
341d61131a
commit
e92b5a59b2
@@ -1358,6 +1358,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-08-29] get_vat_ruta_source_lines ACL restored in a NEW migration (20260829090500) rather than by editing 20260828172003: that file DROPped the 9-arg overload and CREATEd the 11-arg one without restating REVOKE/GRANT, and DROP FUNCTION discards the ACL, so the new signature silently fell back to EXECUTE for PUBLIC (anon included); the migration is already applied on prod, so a follow-up file is the only compliant path. Rule going forward: every DROP + CREATE of an RPC must restate its REVOKE ALL FROM PUBLIC, anon / GRANT EXECUTE TO authenticated, service_role, and tests/pg/vat-ruta-drilldown-reconcile.pg.test.ts now pins it with has_function_privilege (anon false, authenticated and service_role true, exactly one overload).
|
||||
[2026-08-29] PR #1756 replacement (rebind on PSD2 remap, amends the 2026-07-09 #916 entry): when upsertFromPsd2 resolves a duplicate row for the same connection+uid, the duplicate's MOVABLE transactions (unbooked, unmatched, not anchored via transaction_voucher_links or a payment row: the #1570 single-row move gate) are rebound onto the promoted row BEFORE the duplicate is resolved, so categorize/booking proposes the ledger the user just mapped instead of the overflow slot; a duplicate that still holds booked or anchored rows is demoted to manual as before and never deleted (their vouchers carry the old 19xx line, and the #1643 orphan guards handle the released twin). The contributor's unconditional rebind-all-then-delete was narrowed for that reason.
|
||||
[2026-08-29] Database errors now keep their SQLSTATE: new lib/errors/db-error.ts (dbError/errorCauseTag), applied at the 54 `throw new Error(\`Database error: ${err.message}\`)` sites in the MCP server AND, far more importantly, at lib/supabase/fetch-all.ts:74 where `throw new Error(error.message)` was the single highest-traffic strip point in the codebase (31 callers; every paginated read). isTransientFailure() checks the driver code FIRST and 57014 (statement timeout) is already in TRANSIENT_SQLSTATES, so discarding it turned a retryable timeout into UNKNOWN_ERROR ("Något gick fel. Försök igen."), which an agent cannot dispatch on. Traced end to end: gnubok_query_journal -> fetchEntryLines -> fetchAllRows (code stripped here) -> the tool's own sanitizeDbError, which ALREADY had a correct TRANSIENT_ERROR branch with a "retry or narrow with date_from/date_to" hint that could never fire because getStructuredError saw an anonymous Error. Measured on prod over 60 days with bot actors excluded: 1 024 real-agent failures, 645 UNKNOWN_ERROR across 60 actors and 57 companies; query_journal failed 164 times at p50 8 110 ms while every other failing tool sat at 1-315 ms; 82 retry streaks, 462 wasted repeat calls, 53.1% of error calls inside a streak. fetch-all passes context=null so the driver message stays VERBATIM (sanitizeDbError and other callers match on the existing text; this change adds the code, it does not reword). Attaching `code` is safe because extractCode() only accepts /^[A-Z_]+$/ and every SQLSTATE/PostgREST code contains digits, so it cannot hijack the application error registry (pinned by a test). dbError also never renders the literal "undefined": a driver-level failure with no message produced "Database error: undefined", the string that made these unsearchable. errorCauseTag() returns a PII-safe SQLSTATE for telemetry; the raw driver message can quote row values in a constraint violation and belongs in the server log, never in event_log. NOT ratcheted: check:types reports 538 vs baseline 539 because main fixed an unrelated error in own-account-detector.test.ts after the baseline was set; the gate only fails on an INCREASE, so the baseline is left alone rather than adding unrelated churn to this diff.
|
||||
[2026-08-30] Reminder settings disclosure (PR #2033) keeps the cron unscheduled: re-adding the vercel.json entry would fail the crontab ratchet (INTENTIONALLY_UNSCHEDULED) and log daily 503s; the full re-enable checklist incl. idempotency prerequisites lives in lib/invoices/reminders-enabled.ts.
|
||||
[2026-08-30] delete_draft_invoice risk tier 'high' (not 'medium' like update_invoice): both outcomes are irreversible (hard delete removes the row; makulering permanently consumes the F-series number), so the op must never be auto-committed.
|
||||
[2026-08-30] v1 DELETE invoices/{id} returns INVOICE_DELETE_NOT_DRAFT as 409 via the status override (registry maps it to 400 for the cookie route): a state-machine refusal is a conflict on v1, aligned with INVOICE_UPDATE_NOT_DRAFT; web behavior left unchanged.
|
||||
[2026-08-30] book_skattekonto_row(s) tier 'medium' + scope 'transactions:write': rule-driven booking with no caller-supplied lines mirrors book_mileage_period (not create_voucher's 'high'); scope follows reconcile_residual (books an outside row). Commit service gates on SKATTEVERKET_ENABLED for HTTP-dispatcher parity, recoverable so the op stays pending.
|
||||
|
||||
@@ -0,0 +1,146 @@
|
||||
/**
|
||||
* Tests for the invoice reminder cron gate: automatic reminder sending has
|
||||
* been deliberately disabled since May 2026 (PR #583). The route and the
|
||||
* invoice settings UI both read REMINDERS_SENDING_ENABLED, so these tests
|
||||
* pin the contract: while the flag is off the route answers 503 and never
|
||||
* touches the reminder processor; when the flag is flipped on, the route
|
||||
* runs the original sending pipeline. Scheduling the route again (it has
|
||||
* had no vercel.json cron entry since PR #559) and the pre-flip idempotency
|
||||
* work are separate steps; see lib/invoices/reminders-enabled.ts.
|
||||
*/
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
|
||||
const h = vi.hoisted(() => ({
|
||||
logInfo: vi.fn(),
|
||||
logError: vi.fn(),
|
||||
processOverdueReminders: vi.fn(),
|
||||
isConfigured: vi.fn(),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/api/with-cron-context', () => ({
|
||||
withCronContext:
|
||||
(_name: string, handler: (req: Request, ctx: unknown) => Promise<Response>) =>
|
||||
(req: Request) =>
|
||||
handler(req, {
|
||||
log: { info: h.logInfo, error: h.logError, warn: vi.fn() },
|
||||
requestId: 'req_test',
|
||||
}),
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/invoices/reminder-processor', () => ({
|
||||
processOverdueReminders: h.processOverdueReminders,
|
||||
}))
|
||||
|
||||
vi.mock('@/lib/email/service', () => ({
|
||||
getEmailService: () => ({ isConfigured: h.isConfigured }),
|
||||
}))
|
||||
|
||||
import { GET, POST } from '../route'
|
||||
import { REMINDERS_SENDING_ENABLED } from '@/lib/invoices/reminders-enabled'
|
||||
|
||||
function cronRequest(): Request {
|
||||
return new Request('http://localhost:3000/api/invoices/reminders/cron')
|
||||
}
|
||||
|
||||
describe('REMINDERS_SENDING_ENABLED flag', () => {
|
||||
it('is off: re-enabling automatic reminder sending is a deliberate founder decision', () => {
|
||||
expect(REMINDERS_SENDING_ENABLED).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe('GET /api/invoices/reminders/cron with sending disabled', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it('returns 503 and never invokes the reminder processor', async () => {
|
||||
const res = await GET(cronRequest())
|
||||
const body = await res.json()
|
||||
|
||||
expect(res.status).toBe(503)
|
||||
expect(body).toEqual({ disabled: true })
|
||||
expect(h.processOverdueReminders).not.toHaveBeenCalled()
|
||||
expect(h.isConfigured).not.toHaveBeenCalled()
|
||||
expect(h.logInfo).toHaveBeenCalledWith(
|
||||
'invoice reminders feature is disabled; skipping run'
|
||||
)
|
||||
})
|
||||
|
||||
it('exposes POST as the same gated handler (manual dashboard trigger)', async () => {
|
||||
expect(POST).toBe(GET)
|
||||
|
||||
const res = await POST(cronRequest())
|
||||
expect(res.status).toBe(503)
|
||||
expect(h.processOverdueReminders).not.toHaveBeenCalled()
|
||||
})
|
||||
})
|
||||
|
||||
describe('GET /api/invoices/reminders/cron with sending enabled (flag flipped)', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
vi.resetModules()
|
||||
})
|
||||
|
||||
async function importRouteWithFlagOn() {
|
||||
vi.doMock('@/lib/invoices/reminders-enabled', () => ({
|
||||
REMINDERS_SENDING_ENABLED: true,
|
||||
}))
|
||||
return import('../route')
|
||||
}
|
||||
|
||||
it('runs the reminder pipeline and reports the summary', async () => {
|
||||
h.isConfigured.mockReturnValue(true)
|
||||
h.processOverdueReminders.mockResolvedValue({
|
||||
processed: 2,
|
||||
sent: 1,
|
||||
failed: 1,
|
||||
results: [
|
||||
{
|
||||
invoiceId: 'inv-1',
|
||||
invoiceNumber: 'F-1001',
|
||||
customerEmail: 'kund@testbrand.example',
|
||||
reminderLevel: 1,
|
||||
success: true,
|
||||
},
|
||||
{
|
||||
invoiceId: 'inv-2',
|
||||
invoiceNumber: 'F-1002',
|
||||
customerEmail: 'kund2@testbrand.example',
|
||||
reminderLevel: 2,
|
||||
success: false,
|
||||
error: 'bounce',
|
||||
},
|
||||
],
|
||||
})
|
||||
|
||||
const { GET: gatedGet } = await importRouteWithFlagOn()
|
||||
const res = await gatedGet(cronRequest())
|
||||
const body = await res.json()
|
||||
|
||||
expect(res.status).toBe(200)
|
||||
expect(h.processOverdueReminders).toHaveBeenCalledTimes(1)
|
||||
expect(body).toEqual({
|
||||
success: true,
|
||||
processed: 2,
|
||||
sent: 1,
|
||||
failed: 1,
|
||||
results: [
|
||||
{ invoiceNumber: 'F-1001', reminderLevel: 1, success: true },
|
||||
{ invoiceNumber: 'F-1002', reminderLevel: 2, success: false, error: 'bounce' },
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
it('still refuses to run when the email service is not configured', async () => {
|
||||
h.isConfigured.mockReturnValue(false)
|
||||
|
||||
const { GET: gatedGet } = await importRouteWithFlagOn()
|
||||
const res = await gatedGet(cronRequest())
|
||||
|
||||
expect(res.status).toBeGreaterThanOrEqual(400)
|
||||
expect(h.processOverdueReminders).not.toHaveBeenCalled()
|
||||
expect(h.logError).toHaveBeenCalledWith(
|
||||
'email service not configured; skipping reminder run'
|
||||
)
|
||||
})
|
||||
})
|
||||
@@ -1,9 +1,55 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { processOverdueReminders } from '@/lib/invoices/reminder-processor'
|
||||
import { REMINDERS_SENDING_ENABLED } from '@/lib/invoices/reminders-enabled'
|
||||
import { getEmailService } from '@/lib/email/service'
|
||||
import { withCronContext } from '@/lib/api/with-cron-context'
|
||||
import { errorResponseFromCode } from '@/lib/errors/get-structured-error'
|
||||
|
||||
/**
|
||||
* GET/POST /api/invoices/reminders/cron: sends overdue invoice reminders.
|
||||
* Both verbs require the cron secret via withCronContext; POST mirrors GET
|
||||
* so a secret-bearing operator can trigger a run outside the schedule.
|
||||
*
|
||||
* Gated behind REMINDERS_SENDING_ENABLED (off since May 2026, PR #583):
|
||||
* while the flag is off this route answers 503 and nothing is sent, and
|
||||
* the invoice settings UI reads the same flag to disclose that state.
|
||||
* On hosted the route is also unscheduled (no vercel.json cron entry
|
||||
* since PR #559), so the flag flip alone does not resume sending there;
|
||||
* see lib/invoices/reminders-enabled.ts for the full re-enable checklist.
|
||||
*/
|
||||
export const GET = withCronContext('cron.invoice_reminders', async (_request, ctx) => {
|
||||
ctx.log.info('invoice reminders feature is disabled; skipping run')
|
||||
return NextResponse.json({ disabled: true }, { status: 503 })
|
||||
if (!REMINDERS_SENDING_ENABLED) {
|
||||
ctx.log.info('invoice reminders feature is disabled; skipping run')
|
||||
return NextResponse.json({ disabled: true }, { status: 503 })
|
||||
}
|
||||
|
||||
if (!getEmailService().isConfigured()) {
|
||||
ctx.log.error('email service not configured; skipping reminder run')
|
||||
return errorResponseFromCode('INVOICE_SEND_EMAIL_NOT_CONFIGURED', ctx.log, {
|
||||
requestId: ctx.requestId,
|
||||
})
|
||||
}
|
||||
|
||||
const result = await processOverdueReminders()
|
||||
|
||||
ctx.log.info('reminder cron summary', {
|
||||
processed: result.processed,
|
||||
sent: result.sent,
|
||||
failed: result.failed,
|
||||
})
|
||||
|
||||
return NextResponse.json({
|
||||
success: true,
|
||||
processed: result.processed,
|
||||
sent: result.sent,
|
||||
failed: result.failed,
|
||||
results: result.results.map((r) => ({
|
||||
invoiceNumber: r.invoiceNumber,
|
||||
reminderLevel: r.reminderLevel,
|
||||
success: r.success,
|
||||
error: r.error,
|
||||
})),
|
||||
})
|
||||
})
|
||||
|
||||
export const POST = GET
|
||||
|
||||
@@ -3,6 +3,7 @@
|
||||
import { useState } from 'react'
|
||||
import { useTranslations } from 'next-intl'
|
||||
import { Switch } from '@/components/ui/switch'
|
||||
import { AttnLine } from '@/components/ui/attn-line'
|
||||
import {
|
||||
SettingsGroup,
|
||||
SettingsInput,
|
||||
@@ -10,6 +11,7 @@ import {
|
||||
SettingsRow,
|
||||
SettingsTextarea,
|
||||
} from '@/components/settings/SettingsRows'
|
||||
import { REMINDERS_SENDING_ENABLED } from '@/lib/invoices/reminders-enabled'
|
||||
import type { CompanySettings } from '@/types'
|
||||
|
||||
interface InvoiceSettingsFormProps {
|
||||
@@ -104,6 +106,9 @@ export function InvoiceSettingsForm({ settings }: InvoiceSettingsFormProps) {
|
||||
</SettingsGroup>
|
||||
|
||||
<SettingsGroup label={t('reminder_days_heading')} help={t('reminder_days_help')}>
|
||||
{!REMINDERS_SENDING_ENABLED && (
|
||||
<AttnLine className="px-1 pt-2">{t('sending_disabled_notice')}</AttnLine>
|
||||
)}
|
||||
<SettingsRow
|
||||
label={t('send_reminders_label')}
|
||||
htmlFor="send_invoice_reminders"
|
||||
|
||||
@@ -0,0 +1,36 @@
|
||||
/**
|
||||
* Master switch for automatic invoice reminder sending.
|
||||
*
|
||||
* Sending was deliberately gated off in May 2026 (PR #583): the reminder
|
||||
* cron route answers 503 and no reminder emails go out, while the schedule
|
||||
* settings (send_invoice_reminders, reminder_days_level_1/2/3) remain
|
||||
* editable and are honored per company once sending is on.
|
||||
*
|
||||
* Both sides of that gate read this one constant: the cron route uses it as
|
||||
* its 503 gate, and the invoice settings form uses it to disclose to users
|
||||
* that automatic sending is currently disabled. Flipping it opens the route
|
||||
* and removes the notice in the same change.
|
||||
*
|
||||
* Re-enabling sending is a founder product decision and takes MORE than
|
||||
* this flip. The checklist (verified against the repo 2026-08-30):
|
||||
*
|
||||
* 1. Flip this flag to true.
|
||||
* 2. Hosted: the route has had no vercel.json cron entry since PR #559,
|
||||
* and scripts/__tests__/generate-crontabs.test.ts pins it in
|
||||
* INTENTIONALLY_UNSCHEDULED (the ratchet fails if it is scheduled).
|
||||
* Add the cron entry back and drop the pin together. Self-hosted
|
||||
* crontabs built from docs/SELF-HOSTING.md may already hit the route,
|
||||
* so those installs resume on the flag flip alone.
|
||||
* 3. Before any flip, make the reminder run idempotent: invoice_reminders
|
||||
* has no unique (invoice_id, reminder_level) constraint and
|
||||
* processOverdueReminders books the reminder fee entry BEFORE inserting
|
||||
* the invoice_reminders row, so a run dying mid-batch double-books the
|
||||
* fee (Dr 1510 / Cr 3990) on the next run. Also decide how to handle
|
||||
* the backlog: determineReminderLevel sends the highest eligible level
|
||||
* first, so long-overdue customers would get a final notice with fee
|
||||
* and interest as their first-ever reminder.
|
||||
*
|
||||
* Kept dependency-free on purpose: it is imported from both server code
|
||||
* (the cron route) and client components (the settings form).
|
||||
*/
|
||||
export const REMINDERS_SENDING_ENABLED = false as boolean
|
||||
@@ -2272,6 +2272,7 @@
|
||||
"default_notes_help": "Suggested automatically for new invoices.",
|
||||
"reminder_days_heading": "Automatic reminders",
|
||||
"reminder_days_help": "Choose how many days after the due date each reminder is sent. The days must be in ascending order.",
|
||||
"sending_disabled_notice": "Automatic reminder sending is currently disabled in Accounted and no reminder emails go out. Your schedule is saved and takes effect when sending is enabled.",
|
||||
"send_reminders_label": "Send automatic reminders",
|
||||
"send_reminders_help": "When off, no automatic reminder emails are sent to customers with overdue invoices.",
|
||||
"reminder_days_level_1": "First reminder (days)",
|
||||
|
||||
@@ -2272,6 +2272,7 @@
|
||||
"default_notes_help": "Föreslås automatiskt vid ny faktura.",
|
||||
"reminder_days_heading": "Automatiska påminnelser",
|
||||
"reminder_days_help": "Ange hur många dagar efter förfallodatum varje påminnelse skickas. Dagarna måste vara i stigande ordning.",
|
||||
"sending_disabled_notice": "Automatiska påminnelser är för närvarande inaktiverade i Accounted och inga påminnelsemejl skickas. Ditt schema sparas och börjar gälla när utskicken aktiveras.",
|
||||
"send_reminders_label": "Skicka automatiska påminnelser",
|
||||
"send_reminders_help": "När detta är avstängt skickas inga automatiska påminnelsemejl till kunder med förfallna fakturor.",
|
||||
"reminder_days_level_1": "Första påminnelsen (dagar)",
|
||||
|
||||
Reference in New Issue
Block a user