feat(ui): add confirmation modal for destructive default user settings changes

When removing teams, clearing budgets, or changing roles in Default User
Settings, a "Review Changes" modal now appears summarizing what will change
before persisting. Non-destructive changes (additions only) save directly.
This commit is contained in:
Ryan Crabbe 2026-03-21 09:17:55 -07:00
parent c68a8aa356
commit c63ad09a86
3 changed files with 606 additions and 40 deletions

View file

@ -1,6 +1,6 @@
import { act, fireEvent, render, screen, waitFor } from "@testing-library/react";
import { beforeEach, describe, expect, it, vi } from "vitest";
import DefaultUserSettings from "./DefaultUserSettings";
import DefaultUserSettings, { computeSettingsDiff } from "./DefaultUserSettings";
import * as networking from "./networking";
vi.mock("./networking", () => ({
@ -20,6 +20,10 @@ vi.mock("./common_components/budget_duration_dropdown", () => ({
getBudgetDurationLabel: (value: string) => value,
}));
vi.mock("@/utils/dataUtils", () => ({
formatNumberWithCommas: (n: number) => String(n),
}));
vi.mock("./key_team_helpers/fetch_available_models_team_key", () => ({
getModelDisplayName: (model: string) => model,
}));
@ -115,13 +119,11 @@ describe("DefaultUserSettings", () => {
expect(screen.queryByText("Edit Settings")).not.toBeInTheDocument();
});
it("should save settings when save button is clicked", async () => {
it("should save settings directly when no destructive changes", async () => {
// No changes at all → direct save (no modal)
mockGetInternalUserSettings.mockResolvedValue(mockSettings);
mockUpdateInternalUserSettings.mockResolvedValue({
settings: {
...mockSettings.values,
max_budget: 2000,
},
settings: mockSettings.values,
});
render(<DefaultUserSettings {...defaultProps} />);
@ -148,6 +150,295 @@ describe("DefaultUserSettings", () => {
expect(mockUpdateInternalUserSettings).toHaveBeenCalled();
});
// No modal should appear
expect(screen.queryByText("Review Changes")).not.toBeInTheDocument();
expect(screen.getByText("Edit Settings")).toBeInTheDocument();
});
it("should show confirmation modal when a team is removed", async () => {
const settingsWithTeams = {
...mockSettings,
values: {
...mockSettings.values,
teams: [
{ team_id: "team-alpha", max_budget_in_team: 50, user_role: "user" },
{ team_id: "team-beta", max_budget_in_team: 25, user_role: "admin" },
],
},
};
mockGetInternalUserSettings.mockResolvedValue(settingsWithTeams);
render(<DefaultUserSettings {...defaultProps} />);
await waitFor(() => {
expect(screen.getByText("Edit Settings")).toBeInTheDocument();
});
// Enter edit mode
act(() => {
fireEvent.click(screen.getByText("Edit Settings"));
});
// Remove the first team
const removeButtons = screen.getAllByText("Remove");
act(() => {
fireEvent.click(removeButtons[0]);
});
// Click Save Changes
act(() => {
fireEvent.click(screen.getByText("Save Changes"));
});
// Modal should appear
await waitFor(() => {
expect(screen.getByText("Review Changes")).toBeInTheDocument();
});
// Should mention the removed team
expect(screen.getByText(/team-alpha/)).toBeInTheDocument();
// Save should NOT have been called yet
expect(mockUpdateInternalUserSettings).not.toHaveBeenCalled();
});
it("should save after confirming in the modal", async () => {
const settingsWithTeams = {
...mockSettings,
values: {
...mockSettings.values,
teams: [
{ team_id: "team-alpha", max_budget_in_team: 50, user_role: "user" },
{ team_id: "team-beta", max_budget_in_team: 25, user_role: "admin" },
],
},
};
mockGetInternalUserSettings.mockResolvedValue(settingsWithTeams);
mockUpdateInternalUserSettings.mockResolvedValue({
settings: {
...settingsWithTeams.values,
teams: [{ team_id: "team-beta", max_budget_in_team: 25, user_role: "admin" }],
},
});
render(<DefaultUserSettings {...defaultProps} />);
await waitFor(() => {
expect(screen.getByText("Edit Settings")).toBeInTheDocument();
});
// Enter edit mode, remove first team, click Save
act(() => {
fireEvent.click(screen.getByText("Edit Settings"));
});
const removeButtons = screen.getAllByText("Remove");
act(() => {
fireEvent.click(removeButtons[0]);
});
act(() => {
fireEvent.click(screen.getByText("Save Changes"));
});
// Wait for modal
await waitFor(() => {
expect(screen.getByText("Review Changes")).toBeInTheDocument();
});
// Click Confirm
await act(async () => {
fireEvent.click(screen.getByText("Confirm Changes"));
});
// Save should have been called
await waitFor(() => {
expect(mockUpdateInternalUserSettings).toHaveBeenCalledTimes(1);
});
// The saved teams should not include team-alpha
const sentValues = mockUpdateInternalUserSettings.mock.calls[0][1];
const sentTeamIds = sentValues.teams.map((t: any) => t.team_id);
expect(sentTeamIds).not.toContain("team-alpha");
expect(sentTeamIds).toContain("team-beta");
});
it("should NOT save when modal is cancelled", async () => {
const settingsWithTeams = {
...mockSettings,
values: {
...mockSettings.values,
teams: [
{ team_id: "team-alpha", max_budget_in_team: 50, user_role: "user" },
],
},
};
mockGetInternalUserSettings.mockResolvedValue(settingsWithTeams);
render(<DefaultUserSettings {...defaultProps} />);
await waitFor(() => {
expect(screen.getByText("Edit Settings")).toBeInTheDocument();
});
// Enter edit mode, remove team, click Save
act(() => {
fireEvent.click(screen.getByText("Edit Settings"));
});
const removeButtons = screen.getAllByText("Remove");
act(() => {
fireEvent.click(removeButtons[0]);
});
act(() => {
fireEvent.click(screen.getByText("Save Changes"));
});
// Wait for modal
await waitFor(() => {
expect(screen.getByText("Review Changes")).toBeInTheDocument();
});
// Click Cancel inside the modal (there are two Cancel buttons: one in the
// edit header and one in the modal footer). The modal's Cancel is the last
// one rendered.
const cancelButtons = screen.getAllByText("Cancel");
await act(async () => {
fireEvent.click(cancelButtons[cancelButtons.length - 1]);
});
// Save should NOT have been called
expect(mockUpdateInternalUserSettings).not.toHaveBeenCalled();
// Should still be in edit mode (Save Changes button visible)
expect(screen.getByText("Save Changes")).toBeInTheDocument();
});
});
// ---------------------------------------------------------------------------
// Tests: computeSettingsDiff (pure function)
// ---------------------------------------------------------------------------
describe("computeSettingsDiff", () => {
it("returns no changes when values are identical", () => {
const values = { max_budget: 100, models: ["gpt-4"], teams: [] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(values, { ...values });
expect(changes).toHaveLength(0);
expect(hasDestructiveChanges).toBe(false);
});
it("detects team removal as destructive", () => {
const original = {
teams: [
{ team_id: "team-alpha", user_role: "user" },
{ team_id: "team-beta", user_role: "admin" },
],
};
const edited = {
teams: [{ team_id: "team-alpha", user_role: "user" }],
};
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes).toEqual(
expect.arrayContaining([
expect.objectContaining({
type: "removed",
details: expect.stringContaining("team-beta"),
}),
]),
);
});
it("detects team addition as non-destructive", () => {
const original = { teams: [] };
const edited = { teams: [{ team_id: "new-team", user_role: "user" }] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(false);
expect(changes).toEqual(
expect.arrayContaining([expect.objectContaining({ type: "added" })]),
);
});
it("detects team budget change as destructive", () => {
const original = { teams: [{ team_id: "t1", max_budget_in_team: 100, user_role: "user" }] };
const edited = { teams: [{ team_id: "t1", max_budget_in_team: 50, user_role: "user" }] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes[0].type).toBe("changed");
expect(changes[0].details).toContain("$100");
expect(changes[0].details).toContain("$50");
});
it("detects model removal as destructive", () => {
const original = { models: ["gpt-4", "gpt-3.5-turbo"] };
const edited = { models: ["gpt-4"] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes).toEqual(
expect.arrayContaining([
expect.objectContaining({ type: "removed", details: expect.stringContaining("gpt-3.5-turbo") }),
]),
);
});
it("detects model addition as non-destructive", () => {
const original = { models: ["gpt-4"] };
const edited = { models: ["gpt-4", "claude-3"] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(false);
expect(changes).toEqual(
expect.arrayContaining([expect.objectContaining({ type: "added" })]),
);
});
it("detects scalar field cleared as destructive", () => {
const original = { max_budget: 100 };
const edited = { max_budget: null };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes[0].type).toBe("removed");
expect(changes[0].details).toContain("Cleared");
});
it("detects scalar field set as non-destructive", () => {
const original = { max_budget: null };
const edited = { max_budget: 200 };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(false);
expect(changes[0].type).toBe("added");
});
it("detects scalar value change as destructive", () => {
const original = { user_role: "internal_user" };
const edited = { user_role: "internal_user_view_only" };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes[0].type).toBe("changed");
});
it("handles string team_ids in original", () => {
const original = { teams: ["team-alpha", "team-beta"] };
const edited = { teams: [{ team_id: "team-alpha", user_role: "user" }] };
const { changes, hasDestructiveChanges } = computeSettingsDiff(original, edited);
expect(hasDestructiveChanges).toBe(true);
expect(changes).toEqual(
expect.arrayContaining([
expect.objectContaining({ type: "removed", details: expect.stringContaining("team-beta") }),
]),
);
});
it("treats empty string same as null for scalars", () => {
const original = { max_budget: "" };
const edited = { max_budget: null };
const { changes } = computeSettingsDiff(original, edited);
expect(changes).toHaveLength(0);
});
});

View file

@ -4,6 +4,7 @@ import { Button, Typography, Spin, Switch, Select, InputNumber } from "antd";
import { PlusOutlined, DeleteOutlined } from "@ant-design/icons";
import { getInternalUserSettings, updateInternalUserSettings, modelAvailableCall } from "./networking";
import BudgetDurationDropdown, { getBudgetDurationLabel } from "./common_components/budget_duration_dropdown";
import ConfirmSettingsChangeModal, { SettingsChange } from "./common_components/ConfirmSettingsChangeModal";
import { getModelDisplayName } from "./key_team_helpers/fetch_available_models_team_key";
import { formatNumberWithCommas } from "@/utils/dataUtils";
import NotificationManager from "./molecules/notifications_manager";
@ -21,6 +22,162 @@ interface TeamEntry {
user_role: "user" | "admin";
}
/** Normalize heterogeneous team arrays (strings or objects) to TeamEntry[]. */
function normalizeTeams(teams: any[]): TeamEntry[] {
if (!teams || !Array.isArray(teams)) return [];
return teams.map((team) => {
if (typeof team === "string") {
return { team_id: team, user_role: "user" as const };
} else if (typeof team === "object" && team.team_id) {
return {
team_id: team.team_id,
max_budget_in_team: team.max_budget_in_team,
user_role: team.user_role || "user",
};
}
return { team_id: "", user_role: "user" as const };
});
}
/**
* Compare original settings values against edited values and return a list of
* human-readable changes. A change is "destructive" if something was removed,
* cleared, or reduced.
*/
export function computeSettingsDiff(
original: Record<string, any>,
edited: Record<string, any>,
): { changes: SettingsChange[]; hasDestructiveChanges: boolean } {
const changes: SettingsChange[] = [];
const allKeys = new Set([...Object.keys(original), ...Object.keys(edited)]);
for (const key of allKeys) {
const oldVal = original[key];
const newVal = edited[key];
const displayName = key.replace(/_/g, " ").replace(/\b\w/g, (l) => l.toUpperCase());
if (key === "teams") {
const oldTeams = normalizeTeams(oldVal || []);
const newTeams = normalizeTeams(newVal || []);
const oldIds = new Set(oldTeams.map((t) => t.team_id));
const newIds = new Set(newTeams.map((t) => t.team_id));
// Removed teams
for (const t of oldTeams) {
if (!newIds.has(t.team_id)) {
changes.push({
field: displayName,
type: "removed",
details: `Team "${t.team_id}" removed`,
});
}
}
// Added teams
for (const t of newTeams) {
if (t.team_id && !oldIds.has(t.team_id)) {
changes.push({
field: displayName,
type: "added",
details: `Team "${t.team_id}" added`,
});
}
}
// Budget/role changes for teams that still exist
for (const newTeam of newTeams) {
if (!oldIds.has(newTeam.team_id)) continue;
const oldTeam = oldTeams.find((t) => t.team_id === newTeam.team_id);
if (!oldTeam) continue;
if (oldTeam.max_budget_in_team !== newTeam.max_budget_in_team) {
const oldBudget = oldTeam.max_budget_in_team !== undefined ? `$${oldTeam.max_budget_in_team}` : "No limit";
const newBudget = newTeam.max_budget_in_team !== undefined ? `$${newTeam.max_budget_in_team}` : "No limit";
changes.push({
field: displayName,
type: "changed",
details: `Team "${newTeam.team_id}" budget: ${oldBudget} → ${newBudget}`,
});
}
if (oldTeam.user_role !== newTeam.user_role) {
changes.push({
field: displayName,
type: "changed",
details: `Team "${newTeam.team_id}" role: ${oldTeam.user_role} → ${newTeam.user_role}`,
});
}
}
continue;
}
if (key === "models") {
const oldModels: string[] = Array.isArray(oldVal) ? oldVal : [];
const newModels: string[] = Array.isArray(newVal) ? newVal : [];
const oldSet = new Set(oldModels);
const newSet = new Set(newModels);
const removed = oldModels.filter((m) => !newSet.has(m));
const added = newModels.filter((m) => !oldSet.has(m));
if (removed.length > 0) {
changes.push({
field: displayName,
type: "removed",
details: `${removed.join(", ")} removed`,
});
}
if (added.length > 0) {
changes.push({
field: displayName,
type: "added",
details: `${added.join(", ")} added`,
});
}
continue;
}
// Scalar fields
const oldNorm = oldVal === "" ? null : oldVal;
const newNorm = newVal === "" ? null : newVal;
if (oldNorm === newNorm) continue;
// Both null/undefined — no change
if (oldNorm == null && newNorm == null) continue;
if (oldNorm != null && newNorm == null) {
// Value cleared
changes.push({
field: displayName,
type: "removed",
details: `Cleared (was "${oldNorm}")`,
});
} else if (oldNorm == null && newNorm != null) {
// Value set
changes.push({
field: displayName,
type: "added",
details: `Set to "${newNorm}"`,
});
} else if (oldNorm !== newNorm) {
changes.push({
field: displayName,
type: "changed",
details: `"${oldNorm}" → "${newNorm}"`,
});
}
}
const hasDestructiveChanges = changes.some((c) => c.type === "removed" || c.type === "changed");
return { changes, hasDestructiveChanges };
}
const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
accessToken,
possibleUIRoles,
@ -33,6 +190,9 @@ const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
const [editedValues, setEditedValues] = useState<any>({});
const [saving, setSaving] = useState<boolean>(false);
const [availableModels, setAvailableModels] = useState<string[]>([]);
const [showConfirmModal, setShowConfirmModal] = useState<boolean>(false);
const [pendingChanges, setPendingChanges] = useState<SettingsChange[]>([]);
const [pendingProcessedValues, setPendingProcessedValues] = useState<Record<string, any> | null>(null);
const { Paragraph } = Typography;
const { Option } = Select;
@ -71,20 +231,12 @@ const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
fetchSSOSettings();
}, [accessToken]);
const handleSaveSettings = async () => {
/** Perform the actual API save with the given processed values. */
const executeSave = async (processedValues: Record<string, any>) => {
if (!accessToken) return;
setSaving(true);
try {
// Convert empty strings to null
const processedValues = Object.entries(editedValues).reduce(
(acc, [key, value]) => {
acc[key] = value === "" ? null : value;
return acc;
},
{} as Record<string, any>,
);
const updatedSettings = await updateInternalUserSettings(accessToken, processedValues);
setSettings({ ...settings, values: updatedSettings.settings });
setIsEditing(false);
@ -96,6 +248,44 @@ const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
}
};
/** Called when user clicks "Save Changes". Shows modal if destructive. */
const handleSaveSettings = async () => {
if (!accessToken) return;
// Convert empty strings to null
const processedValues = Object.entries(editedValues).reduce(
(acc, [key, value]) => {
acc[key] = value === "" ? null : value;
return acc;
},
{} as Record<string, any>,
);
const { changes, hasDestructiveChanges } = computeSettingsDiff(
settings?.values || {},
processedValues,
);
if (hasDestructiveChanges) {
setPendingChanges(changes);
setPendingProcessedValues(processedValues);
setShowConfirmModal(true);
return;
}
// No destructive changes — save directly
await executeSave(processedValues);
};
/** Called when user confirms changes in the modal. */
const handleConfirmSave = async () => {
if (!pendingProcessedValues) return;
await executeSave(pendingProcessedValues);
setShowConfirmModal(false);
setPendingChanges([]);
setPendingProcessedValues(null);
};
const handleTextInputChange = (key: string, value: any) => {
setEditedValues((prev: Record<string, any>) => ({
...prev,
@ -103,30 +293,6 @@ const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
}));
};
// Helper function to normalize teams array to consistent format
const normalizeTeams = (teams: any[]): TeamEntry[] => {
if (!teams || !Array.isArray(teams)) return [];
return teams.map((team) => {
if (typeof team === "string") {
return {
team_id: team,
user_role: "user" as const,
};
} else if (typeof team === "object" && team.team_id) {
return {
team_id: team.team_id,
max_budget_in_team: team.max_budget_in_team,
user_role: team.user_role || "user",
};
}
return {
team_id: "",
user_role: "user" as const,
};
});
};
// Teams editor component
const renderTeamsEditor = (teams: any[]) => {
const normalizedTeams = normalizeTeams(teams);
@ -484,6 +650,18 @@ const DefaultUserSettings: React.FC<DefaultUserSettingsProps> = ({
<Divider />
<div className="mt-4 space-y-4">{renderSettings()}</div>
<ConfirmSettingsChangeModal
isOpen={showConfirmModal}
changes={pendingChanges}
onConfirm={handleConfirmSave}
onCancel={() => {
setShowConfirmModal(false);
setPendingChanges([]);
setPendingProcessedValues(null);
}}
confirmLoading={saving}
/>
</Card>
);
};

View file

@ -0,0 +1,97 @@
import React from "react";
import { Alert, Modal, Typography } from "antd";
import {
MinusCircleOutlined,
PlusCircleOutlined,
EditOutlined,
} from "@ant-design/icons";
export interface SettingsChange {
field: string;
type: "removed" | "added" | "changed";
details: string;
}
interface ConfirmSettingsChangeModalProps {
isOpen: boolean;
changes: SettingsChange[];
onConfirm: () => void;
onCancel: () => void;
confirmLoading: boolean;
}
const CHANGE_CONFIG: Record<
SettingsChange["type"],
{ color: string; icon: React.ReactNode; label: string }
> = {
removed: {
color: "#cf1322",
icon: <MinusCircleOutlined style={{ color: "#cf1322" }} />,
label: "Removed",
},
added: {
color: "#389e0d",
icon: <PlusCircleOutlined style={{ color: "#389e0d" }} />,
label: "Added",
},
changed: {
color: "#d48806",
icon: <EditOutlined style={{ color: "#d48806" }} />,
label: "Changed",
},
};
export default function ConfirmSettingsChangeModal({
isOpen,
changes,
onConfirm,
onCancel,
confirmLoading,
}: ConfirmSettingsChangeModalProps) {
const { Text } = Typography;
return (
<Modal
title="Review Changes"
open={isOpen}
onOk={onConfirm}
onCancel={onCancel}
confirmLoading={confirmLoading}
okText={confirmLoading ? "Saving..." : "Confirm Changes"}
cancelText="Cancel"
okButtonProps={{ disabled: confirmLoading }}
cancelButtonProps={{ disabled: confirmLoading }}
>
<div className="space-y-4">
<Alert
message="These defaults apply to all future users created via SSO, API, or SCIM."
type="warning"
showIcon
/>
<div className="mt-4 space-y-2">
{changes.map((change, index) => {
const config = CHANGE_CONFIG[change.type];
return (
<div
key={index}
className="flex items-start gap-2 p-2 rounded"
style={{ backgroundColor: `${config.color}08` }}
>
<span className="mt-0.5 flex-shrink-0">{config.icon}</span>
<div>
<Text strong style={{ color: config.color }}>
{change.field}
</Text>
<Text className="ml-1" style={{ color: config.color }}>
— {change.details}
</Text>
</div>
</div>
);
})}
</div>
</div>
</Modal>
);
}