fix(settings): preserve Rules drafts across internal tabs
This commit is contained in:
@@ -1,13 +1,11 @@
|
||||
import React, { useEffect, useState } from "react";
|
||||
import React from "react";
|
||||
|
||||
import { Alert, Box, Button, TextField, Typography } from "@mui/material";
|
||||
import { PageSection } from "../layout/PageLayout";
|
||||
|
||||
import { api, getApiErrorMessage } from "../api";
|
||||
import { useToast } from "../toast";
|
||||
import { useI18n } from "../i18n/I18nProvider";
|
||||
|
||||
type RuleSettings = {
|
||||
export type RuleSettings = {
|
||||
id: number;
|
||||
appliedFollowUpDays: number;
|
||||
appliedGhostDays: number;
|
||||
@@ -17,61 +15,47 @@ type RuleSettings = {
|
||||
feedbackGhostDays: number;
|
||||
};
|
||||
|
||||
export default function RulesSettingsCard() {
|
||||
const { toast } = useToast();
|
||||
export const RULE_FIELDS = ["appliedFollowUpDays", "appliedGhostDays", "offerFollowUpDays", "offerGhostDays", "feedbackFollowUpDays", "feedbackGhostDays"] as const;
|
||||
export type RuleField = typeof RULE_FIELDS[number];
|
||||
export type RuleDraft = Record<RuleField, string>;
|
||||
|
||||
type Props = {
|
||||
draft: RuleDraft | null;
|
||||
loadError: string | null;
|
||||
saveError: boolean;
|
||||
invalid: RuleField[];
|
||||
saving: boolean;
|
||||
dirty: boolean;
|
||||
onChange: (field: RuleField, value: string) => void;
|
||||
onSave: () => void;
|
||||
onDiscard: () => void;
|
||||
onRetry: () => void;
|
||||
};
|
||||
|
||||
export default function RulesSettingsCard({ draft, loadError, saveError, invalid, saving, dirty, onChange, onSave, onDiscard, onRetry }: Props) {
|
||||
const { t } = useI18n();
|
||||
const [s, setS] = useState<RuleSettings | null>(null);
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [loadError, setLoadError] = useState<string | null>(null);
|
||||
|
||||
useEffect(() => {
|
||||
let active = true;
|
||||
api.get<RuleSettings>("/rules")
|
||||
.then((r) => {
|
||||
if (!active) return;
|
||||
setS(r.data);
|
||||
setLoadError(null);
|
||||
})
|
||||
.catch((e: any) => {
|
||||
if (!active) return;
|
||||
setS(null);
|
||||
setLoadError(getApiErrorMessage(e, t("rulesLoadFailed")));
|
||||
});
|
||||
return () => { active = false; };
|
||||
}, [t]);
|
||||
|
||||
const save = async () => {
|
||||
if (!s) return;
|
||||
setSaving(true);
|
||||
try {
|
||||
await api.put("/rules", s);
|
||||
toast(t("rulesSave"), "success");
|
||||
} catch {
|
||||
toast(t("rulesSaveFailed"), "error");
|
||||
} finally {
|
||||
setSaving(false);
|
||||
}
|
||||
};
|
||||
|
||||
if (!s) {
|
||||
if (!draft) {
|
||||
return (
|
||||
<PageSection surface title={t("rulesTitle")}>
|
||||
{loadError ? <Alert severity="error">{loadError}</Alert> : <Typography sx={{ color: "text.secondary" }}>{t("rulesLoading")}</Typography>}
|
||||
{loadError ? <Alert severity="error" action={<Button color="inherit" onClick={onRetry}>{t("retry")}</Button>}>{loadError}</Alert> : <Typography role="status" sx={{ color: "text.secondary" }}>{t("rulesLoading")}</Typography>}
|
||||
</PageSection>
|
||||
);
|
||||
}
|
||||
|
||||
const num = (k: keyof RuleSettings) => ({
|
||||
value: s[k],
|
||||
const num = (k: RuleField) => ({
|
||||
value: draft[k],
|
||||
onChange: (e: React.ChangeEvent<HTMLInputElement>) =>
|
||||
setS((p) => (p ? { ...p, [k]: Number(e.target.value) } : p)),
|
||||
onChange(k, e.target.value),
|
||||
type: "number" as const,
|
||||
inputProps: { min: 1, max: 365 },
|
||||
error: invalid.includes(k),
|
||||
helperText: invalid.includes(k) ? t("rulesDaysInvalid") : undefined,
|
||||
inputProps: { min: 1, max: 365, step: 1, required: true },
|
||||
});
|
||||
|
||||
return (
|
||||
<PageSection surface title={t("rulesTitle")} description={t("rulesBody")}>
|
||||
|
||||
<Box component="form" noValidate onSubmit={(event) => { event.preventDefault(); onSave(); }}>
|
||||
{saveError ? <Alert severity="error" sx={{ mb: 2 }}>{t("rulesSaveFailed")}</Alert> : null}
|
||||
<Box sx={{ display: "grid", gridTemplateColumns: { xs: "1fr", md: "1fr 1fr 1fr" }, gap: 2 }}>
|
||||
<TextField label={t("rulesAppliedFollowUpDays")} {...num("appliedFollowUpDays")} />
|
||||
<TextField label={t("rulesAppliedGhostDays")} {...num("appliedGhostDays")} />
|
||||
@@ -84,11 +68,13 @@ export default function RulesSettingsCard() {
|
||||
<Box />
|
||||
</Box>
|
||||
|
||||
<Box sx={{ display: "flex", justifyContent: "flex-end", mt: 2 }}>
|
||||
<Button variant="contained" onClick={save} disabled={saving}>
|
||||
<Box sx={{ display: "flex", justifyContent: "flex-end", flexWrap: "wrap", gap: 1, mt: 2 }}>
|
||||
<Button onClick={onDiscard} disabled={!dirty || saving}>{t("assetsDiscardChanges")}</Button>
|
||||
<Button variant="contained" type="submit" disabled={saving}>
|
||||
{saving ? t("rulesSaving") : t("rulesSave")}
|
||||
</Button>
|
||||
</Box>
|
||||
</Box>
|
||||
</PageSection>
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1,4 +1,4 @@
|
||||
import React, { useEffect, useState } from "react";
|
||||
import React, { useEffect, useRef, useState } from "react";
|
||||
|
||||
import {
|
||||
Box,
|
||||
@@ -20,7 +20,7 @@ import { useNavigate } from "react-router-dom";
|
||||
|
||||
import { JobTableColumns } from "./JobTable";
|
||||
import ImportExportJobs from "./ImportExportJobs";
|
||||
import RulesSettingsCard from "./RulesSettingsCard";
|
||||
import RulesSettingsCard, { RULE_FIELDS, RuleDraft, RuleField, RuleSettings } from "./RulesSettingsCard";
|
||||
import BackupCard from "./BackupCard";
|
||||
import QuickCaptureCard from "./QuickCaptureCard";
|
||||
import AiUsageCard from "./AiUsageCard";
|
||||
@@ -49,6 +49,10 @@ type NotificationPrefs = {
|
||||
emailFollowUpRemindersEnabled: boolean;
|
||||
};
|
||||
|
||||
function ruleDraft(settings: RuleSettings): RuleDraft {
|
||||
return Object.fromEntries(RULE_FIELDS.map(field => [field, String(settings[field])])) as RuleDraft;
|
||||
}
|
||||
|
||||
export default function SettingsView({
|
||||
pageSize,
|
||||
onPageSizeChange,
|
||||
@@ -64,6 +68,73 @@ export default function SettingsView({
|
||||
const [notificationPrefs, setNotificationPrefs] = useState<NotificationPrefs | null>(null);
|
||||
const [notificationError, setNotificationError] = useState<string | null>(null);
|
||||
const [savingNotifications, setSavingNotifications] = useState(false);
|
||||
const [savedRules, setSavedRules] = useState<RuleSettings | null>(null);
|
||||
const [rulesDraft, setRulesDraft] = useState<RuleDraft | null>(null);
|
||||
const [rulesLoadError, setRulesLoadError] = useState<string | null>(null);
|
||||
const [rulesSaveError, setRulesSaveError] = useState(false);
|
||||
const [invalidRules, setInvalidRules] = useState<RuleField[]>([]);
|
||||
const [savingRules, setSavingRules] = useState(false);
|
||||
const rulesSession = useRef({ mounted: true, loading: false, saving: false });
|
||||
const rulesDirty = Boolean(rulesDraft && savedRules && RULE_FIELDS.some(field => rulesDraft[field] !== String(savedRules[field])));
|
||||
|
||||
useEffect(() => {
|
||||
const session = rulesSession.current;
|
||||
session.mounted = true;
|
||||
return () => { session.mounted = false; };
|
||||
}, []);
|
||||
|
||||
const loadRules = async () => {
|
||||
const session = rulesSession.current;
|
||||
if (rulesDraft || session.loading) return;
|
||||
session.loading = true;
|
||||
setRulesLoadError(null);
|
||||
try {
|
||||
const { data } = await api.get<RuleSettings>("/rules");
|
||||
if (!session.mounted) return;
|
||||
setSavedRules(data);
|
||||
setRulesDraft(ruleDraft(data));
|
||||
} catch (error) {
|
||||
if (session.mounted) setRulesLoadError(getApiErrorMessage(error, t("rulesLoadFailed")));
|
||||
} finally { session.loading = false; }
|
||||
};
|
||||
|
||||
const selectSection = (next: number) => {
|
||||
setTab(next);
|
||||
if (next === 1) void loadRules();
|
||||
};
|
||||
|
||||
const saveRules = async () => {
|
||||
const session = rulesSession.current;
|
||||
if (!rulesDraft || !savedRules || session.saving) return;
|
||||
const invalid = RULE_FIELDS.filter(field => !rulesDraft[field].trim() || !Number.isInteger(Number(rulesDraft[field])) || Number(rulesDraft[field]) < 1 || Number(rulesDraft[field]) > 365);
|
||||
setInvalidRules(invalid);
|
||||
if (invalid.length) return;
|
||||
const submittedDraft = rulesDraft;
|
||||
const submitted = { id: savedRules.id, ...Object.fromEntries(RULE_FIELDS.map(field => [field, Number(submittedDraft[field])])) } as RuleSettings;
|
||||
session.saving = true;
|
||||
setSavingRules(true);
|
||||
setRulesSaveError(false);
|
||||
try {
|
||||
await api.put("/rules", submitted);
|
||||
if (!session.mounted) return;
|
||||
setSavedRules(submitted);
|
||||
// A completed save acknowledges its snapshot, not edits made while it was pending.
|
||||
setRulesDraft(current => current && RULE_FIELDS.every(field => current[field] === submittedDraft[field]) ? ruleDraft(submitted) : current);
|
||||
toast(t("rulesSaved"), "success");
|
||||
} catch {
|
||||
if (session.mounted) setRulesSaveError(true);
|
||||
} finally {
|
||||
session.saving = false;
|
||||
if (session.mounted) setSavingRules(false);
|
||||
}
|
||||
};
|
||||
|
||||
const discardRules = () => {
|
||||
if (!savedRules || savingRules) return;
|
||||
setRulesDraft(ruleDraft(savedRules));
|
||||
setInvalidRules([]);
|
||||
setRulesSaveError(false);
|
||||
};
|
||||
|
||||
useEffect(() => {
|
||||
let active = true;
|
||||
@@ -94,7 +165,7 @@ export default function SettingsView({
|
||||
labelId="settings-section-label"
|
||||
value={tab}
|
||||
label={t("settingsNavigation")}
|
||||
onChange={(event) => setTab(Number(event.target.value))}
|
||||
onChange={(event) => selectSection(Number(event.target.value))}
|
||||
>
|
||||
<MenuItem value={0}>{t("settingsTabGeneral")}</MenuItem>
|
||||
<MenuItem value={1}>{t("settingsTabFollowUps")}</MenuItem>
|
||||
@@ -103,13 +174,15 @@ export default function SettingsView({
|
||||
</Select>
|
||||
</FormControl>
|
||||
|
||||
<Tabs aria-label={t("settings")} value={tab} onChange={(_, v) => setTab(v)} variant="scrollable" scrollButtons="auto" sx={{ display: { xs: "none", sm: "flex" }, mb: 1 }}>
|
||||
<Tabs aria-label={t("settings")} value={tab} onChange={(_, v) => selectSection(v)} variant="scrollable" scrollButtons="auto" sx={{ display: { xs: "none", sm: "flex" }, mb: 1 }}>
|
||||
<Tab label={t("settingsTabGeneral")} />
|
||||
<Tab label={t("settingsTabFollowUps")} />
|
||||
<Tab label={t("settingsTabNotifications")} />
|
||||
<Tab label={t("settingsTabBackup")} />
|
||||
</Tabs>
|
||||
|
||||
{rulesDirty ? <Typography role="status" variant="body2" sx={{ mt: 1, color: "text.secondary" }}>{t("assetsUnsavedChanges")}. {t("rulesDraftScope")}</Typography> : null}
|
||||
|
||||
<TabPanel value={tab} index={0}>
|
||||
<Box sx={{ display: "grid", gap: "var(--app-section-gap)", minWidth: 0 }}>
|
||||
<Box sx={{ display: "grid", gridTemplateColumns: { xs: "minmax(0, 1fr)", md: "repeat(2, minmax(0, 1fr))" }, gap: "var(--app-section-gap)" }}>
|
||||
@@ -201,7 +274,12 @@ export default function SettingsView({
|
||||
</TabPanel>
|
||||
|
||||
<TabPanel value={tab} index={1}>
|
||||
<RulesSettingsCard />
|
||||
<RulesSettingsCard draft={rulesDraft} loadError={rulesLoadError} saveError={rulesSaveError} invalid={invalidRules}
|
||||
saving={savingRules} dirty={rulesDirty} onSave={() => void saveRules()} onRetry={() => void loadRules()} onDiscard={discardRules}
|
||||
onChange={(field, value) => {
|
||||
setRulesDraft(current => current ? { ...current, [field]: value } : current);
|
||||
setInvalidRules(current => current.filter(key => key !== field));
|
||||
}} />
|
||||
</TabPanel>
|
||||
|
||||
<TabPanel value={tab} index={2}>
|
||||
|
||||
@@ -2453,6 +2453,9 @@ export const translations = {
|
||||
rulesSaving: "Saving...",
|
||||
rulesSave: "Save Rules",
|
||||
rulesSaveFailed: "Failed to save rules.",
|
||||
rulesSaved: "Rules saved.",
|
||||
rulesDaysInvalid: "Enter a whole number from 1 to 365.",
|
||||
rulesDraftScope: "Rules changes stay while you switch Settings sections. Save or discard them before leaving Settings; leaving discards unsaved changes.",
|
||||
workspace: "Workspace",
|
||||
workspaceBack: "Back to applications",
|
||||
workspaceOpenFull: "Open full-page workspace",
|
||||
@@ -5033,6 +5036,9 @@ export const translations = {
|
||||
rulesSaving: "Lagrer...",
|
||||
rulesSave: "Lagre regler",
|
||||
rulesSaveFailed: "Kunne ikke lagre regler.",
|
||||
rulesSaved: "Reglene er lagret.",
|
||||
rulesDaysInvalid: "Skriv et helt tall fra 1 til 365.",
|
||||
rulesDraftScope: "Regelendringer beholdes når du bytter del i Innstillinger. Lagre eller forkast dem før du går ut; ulagrede endringer går tapt når du forlater Innstillinger.",
|
||||
workspace: "Arbeidsområde",
|
||||
workspaceBack: "Tilbake til søknader",
|
||||
workspaceOpenFull: "Åpne arbeidsområdet på helside",
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import React from 'react';
|
||||
import '@testing-library/jest-dom';
|
||||
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
|
||||
import { MemoryRouter } from 'react-router-dom';
|
||||
|
||||
import SettingsView from './components/SettingsView';
|
||||
@@ -181,3 +181,122 @@ test('[PRES-S03 PRES-J05] table and theme preferences keep their independent cal
|
||||
expect(onThemeModeChange).toHaveBeenCalledWith('light');
|
||||
expect(mockedApi.put).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
async function openRules() {
|
||||
fireEvent.click(screen.getByRole('tab', { name: /follow-ups/i }));
|
||||
const field = await screen.findByLabelText('Applied: follow-up days');
|
||||
await waitFor(() => expect(field).toHaveValue(14));
|
||||
return field;
|
||||
}
|
||||
|
||||
function rulesRoundTrip() {
|
||||
fireEvent.click(screen.getByRole('tab', { name: /notifications/i }));
|
||||
fireEvent.click(screen.getByRole('tab', { name: /follow-ups/i }));
|
||||
}
|
||||
|
||||
test('[PRES-S03 W11] the Settings session retains independent pending Rules fields across tab unmounts', async () => {
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
fireEvent.change(screen.getByLabelText('Offer: follow-up days'), { target: { value: '9' } });
|
||||
rulesRoundTrip();
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(5);
|
||||
expect(screen.getByLabelText('Offer: follow-up days')).toHaveValue(9);
|
||||
expect(screen.getByLabelText('Feedback: follow-up days')).toHaveValue(7);
|
||||
expect(screen.getByRole('status')).toHaveTextContent(/unsaved/i);
|
||||
expect(mockedApi.put).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('[PRES-S03] failed Rules save remains actionable after tab return and explicit retry clears dirty state', async () => {
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
mockedApi.put.mockRejectedValueOnce(new Error('offline'));
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
expect(await screen.findByRole('alert')).toHaveTextContent(/failed to save rules/i);
|
||||
rulesRoundTrip();
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(5);
|
||||
expect(screen.getByRole('alert')).toHaveTextContent(/failed to save rules/i);
|
||||
expect(screen.getByRole('status')).toHaveTextContent(/unsaved/i);
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
await waitFor(() => expect(mockedApi.put).toHaveBeenCalledTimes(2));
|
||||
await waitFor(() => expect(screen.queryByRole('status')).not.toBeInTheDocument());
|
||||
expect(screen.queryByText('Failed to save rules.')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('[PRES-S03] invalid and empty Rules input and associated validation survive internal navigation', async () => {
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '' } });
|
||||
fireEvent.change(screen.getByLabelText('Offer: follow-up days'), { target: { value: '366' } });
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
expect(mockedApi.put).not.toHaveBeenCalled();
|
||||
rulesRoundTrip();
|
||||
const empty = screen.getByLabelText('Applied: follow-up days');
|
||||
expect(empty).toHaveValue(null);
|
||||
expect(empty).toHaveAttribute('aria-invalid', 'true');
|
||||
expect(empty).toHaveAccessibleDescription(/whole number.*1.*365/i);
|
||||
expect(screen.getByLabelText('Offer: follow-up days')).toHaveValue(366);
|
||||
fireEvent.change(empty, { target: { value: '5' } });
|
||||
fireEvent.change(screen.getByLabelText('Offer: follow-up days'), { target: { value: '9' } });
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
await waitFor(() => expect(mockedApi.put).toHaveBeenCalledTimes(1));
|
||||
});
|
||||
|
||||
test('[PRES-S03] explicit discard resets only the Rules draft to its saved baseline without writing', async () => {
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
rulesRoundTrip();
|
||||
fireEvent.click(screen.getByRole('button', { name: /discard changes/i }));
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(14);
|
||||
expect(screen.queryByRole('status')).not.toBeInTheDocument();
|
||||
expect(mockedApi.put).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('[PRES-S03] later edits stay dirty when an earlier Rules save completes after a tab switch', async () => {
|
||||
let finish!: (value: any) => void;
|
||||
mockedApi.put.mockImplementationOnce(() => new Promise(resolve => { finish = resolve; }));
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
fireEvent.change(screen.getByLabelText('Applied: follow-up days'), { target: { value: '6' } });
|
||||
rulesRoundTrip();
|
||||
await act(async () => finish({ status: 204 }));
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(6);
|
||||
expect(screen.getByRole('status')).toHaveTextContent(/unsaved/i);
|
||||
fireEvent.click(screen.getByRole('button', { name: /discard changes/i }));
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(5);
|
||||
});
|
||||
|
||||
test('[PRES-S03 E09] leaving the Settings session disposes its draft and pending responses', async () => {
|
||||
let finish!: (value: any) => void;
|
||||
mockedApi.put.mockImplementationOnce(() => new Promise(resolve => { finish = resolve; }));
|
||||
const first = renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
fireEvent.click(screen.getByRole('button', { name: /save rules/i }));
|
||||
first.unmount();
|
||||
renderView();
|
||||
await openRules();
|
||||
await act(async () => finish({ status: 204 }));
|
||||
expect(screen.getByLabelText('Applied: follow-up days')).toHaveValue(14);
|
||||
expect(screen.queryByRole('status')).not.toBeInTheDocument();
|
||||
});
|
||||
|
||||
test('[PRES-S03] switching locale while Rules is hidden does not reload or overwrite its pending draft', async () => {
|
||||
renderView();
|
||||
fireEvent.change(await openRules(), { target: { value: '5' } });
|
||||
const reads = mockedApi.get.mock.calls.filter(([url]) => url === '/rules').length;
|
||||
fireEvent.click(screen.getByRole('tab', { name: /general/i }));
|
||||
fireEvent.mouseDown(screen.getByRole('combobox', { name: 'Preferred language' }));
|
||||
fireEvent.click(screen.getByRole('option', { name: 'Norwegian Bokmål' }));
|
||||
fireEvent.click(screen.getByRole('tab', { name: 'Oppfølging' }));
|
||||
expect(screen.getByLabelText('Søkt: oppfølgingsdager')).toHaveValue(5);
|
||||
expect(mockedApi.get.mock.calls.filter(([url]) => url === '/rules')).toHaveLength(reads);
|
||||
});
|
||||
|
||||
test('[PRES-S03] Rules draft updates do not refetch an unrelated notification preference', async () => {
|
||||
renderView();
|
||||
const field = await openRules();
|
||||
const reads = mockedApi.get.mock.calls.filter(([url]) => url === '/notification-settings').length;
|
||||
fireEvent.change(field, { target: { value: '5' } });
|
||||
fireEvent.change(field, { target: { value: '6' } });
|
||||
await act(async () => undefined);
|
||||
expect(mockedApi.get.mock.calls.filter(([url]) => url === '/notification-settings')).toHaveLength(reads);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user