feat(notices): one aggregated notice line instead of stacked degraded-state banners (#1733)
* feat(notices): lib/notices aggregator + single notice line on Hem
Degraded-state surfaces (broken/expiring bank connections, Skatteverket
reconnect, failing cloud backups, wrong-account hint) each hand-rolled
their own detection and stacked independently on the dashboard. This adds
lib/notices, mirroring lib/worklist, as the single owner of every health
predicate, and de-clutters the surfaces:
- lib/notices/{types,predicates,categories,aggregate}: five documented
categories with a fixed priority order; every predicate soft-fails to
null; pure decision helpers live in predicates.ts so 'use client' pages
can import them without pulling server-only modules. Broken supersedes
expiring for the same bank connection by construction (status filter).
- GET /api/notices + POST /api/notices/dismiss (withRouteContext), and a
notice_dismissals table (per company+user+notice_id, RLS user-scoped).
Notice ids embed a state discriminator, so a dismissal hides exactly
the state the user saw and a NEW failure surfaces again.
- Hem renders only the highest-priority notice as ONE AttnLine where the
boxed BackupHealthBanner card sat (banner deleted; its multi-provider
sentence logic moved into the backup_failing predicate), with a quiet
"+N till" inline expander. otherAccountHint joins the same list as the
lowest-priority category instead of an unconditional extra line.
- transactions and skattekonto keep their own AttnLine copy/CTA but source
the reconnect decision from the shared skvStatusNeedsReconnect /
skvAuthErrorNeedsReconnect predicates; Hem's Bevaka row imports the
expiring-consent day-math instead of duplicating it.
- design.md convention 6 addendum: max one global notice line + max one
page-domain attn line (locked convention: needs founder sign-off).
- i18n: new notices namespace in sv+en; moved banner/hint keys deleted.
- notice_dismissals classified as archive-excluded (UI state, not
räkenskapsinformation) to satisfy the full-archive contract.
SkatteverketPromoCard keeps its localStorage dismiss for now; migrating it
to notice_dismissals is a follow-up.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* fix(notices): stable dismissals with reaping, bounded ids, unnamed-bank copy
Review fixes on the notice aggregator:
- Migration renamed 20260819080000 -> 20260819190000_notice_dismissals.sql
(version collision with another in-flight PR; content unchanged).
- backup_failing dismissal stability: the id no longer embeds
last_auto_sync_at / needs_reauth_at, which the cron re-stamps while the
SAME incident persists and so resurrected a dismissed notice daily. The
id is now stable per (provider, reason), and the opposite direction is
kept correct by stale-dismissal reaping in getCompanyNotices: when a
category is currently healthy, the caller's stored dismissals for that
category (matched on the 'category:' id prefix) are best-effort deleted,
so error -> dismiss -> healthy (reaped) -> new error resurfaces. Audit of
the other ids: bank ids embed connection id + status/expiry and skv
embeds the incident's first-error/expiry timestamp (markNeedsReconsent
only fires post-connect), all stable per incident; they get the same
reaping as hygiene. Contract documented on Notice.id in types.ts.
- NULL bank_name no longer interpolates the Swedish fallback 'banken' into
the English message: a bank_broken_one_unnamed message variant (sv + en)
is selected instead of a name param.
- Bounded notice ids: folding several connections into one discriminator
now collapses to count + first 8 hex of a sha256 over the sorted parts
(node:crypto, server-only) instead of concatenating uuids; single
connection ids stay human-readable. Dismiss schema cap tightened to 200
with an updated rationale.
- Tests: persisting failure stays dismissed across two aggregations,
healthy state reaps, new failure after reap resurfaces, hint never
reaped, failed reap swallowed, 30-connection id under 200 chars and
stable across orderings, unnamed-bank variant, sorted backup id stable
across cron re-stamps.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* test(notices): pg-real coverage for the notice_dismissals policies
The coverage gate is right to flag the migration: every policy on this table
binds company membership AND auth.uid(), and nothing exercised it. The suite
pins the property that makes the table different from the rest of the schema:
a dismissal is personal, so a colleague in the same company keeps seeing a
notice the other member hid. It also covers the upsert re-stamp (which needs
the UPDATE policy), cross-tenant refusal, dismissing on behalf of another
user, the caller-scoped DELETE that reaping relies on, and the composite key.
Falsification-verified against a real Postgres: weakening the SELECT policy
to company-only scoping fails the colleague-isolation test.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
Jakob Wennberg
parent
4cf227001d
commit
9fc05c383f
@@ -0,0 +1,65 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { createMockRequest, createQueuedMockSupabase, parseJsonResponse } from '@/tests/helpers'
|
||||
import type { Notice } from '@/lib/notices/types'
|
||||
|
||||
const { supabase, reset } = createQueuedMockSupabase()
|
||||
const requireAuthMock = vi.fn()
|
||||
const getCompanyNoticesMock = vi.hoisted(() => vi.fn())
|
||||
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: (...args: unknown[]) => requireAuthMock(...args),
|
||||
}))
|
||||
vi.mock('@/lib/company/context', () => ({
|
||||
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
|
||||
}))
|
||||
vi.mock('@/lib/notices', () => ({
|
||||
getCompanyNotices: getCompanyNoticesMock,
|
||||
}))
|
||||
|
||||
import { GET } from '../route'
|
||||
|
||||
const notice: Notice = {
|
||||
id: 'skv_disconnected:needs_reconsent@2026-08-18T03:00:00Z',
|
||||
category: 'skv_disconnected',
|
||||
severity: 'error',
|
||||
messageKey: 'skv_disconnected',
|
||||
actionKey: 'skv_disconnected_action',
|
||||
actionHref: '/settings/tax',
|
||||
}
|
||||
|
||||
describe('GET /api/notices', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: { id: 'user-1' },
|
||||
supabase,
|
||||
error: null,
|
||||
})
|
||||
getCompanyNoticesMock.mockResolvedValue([notice])
|
||||
})
|
||||
|
||||
it('returns 401 when the user is not authenticated', async () => {
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: null,
|
||||
supabase,
|
||||
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
|
||||
})
|
||||
|
||||
const response = await GET(createMockRequest('/api/notices'), { params: Promise.resolve({}) })
|
||||
expect(response.status).toBe(401)
|
||||
expect(getCompanyNoticesMock).not.toHaveBeenCalled()
|
||||
})
|
||||
|
||||
it('returns the ordered notices for the active company and calling user', async () => {
|
||||
const { status, body } = await parseJsonResponse<{ data: { notices: Notice[] } }>(
|
||||
await GET(createMockRequest('/api/notices'), { params: Promise.resolve({}) }),
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.notices).toEqual([notice])
|
||||
expect(getCompanyNoticesMock).toHaveBeenCalledWith(supabase, 'company-1', {
|
||||
userId: 'user-1',
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,93 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import { NextResponse } from 'next/server'
|
||||
import { createMockRequest, createQueuedMockSupabase, parseJsonResponse } from '@/tests/helpers'
|
||||
|
||||
const { supabase, enqueue, reset, findCall } = createQueuedMockSupabase()
|
||||
const requireAuthMock = vi.fn()
|
||||
|
||||
vi.mock('@/lib/auth/require-auth', () => ({
|
||||
requireAuth: (...args: unknown[]) => requireAuthMock(...args),
|
||||
}))
|
||||
vi.mock('@/lib/company/context', () => ({
|
||||
getActiveCompanyId: vi.fn().mockResolvedValue('company-1'),
|
||||
}))
|
||||
|
||||
import { POST } from '../route'
|
||||
|
||||
describe('POST /api/notices/dismiss', () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
reset()
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: { id: 'user-1' },
|
||||
supabase,
|
||||
error: null,
|
||||
})
|
||||
})
|
||||
|
||||
it('returns 401 when the user is not authenticated', async () => {
|
||||
requireAuthMock.mockResolvedValue({
|
||||
user: null,
|
||||
supabase,
|
||||
error: NextResponse.json({ error: 'Unauthorized' }, { status: 401 }),
|
||||
})
|
||||
|
||||
const response = await POST(
|
||||
createMockRequest('/api/notices/dismiss', {
|
||||
method: 'POST',
|
||||
body: { notice_id: 'skv_disconnected:x' },
|
||||
}),
|
||||
{ params: Promise.resolve({}) },
|
||||
)
|
||||
expect(response.status).toBe(401)
|
||||
})
|
||||
|
||||
it('returns 400 when notice_id is missing or empty', async () => {
|
||||
const missing = await POST(
|
||||
createMockRequest('/api/notices/dismiss', { method: 'POST', body: {} }),
|
||||
{ params: Promise.resolve({}) },
|
||||
)
|
||||
expect(missing.status).toBe(400)
|
||||
|
||||
const empty = await POST(
|
||||
createMockRequest('/api/notices/dismiss', { method: 'POST', body: { notice_id: '' } }),
|
||||
{ params: Promise.resolve({}) },
|
||||
)
|
||||
expect(empty.status).toBe(400)
|
||||
})
|
||||
|
||||
it('upserts the dismissal scoped to the calling user and company', async () => {
|
||||
enqueue({ data: null, error: null })
|
||||
const { status, body } = await parseJsonResponse<{ data: { dismissed: boolean } }>(
|
||||
await POST(
|
||||
createMockRequest('/api/notices/dismiss', {
|
||||
method: 'POST',
|
||||
body: { notice_id: 'bank_connection_broken:c1=expired' },
|
||||
}),
|
||||
{ params: Promise.resolve({}) },
|
||||
),
|
||||
)
|
||||
expect(status).toBe(200)
|
||||
expect(body.data.dismissed).toBe(true)
|
||||
|
||||
const upsertArgs = findCall('notice_dismissals', 'upsert')
|
||||
expect(upsertArgs?.[0]).toMatchObject({
|
||||
company_id: 'company-1',
|
||||
user_id: 'user-1',
|
||||
notice_id: 'bank_connection_broken:c1=expired',
|
||||
})
|
||||
expect(upsertArgs?.[1]).toEqual({ onConflict: 'company_id,user_id,notice_id' })
|
||||
})
|
||||
|
||||
it('maps a failed write to a 500 error envelope', async () => {
|
||||
enqueue({ data: null, error: { message: 'boom' } })
|
||||
const response = await POST(
|
||||
createMockRequest('/api/notices/dismiss', {
|
||||
method: 'POST',
|
||||
body: { notice_id: 'x' },
|
||||
}),
|
||||
{ params: Promise.resolve({}) },
|
||||
)
|
||||
expect(response.status).toBe(500)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,35 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { validateBody } from '@/lib/api/validate'
|
||||
import { NoticeDismissSchema } from '@/lib/api/schemas'
|
||||
|
||||
/**
|
||||
* POST /api/notices/dismiss: hide one notice for the calling user + company.
|
||||
*
|
||||
* Dismissals are per user (a colleague still sees the notice) and per notice
|
||||
* id; ids embed a state discriminator (lib/notices/types.ts), so a dismissed
|
||||
* notice stays hidden only until the underlying condition changes: a NEW
|
||||
* failure mints a new id and surfaces again. Idempotent via upsert.
|
||||
*
|
||||
* No requireWrite: dismissing a personal notice is a per-user preference,
|
||||
* not a company-data mutation, so viewers may dismiss too.
|
||||
*/
|
||||
export const POST = withRouteContext('notices.dismiss', async (request, ctx) => {
|
||||
const validation = await validateBody(request, NoticeDismissSchema)
|
||||
if (!validation.success) return validation.response
|
||||
|
||||
const { supabase, companyId, user } = ctx
|
||||
const { error } = await supabase.from('notice_dismissals').upsert(
|
||||
{
|
||||
company_id: companyId,
|
||||
user_id: user.id,
|
||||
notice_id: validation.data.notice_id,
|
||||
dismissed_at: new Date().toISOString(),
|
||||
},
|
||||
{ onConflict: 'company_id,user_id,notice_id' },
|
||||
)
|
||||
if (error) {
|
||||
throw new Error(`notice dismissal failed: ${error.message}`)
|
||||
}
|
||||
return NextResponse.json({ data: { dismissed: true } })
|
||||
})
|
||||
@@ -0,0 +1,19 @@
|
||||
import { NextResponse } from 'next/server'
|
||||
import { withRouteContext } from '@/lib/api/with-route-context'
|
||||
import { getCompanyNotices } from '@/lib/notices'
|
||||
|
||||
/**
|
||||
* GET /api/notices: active degraded-state notices for the active company,
|
||||
* in priority order (lib/notices, the health sibling of /api/worklist/counts).
|
||||
*
|
||||
* Read-only; every predicate is one bounded query that soft-fails to null,
|
||||
* and per-user dismissals are already filtered out. No events are emitted,
|
||||
* so ensureInitialized() is deliberately absent.
|
||||
*
|
||||
* Response: { data: { notices: Notice[] } }
|
||||
*/
|
||||
export const GET = withRouteContext('notices.list', async (_request, ctx) => {
|
||||
const { supabase, companyId, user } = ctx
|
||||
const notices = await getCompanyNotices(supabase, companyId, { userId: user.id })
|
||||
return NextResponse.json({ data: { notices } })
|
||||
})
|
||||
Reference in New Issue
Block a user