From 2f4d66768bfd55307a58195f41aae5b548edd38a Mon Sep 17 00:00:00 2001 From: Dmitry Rumyantsev Date: Wed, 22 Jul 2026 15:04:44 +0300 Subject: [PATCH 1/2] SS-9721: State protection strategy in App Builder --- .../config/shapediverStoreParameters.ts | 23 ++- ...rameterImportExport.unsavedChanges.test.ts | 177 ++++++++++++++++++ ...iverStoreParameters.unsavedChanges.test.ts | 86 +++++++++ .../parameter/model/useParameterHistory.ts | 2 +- .../model/useParameterImportExport.ts | 22 ++- .../model/useShapeDiverStoreParameters.ts | 29 ++- .../model/useUnsavedChangesProtection.ts | 32 ++++ .../valueSources/useModelStateSources.ts | 21 ++- ...useCreateModelState.unsavedChanges.test.ts | 130 +++++++++++++ ...useImportModelState.unsavedChanges.test.ts | 155 +++++++++++++++ .../model-state/model/useCreateModelState.ts | 24 ++- .../model-state/model/useImportModelState.ts | 17 +- pages/appbuilder/AppBuilderPage.tsx | 4 + 13 files changed, 696 insertions(+), 26 deletions(-) create mode 100644 entities/parameter/model/__tests__/useParameterImportExport.unsavedChanges.test.ts create mode 100644 entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts create mode 100644 entities/parameter/model/useUnsavedChangesProtection.ts create mode 100644 features/model-state/model/__tests__/useCreateModelState.unsavedChanges.test.ts create mode 100644 features/model-state/model/__tests__/useImportModelState.unsavedChanges.test.ts diff --git a/entities/parameter/config/shapediverStoreParameters.ts b/entities/parameter/config/shapediverStoreParameters.ts index 1fb940ca..77cb4f59 100644 --- a/entities/parameter/config/shapediverStoreParameters.ts +++ b/entities/parameter/config/shapediverStoreParameters.ts @@ -63,6 +63,12 @@ export interface IHistoryEntry { state: ISessionsHistoryState; /** The time of the history entry (return value of Date.now()). */ time: number; + /** + * Whether the parameter values in this history entry have been changed + * since the last time a model state or parameter JSON file was created or + * imported. Used to drive the `beforeunload` protection prompt. + */ + unsavedChanges: boolean; } /** @@ -517,8 +523,23 @@ export interface IShapeDiverStoreParameters { * Push a state of parameter values to the history at the current index. * In case the history index is not at the end of the history, * all history entries after the current index are removed. + * + * @param state + * @param unsavedChanges Whether the new entry represents unsaved parameter + * changes. Defaults to `true` (parameter change). Pass `false` for the + * initial default state entry. + */ + readonly pushHistoryState: ( + state: ISessionsHistoryState, + unsavedChanges?: boolean, + ) => IHistoryEntry; + + /** + * Clear the `unsavedChanges` flag of the current history entry. + * Called after creating or importing a model state, and after creating or + * importing a parameter JSON file. */ - readonly pushHistoryState: (state: ISessionsHistoryState) => IHistoryEntry; + readonly clearUnsavedChanges: () => void; /** * Replace the transient derived namespace state used during history restore. diff --git a/entities/parameter/model/__tests__/useParameterImportExport.unsavedChanges.test.ts b/entities/parameter/model/__tests__/useParameterImportExport.unsavedChanges.test.ts new file mode 100644 index 00000000..573e7d46 --- /dev/null +++ b/entities/parameter/model/__tests__/useParameterImportExport.unsavedChanges.test.ts @@ -0,0 +1,177 @@ +/** + * @jest-environment jsdom + */ +import {act, renderHook} from "@testing-library/react"; +import * as React from "react"; +import {useParameterImportExport} from "../useParameterImportExport"; + +jest.mock("@mantine/core", () => { + const actual = jest.requireActual("@mantine/core"); + return { + ...actual, + useProps: (_name: string, _defaults: any, props: any) => props ?? {}, + }; +}); + +const notificationMock = { + success: jest.fn(), + error: jest.fn(), + warning: jest.fn(), + info: jest.fn(), +}; +jest.mock( + "@AppBuilderLib/features/notifications/model/useNotificationStore", + () => ({ + useNotificationStore: () => notificationMock, + }), +); + +jest.mock("@AppBuilderLib/shared/lib/ErrorReportingContext", () => ({ + ErrorReportingContext: React.createContext({captureException: jest.fn()}), +})); + +jest.mock("@AppBuilderLib/shared/model/useShapeDiverStorePlatform", () => ({ + useShapeDiverStorePlatform: (selector: any) => + selector({currentModel: undefined}), +})); + +const filterAndValidateParameters = jest.fn(); +const generateParameterFeedback = jest.fn(); +const isImportParameterArray = jest.fn(); +jest.mock("@AppBuilderLib/entities/parameter/lib/parametersFilter", () => ({ + filterAndValidateParameters: (...a: any[]) => + filterAndValidateParameters(...a), + generateParameterFeedback: (...a: any[]) => generateParameterFeedback(...a), + isImportParameterArray: (...a: any[]) => isImportParameterArray(...a), +})); + +jest.mock("@AppBuilderLib/entities/parameter/lib/parameterStates", () => ({ + getParameterStates: () => [], +})); + +jest.mock( + "@AppBuilderLib/entities/parameter/lib/resolveParameterExportValue", + () => ({ + resolveParameterExportValue: () => "exported-value", + }), +); + +// Real parameter store. +import {useShapeDiverStoreParameters} from "../useShapeDiverStoreParameters"; + +const store = useShapeDiverStoreParameters; + +function seedUnsaved() { + store.getState().resetHistory(); + store.getState().pushHistoryState({}, false); + store.getState().pushHistoryState({ns: {p: "changed"}}); +} + +function currentUnsaved() { + const {history, historyIndex} = store.getState(); + return history[historyIndex]?.unsavedChanges; +} + +describe("useParameterImportExport unsavedChanges wiring", () => { + beforeEach(() => { + jest.clearAllMocks(); + // jsdom lacks URL.createObjectURL + (URL as any).createObjectURL = jest.fn(() => "blob:fake"); + (URL as any).revokeObjectURL = jest.fn(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + describe("exportParameters", () => { + it("clears unsavedChanges after exporting parameters to JSON", async () => { + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + const {result} = renderHook(() => useParameterImportExport("ns")); + + await act(async () => { + await result.current.exportParameters(); + }); + + expect(notificationMock.success).toHaveBeenCalled(); + expect(currentUnsaved()).toBe(false); + }); + }); + + describe("importParameters", () => { + function installFakeFileInput(fileContents: string) { + const fakeFile = { + text: () => Promise.resolve(fileContents), + }; + const realCreate = document.createElement.bind(document); + const spy = jest + .spyOn(document, "createElement") + .mockImplementation((tag: string) => { + if (tag === "input") { + const el = realCreate("input") as HTMLInputElement; + el.click = jest.fn(() => { + // simulate the user selecting a file + Promise.resolve().then(() => { + el.onchange?.({ + target: {files: [fakeFile as any]}, + } as any); + }); + }); + return el; + } + return realCreate(tag); + }); + return spy; + } + + it("clears unsavedChanges after importing a valid parameter JSON file", async () => { + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + isImportParameterArray.mockReturnValue(true); + filterAndValidateParameters.mockReturnValue({ + hasValidParameters: true, + validParameters: {paramA: 1}, + }); + generateParameterFeedback.mockReturnValue({ + type: "success", + message: "imported", + }); + + installFakeFileInput( + JSON.stringify({parameters: [{id: "paramA"}]}), + ); + + const {result} = renderHook(() => useParameterImportExport("ns")); + + await act(async () => { + await result.current.importParameters(); + }); + + expect(currentUnsaved()).toBe(false); + }); + + it("does not clear unsavedChanges when the imported JSON is invalid", async () => { + seedUnsaved(); + const before = currentUnsaved(); + + isImportParameterArray.mockReturnValue(false); + + installFakeFileInput( + JSON.stringify({parameters: [{id: "paramA"}]}), + ); + + const {result} = renderHook(() => useParameterImportExport("ns")); + + await act(async () => { + await expect( + result.current.importParameters(), + ).rejects.toThrow(); + }); + + expect(currentUnsaved()).toBe(before); + }); + }); +}); diff --git a/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts b/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts new file mode 100644 index 00000000..bab0b3b6 --- /dev/null +++ b/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts @@ -0,0 +1,86 @@ +/** + * @jest-environment jsdom + */ +import {useShapeDiverStoreParameters} from "../useShapeDiverStoreParameters"; + +/** + * Tests for the `unsavedChanges` flag on parameter history entries, which + * drives the `beforeunload` protection prompt (SS-9721). + * + * The store is a singleton created at module load. Each test resets the + * history before asserting. + */ +describe("useShapeDiverStoreParameters unsavedChanges", () => { + const store = useShapeDiverStoreParameters; + + beforeEach(() => { + store.getState().resetHistory(); + }); + + it("initial default state entry has unsavedChanges=false", () => { + const entry = store.getState().pushHistoryState({}, false); + expect(entry.unsavedChanges).toBe(false); + expect(store.getState().historyIndex).toBe(0); + expect(store.getState().history[0].unsavedChanges).toBe(false); + }); + + it("parameter change entries default to unsavedChanges=true", () => { + store.getState().pushHistoryState({}, false); + const entry = store.getState().pushHistoryState({ns: {p: "v"}}); + expect(entry.unsavedChanges).toBe(true); + expect(store.getState().history[store.getState().historyIndex] + .unsavedChanges).toBe(true); + }); + + it("clearUnsavedChanges clears the flag on the current entry", () => { + store.getState().pushHistoryState({}, false); + store.getState().pushHistoryState({ns: {p: "v"}}); + expect( + store.getState().history[store.getState().historyIndex] + .unsavedChanges, + ).toBe(true); + + store.getState().clearUnsavedChanges(); + + expect( + store.getState().history[store.getState().historyIndex] + .unsavedChanges, + ).toBe(false); + }); + + it("clearUnsavedChanges is a no-op when there is no current entry", () => { + // history is empty after reset + expect(store.getState().historyIndex).toBe(-1); + expect(() => store.getState().clearUnsavedChanges()).not.toThrow(); + expect(store.getState().historyIndex).toBe(-1); + }); + + it("clearUnsavedChanges is a no-op when the flag is already false", () => { + store.getState().pushHistoryState({}, false); + const initialHistory = store.getState().history; + store.getState().clearUnsavedChanges(); + // reference unchanged because no mutation was needed + expect(store.getState().history).toBe(initialHistory); + }); + + it("clearing does not affect earlier entries", () => { + store.getState().pushHistoryState({}, false); // index 0, clean + store.getState().pushHistoryState({ns: {p: "a"}}); // index 1, unsaved + store.getState().pushHistoryState({ns: {p: "b"}}); // index 2, unsaved + + store.getState().clearUnsavedChanges(); + + expect(store.getState().history[0].unsavedChanges).toBe(false); + expect(store.getState().history[1].unsavedChanges).toBe(true); + expect(store.getState().history[2].unsavedChanges).toBe(false); + }); + + it("a new change after clearing marks the new entry as unsaved", () => { + store.getState().pushHistoryState({}, false); + store.getState().pushHistoryState({ns: {p: "a"}}); + store.getState().clearUnsavedChanges(); + + const entry = store.getState().pushHistoryState({ns: {p: "b"}}); + expect(entry.unsavedChanges).toBe(true); + }); +}); diff --git a/entities/parameter/model/useParameterHistory.ts b/entities/parameter/model/useParameterHistory.ts index 50301193..45bccc9c 100644 --- a/entities/parameter/model/useParameterHistory.ts +++ b/entities/parameter/model/useParameterHistory.ts @@ -51,7 +51,7 @@ export function useParameterHistory(props: Props) { if (!loaded) return; const defaultState = getDefaultState(); - const entry = pushHistoryState(defaultState); + const entry = pushHistoryState(defaultState, false); history.replaceState(entry, "", ""); return () => { diff --git a/entities/parameter/model/useParameterImportExport.ts b/entities/parameter/model/useParameterImportExport.ts index dddaeaf3..160eded4 100644 --- a/entities/parameter/model/useParameterImportExport.ts +++ b/entities/parameter/model/useParameterImportExport.ts @@ -20,11 +20,13 @@ import {useShapeDiverStoreParameters} from "./useShapeDiverStoreParameters"; * Hook for managing parameter import/export and reset functionality. */ export function useParameterImportExport(namespace: string) { - const {batchParameterValueUpdate} = useShapeDiverStoreParameters( - useShallow((state) => ({ - batchParameterValueUpdate: state.batchParameterValueUpdate, - })), - ); + const {batchParameterValueUpdate, clearUnsavedChanges} = + useShapeDiverStoreParameters( + useShallow((state) => ({ + batchParameterValueUpdate: state.batchParameterValueUpdate, + clearUnsavedChanges: state.clearUnsavedChanges, + })), + ); const {currentModel} = useShapeDiverStorePlatform( useShallow((state) => ({ @@ -67,7 +69,10 @@ export function useParameterImportExport(namespace: string) { notifications.success({ message: "Parameter values exported successfully", }); - }, [namespace, currentModel]); + + // creating a parameter JSON file persists the current configuration + clearUnsavedChanges(); + }, [namespace, currentModel, clearUnsavedChanges]); /** * Import parameters from JSON file @@ -161,6 +166,9 @@ export function useParameterImportExport(namespace: string) { [namespace]: validationResult.validParameters, }); + // importing a parameter JSON file reverts the unsaved changes flag + clearUnsavedChanges(); + // Provide user feedback const feedback = generateParameterFeedback( validationResult, @@ -176,7 +184,7 @@ export function useParameterImportExport(namespace: string) { fileInput.click(); }); - }, [namespace, notifications]); + }, [namespace, notifications, clearUnsavedChanges]); /** * Reset parameters to default values diff --git a/entities/parameter/model/useShapeDiverStoreParameters.ts b/entities/parameter/model/useShapeDiverStoreParameters.ts index 727f1e05..5dcaa829 100644 --- a/entities/parameter/model/useShapeDiverStoreParameters.ts +++ b/entities/parameter/model/useShapeDiverStoreParameters.ts @@ -1691,9 +1691,16 @@ export const useShapeDiverStoreParameters = ); }, - pushHistoryState(state: ISessionsHistoryState) { + pushHistoryState( + state: ISessionsHistoryState, + unsavedChanges = true, + ) { const {history, historyIndex} = get(); - const entry: IHistoryEntry = {state, time: Date.now()}; + const entry: IHistoryEntry = { + state, + time: Date.now(), + unsavedChanges, + }; const newHistory = history .slice(0, historyIndex + 1) .concat(entry); @@ -1709,6 +1716,24 @@ export const useShapeDiverStoreParameters = return entry; }, + clearUnsavedChanges() { + const {history, historyIndex} = get(); + if (historyIndex < 0 || historyIndex >= history.length) + return; + const current = history[historyIndex]; + if (!current.unsavedChanges) return; + const newHistory = history.slice(); + newHistory[historyIndex] = { + ...current, + unsavedChanges: false, + }; + set( + () => ({history: newHistory}), + false, + "clearUnsavedChanges", + ); + }, + setPendingHistoryDerivedState(state: ISessionsHistoryState) { set( () => ({ diff --git a/entities/parameter/model/useUnsavedChangesProtection.ts b/entities/parameter/model/useUnsavedChangesProtection.ts new file mode 100644 index 00000000..a9c004a4 --- /dev/null +++ b/entities/parameter/model/useUnsavedChangesProtection.ts @@ -0,0 +1,32 @@ +import {useEffect} from "react"; +import {useShapeDiverStoreParameters} from "./useShapeDiverStoreParameters"; + +/** + * Hook that installs a `beforeunload` handler preventing accidental loss of + * unsaved parameter changes (browser crash, tab close, navigation). + * + * The handler reads the `unsavedChanges` flag of the current parameter history + * entry directly from the store, so it stays in sync with parameter changes + * and model state / parameter JSON creation or import without re-subscribing + * on every state update. + */ +export function useUnsavedChangesProtection() { + useEffect(() => { + const handleBeforeUnload = (event: BeforeUnloadEvent) => { + const {history, historyIndex} = + useShapeDiverStoreParameters.getState(); + const current = history[historyIndex]; + if (current?.unsavedChanges) { + event.preventDefault(); + // Required for legacy browsers to trigger the confirmation prompt + event.returnValue = ""; + } + }; + + window.addEventListener("beforeunload", handleBeforeUnload); + + return () => { + window.removeEventListener("beforeunload", handleBeforeUnload); + }; + }, []); +} diff --git a/entities/parameter/model/valueSources/useModelStateSources.ts b/entities/parameter/model/valueSources/useModelStateSources.ts index c9550f0a..17b3ed5f 100644 --- a/entities/parameter/model/valueSources/useModelStateSources.ts +++ b/entities/parameter/model/valueSources/useModelStateSources.ts @@ -38,14 +38,19 @@ export function useModelStateSources(props: { parameterNamesToExclude, } = source; - const promise = createModelState({ - parameterNamesToInclude, - parameterNamesToExclude, - includeImage, - image, - data: undefined, - includeGltf, - }) + const promise = createModelState( + { + parameterNamesToInclude, + parameterNamesToExclude, + includeImage, + image, + data: undefined, + includeGltf, + }, + // value-source model states are generated, not user saves; + // do not clear the unsaved changes flag + {markSaved: false}, + ) .then(async ({modelStateId}) => { if (!modelStateId) return; // in case we are not running inside an iframe, the instance of diff --git a/features/model-state/model/__tests__/useCreateModelState.unsavedChanges.test.ts b/features/model-state/model/__tests__/useCreateModelState.unsavedChanges.test.ts new file mode 100644 index 00000000..98075f4b --- /dev/null +++ b/features/model-state/model/__tests__/useCreateModelState.unsavedChanges.test.ts @@ -0,0 +1,130 @@ +/** + * @jest-environment jsdom + */ +import {act, renderHook} from "@testing-library/react"; +import {useCreateModelState} from "../useCreateModelState"; + +// useViewportId reads from context; mock it to a fixed viewport id. +jest.mock("@AppBuilderLib/entities/viewport/model/useViewportId", () => ({ + useViewportId: () => ({viewportId: "vp1"}), +})); + +// useProps needs a Mantine provider; override only useProps, keep the rest real +// (getDefaultZIndex etc. are used transitively via the notification store). +jest.mock("@mantine/core", () => { + const actual = jest.requireActual("@mantine/core"); + return { + ...actual, + useProps: (_name: string, _defaults: any, props: any) => props ?? {}, + }; +}); + +// Real stores — assertions and session state go directly against them. +import {useShapeDiverStoreParameters} from "@AppBuilderLib/entities/parameter/model/useShapeDiverStoreParameters"; +import {useShapeDiverStoreSession} from "@AppBuilderLib/entities/session/model/useShapeDiverStoreSession"; + +const paramStore = useShapeDiverStoreParameters; +const sessionStore = useShapeDiverStoreSession; + +const sessionApiMock: any = {}; + +function setSessionApi(overrides: Partial = {}) { + Object.keys(sessionApiMock).forEach((k) => delete sessionApiMock[k]); + Object.assign(sessionApiMock, { + modelViewUrl: "https://backend.example.com/", + parameters: { + paramA: { + id: "paramA", + name: "Param A", + displayname: "Param A", + value: 1, + }, + }, + createModelState: jest.fn().mockResolvedValue("ms-id-123"), + ...overrides, + }); + sessionStore.setState({sessions: {ns: sessionApiMock}}); +} + +function seedUnsaved() { + paramStore.getState().resetHistory(); + paramStore.getState().pushHistoryState({}, false); + paramStore.getState().pushHistoryState({ns: {p: "changed"}}); +} + +function currentUnsaved() { + const {history, historyIndex} = paramStore.getState(); + return history[historyIndex]?.unsavedChanges; +} + +describe("useCreateModelState unsavedChanges wiring", () => { + beforeEach(() => { + setSessionApi(); + }); + + it("clears unsavedChanges after a successful user-initiated model state creation", async () => { + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + const {result} = renderHook(() => + useCreateModelState({namespace: "ns"}), + ); + + await act(async () => { + await result.current.createModelState({}); + }); + + expect(sessionApiMock.createModelState).toHaveBeenCalled(); + expect(currentUnsaved()).toBe(false); + }); + + it("does not clear unsavedChanges when markSaved is false (value-source usage)", async () => { + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + const {result} = renderHook(() => + useCreateModelState({namespace: "ns"}), + ); + + await act(async () => { + await result.current.createModelState({}, {markSaved: false}); + }); + + expect(sessionApiMock.createModelState).toHaveBeenCalled(); + expect(currentUnsaved()).toBe(true); + }); + + it("does not clear unsavedChanges when no model state id is returned", async () => { + setSessionApi({ + createModelState: jest.fn().mockResolvedValue(undefined), + }); + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + const {result} = renderHook(() => + useCreateModelState({namespace: "ns"}), + ); + + await act(async () => { + await result.current.createModelState({}); + }); + + expect(currentUnsaved()).toBe(true); + }); + + it("returns empty result and does not touch the store when the session is missing", async () => { + seedUnsaved(); + const before = currentUnsaved(); + + const {result} = renderHook(() => + useCreateModelState({namespace: "other"}), + ); + + await act(async () => { + const res = await result.current.createModelState({}); + expect(res).toEqual({}); + }); + + expect(currentUnsaved()).toBe(before); + }); +}); diff --git a/features/model-state/model/__tests__/useImportModelState.unsavedChanges.test.ts b/features/model-state/model/__tests__/useImportModelState.unsavedChanges.test.ts new file mode 100644 index 00000000..d8864697 --- /dev/null +++ b/features/model-state/model/__tests__/useImportModelState.unsavedChanges.test.ts @@ -0,0 +1,155 @@ +/** + * @jest-environment jsdom + */ +import {act, renderHook} from "@testing-library/react"; +import * as React from "react"; +import {useImportModelState} from "../useImportModelState"; + +// Mock peer dependencies of useImportModelState. +jest.mock("@mantine/core", () => { + const actual = jest.requireActual("@mantine/core"); + return { + ...actual, + useProps: (_name: string, _defaults: any, props: any) => props ?? {}, + }; +}); + +const notificationMock = { + success: jest.fn(), + error: jest.fn(), + warning: jest.fn(), + info: jest.fn(), +}; +jest.mock( + "@AppBuilderLib/features/notifications/model/useNotificationStore", + () => ({ + useNotificationStore: () => notificationMock, + }), +); + +jest.mock("@AppBuilderLib/shared/lib/ErrorReportingContext", () => ({ + ErrorReportingContext: React.createContext({captureException: jest.fn()}), +})); + +const filterAndValidateModelStateParameters = jest.fn(); +const generateParameterFeedback = jest.fn(); +jest.mock("@AppBuilderLib/entities/parameter/lib/parametersFilter", () => ({ + filterAndValidateModelStateParameters: (...args: any[]) => + filterAndValidateModelStateParameters(...args), + generateParameterFeedback: (...args: any[]) => + generateParameterFeedback(...args), +})); + +jest.mock("@AppBuilderLib/entities/parameter/lib/parameterStates", () => ({ + getParameterStates: () => [], +})); + +// Real parameter + session stores. +import {useShapeDiverStoreParameters} from "@AppBuilderLib/entities/parameter/model/useShapeDiverStoreParameters"; +import {useShapeDiverStoreSession} from "@AppBuilderLib/entities/session/model/useShapeDiverStoreSession"; + +const paramStore = useShapeDiverStoreParameters; +const sessionStore = useShapeDiverStoreSession; + +const sessionApiMock: any = {}; + +function seedUnsaved() { + paramStore.getState().resetHistory(); + paramStore.getState().pushHistoryState({}, false); + paramStore.getState().pushHistoryState({ns: {p: "changed"}}); +} + +function currentUnsaved() { + const {history, historyIndex} = paramStore.getState(); + return history[historyIndex]?.unsavedChanges; +} + +describe("useImportModelState unsavedChanges wiring", () => { + beforeEach(() => { + jest.clearAllMocks(); + Object.keys(sessionApiMock).forEach((k) => delete sessionApiMock[k]); + Object.assign(sessionApiMock, { + getModelState: jest + .fn() + .mockResolvedValue({modelState: {parameters: {paramA: 1}}}), + }); + sessionStore.setState({sessions: {ns: sessionApiMock}}); + + filterAndValidateModelStateParameters.mockReturnValue({ + hasValidParameters: true, + validParameters: {paramA: 1}, + }); + generateParameterFeedback.mockReturnValue({ + type: "success", + message: "imported", + }); + }); + + it("clears unsavedChanges after a successful model state import", async () => { + seedUnsaved(); + expect(currentUnsaved()).toBe(true); + + const {result} = renderHook(() => + useImportModelState({namespace: "ns"}), + ); + + await act(async () => { + const res = await result.current.importModelState({ + modelStateId: "abc", + }); + expect(res.success).toBe(true); + }); + + expect(sessionApiMock.getModelState).toHaveBeenCalledWith("abc"); + expect(currentUnsaved()).toBe(false); + }); + + it("does not clear unsavedChanges when the model state fetch fails", async () => { + sessionApiMock.getModelState = jest + .fn() + .mockResolvedValue({error: new Error("boom")}); + + seedUnsaved(); + const before = currentUnsaved(); + + const {result} = renderHook(() => + useImportModelState({namespace: "ns"}), + ); + + await act(async () => { + const res = await result.current.importModelState({ + modelStateId: "abc", + }); + expect(res.success).toBe(false); + }); + + expect(currentUnsaved()).toBe(before); + }); + + it("does not clear unsavedChanges when parameters are invalid", async () => { + filterAndValidateModelStateParameters.mockReturnValue({ + hasValidParameters: false, + validParameters: {}, + }); + generateParameterFeedback.mockReturnValue({ + type: "error", + message: "invalid", + }); + + seedUnsaved(); + const before = currentUnsaved(); + + const {result} = renderHook(() => + useImportModelState({namespace: "ns"}), + ); + + await act(async () => { + const res = await result.current.importModelState({ + modelStateId: "abc", + }); + expect(res.success).toBe(false); + }); + + expect(currentUnsaved()).toBe(before); + }); +}); diff --git a/features/model-state/model/useCreateModelState.ts b/features/model-state/model/useCreateModelState.ts index 78859ce4..eb94979c 100644 --- a/features/model-state/model/useCreateModelState.ts +++ b/features/model-state/model/useCreateModelState.ts @@ -1,3 +1,4 @@ +import {useShapeDiverStoreParameters} from "@AppBuilderLib/entities/parameter/model/useShapeDiverStoreParameters"; import {useShapeDiverStoreSession} from "@AppBuilderLib/entities/session/model/useShapeDiverStoreSession"; import {useShapeDiverStoreViewportAccessFunctions} from "@AppBuilderLib/entities/viewport/model/useShapeDiverStoreViewportAccessFunctions"; import {useViewportId} from "@AppBuilderLib/entities/viewport/model/useViewportId"; @@ -9,7 +10,6 @@ import { ICreateModelStateResult, } from "../config/createModelState"; import type {CreateModelStateHookThemeDefaultProps} from "./useCreateModelState.types"; - type CreateModelStateHookThemePropsType = Partial; @@ -65,10 +65,27 @@ export function useCreateModelState(props: Props) { })), ); + const {clearUnsavedChanges} = useShapeDiverStoreParameters( + useShallow((state) => ({ + clearUnsavedChanges: state.clearUnsavedChanges, + })), + ); + const createModelState = useCallback( async ( props: ICreateModelStateData, + options?: { + /** + * Whether creating this model state marks the current + * configuration as saved (clears the `unsavedChanges` flag). + * Defaults to `true`. Pass `false` for internal model state + * creations that are not user-initiated saves (e.g. parameter + * value sources). + */ + markSaved?: boolean; + }, ): Promise => { + const {markSaved = true} = options ?? {}; const { parameterNamesToInclude = parameterNamesToIncludeDefault, parameterNamesToExclude = parameterNamesToExcludeDefault, @@ -155,6 +172,10 @@ export function useCreateModelState(props: Props) { ) : undefined; + // creating a model state persists the current configuration, + // so there are no unsaved changes anymore (unless the caller opted out) + if (modelStateId && markSaved) clearUnsavedChanges(); + const modelViewUrl = sessionApi.modelViewUrl.endsWith("/") ? sessionApi.modelViewUrl.substring( 0, @@ -189,6 +210,7 @@ export function useCreateModelState(props: Props) { parameterNamesToIncludeDefault, parameterNamesToExcludeDefault, parameterNamesToAlwaysExclude, + clearUnsavedChanges, ], ); diff --git a/features/model-state/model/useImportModelState.ts b/features/model-state/model/useImportModelState.ts index 3d3a3711..66d7c00a 100644 --- a/features/model-state/model/useImportModelState.ts +++ b/features/model-state/model/useImportModelState.ts @@ -35,11 +35,13 @@ export function useImportModelState({namespace}: Props) { const notifications = useNotificationStore(); const errorReporting = useContext(ErrorReportingContext); - const {batchParameterValueUpdate} = useShapeDiverStoreParameters( - useShallow((state) => ({ - batchParameterValueUpdate: state.batchParameterValueUpdate, - })), - ); + const {batchParameterValueUpdate, clearUnsavedChanges} = + useShapeDiverStoreParameters( + useShallow((state) => ({ + batchParameterValueUpdate: state.batchParameterValueUpdate, + clearUnsavedChanges: state.clearUnsavedChanges, + })), + ); /** * Import a model state by ID @@ -119,6 +121,9 @@ export function useImportModelState({namespace}: Props) { [namespace]: validationResult.validParameters, }); + // importing a model state reverts the unsaved changes flag + clearUnsavedChanges(); + // set as modelStateId in the URL applyModelStateToUrl(modelStateId, true); @@ -137,7 +142,7 @@ export function useImportModelState({namespace}: Props) { data: response.data, }; }, - [sessionApi, namespace], + [sessionApi, namespace, clearUnsavedChanges], ); return { diff --git a/pages/appbuilder/AppBuilderPage.tsx b/pages/appbuilder/AppBuilderPage.tsx index c79fefed..23205812 100644 --- a/pages/appbuilder/AppBuilderPage.tsx +++ b/pages/appbuilder/AppBuilderPage.tsx @@ -1,4 +1,5 @@ import {useParameterHistory} from "@AppBuilderLib/entities/parameter/model/useParameterHistory"; +import {useUnsavedChangesProtection} from "@AppBuilderLib/entities/parameter/model/useUnsavedChangesProtection"; import useDefaultSessionDto from "@AppBuilderLib/entities/session/model/useDefaultSessionDto"; import {IUseSessionDto} from "@AppBuilderLib/entities/session/model/useSession"; import {useSessions} from "@AppBuilderLib/entities/session/model/useSessions"; @@ -203,6 +204,9 @@ export default function AppBuilderPage(props: Partial) { // use parameter history useParameterHistory({loaded: show && customParametersLoaded}); + // protect unsaved parameter changes against accidental tab close / navigation + useUnsavedChangesProtection(); + // key bindings useKeyBindings({ namespace, From b5a9cc85e0f8c2b063947e058cc0824aae0e92ec Mon Sep 17 00:00:00 2001 From: Dmitry Rumyantsev Date: Thu, 23 Jul 2026 10:19:21 +0300 Subject: [PATCH 2/2] SS-9721: Add history sync --- .../config/shapediverStoreParameters.ts | 3 +- ...iverStoreParameters.unsavedChanges.test.ts | 39 ++++++++++++++++++- .../model/useShapeDiverStoreParameters.ts | 22 ++++++++++- 3 files changed, 59 insertions(+), 5 deletions(-) diff --git a/entities/parameter/config/shapediverStoreParameters.ts b/entities/parameter/config/shapediverStoreParameters.ts index 77cb4f59..24642494 100644 --- a/entities/parameter/config/shapediverStoreParameters.ts +++ b/entities/parameter/config/shapediverStoreParameters.ts @@ -535,7 +535,8 @@ export interface IShapeDiverStoreParameters { ) => IHistoryEntry; /** - * Clear the `unsavedChanges` flag of the current history entry. + * Clear the `unsavedChanges` flag of the current history entry and sync + * `window.history.state` when it matches that entry (by `time`). * Called after creating or importing a model state, and after creating or * importing a parameter JSON file. */ diff --git a/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts b/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts index bab0b3b6..263bbb50 100644 --- a/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts +++ b/entities/parameter/model/__tests__/useShapeDiverStoreParameters.unsavedChanges.test.ts @@ -28,8 +28,10 @@ describe("useShapeDiverStoreParameters unsavedChanges", () => { store.getState().pushHistoryState({}, false); const entry = store.getState().pushHistoryState({ns: {p: "v"}}); expect(entry.unsavedChanges).toBe(true); - expect(store.getState().history[store.getState().historyIndex] - .unsavedChanges).toBe(true); + expect( + store.getState().history[store.getState().historyIndex] + .unsavedChanges, + ).toBe(true); }); it("clearUnsavedChanges clears the flag on the current entry", () => { @@ -48,6 +50,39 @@ describe("useShapeDiverStoreParameters unsavedChanges", () => { ).toBe(false); }); + it("clearUnsavedChanges syncs window.history.state when it matches the current entry", () => { + store.getState().pushHistoryState({}, false); + const entry = store.getState().pushHistoryState({ns: {p: "v"}}); + // Mimic historyPusher / useParameterHistory writing the entry into the + // browser history stack. + window.history.replaceState(entry, ""); + expect( + (window.history.state as {unsavedChanges?: boolean}).unsavedChanges, + ).toBe(true); + + store.getState().clearUnsavedChanges(); + + expect( + (window.history.state as {unsavedChanges?: boolean}).unsavedChanges, + ).toBe(false); + expect((window.history.state as {time?: number}).time).toBe(entry.time); + }); + + it("clearUnsavedChanges does not overwrite unrelated window.history.state", () => { + store.getState().pushHistoryState({}, false); + store.getState().pushHistoryState({ns: {p: "v"}}); + const unrelated = {foo: "bar"}; + window.history.replaceState(unrelated, ""); + + store.getState().clearUnsavedChanges(); + + expect(window.history.state).toEqual(unrelated); + expect( + store.getState().history[store.getState().historyIndex] + .unsavedChanges, + ).toBe(false); + }); + it("clearUnsavedChanges is a no-op when there is no current entry", () => { // history is empty after reset expect(store.getState().historyIndex).toBe(-1); diff --git a/entities/parameter/model/useShapeDiverStoreParameters.ts b/entities/parameter/model/useShapeDiverStoreParameters.ts index 5dcaa829..4db0c332 100644 --- a/entities/parameter/model/useShapeDiverStoreParameters.ts +++ b/entities/parameter/model/useShapeDiverStoreParameters.ts @@ -1722,16 +1722,34 @@ export const useShapeDiverStoreParameters = return; const current = history[historyIndex]; if (!current.unsavedChanges) return; - const newHistory = history.slice(); - newHistory[historyIndex] = { + const updated: IHistoryEntry = { ...current, unsavedChanges: false, }; + const newHistory = history.slice(); + newHistory[historyIndex] = updated; set( () => ({history: newHistory}), false, "clearUnsavedChanges", ); + + // Keep window.history.state in sync so popstate / URL helpers + // (e.g. modifyUrl replaceState) do not keep a stale flag. + if (typeof window !== "undefined") { + const browserState = window.history + .state as IHistoryEntry | null; + if ( + browserState && + typeof browserState === "object" && + browserState.time === updated.time + ) { + window.history.replaceState( + {...browserState, unsavedChanges: false}, + "", + ); + } + } }, setPendingHistoryDerivedState(state: ISessionsHistoryState) {