From 0b9ac0f69efc3e5be28230a784fa6c7e5faf92fb Mon Sep 17 00:00:00 2001 From: Lakatos Andrei Date: Fri, 29 May 2026 14:56:45 +0300 Subject: [PATCH] fix: called onModelChanged with only the change, to no override changes done fast [PIE-509][PIE-562][PIE-563] --- .../configure/src/__tests__/index.test.js | 157 ++++++++++++++++++ .../configure/src/__tests__/main.test.jsx | 52 +++--- .../configure/src/index.js | 15 +- .../configure/src/main.jsx | 25 ++- 4 files changed, 207 insertions(+), 42 deletions(-) create mode 100644 packages/explicit-constructed-response/configure/src/__tests__/index.test.js diff --git a/packages/explicit-constructed-response/configure/src/__tests__/index.test.js b/packages/explicit-constructed-response/configure/src/__tests__/index.test.js new file mode 100644 index 0000000000..b7a2a4142e --- /dev/null +++ b/packages/explicit-constructed-response/configure/src/__tests__/index.test.js @@ -0,0 +1,157 @@ +import React from 'react'; +import { createRoot } from 'react-dom/client'; +import { ModelUpdatedEvent } from '@pie-framework/pie-configure-events'; + +import sensibleDefaults from '../defaults'; + +jest.mock('@pie-lib/config-ui', () => ({ + settings: { + Panel: (props) =>
, + toggle: jest.fn(), + radio: jest.fn(), + dropdown: jest.fn(), + }, + layout: { + ConfigLayout: (props) =>
{props.children}
, + }, + InputContainer: (props) =>
{props.children}
, +})); + +jest.mock('@pie-lib/editable-html-tip-tap', () => ({ + __esModule: true, + default: () =>
, + ALL_PLUGINS: [], +})); + +jest.mock('../ecr-toolbar', () => ({ + __esModule: true, + default: () =>
, +})); + +jest.mock('../alternateResponses', () => ({ + __esModule: true, + default: () =>
, +})); + +const mockRender = jest.fn(); +const mockUnmount = jest.fn(); + +jest.mock('react-dom/client', () => ({ + createRoot: jest.fn(() => ({ + render: mockRender, + unmount: mockUnmount, + })), +})); + +const model = { + markup: '

The {{0}} jumped {{1}} the {{2}}

', + choices: { + 0: [{ label: 'cow', value: '0' }], + 1: [{ label: 'over', value: '0' }], + 2: [{ label: 'moon', value: '0' }], + }, + prompt: 'Complete the sentence', +}; + +describe('ExplicitConstructedResponse configure index', () => { + let Def; + let element; + + beforeAll(() => { + Def = require('../index').default; + + if (!customElements.get('explicit-constructed-response-configure-test')) { + customElements.define('explicit-constructed-response-configure-test', Def); + } + }); + + beforeEach(() => { + jest.clearAllMocks(); + element = document.createElement('explicit-constructed-response-configure-test'); + element.model = model; + }); + + describe('prepareModel', () => { + it('adds value to choices missing the value property', () => { + const result = Def.prepareModel({ + choices: { + 0: [{ label: 'test' }], + }, + }); + + expect(result.choices[0][0]).toEqual({ value: '0', label: 'test' }); + }); + }); + + describe('onModelChanged', () => { + it('merges partial updates into the existing model', () => { + const initialModel = Def.prepareModel(model); + element._model = initialModel; + + element.onModelChanged({ prompt: 'Updated prompt' }); + + expect(element._model.prompt).toBe('Updated prompt'); + expect(element._model.choices).toEqual(initialModel.choices); + expect(element._model.slateMarkup).toEqual(initialModel.slateMarkup); + }); + + it('dispatches ModelUpdatedEvent with the merged model', () => { + const dispatchSpy = jest.spyOn(element, 'dispatchEvent'); + const initialModel = Def.prepareModel(model); + element._model = initialModel; + + element.onModelChanged({ prompt: 'Updated prompt' }, true); + + expect(dispatchSpy).toHaveBeenCalledWith(new ModelUpdatedEvent(element._model, true)); + }); + + it('preserves existing fields when receiving partial updates from Main', () => { + const initialModel = Def.prepareModel(model); + element._model = initialModel; + + element.onModelChanged({ rationale: 'New rationale' }); + + expect(element._model.rationale).toBe('New rationale'); + expect(element._model.prompt).toBe(model.prompt); + expect(element._model.choices).toEqual(initialModel.choices); + }); + }); + + describe('onConfigurationChanged', () => { + it('updates responseAreaInputConfiguration on the model', () => { + const initialModel = Def.prepareModel(model); + element._model = initialModel; + + const newConfiguration = { + ...sensibleDefaults.configuration, + responseAreaInputConfiguration: { + inputConfiguration: { + characters: { disabled: false }, + }, + }, + }; + + element.onConfigurationChanged(newConfiguration); + + expect(element._model.responseAreaInputConfiguration).toEqual( + newConfiguration.responseAreaInputConfiguration.inputConfiguration, + ); + expect(element._model.prompt).toBe(model.prompt); + }); + }); + + describe('set model', () => { + it('renders the configure view', () => { + expect(createRoot).toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); + }); + }); + + describe('disconnectedCallback', () => { + it('unmounts the react root', () => { + element.disconnectedCallback(); + + expect(mockUnmount).toHaveBeenCalled(); + }); + }); +}); diff --git a/packages/explicit-constructed-response/configure/src/__tests__/main.test.jsx b/packages/explicit-constructed-response/configure/src/__tests__/main.test.jsx index 5b744b854c..461a9f6855 100644 --- a/packages/explicit-constructed-response/configure/src/__tests__/main.test.jsx +++ b/packages/explicit-constructed-response/configure/src/__tests__/main.test.jsx @@ -120,10 +120,7 @@ describe('Main', () => { testInstance.onModelChange({ promptEnabled: false }); - expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - promptEnabled: false, - }); + expect(onModelChanged).toBeCalledWith({ promptEnabled: false }); }); }); @@ -139,10 +136,7 @@ describe('Main', () => { testInstance.onPromptChanged('This is the new prompt'); - expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - prompt: 'This is the new prompt', - }); + expect(onModelChanged).toBeCalledWith({ prompt: 'This is the new prompt' }); }); }); @@ -158,10 +152,7 @@ describe('Main', () => { testInstance.onRationaleChanged('New Rationale'); - expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - rationale: 'New Rationale', - }); + expect(onModelChanged).toBeCalledWith({ rationale: 'New Rationale' }); }); }); @@ -177,10 +168,7 @@ describe('Main', () => { testInstance.onTeacherInstructionsChanged('New Teacher Instructions'); - expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - teacherInstructions: 'New Teacher Instructions', - }); + expect(onModelChanged).toBeCalledWith({ teacherInstructions: 'New Teacher Instructions' }); }); }); @@ -199,10 +187,7 @@ describe('Main', () => { testInstance.onMarkupChanged(slateMarkup); - expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - slateMarkup, - }); + expect(onModelChanged).toBeCalledWith({ slateMarkup }); }); }); @@ -232,9 +217,32 @@ describe('Main', () => { testInstance.onResponsesChanged(newChoices); + expect(onModelChanged).toBeCalledWith({ choices: newChoices }); + }); + }); + + describe('onLengthChanged', () => { + it('changes maxLengthPerChoice value', () => { + const testInstance = new Main({ + onModelChanged, + onConfigurationChanged, + classes: {}, + model: prepareModel(model), + configuration: sensibleDefaults.configuration, + }); + + testInstance.onLengthChanged([8, 8, 6]); + + expect(onModelChanged).toBeCalledWith({ maxLengthPerChoice: [8, 8, 6] }); + }); + }); + + describe('componentDidMount', () => { + it('calls onModelChanged with maxLengthPerChoice only', () => { + renderMain(); + expect(onModelChanged).toBeCalledWith({ - ...prepareModel(model), - choices: newChoices, + maxLengthPerChoice: expect.arrayContaining([6, 6, 4]), }); }); }); diff --git a/packages/explicit-constructed-response/configure/src/index.js b/packages/explicit-constructed-response/configure/src/index.js index ef799cdf7b..a8d15c4317 100644 --- a/packages/explicit-constructed-response/configure/src/index.js +++ b/packages/explicit-constructed-response/configure/src/index.js @@ -55,8 +55,6 @@ export default class ExplicitConstructedResponse extends HTMLElement { this._root = null; this._model = ExplicitConstructedResponse.prepareModel(); this._configuration = sensibleDefaults.configuration; - this.onModelChanged = this.onModelChanged.bind(this); - this.onConfigurationChanged = this.onConfigurationChanged.bind(this); } set model(s) { @@ -112,13 +110,16 @@ export default class ExplicitConstructedResponse extends HTMLElement { this.dispatchEvent(new ModelUpdatedEvent(this._model, resetValue)); } - onModelChanged(m, reset) { - this._model = ExplicitConstructedResponse.prepareModel(m); + onModelChanged = (m, reset)=> { + this._model = ExplicitConstructedResponse.prepareModel({ + ...this._model, + ...m, + }); this._render(); this.dispatchModelUpdated(reset); - } + }; - onConfigurationChanged(c) { + onConfigurationChanged =(c) => { this._configuration = c; const newInputConfig = this._configuration?.responseAreaInputConfiguration?.inputConfiguration; @@ -129,7 +130,7 @@ export default class ExplicitConstructedResponse extends HTMLElement { }; this.onModelChanged(this._model); - } + }; /** @param {done, progress, file} handler */ insertImage(handler) { diff --git a/packages/explicit-constructed-response/configure/src/main.jsx b/packages/explicit-constructed-response/configure/src/main.jsx index fa1d05f6e8..e6911102ec 100644 --- a/packages/explicit-constructed-response/configure/src/main.jsx +++ b/packages/explicit-constructed-response/configure/src/main.jsx @@ -107,39 +107,39 @@ export class Main extends React.Component { } }); - onModelChanged({ ...this.props.model, maxLengthPerChoice }); + onModelChanged({ maxLengthPerChoice }); } onModelChange = (newVal) => { - this.props.onModelChanged({ ...this.props.model, ...newVal }); + this.props.onModelChanged({ ...newVal }); }; onPromptChanged = (prompt) => { - this.props.onModelChanged({ ...this.props.model, prompt }); + this.props.onModelChanged({ prompt }); }; onRationaleChanged = (rationale) => { - this.props.onModelChanged({ ...this.props.model, rationale }); + this.props.onModelChanged({ rationale }); }; onTeacherInstructionsChanged = (teacherInstructions) => { - const { model, onModelChanged } = this.props; + const { onModelChanged } = this.props; - onModelChanged({ ...model, teacherInstructions }); + onModelChanged({ teacherInstructions }); }; onMarkupChanged = (slateMarkup) => { - this.props.onModelChanged({ ...this.props.model, slateMarkup }); + this.props.onModelChanged({ slateMarkup }); }; onResponsesChanged = (choices) => { - this.props.onModelChanged({ ...this.props.model, choices }); + this.props.onModelChanged({ choices }); }; onLengthChanged = (maxLengthPerChoice) => { - const { model, onModelChanged } = this.props; + const { onModelChanged } = this.props; - onModelChanged({ ...model, maxLengthPerChoice }); + onModelChanged({ maxLengthPerChoice }); }; onChangeResponse = (index, newVal) => { @@ -168,7 +168,7 @@ export class Main extends React.Component { } } - onModelChanged({ ...model, choices, maxLengthPerChoice }); + onModelChanged({ choices, maxLengthPerChoice }); }; onChange = (markup) => { @@ -200,7 +200,6 @@ export class Main extends React.Component { const callback = () => onModelChanged({ - ...this.props.model, choices: allChoices, slateMarkup: domMarkup.innerHTML, maxLengthPerChoice: updatedMaxLengthPerChoice, @@ -242,7 +241,7 @@ export class Main extends React.Component { } }); - const callback = () => onModelChanged({ ...this.props.model, choices: newChoices }); + const callback = () => onModelChanged({ choices: newChoices }); this.setState({ cachedChoices: newCachedChoices }, callback); },