Skip to content

Commit 1c7148b

Browse files
authored
Improve union type default handling in values schema (#4698)
Closes #4688 Signed-off-by: Cintia Sánchez García <cynthiasg@icloud.com>
1 parent 6747492 commit 1c7148b

File tree

15 files changed

+228
-38
lines changed

15 files changed

+228
-38
lines changed

web/src/layout/package/changelog/Content.test.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,23 @@ const defaultProps = {
7070
state: null,
7171
};
7272

73+
// Fixed date for stable snapshot tests (~1 month after fixture timestamp 1604048487)
74+
const fixedDate = new Date('2020-11-30T00:00:00Z');
75+
7376
describe('Changelog content ', () => {
74-
afterEach(() => {
75-
jest.resetAllMocks();
77+
beforeAll(() => {
78+
vi.useFakeTimers({
79+
now: fixedDate,
80+
toFake: ['Date'],
81+
});
7682
});
7783

78-
beforeEach(() => {
79-
jest.spyOn(Date, 'now').mockReturnValueOnce(new Date('2019/11/24').getTime());
84+
afterAll(() => {
85+
vi.useRealTimers();
86+
});
87+
88+
afterEach(() => {
89+
jest.resetAllMocks();
8090
});
8191

8292
it('creates snapshot', () => {

web/src/layout/package/changelog/Modal.test.tsx

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@ vi.mock('react-router-dom', () => ({
2121
const scrollIntoViewMock = jest.fn();
2222
window.HTMLElement.prototype.scrollIntoView = scrollIntoViewMock;
2323

24+
// Fixed date for stable snapshot tests (~1 month after fixture timestamp 1604048487)
25+
const fixedDate = new Date('2020-11-30T00:00:00Z');
26+
2427
const getMockChangelog = (fixtureId: string): ChangeLog[] => {
2528
// eslint-disable-next-line @typescript-eslint/no-require-imports
2629
return require(`./__fixtures__/Modal/${fixtureId}.json`) as ChangeLog[];
@@ -58,12 +61,19 @@ const defaultProps = {
5861
};
5962

6063
describe('ChangelogModal', () => {
61-
afterEach(() => {
62-
jest.resetAllMocks();
64+
beforeAll(() => {
65+
vi.useFakeTimers({
66+
now: fixedDate,
67+
toFake: ['Date'],
68+
});
6369
});
6470

65-
beforeEach(() => {
66-
jest.spyOn(Date, 'now').mockReturnValueOnce(new Date('2019/11/24').getTime());
71+
afterAll(() => {
72+
vi.useRealTimers();
73+
});
74+
75+
afterEach(() => {
76+
jest.resetAllMocks();
6777
});
6878

6979
it('creates snapshot', async () => {

web/src/layout/package/changelog/__snapshots__/Content.test.tsx.snap

Lines changed: 2 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

web/src/layout/package/changelog/__snapshots__/Modal.test.tsx.snap

Lines changed: 7 additions & 7 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

web/src/layout/package/valuesSchema/SchemaDefinition.test.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,5 +282,23 @@ describe('SchemaDefinition', () => {
282282
expect(screen.getByText('0')).toBeInTheDocument();
283283
expect(screen.getAllByText('-')).toHaveLength(3);
284284
});
285+
286+
it('renders type union (string|integer) with integer default - resolves to integer', () => {
287+
const props = getProps('14');
288+
render(<SchemaDefinition {...props} {...defaultProps} />);
289+
290+
const select = screen.getByRole('combobox', { name: 'Type selection' });
291+
expect(select).toBeInTheDocument();
292+
expect(select).toHaveValue('integer');
293+
});
294+
295+
it('renders type union (string|integer) with string default - resolves to string', () => {
296+
const props = getProps('15');
297+
render(<SchemaDefinition {...props} {...defaultProps} />);
298+
299+
const select = screen.getByRole('combobox', { name: 'Type selection' });
300+
expect(select).toBeInTheDocument();
301+
expect(select).toHaveValue('string');
302+
});
285303
});
286304
});

web/src/layout/package/valuesSchema/SchemaDefinition.tsx

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { JSONSchema } from '../../../jsonschema';
1313
import { ActiveJSONSchemaValue } from '../../../types';
1414
import checkIfPropIsRequiredInSchema from '../../../utils/checkIfPropIsRequiredInSchema';
1515
import detectLinksInText from '../../../utils/detectLinksInText';
16+
import resolveSchemaTypeFromDefault from '../../../utils/resolveSchemaTypeFromDefault';
1617
import ButtonCopyToClipboard from '../../common/ButtonCopyToClipboard';
1718
import styles from './SchemaDefinition.module.css';
1819

@@ -114,27 +115,25 @@ const SCHEMA_PROPS_PER_TYPE: KeywordPropsByType = {
114115
],
115116
};
116117

118+
const getInitialType = (def: JSONSchema): string | undefined => {
119+
let currentType: string | undefined;
120+
if (def.type) {
121+
if (isArray(def.type)) {
122+
currentType = resolveSchemaTypeFromDefault(def.type as string[], def.default);
123+
} else {
124+
currentType = def.type;
125+
}
126+
} else if (def.properties) {
127+
currentType = 'object';
128+
}
129+
return currentType;
130+
};
131+
117132
const SchemaDefinition = (props: Props) => {
118133
const ref = useRef<HTMLDivElement>(null);
119134
const def = props.def.options[props.def.active];
120135

121-
const getInitialType = (): string | undefined => {
122-
let currentType: string | undefined;
123-
if (def.type) {
124-
if (isArray(def.type)) {
125-
currentType = def.type[0] as string;
126-
} else {
127-
currentType = def.type;
128-
}
129-
} else {
130-
if (def.properties) {
131-
currentType = 'object';
132-
}
133-
}
134-
return currentType;
135-
};
136-
137-
const [activeType, setActiveType] = useState<string | undefined>(getInitialType());
136+
const [activeType, setActiveType] = useState<string | undefined>(() => getInitialType(def));
138137

139138
useEffect(() => {
140139
// Scrolls content into view when a definition is expanded
@@ -144,8 +143,8 @@ const SchemaDefinition = (props: Props) => {
144143
}, [props.isExpanded, ref]);
145144

146145
useEffect(() => {
147-
setActiveType(getInitialType());
148-
}, [props.def.active, props.def.combinationType]);
146+
setActiveType(getInitialType(def));
147+
}, [def.default, def.type, def.properties, props.def.active, props.def.combinationType]);
149148

150149
const typeDef = activeType ? SCHEMA_PROPS_PER_TYPE[activeType] : null;
151150

web/src/layout/package/valuesSchema/SchemaLine.test.tsx

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,5 +144,33 @@ describe('SchemaLine', () => {
144144
expect(onActivePathChangeMock).toHaveBeenCalledWith('db.host');
145145
});
146146
});
147+
148+
it('renders union type (string|integer) with integer default value', () => {
149+
render(
150+
<SchemaLine {...getProps('12')} onActivePathChange={onActivePathChangeMock} saveSelectedOption={jest.fn()} />
151+
);
152+
153+
expect(screen.getByText('# CPU request')).toBeInTheDocument();
154+
expect(screen.getByText('cpu:')).toBeInTheDocument();
155+
156+
const defaultValue = screen.getByTestId('defaultValue');
157+
expect(defaultValue).toBeInTheDocument();
158+
expect(defaultValue).toHaveTextContent('2');
159+
expect(defaultValue).toHaveClass('text-danger');
160+
});
161+
162+
it('renders union type (string|integer) with string default value', () => {
163+
render(
164+
<SchemaLine {...getProps('13')} onActivePathChange={onActivePathChangeMock} saveSelectedOption={jest.fn()} />
165+
);
166+
167+
expect(screen.getByText('# CPU request')).toBeInTheDocument();
168+
expect(screen.getByText('cpu:')).toBeInTheDocument();
169+
170+
const defaultValue = screen.getByTestId('defaultValue');
171+
expect(defaultValue).toBeInTheDocument();
172+
expect(defaultValue).toHaveTextContent('500m');
173+
expect(defaultValue).toHaveClass('text-warning');
174+
});
147175
});
148176
});

web/src/layout/package/valuesSchema/SchemaLine.tsx

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { ActiveJSONSchemaValue } from '../../../types';
1111
import checkIfPropIsRequiredInSchema from '../../../utils/checkIfPropIsRequiredInSchema';
1212
import formatStringForYAML from '../../../utils/formatStringForYAML';
1313
import getJMESPathForValuesSchema from '../../../utils/getJMESPathForValuesSchema';
14+
import resolveSchemaTypeFromDefault from '../../../utils/resolveSchemaTypeFromDefault';
1415
import SchemaDefinition from './SchemaDefinition';
1516
import styles from './SchemaLine.module.css';
1617

@@ -54,8 +55,14 @@ const getValue = (newValue: any): ValueProp => {
5455
};
5556
}
5657

58+
const defaultValue = valueToCheck.default;
59+
const typeOptions = isArray(valueToCheck.type) ? valueToCheck.type : [valueToCheck.type];
60+
const resolvedType = isArray(valueToCheck.type)
61+
? resolveSchemaTypeFromDefault(typeOptions as string[], defaultValue)
62+
: typeOptions[0];
63+
5764
let isLongText;
58-
switch (isArray(valueToCheck.type) ? valueToCheck.type[0] : valueToCheck.type) {
65+
switch (resolvedType) {
5966
case 'object':
6067
return {
6168
content: <span>{isEmpty(valueToCheck.default) ? '{}' : JSON.stringify(valueToCheck.default)}</span>,
@@ -87,6 +94,7 @@ const getValue = (newValue: any): ValueProp => {
8794
};
8895
case 'boolean':
8996
case 'integer':
97+
case 'number':
9098
return {
9199
content: <span>{valueToCheck.default!.toString()}</span>,
92100
className: 'text-danger',
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"def": {
3+
"active": 0,
4+
"combinationType": null,
5+
"options": [
6+
{
7+
"type": ["string", "integer"],
8+
"title": "CPU request",
9+
"description": "CPU request for the container",
10+
"default": 2
11+
}
12+
],
13+
"error": false
14+
},
15+
"defaultValue": null
16+
}
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"def": {
3+
"active": 0,
4+
"combinationType": null,
5+
"options": [
6+
{
7+
"type": ["string", "integer"],
8+
"title": "CPU request",
9+
"description": "CPU request for the container",
10+
"default": "500m"
11+
}
12+
],
13+
"error": false
14+
},
15+
"defaultValue": null
16+
}

0 commit comments

Comments
 (0)