Skip to content

Commit 19a7954

Browse files
(fix) O3-4898: Implementer tools: Prevent crash when hovering over tree-level descriptions (#1485)
* (fix) Implementer tools: Prevent crash when hovering over tree-level descriptions * fix: use Object.hasOwn() and revert en.json changes * fix: add type guards and fix lodash.unset usage in implementer tools - Add type checks before Object.hasOwn() to prevent TypeError on null/undefined values - Fix improper use of lodash.unset by cloning state before mutation - Follows immutable state update patterns to prevent store corruption * test: add unit tests for type guards and lodash.unset fix - Add tests to verify type guards prevent TypeError on null/undefined values - Add tests to verify immutable state update pattern with lodash.unset - Ensures changes follow best practices and prevent store corruption * Fixup: Complete type guards and improve type safety in config-subtree component Adds type guards for _source and _description properties to prevent runtime crashes when accessing metadata on non-leaf config nodes. Also removes unused parameters and adds explicit TypeScript type annotations for better type safety. * Normalize line endings in translation files to LF Converts CRLF (Windows) line endings to LF (Unix) to match repository convention. * Replace non-canonical tests with canonical component-level test Previous tests were not canonical and did not test the component-level behavior. Rather, they test implementation details. They also used non-canonical imports instead of Jest globals and are not colocated properly to the code they were testing. --------- Co-authored-by: Dennis Kigen <kigen.work@gmail.com>
1 parent b2605ec commit 19a7954

File tree

3 files changed

+64
-25
lines changed

3 files changed

+64
-25
lines changed

packages/apps/esm-implementer-tools-app/src/configuration/configuration.test.tsx

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -361,4 +361,38 @@ describe('Configuration', () => {
361361
// expect(mockSetTemporaryConfigValue).toHaveBeenCalledWith(["@openmrs/luigi", "favoriteNumbers"], [5, 11, 13]);
362362
}
363363
});
364+
365+
it('handles hovering over config tree items without crashing', async () => {
366+
const user = userEvent.setup();
367+
368+
implementerToolsConfigStore.setState({
369+
config: {
370+
'@openmrs/mario': {
371+
hasHat: mockImplToolsConfig['@openmrs/mario'].hasHat,
372+
weapons: {
373+
gloves: {
374+
_type: Type.Number,
375+
_default: 0,
376+
_value: 2,
377+
_source: 'provided',
378+
},
379+
},
380+
},
381+
},
382+
});
383+
384+
renderConfiguration();
385+
386+
// Find and hover over a leaf node (hasHat)
387+
const hasHatElement = await screen.findByText('hasHat');
388+
await user.hover(hasHatElement);
389+
390+
// Find and hover over a branch node (weapons) - this should not crash
391+
const weaponsElement = await screen.findByText('weapons');
392+
await user.hover(weaponsElement);
393+
394+
// Both elements should still be in the document (no crash occurred)
395+
expect(hasHatElement).toBeInTheDocument();
396+
expect(weaponsElement).toBeInTheDocument();
397+
});
364398
});

packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/config-subtree.component.tsx

Lines changed: 27 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -10,20 +10,21 @@ export interface ConfigSubtreeProps {
1010
}
1111

1212
export function ConfigSubtree({ config, path = [] }: ConfigSubtreeProps) {
13-
function setActiveItemDescriptionOnMouseEnter(thisPath, key, value) {
13+
function setActiveItemDescriptionOnMouseEnter(thisPath: Array<string>, value: any) {
1414
if (!implementerToolsStore.getState().configPathBeingEdited) {
15+
const isLeaf = value && typeof value === 'object' && Object.hasOwn(value, '_value');
1516
implementerToolsStore.setState({
1617
activeItemDescription: {
1718
path: thisPath,
18-
source: value._source,
19-
description: value._description,
20-
value: JSON.stringify(value._value),
19+
source: isLeaf ? value._source : undefined,
20+
description: isLeaf ? value._description : undefined,
21+
value: isLeaf ? JSON.stringify(value._value) : undefined,
2122
},
2223
});
2324
}
2425
}
2526

26-
function removeActiveItemDescriptionOnMouseLeave(thisPath) {
27+
function removeActiveItemDescriptionOnMouseLeave(thisPath: Array<string>) {
2728
const state = implementerToolsStore.getState();
2829
if (isEqual(state.activeItemDescription?.path, thisPath) && !isEqual(state.configPathBeingEdited, thisPath)) {
2930
implementerToolsStore.setState({ activeItemDescription: undefined });
@@ -32,25 +33,27 @@ export function ConfigSubtree({ config, path = [] }: ConfigSubtreeProps) {
3233

3334
return (
3435
<>
35-
{Object.entries(config).map(([key, value], i) => {
36-
const thisPath = path.concat([key]);
37-
const isLeaf = value.hasOwnProperty('_value') || value.hasOwnProperty('_type');
38-
return (
39-
<Subtree
40-
label={key}
41-
leaf={isLeaf}
42-
onMouseEnter={() => setActiveItemDescriptionOnMouseEnter(thisPath, key, value)}
43-
onMouseLeave={() => removeActiveItemDescriptionOnMouseLeave(thisPath)}
44-
key={`subtree-${thisPath.join('.')}`}
45-
>
46-
{isLeaf ? (
47-
<EditableValue path={thisPath} element={value} />
48-
) : (
49-
<ConfigSubtree config={value} path={thisPath} />
50-
)}
51-
</Subtree>
52-
);
53-
})}
36+
{Object.entries(config)
37+
.filter(([key]) => !key.startsWith('_'))
38+
.map(([key, value]) => {
39+
const thisPath = path.concat([key]);
40+
const isLeaf = value && typeof value === 'object' && Object.hasOwn(value, '_value');
41+
return (
42+
<Subtree
43+
label={key}
44+
leaf={isLeaf}
45+
onMouseEnter={() => setActiveItemDescriptionOnMouseEnter(thisPath, value)}
46+
onMouseLeave={() => removeActiveItemDescriptionOnMouseLeave(thisPath)}
47+
key={`subtree-${thisPath.join('.')}`}
48+
>
49+
{isLeaf ? (
50+
<EditableValue path={thisPath} element={value} />
51+
) : (
52+
<ConfigSubtree config={value} path={thisPath} />
53+
)}
54+
</Subtree>
55+
);
56+
})}
5457
</>
5558
);
5659
}

packages/apps/esm-implementer-tools-app/src/configuration/interactive-editor/editable-value.component.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,9 @@ export default function EditableValue({ path, element, customType }: EditableVal
125125
hasIconOnly
126126
onClick={() => {
127127
clearConfigErrors(path.join('.'));
128-
temporaryConfigStore.setState(unset(temporaryConfigStore.getState(), ['config', ...path]) as any);
128+
const state = cloneDeep(temporaryConfigStore.getState());
129+
unset(state, ['config', ...path]);
130+
temporaryConfigStore.setState(state);
129131
}}
130132
/>
131133
) : null}

0 commit comments

Comments
 (0)