fix(salary): one save per surface on the employee form (#1623)
* fix(salary): one save per surface on the employee form The employee edit page stacked two competing saves: the opening-balances Card ended in "Spara ingaende saldon" and the page ended in "Spara andringar" 80px below, with no visible boundary between their scopes. Worse, both self-saving panels lived INSIDE the page <form> and shadcn Button sets no default type, so every panel button (save opening balances, add/remove benefit) implicitly submitted the outer form too, firing the full employee PATCH alongside the panel's own request. Restructure so each surface owns exactly one save: - The employee <form> now closes right after the Bank card, with Avbryt + "Spara andringar" directly under the fields it actually saves. - Formaner and Ingaende saldon move below the form into a "Sparas separat" section (uppercase kicker + one-line scope hint) so the page save structurally cannot include them and their buttons can no longer leak submits into the employee form. - OpeningBalancesPanel becomes its own <form>: Enter saves the panel, and the save button enables only when its fields are actually dirty (fingerprint of loaded values, reset on successful save). - EmployeeBenefitsPanel buttons get explicit type="button". New strings in both messages/sv.json and messages/en.json. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(salary): release the loading skeleton when the balances fetch fails CodeRabbit on #1623: a rejected fetch or JSON parse skipped the setLoading(false) line, holding the skeleton forever. The load now wraps in try/finally; a failed load falls back to the empty form. 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:
Jakob Wennberg
co-authored by
Claude Fable 5
Jakob Wennberg
Claude Fable 5
Jakob Wennberg
parent
dd4ced1f93
commit
51539b93ed
@@ -993,6 +993,7 @@ One line per decision: `[YYYY-MM-DD] <decision>: <why>`. Appended by agents and
|
||||
[2026-08-14] The onboarding branch question ("Var fanns bokföringen innan?") became its own journey step at the existing KLART station (done -> source, same station grammar as momsyn/moms under MOMSEN) instead of a sixth rail station: a 6-point rail crowds the 680px band's 150px label slots, "BOKFÖRINGEN INNAN" would sit next to the existing "BOKFÖRINGEN" station, and mode='add' (which never sees the branch question) would need an asymmetric rail. The done screen ends in a revealed Fortsätt that dispatches DONE_CONTINUE (mode='first' only, reducer-guarded).
|
||||
[2026-08-14] Migration /preview fetches ALL allowed fiscal years (dropped latestOnly): the connect step's "Hittade X konton och Y verifikationer" renders from /preview's sieStats, not /sie-data's generateImportPreview, so fixing only /sie-data would have left the founder-reported "0 verifikationer" (actual: 4153) on screen. Costs one SIE export per extra year at connect time, the same work /sie-data repeats right after; honest counts won over latency.
|
||||
[2026-08-14] /sie-data validation stays newest-file-only (not per-file, not on the merged parse): preserves exactly which datasets are accepted today, and validateSIEFile assumes single-file invariants (balance yearIndexes relative to ONE current year) that mergeParsedSIEFiles deliberately does not preserve. Older files' problems still surface per-file at import time.
|
||||
[2026-08-15] Employee form save-scope fix: moved the self-saving sections (Förmåner, Ingående saldon) OUTSIDE the page <form> under a "Sparas separat" kicker with a one-line scope hint, kept them as Cards (locked section pattern; the task-suggested hairline no-box region would deviate from every sibling section) and rejected the tab/sub-page shape (page has no tab structure, would be more invasive). This also fixes a real double-fire: shadcn Button has no default type, so the panels' save/add/delete buttons inside the form implicitly submitted the whole employee PATCH on every click; opening-balances now has its own <form> whose save enables only when dirty.
|
||||
[2026-08-14] SKV manual-verifikat deep link payload moved from URL params to single-use sessionStorage (supersedes same-day URL-params decision): compliance swarm flagged financial data in query strings landing in history/access logs/Referer (GDPR Art.5(1)(f), ISO A.8.12); URL now carries only the opaque row id.
|
||||
[2026-08-14] SKV prefill sessionStorage XSS window accepted as residual risk (ISO A.8.12 low, swarm PR #1621): script execution already implies full ledger read via authenticated APIs; a server-issued staging token adds a roundtrip, not protection. Documented in manual-verifikat-prefill.ts header.
|
||||
[2026-08-15] BankID mobile stranded-tab fix: the session id never reaches the browser; it lives in a signed `__Host-` HttpOnly cookie (extensions/general/tic/lib/bankid-flow-cookie.ts) that /poll, /complete, /link and /cancel read. Reported bug: BankID does not reliably return the user to the tab that started the flow (outside plain Safari the iOS https redirect goes to the OS, which opens a NEW tab), the session lived in per-tab sessionStorage, so the new tab rendered the start button and the completed signup was stranded. Login hid it because its completion is self-finishing and the Supabase session is a cookie every tab shares. Cookies being origin-wide is exactly what the handoff needed, so there is no client storage, no cross-tab lock and no heartbeat. TWO REJECTED ATTEMPTS, both killed by adversarial review before merge, both worth not re-deriving. (1) localStorage plus an owner/heartbeat lock: a TIC sessionId is an unauthenticated bearer credential (/bankid/poll is skipAuth and returned user.personalNumber verbatim; /bankid/complete with mode 'login' returns a tokenHash that verifyOtp turns into a session, MFA skipped via bankid_linked), so origin-wide persistent storage turned ANY same-origin XSS into a login-fixation primitive, and the design also deadlocked in the very flow it fixed (the hidden tab stopped polling, the new tab cleared the record, and the storage event never reaches a tab iOS has frozen). (2) The first cookie version: it pinned `mode` but not the USER, so on a shared browser an abandoned link flow bound the first person's personnummer to the second person's account, which is the same class of flaw that killed (1); its cookie also used a narrow Path, which a script can shadow by setting the same name at a LONGER path (RFC 6265 sends longer paths first, and a Max-Age=0 at the shorter path cannot delete it); and its "single-use" was a Set-Cookie, which does not serialise two requests that already carried the cookie. Consequences now, all deliberate: `__Host-` with Path=/ and unconditional Secure, because the prefix forbids Domain and forces Path=/, leaving exactly one possible (name, domain, path) and making both the longer-path shadow and a subdomain toss unsettable (Secure costs nothing since isBankIdEnabled() is false when NEXT_PUBLIC_SELF_HOSTED is set, so the earlier conditional-Secure only risked shipping unprotected behind a proxy that omits x-forwarded-proto); readBankIdFlow fails closed on two cookies of the name rather than picking a winner; SameSite=Lax not Strict, because the BankID return is a top-level navigation from outside the site; a `link` flow requires auth at /start and pins userId, and /link rejects a flow belonging to anyone else; single-use is a unique index (bankid_consumed_sessions, migration 20260815120000) claimed BEFORE generateLink, since serverless has no shared memory and a second magic link invalidates the first; the claim fails closed on any error other than 23505; account_exists deliberately does NOT consume, so a mistyped e-mail is correctable; the window is 300s for the order and re-issued at 900s once /poll observes completion, because the signup e-mail step is a person typing and on the old design it was bounded only by TIC's retention; /poll whitelists its response fields, so the personnummer is no longer returned to anyone, and clears the flow when TIC 404/410s or answers without a status, so a dead session cannot make BankID look unavailable for the rest of the window. The client resumes from ONE probe against /poll, never a readable hint cookie: a hint goes stale, cannot tell a live order from a dead one, and made the component poll on mount in states where nothing was in flight. `link` never auto-resumes at all, since picking one up without a click is how an abandoned flow binds to the next person. Android stays on redirect=null; no Android report motivates changing it and #194 closed that path deliberately.
|
||||
|
||||
@@ -487,12 +487,6 @@ export default function EmployeeDetailPage({ params }: { params: Promise<{ id: s
|
||||
</CardContent>
|
||||
</Card>
|
||||
|
||||
{/* Benefits */}
|
||||
<EmployeeBenefitsPanel employeeId={id} canWrite={canWrite} />
|
||||
|
||||
{/* Ingående saldon (payroll cutover) */}
|
||||
<OpeningBalancesPanel employeeId={id} canWrite={canWrite} />
|
||||
|
||||
{canWrite && (
|
||||
<div className="flex justify-end gap-3">
|
||||
<Button variant="outline" asChild>
|
||||
@@ -506,6 +500,25 @@ export default function EmployeeDetailPage({ params }: { params: Promise<{ id: s
|
||||
)}
|
||||
</form>
|
||||
|
||||
{/* Self-saving sections. These live OUTSIDE the employee form on purpose:
|
||||
benefits and opening balances write to their own endpoints, so the
|
||||
page-level "Spara ändringar" never touches them, and their buttons can
|
||||
no longer implicitly submit the employee form (default type="submit"). */}
|
||||
<section className="space-y-4">
|
||||
<div>
|
||||
<h2 className="text-sm font-medium uppercase tracking-wider text-muted-foreground">
|
||||
{t('detail_self_saving_heading')}
|
||||
</h2>
|
||||
<p className="text-sm text-muted-foreground mt-1">{t('detail_self_saving_hint')}</p>
|
||||
</div>
|
||||
|
||||
{/* Benefits */}
|
||||
<EmployeeBenefitsPanel employeeId={id} canWrite={canWrite} />
|
||||
|
||||
{/* Ingående saldon (payroll cutover) */}
|
||||
<OpeningBalancesPanel employeeId={id} canWrite={canWrite} />
|
||||
</section>
|
||||
|
||||
<DestructiveConfirmDialog {...dialogProps} />
|
||||
</div>
|
||||
)
|
||||
|
||||
@@ -132,7 +132,7 @@ export function EmployeeBenefitsPanel({ employeeId, canWrite }: { employeeId: st
|
||||
<CardHeader className="flex flex-row items-center justify-between">
|
||||
<CardTitle className="text-base">{t('benefits_title')}</CardTitle>
|
||||
{canWrite && !adding && (
|
||||
<Button size="sm" variant="outline" onClick={() => setAdding(true)}>
|
||||
<Button type="button" size="sm" variant="outline" onClick={() => setAdding(true)}>
|
||||
<Plus className="mr-2 h-4 w-4" />
|
||||
{t('benefits_add')}
|
||||
</Button>
|
||||
@@ -169,7 +169,7 @@ export function EmployeeBenefitsPanel({ employeeId, canWrite }: { employeeId: st
|
||||
</TableCell>
|
||||
<TableCell className="text-right">
|
||||
{canWrite && (
|
||||
<Button size="icon" variant="ghost" onClick={() => handleDelete(b.id)} aria-label={t('benefits_remove')}>
|
||||
<Button type="button" size="icon" variant="ghost" onClick={() => handleDelete(b.id)} aria-label={t('benefits_remove')}>
|
||||
<Trash2 className="h-4 w-4" />
|
||||
</Button>
|
||||
)}
|
||||
@@ -257,8 +257,8 @@ export function EmployeeBenefitsPanel({ employeeId, canWrite }: { employeeId: st
|
||||
</div>
|
||||
|
||||
<div className="flex justify-end gap-2">
|
||||
<Button variant="outline" size="sm" onClick={reset} disabled={submitting}>{t('form_cancel')}</Button>
|
||||
<Button size="sm" onClick={handleAdd} disabled={submitting}>
|
||||
<Button type="button" variant="outline" size="sm" onClick={reset} disabled={submitting}>{t('form_cancel')}</Button>
|
||||
<Button type="button" size="sm" onClick={handleAdd} disabled={submitting}>
|
||||
{submitting && <Loader2 className="mr-2 h-4 w-4 animate-spin" />}
|
||||
{t('form_save')}
|
||||
</Button>
|
||||
|
||||
@@ -40,6 +40,31 @@ const currentYear = new Date().getFullYear()
|
||||
/** Sparade dagar origin years: Semesterlagen allows saving max 5 years. */
|
||||
const SAVED_YEARS = Array.from({ length: 5 }, (_, i) => String(currentYear - 1 - i))
|
||||
|
||||
interface PanelValues {
|
||||
cutoverDate: string
|
||||
ytdGross: string
|
||||
ytdTax: string
|
||||
ytdNet: string
|
||||
daysRemaining: string
|
||||
daysTaken: string
|
||||
savedByYear: Record<string, string>
|
||||
liability: string
|
||||
liabilityAvgifter: string
|
||||
karens: string
|
||||
}
|
||||
|
||||
/**
|
||||
* Stable serialization of the panel's field values, used to gate the save
|
||||
* button on actual edits. Empty saved-days entries equal absent entries, so
|
||||
* typing into a year and clearing it again returns the panel to clean.
|
||||
*/
|
||||
function fingerprint(v: PanelValues): string {
|
||||
const saved = Object.entries(v.savedByYear)
|
||||
.filter(([, days]) => days.trim() !== '')
|
||||
.sort(([a], [b]) => a.localeCompare(b))
|
||||
return JSON.stringify({ ...v, savedByYear: saved })
|
||||
}
|
||||
|
||||
export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: string; canWrite: boolean }) {
|
||||
const t = useTranslations('salary_employee')
|
||||
const { toast } = useToast()
|
||||
@@ -58,37 +83,83 @@ export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: str
|
||||
const [liability, setLiability] = useState('')
|
||||
const [liabilityAvgifter, setLiabilityAvgifter] = useState('')
|
||||
const [karens, setKarens] = useState('')
|
||||
// Fingerprint of the last loaded/saved values: the save button stays
|
||||
// disabled until the fields actually differ from it.
|
||||
const [baseline, setBaseline] = useState(() =>
|
||||
fingerprint({
|
||||
cutoverDate: `${currentYear}-01-01`,
|
||||
ytdGross: '',
|
||||
ytdTax: '',
|
||||
ytdNet: '',
|
||||
daysRemaining: '',
|
||||
daysTaken: '',
|
||||
savedByYear: {},
|
||||
liability: '',
|
||||
liabilityAvgifter: '',
|
||||
karens: '',
|
||||
}),
|
||||
)
|
||||
|
||||
useEffect(() => {
|
||||
async function load() {
|
||||
setLoading(true)
|
||||
const res = await fetch(`/api/salary/employees/${employeeId}/opening-balances`)
|
||||
if (res.ok) {
|
||||
try {
|
||||
const res = await fetch(`/api/salary/employees/${employeeId}/opening-balances`)
|
||||
if (!res.ok) return
|
||||
const { data } = (await res.json()) as { data: OpeningBalancesData | null }
|
||||
if (data) {
|
||||
setHasRow(true)
|
||||
setLocked(data.locked)
|
||||
setCutoverDate(data.cutover_date)
|
||||
setYtdGross(String(data.ytd_gross))
|
||||
setYtdTax(String(data.ytd_tax))
|
||||
setYtdNet(String(data.ytd_net))
|
||||
setDaysRemaining(String(data.vacation_paid_days_remaining))
|
||||
setDaysTaken(String(data.vacation_days_taken_this_year ?? 0))
|
||||
setSavedByYear(
|
||||
Object.fromEntries(
|
||||
const values: PanelValues = {
|
||||
cutoverDate: data.cutover_date,
|
||||
ytdGross: String(data.ytd_gross),
|
||||
ytdTax: String(data.ytd_tax),
|
||||
ytdNet: String(data.ytd_net),
|
||||
daysRemaining: String(data.vacation_paid_days_remaining),
|
||||
daysTaken: String(data.vacation_days_taken_this_year ?? 0),
|
||||
savedByYear: Object.fromEntries(
|
||||
Object.entries(data.vacation_saved_days_by_year ?? {}).map(([y, d]) => [y, String(d)]),
|
||||
),
|
||||
)
|
||||
setLiability(String(data.opening_semester_liability))
|
||||
setLiabilityAvgifter(String(data.opening_semester_liability_avgifter))
|
||||
setKarens(String(data.karens_periods_adjustment))
|
||||
liability: String(data.opening_semester_liability),
|
||||
liabilityAvgifter: String(data.opening_semester_liability_avgifter),
|
||||
karens: String(data.karens_periods_adjustment),
|
||||
}
|
||||
setHasRow(true)
|
||||
setLocked(data.locked)
|
||||
setCutoverDate(values.cutoverDate)
|
||||
setYtdGross(values.ytdGross)
|
||||
setYtdTax(values.ytdTax)
|
||||
setYtdNet(values.ytdNet)
|
||||
setDaysRemaining(values.daysRemaining)
|
||||
setDaysTaken(values.daysTaken)
|
||||
setSavedByYear(values.savedByYear)
|
||||
setLiability(values.liability)
|
||||
setLiabilityAvgifter(values.liabilityAvgifter)
|
||||
setKarens(values.karens)
|
||||
setBaseline(fingerprint(values))
|
||||
}
|
||||
} catch {
|
||||
// Network failure: the panel falls back to its empty form rather
|
||||
// than holding the skeleton forever; a retry happens on remount.
|
||||
} finally {
|
||||
setLoading(false)
|
||||
}
|
||||
setLoading(false)
|
||||
}
|
||||
load()
|
||||
}, [employeeId])
|
||||
|
||||
const currentValues: PanelValues = {
|
||||
cutoverDate,
|
||||
ytdGross,
|
||||
ytdTax,
|
||||
ytdNet,
|
||||
daysRemaining,
|
||||
daysTaken,
|
||||
savedByYear,
|
||||
liability,
|
||||
liabilityAvgifter,
|
||||
karens,
|
||||
}
|
||||
const dirty = fingerprint(currentValues) !== baseline
|
||||
|
||||
async function handleSave() {
|
||||
setSaving(true)
|
||||
const saved: Record<string, number> = {}
|
||||
@@ -116,6 +187,7 @@ export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: str
|
||||
|
||||
if (res.ok) {
|
||||
setHasRow(true)
|
||||
setBaseline(fingerprint(currentValues))
|
||||
toast({ title: t('opening_balances_saved') })
|
||||
} else {
|
||||
const result = await res.json()
|
||||
@@ -150,7 +222,15 @@ export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: str
|
||||
<CardTitle className="text-base">{t('opening_balances_title')}</CardTitle>
|
||||
<p className="text-sm text-muted-foreground">{t('opening_balances_description')}</p>
|
||||
</CardHeader>
|
||||
<CardContent className="space-y-6">
|
||||
{/* Own form: this panel is its own save scope. It must never be nested
|
||||
inside another form (invalid HTML, and its submit would leak). */}
|
||||
<form
|
||||
onSubmit={(e) => {
|
||||
e.preventDefault()
|
||||
handleSave()
|
||||
}}
|
||||
>
|
||||
<CardContent className="space-y-6">
|
||||
{locked && (
|
||||
<p className="text-sm text-muted-foreground border border-border rounded-lg p-3">
|
||||
{t('opening_balances_locked_notice')}
|
||||
@@ -265,7 +345,7 @@ export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: str
|
||||
|
||||
{!readOnly && (
|
||||
<div className="flex justify-end">
|
||||
<Button onClick={handleSave} disabled={saving}>
|
||||
<Button type="submit" disabled={saving || !dirty}>
|
||||
<Save className="h-4 w-4 mr-2" />
|
||||
{saving
|
||||
? t('opening_balances_saving')
|
||||
@@ -275,7 +355,8 @@ export function OpeningBalancesPanel({ employeeId, canWrite }: { employeeId: str
|
||||
</Button>
|
||||
</div>
|
||||
)}
|
||||
</CardContent>
|
||||
</CardContent>
|
||||
</form>
|
||||
</Card>
|
||||
)
|
||||
}
|
||||
|
||||
@@ -6750,6 +6750,8 @@
|
||||
"detail_f_skatt_verified": "F-skatt verified: {date}",
|
||||
"detail_vacation_none_hint": "No accrual to 2920 is booked. Use if vacation is included in the monthly salary — common for owners who are the only employee.",
|
||||
"detail_save_changes": "Save changes",
|
||||
"detail_self_saving_heading": "Saved separately",
|
||||
"detail_self_saving_hint": "Benefits and opening balances are saved directly in each section and are not included in Save changes above.",
|
||||
"tax_title": "Tax",
|
||||
"tax_form_label": "Tax status",
|
||||
"tax_form_tooltip": "A-skatt: you withhold preliminary tax according to the tax table (skattetabell). F-skatt/FA-skatt: the person handles their own tax — no tax is withheld.",
|
||||
|
||||
@@ -6750,6 +6750,8 @@
|
||||
"detail_f_skatt_verified": "F-skatt verifierad: {date}",
|
||||
"detail_vacation_none_hint": "Ingen avsättning till 2920 bokas. Använd om semester ingår i månadslönen — vanligt för ägare som är enda anställd.",
|
||||
"detail_save_changes": "Spara ändringar",
|
||||
"detail_self_saving_heading": "Sparas separat",
|
||||
"detail_self_saving_hint": "Förmåner och ingående saldon sparas direkt i respektive sektion och ingår inte i Spara ändringar ovan.",
|
||||
"tax_title": "Skatt",
|
||||
"tax_form_label": "Skatteform",
|
||||
"tax_form_tooltip": "A-skatt: du drar preliminärskatt enligt skattetabell. F-skatt/FA-skatt: personen sköter sin egen skatt — inget skatteavdrag görs.",
|
||||
|
||||
Reference in New Issue
Block a user