* feat(salary): recurring payroll lines per employee (#2042) A standing per-employee payslip row derived into every salary run inside its validity window, e.g. a benefit-bike bruttolöneavdrag of -670 kr/month. Mirrors the employee_benefits pattern end to end: - employee_recurring_lines table with RLS, audit + updated_at triggers, and a salary_line_items.source_recurring_line_id back-link; amount sign and account format enforced by CHECKs - run-calculation step 8d3 derives rows with flags computed from the item type (gross deductions reduce tax + AGA bases, net deductions post-tax); derived rows are excluded from the manual-line set like benefit rows - CRUD routes under /api/salary/employees/[id]/recurring-lines with the same 401/403/404/400 contract as the benefits routes - EmployeeRecurringLinesPanel on the employee page, sv/en strings - registered in the BFL full-archive export Closes #2042 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(salary): address #2044 review: feed recurring rows to the engine, guard deletes - Derived recurring rows are now appended to the calculateSalary lineItems set: they were inserted into salary_line_items but excluded from the in-memory calculation, so a recurring deduction never affected the payslip math (CodeRabbit, major). - DELETE deactivates a line that has derived rows instead of hard-deleting: ON DELETE SET NULL would turn a draft run's derived row into an apparent manual row that recalculation keeps forever; deactivation preserves the provenance link and lets the next recalculation drop the draft rows (CodeRabbit, major). The panel hides inactive lines. - POST employee lookup uses maybeSingle and answers 500 on lookup failure, 404 only on zero rows. - Panel: try/finally releases loading/submitting on network failure, and a request sequence guard stops a stale load from overwriting a newer list. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(migrations): move employee_recurring_lines off 20260830140000, which upstream now occupies Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(migrations): bind employee_id to company_id with a composite FK (review) The dimensions pattern: UNIQUE (id, company_id) on employees plus a composite FK, so RLS company scoping cannot be sidestepped by pointing a recurring line at another company's employee (IDOR, CWE-639). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(salary): address review: deductions only, race-free delete, engine and pg tests Review round on #2044: - Blocker: recurring 'other' additions removed from the whitelist, the migration CHECK and the panel. calculateSalary only treats ADDITION_TYPES as additions, so a recurring taxable addition rendered on the payslip without entering gross, tax, AGA or AGI. Re-add only together with engine support (recorded in DECISIONS.md). - Delete race: salary_line_items.source_recurring_line_id is now NO ACTION instead of SET NULL; the DELETE route deletes first and falls back to deactivation on 23503, so a deletion racing a concurrent derivation can never orphan a derived row into an apparent manual row. NO ACTION defers to statement end, so company-deletion cascades are unaffected. - Correction runs copy source_benefit_id / source_recurring_line_id, so recalculating a correction no longer derives the copied rows a second time (pre-existing for benefits, now pinned). - Engine tests: gross_deduction_other through calculateSalary asserts gross, taxable income and avgifterBasis drop while the semester base stays; net_deduction_union only moves the paid-out net. - pg-real tests for the new table: RLS membership, composite FK cross-company refusal, deduction-only CHECKs, and the NO ACTION back-link blocking deletes of derived-into lines. - Nice-to-haves: POST rounds the stored amount to ore, the redundant single-column employees FK is dropped (composite carries the cascade), the schemas.ts comment references the real migration version, and the panel explains the validity-window semantics (payment date, bounds inclusive, no proration). - Rebased onto main; the phantom-columns ceiling re-measured at 395 on the merged tree. - DECISIONS.md records the vacation-basis judgment call (semester base not reduced by recurring gross deductions). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(salary): gate recurring-line writes on the writer role, 404 unmatched deletes Two findings from the 2026-09-02 review round: - Superagent P1: the write policies were membership-only, so a read-only viewer could write recurring payroll deductions straight through PostgREST, bypassing the route's requireWrite. The table now carries aa_enforce_company_writer_role, the same gate 20260902093000 attaches to every company-scoped table (it also fires inside SECURITY DEFINER bodies, where RLS does not apply). The migration is re-versioned to 20260902140000 so the function exists when a fresh database replays the folder in order. - CodeRabbit: a filtered DELETE reports no error when nothing matches, so an unknown or cross-company line answered 200 deleted: true. The delete now selects the removed row and answers 404 when it is null. Tests: pg-real asserts a viewer is refused insert, update and delete with 42501 while the row survives unchanged, plus a non-member case; the route tests pin the 404. 896 salary tests green, rebased on main. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(salary): pin the recurring-line payload column sets Answers the phantom-column ceiling finding with scoped assertions rather than a bare ceiling raise: the PATCH route test now asserts the exact writable column set, and the comment records that the pg-real test covers the derived-row shape against the real table. Making the PATCH payload a literal would turn a partial update into last-write-wins, which is why the shape stays unresolved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(salary): round recurring line amounts with roundOre check:guards naive-ore-round ratchet: the derived recurring row used Math.round(x * 100) / 100 (baseline 615, +1); roundOre is already imported in run-calculation.ts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * fix(migrations): guard the employees unique-key add against #2145 merge order #2145 (expense claims) also adds employees_id_company_id_key. Wrap this migration's ADD CONSTRAINT in an idempotent DO block so whichever of the two PRs merges second does not fail on a duplicate constraint. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Jakob Wennberg <311770904+jakobwennberg-oss@users.noreply.github.com>
This commit is contained in:
Joakim Hansson
co-authored by
Claude Fable 5
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
parent
50b6299699
commit
4eb1626129
@@ -0,0 +1,208 @@
|
||||
import { randomUUID } from 'node:crypto'
|
||||
import { describe, it, expect } from 'vitest'
|
||||
import { getPool, withUserContext } from './setup'
|
||||
import { seedCompany, insertAuthUser, insertCompanyMember } from './fixtures'
|
||||
|
||||
// pg-real coverage for 20260902140000_employee_recurring_lines: RLS (member
|
||||
// read, stranger blind, viewers denied every write), the composite FK
|
||||
// (cross-company insert refused), the CHECKs (deduction-only item types,
|
||||
// negative amount, period order, account format) and the NO ACTION back-link
|
||||
// FK from salary_line_items (delete of a derived-into line fails with 23503).
|
||||
|
||||
async function seedEmployee(): Promise<{ userId: string; companyId: string; employeeId: string }> {
|
||||
const { userId, companyId } = await seedCompany()
|
||||
const employeeId = randomUUID()
|
||||
await getPool().query(
|
||||
`INSERT INTO public.employees
|
||||
(id, company_id, user_id, first_name, last_name, personnummer, personnummer_last4, employment_start)
|
||||
VALUES ($1, $2, $3, 'Test', 'Testsson', 'enc-payload', '0000', '2026-01-01')`,
|
||||
[employeeId, companyId, userId],
|
||||
)
|
||||
return { userId, companyId, employeeId }
|
||||
}
|
||||
|
||||
async function insertLine(
|
||||
companyId: string,
|
||||
employeeId: string,
|
||||
userId: string,
|
||||
overrides: Partial<{ itemType: string; amount: number; validTo: string | null; account: string | null }> = {},
|
||||
): Promise<string> {
|
||||
const id = randomUUID()
|
||||
await getPool().query(
|
||||
`INSERT INTO public.employee_recurring_lines
|
||||
(id, employee_id, company_id, user_id, item_type, description, amount, account_number, valid_from, valid_to)
|
||||
VALUES ($1, $2, $3, $4, $5, 'Förmånscykel bruttolöneavdrag', $6, $7, '2026-01-01', $8)`,
|
||||
[
|
||||
id,
|
||||
employeeId,
|
||||
companyId,
|
||||
userId,
|
||||
overrides.itemType ?? 'gross_deduction_other',
|
||||
overrides.amount ?? -670.17,
|
||||
overrides.account ?? null,
|
||||
overrides.validTo === undefined ? null : overrides.validTo,
|
||||
],
|
||||
)
|
||||
return id
|
||||
}
|
||||
|
||||
describe('employee_recurring_lines RLS', () => {
|
||||
it('lets company members read, strangers see nothing', async () => {
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
const lineId = await insertLine(companyId, employeeId, userId)
|
||||
const stranger = await insertAuthUser()
|
||||
|
||||
const memberView = await withUserContext(userId, (client) =>
|
||||
client.query<{ id: string }>(`SELECT id FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
)
|
||||
expect(memberView.rows).toHaveLength(1)
|
||||
|
||||
const strangerView = await withUserContext(stranger, (client) =>
|
||||
client.query<{ id: string }>(`SELECT id FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
)
|
||||
expect(strangerView.rows).toHaveLength(0)
|
||||
})
|
||||
|
||||
it('viewers read but are denied insert, update and delete', async () => {
|
||||
// The write policies AND in current_user_can_write(), so a read-only
|
||||
// viewer cannot write straight through PostgREST even though the route's
|
||||
// requireWrite would also refuse.
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
const lineId = await insertLine(companyId, employeeId, userId)
|
||||
const viewer = await insertAuthUser()
|
||||
await insertCompanyMember({ companyId, userId: viewer, role: 'viewer' })
|
||||
|
||||
const viewerRead = await withUserContext(viewer, (client) =>
|
||||
client.query<{ id: string }>(`SELECT id FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
)
|
||||
expect(viewerRead.rows).toHaveLength(1)
|
||||
|
||||
await expect(
|
||||
withUserContext(viewer, (client) =>
|
||||
client.query(
|
||||
`INSERT INTO public.employee_recurring_lines
|
||||
(employee_id, company_id, user_id, item_type, description, amount, valid_from)
|
||||
VALUES ($1, $2, $3, 'net_deduction_union', 'Fackavgift', -100, '2026-01-01')`,
|
||||
[employeeId, companyId, viewer],
|
||||
),
|
||||
),
|
||||
).rejects.toMatchObject({ code: '42501' })
|
||||
|
||||
await expect(
|
||||
withUserContext(viewer, (client) =>
|
||||
client.query(`UPDATE public.employee_recurring_lines SET amount = -1 WHERE id = $1`, [lineId]),
|
||||
),
|
||||
).rejects.toMatchObject({ code: '42501' })
|
||||
await expect(
|
||||
withUserContext(viewer, (client) =>
|
||||
client.query(`DELETE FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
),
|
||||
).rejects.toMatchObject({ code: '42501' })
|
||||
|
||||
const survived = await getPool().query(
|
||||
`SELECT amount FROM public.employee_recurring_lines WHERE id = $1`,
|
||||
[lineId],
|
||||
)
|
||||
expect(survived.rows).toHaveLength(1)
|
||||
expect(Number(survived.rows[0].amount)).toBeCloseTo(-670.17, 2)
|
||||
})
|
||||
|
||||
it('non-members cannot write at all', async () => {
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
const lineId = await insertLine(companyId, employeeId, userId)
|
||||
const stranger = await insertAuthUser()
|
||||
|
||||
await expect(
|
||||
withUserContext(stranger, (client) =>
|
||||
client.query(
|
||||
`INSERT INTO public.employee_recurring_lines
|
||||
(employee_id, company_id, user_id, item_type, description, amount, valid_from)
|
||||
VALUES ($1, $2, $3, 'net_deduction_union', 'Fackavgift', -100, '2026-01-01')`,
|
||||
[employeeId, companyId, stranger],
|
||||
),
|
||||
),
|
||||
).rejects.toThrow(/row-level security|permission|privilege/i)
|
||||
|
||||
const del = await withUserContext(stranger, (client) =>
|
||||
client.query(`DELETE FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
)
|
||||
expect(del.rowCount).toBe(0)
|
||||
})
|
||||
|
||||
it('the composite FK refuses pointing a line at another company employee', async () => {
|
||||
const a = await seedEmployee()
|
||||
const b = await seedEmployee()
|
||||
|
||||
// Insert as superuser (bypasses RLS): the composite (employee_id,
|
||||
// company_id) FK is what must refuse the cross-company pair.
|
||||
await expect(
|
||||
getPool().query(
|
||||
`INSERT INTO public.employee_recurring_lines
|
||||
(employee_id, company_id, user_id, item_type, description, amount, valid_from)
|
||||
VALUES ($1, $2, $3, 'gross_deduction_other', 'IDOR', -100, '2026-01-01')`,
|
||||
[b.employeeId, a.companyId, a.userId],
|
||||
),
|
||||
).rejects.toThrow(/foreign key/)
|
||||
})
|
||||
})
|
||||
|
||||
describe('employee_recurring_lines constraints', () => {
|
||||
it('rejects non-deduction item types (other) and positive amounts', async () => {
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
await expect(
|
||||
insertLine(companyId, employeeId, userId, { itemType: 'other', amount: -500 }),
|
||||
).rejects.toThrow(/item_type/)
|
||||
await expect(
|
||||
insertLine(companyId, employeeId, userId, { amount: 500 }),
|
||||
).rejects.toThrow(/amount_sign/)
|
||||
})
|
||||
|
||||
it('rejects valid_to before valid_from and malformed account overrides', async () => {
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
await expect(
|
||||
insertLine(companyId, employeeId, userId, { validTo: '2025-12-31' }),
|
||||
).rejects.toThrow(/check constraint/i)
|
||||
await expect(
|
||||
insertLine(companyId, employeeId, userId, { account: '73' }),
|
||||
).rejects.toThrow(/account_format/)
|
||||
})
|
||||
})
|
||||
|
||||
describe('salary_line_items back-link FK', () => {
|
||||
it('NO ACTION blocks deleting a line that has been derived into a run', async () => {
|
||||
const { userId, companyId, employeeId } = await seedEmployee()
|
||||
const lineId = await insertLine(companyId, employeeId, userId)
|
||||
|
||||
const runId = randomUUID()
|
||||
await getPool().query(
|
||||
`INSERT INTO public.salary_runs (id, company_id, user_id, period_year, period_month, payment_date, status)
|
||||
VALUES ($1, $2, $3, 2026, 8, '2026-08-25', 'draft')`,
|
||||
[runId, companyId, userId],
|
||||
)
|
||||
const sreId = randomUUID()
|
||||
await getPool().query(
|
||||
`INSERT INTO public.salary_run_employees (id, salary_run_id, employee_id, company_id, salary_type, monthly_salary, employment_degree)
|
||||
VALUES ($1, $2, $3, $4, 'monthly', 35000, 100)`,
|
||||
[sreId, runId, employeeId, companyId],
|
||||
)
|
||||
await getPool().query(
|
||||
`INSERT INTO public.salary_line_items
|
||||
(id, salary_run_employee_id, company_id, item_type, description, quantity, amount,
|
||||
is_taxable, is_avgift_basis, is_vacation_basis, is_gross_deduction, is_net_deduction, source_recurring_line_id)
|
||||
VALUES ($1, $2, $3, 'gross_deduction_other', 'Förmånscykel bruttolöneavdrag', 1, -670.17,
|
||||
true, true, false, true, false, $4)`,
|
||||
[randomUUID(), sreId, companyId, lineId],
|
||||
)
|
||||
|
||||
await expect(
|
||||
getPool().query(`DELETE FROM public.employee_recurring_lines WHERE id = $1`, [lineId]),
|
||||
).rejects.toThrow(/foreign key/)
|
||||
|
||||
// Deactivation (the DELETE route's fallback) still works.
|
||||
const deactivate = await getPool().query(
|
||||
`UPDATE public.employee_recurring_lines SET is_active = false WHERE id = $1`,
|
||||
[lineId],
|
||||
)
|
||||
expect(deactivate.rowCount).toBe(1)
|
||||
})
|
||||
})
|
||||
@@ -130,6 +130,18 @@ const KNOWN_STALE_ON_CONFLICT: Record<string, string> = {}
|
||||
* same patch-shape rationale as webshop-orders ingest: one literal per key
|
||||
* combination is not viable. The field set is pinned by validatePatch and
|
||||
* covered by payroll-executors.test.ts; both selects around it are literals.
|
||||
*
|
||||
* 2026-08-30 recurring payroll lines (#2042): +2 for the same two shapes the
|
||||
* employee_benefits code already carries: the step-8d3 derived-rows insert
|
||||
* (rows built in a .map with literal keys, opaque to the scanner) and the
|
||||
* PATCH route's merged-updates payload (explicit literal keys, but assembled
|
||||
* conditionally into a variable). Both carry scoped assertions instead:
|
||||
* employee-recurring-lines.pg.test.ts inserts the derived-row shape against
|
||||
* the real table, and the PATCH route test pins the exact writable column
|
||||
* set ("writes exactly the patchable columns and nothing else"). Making
|
||||
* either literal would cost a real property: the PATCH would have to write
|
||||
* every column on every request, turning a partial update into
|
||||
* last-write-wins.
|
||||
*/
|
||||
// 2026-08-20: +1 for lib/connect/instance/sync.ts, whose capability_grants
|
||||
// upsert is a per-company x per-scope row array built at runtime (one chunked
|
||||
@@ -153,7 +165,9 @@ const KNOWN_STALE_ON_CONFLICT: Record<string, string> = {}
|
||||
// lines as a row array built from one literal mapper (toInsertRow). Every
|
||||
// header/line update in the module is an object literal. Merged with main
|
||||
// (parties phase 1, #2162/#2168/#2169) at 395: 397.
|
||||
const UNRESOLVED_CEILING = 397
|
||||
// 2026-09-04: +2 recurring lines (#2044, see the 2026-08-30 recurring payroll
|
||||
// lines note above); merged with main (#2141/#2164/#2170/#2192) at 397: 399.
|
||||
const UNRESOLVED_CEILING = 399
|
||||
|
||||
/**
|
||||
* Floor on statically resolved column references. Guards the guard: if a change
|
||||
|
||||
Reference in New Issue
Block a user