Skip to content

Commit 593453a

Browse files
paula-stachopaula-stacho
committed
fixes and empty type prevention
1 parent 1dc7c81 commit 593453a

File tree

4 files changed

+195
-9
lines changed

4 files changed

+195
-9
lines changed

‎packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,20 @@ async function comboboxSelectItem(
6363
});
6464
}
6565

66+
async function multiComboboxToggleItem(
67+
label: string,
68+
value: string,
69+
visibleLabel = value
70+
) {
71+
userEvent.click(screen.getByRole('textbox', { name: label }));
72+
await waitFor(() => {
73+
const listbox = screen.getByRole('listbox');
74+
expect(listbox).to.be.visible;
75+
const option = within(listbox).getByRole('option', { name: visibleLabel });
76+
userEvent.click(option);
77+
});
78+
}
79+
6680
function getMultiComboboxValues(testId: string) {
6781
const combobox = screen.getByTestId(testId);
6882
expect(combobox).to.be.visible;
@@ -261,6 +275,90 @@ describe('DiagramEditorSidePanel', function () {
261275
expect(screen.queryByText('routes.airline.name')).to.exist;
262276
});
263277
});
278+
279+
it('should change the field type', async function () {
280+
const result = renderDrawer();
281+
result.plugin.store.dispatch(
282+
selectField('flights.routes', ['airline', 'name'])
283+
);
284+
285+
await waitForDrawerToOpen();
286+
expect(screen.getByTitle('routes.airline.name')).to.be.visible;
287+
288+
// before - string
289+
const selectedTypesBefore = getMultiComboboxValues(
290+
'lg-combobox-datatype'
291+
);
292+
expect(selectedTypesBefore).to.have.members(['string']);
293+
294+
// add int and bool and remove string
295+
await multiComboboxToggleItem('Datatype', 'int');
296+
await multiComboboxToggleItem('Datatype', 'bool');
297+
await multiComboboxToggleItem('Datatype', 'string');
298+
299+
const modifiedCollection = selectCurrentModelFromState(
300+
result.plugin.store.getState()
301+
).collections.find((coll) => {
302+
return coll.ns === 'flights.routes';
303+
});
304+
expect(
305+
modifiedCollection?.jsonSchema.properties.airline.properties.name
306+
.bsonType
307+
).to.have.members(['int', 'bool']);
308+
});
309+
310+
it('should not completely remove the type', async function () {
311+
const result = renderDrawer();
312+
result.plugin.store.dispatch(
313+
selectField('flights.routes', ['airline', 'name'])
314+
);
315+
316+
await waitForDrawerToOpen();
317+
expect(screen.getByTitle('routes.airline.name')).to.be.visible;
318+
319+
// before - string
320+
const selectedTypesBefore = getMultiComboboxValues(
321+
'lg-combobox-datatype'
322+
);
323+
expect(selectedTypesBefore).to.have.members(['string']);
324+
325+
// remove string without adding anything else
326+
await multiComboboxToggleItem('Datatype', 'string');
327+
328+
await waitFor(() => {
329+
// error message shown
330+
expect(screen.queryByText('Field must have a type.')).to.exist;
331+
const modifiedCollection = selectCurrentModelFromState(
332+
result.plugin.store.getState()
333+
).collections.find((coll) => {
334+
return coll.ns === 'flights.routes';
335+
});
336+
// type remains unchanged
337+
expect(
338+
modifiedCollection?.jsonSchema.properties.airline.properties.name
339+
.bsonType
340+
).to.equal('string');
341+
});
342+
343+
// finally, add some types
344+
await multiComboboxToggleItem('Datatype', 'bool');
345+
await multiComboboxToggleItem('Datatype', 'int');
346+
347+
await waitFor(() => {
348+
// error goes away
349+
expect(screen.queryByText('Field must have a type.')).not.to.exist;
350+
const modifiedCollection = selectCurrentModelFromState(
351+
result.plugin.store.getState()
352+
).collections.find((coll) => {
353+
return coll.ns === 'flights.routes';
354+
});
355+
// new type applied
356+
expect(
357+
modifiedCollection?.jsonSchema.properties.airline.properties.name
358+
.bsonType
359+
).to.have.members(['bool', 'int']);
360+
});
361+
});
264362
});
265363

266364
it('should change the content of the drawer when selecting different items', async function () {

‎packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useMemo } from 'react';
1+
import React, { useEffect, useMemo, useState } from 'react';
22
import { connect } from 'react-redux';
33
import type {
44
FieldPath,
@@ -117,6 +117,15 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
117117
onRenameField,
118118
onChangeFieldType,
119119
}) => {
120+
const [fieldTypeEditErrorMessage, setFieldTypeEditErrorMessage] = useState<
121+
string | undefined
122+
>();
123+
const [fieldTypes, setFieldTypes] = useState<string[]>(types);
124+
125+
useEffect(() => {
126+
setFieldTypes(types);
127+
}, [types]);
128+
120129
const { value: fieldName, ...nameInputProps } = useChangeOnBlur(
121130
fieldPath[fieldPath.length - 1],
122131
(fieldName) => {
@@ -138,6 +147,12 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
138147
);
139148

140149
const handleTypeChange = (newTypes: string[]) => {
150+
setFieldTypes(newTypes);
151+
if (newTypes.length === 0) {
152+
setFieldTypeEditErrorMessage('Field must have a type.');
153+
return;
154+
}
155+
setFieldTypeEditErrorMessage(undefined);
141156
onChangeFieldType(namespace, fieldPath, newTypes);
142157
};
143158

@@ -165,11 +180,13 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
165180
label="Datatype"
166181
aria-label="Datatype"
167182
disabled={isReadOnly}
168-
value={types}
183+
value={fieldTypes}
169184
size="small"
170185
multiselect={true}
171186
clearable={false}
172187
onChange={handleTypeChange}
188+
state={fieldTypeEditErrorMessage ? 'error' : undefined}
189+
errorMessage={fieldTypeEditErrorMessage}
173190
>
174191
{BSON_TYPES.map((type) => (
175192
<ComboboxOption key={type} value={type} />

‎packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,6 +1159,52 @@ describe('getSchemaWithNewTypes', function () {
11591159
// object is no longer part of anyOf, now it is the only type and so the root schema
11601160
expect(result).to.deep.equal(oldSchema.anyOf[0]);
11611161
});
1162+
1163+
it('removes one of many types', function () {
1164+
const newTypes = ['object', 'array', 'string', 'bool']; // removes int
1165+
const oldSchema = {
1166+
anyOf: [
1167+
{
1168+
bsonType: 'object',
1169+
properties: {
1170+
name: { bsonType: 'string' },
1171+
},
1172+
required: ['name'],
1173+
},
1174+
{
1175+
bsonType: 'array',
1176+
items: {
1177+
properties: {
1178+
name: { bsonType: 'string' },
1179+
},
1180+
required: ['name'],
1181+
},
1182+
},
1183+
{
1184+
bsonType: 'string',
1185+
},
1186+
{
1187+
bsonType: 'int',
1188+
},
1189+
{
1190+
bsonType: 'bool',
1191+
},
1192+
],
1193+
};
1194+
const result = getSchemaWithNewTypes(oldSchema, newTypes);
1195+
expect(result).to.not.to.have.property('bsonType');
1196+
expect(result.anyOf).to.have.lengthOf(4);
1197+
expect(result.anyOf).to.have.deep.members([
1198+
oldSchema.anyOf[0],
1199+
oldSchema.anyOf[1],
1200+
oldSchema.anyOf[2],
1201+
// int - is missing
1202+
oldSchema.anyOf[4],
1203+
]);
1204+
expect(result.anyOf).to.not.have.deep.members([
1205+
oldSchema.anyOf[3], // int - is missing
1206+
]);
1207+
});
11621208
});
11631209

11641210
describe('uses anyOf for a mixture of simple and complex types', function () {
@@ -1317,6 +1363,27 @@ describe('getSchemaWithNewTypes', function () {
13171363
expect(result).not.to.have.property('anyOf');
13181364
expect(result).to.deep.equal({ bsonType: newTypes });
13191365
});
1366+
1367+
it('removes string from a mixed type, leaving object', function () {
1368+
const newTypes = ['object'];
1369+
const oldSchema = {
1370+
anyOf: [
1371+
{
1372+
bsonType: 'object',
1373+
properties: {
1374+
name: { bsonType: 'string' },
1375+
},
1376+
required: ['name'],
1377+
},
1378+
{
1379+
bsonType: 'string',
1380+
},
1381+
],
1382+
};
1383+
const result = getSchemaWithNewTypes(oldSchema, newTypes);
1384+
expect(result).not.to.have.property('anyOf');
1385+
expect(result).to.deep.equal(oldSchema.anyOf[0]);
1386+
});
13201387
});
13211388
});
13221389
});

‎packages/compass-data-modeling/src/utils/schema-traversal.tsx‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -331,24 +331,28 @@ const getMin1ObjectVariants = (
331331
};
332332

333333
const getOtherVariants = (
334-
oldSchema: JSONSchema,
334+
oldSchema: MongoDBJSONSchema,
335335
newTypes: string[]
336336
): MongoDBJSONSchema[] => {
337337
const existingAnyOfVariants =
338338
oldSchema.anyOf?.filter(
339-
(variant) => variant.bsonType !== 'object' && variant.bsonType !== 'array'
339+
(variant) =>
340+
typeof variant.bsonType === 'string' &&
341+
variant.bsonType !== 'object' &&
342+
variant.bsonType !== 'array' &&
343+
newTypes.includes(variant.bsonType)
340344
) || [];
341345
const existingAnyOfTypes = existingAnyOfVariants
342346
.map((v) => v.bsonType)
343347
.flat();
344348
const existingBasicTypes = oldSchema.bsonType
345-
? []
346-
: Array.isArray(oldSchema.bsonType)
347-
? oldSchema.bsonType
348-
: [oldSchema.bsonType];
349+
? Array.isArray(oldSchema.bsonType)
350+
? oldSchema.bsonType
351+
: [oldSchema.bsonType]
352+
: [];
349353
const existingBasicVariants = existingBasicTypes
350354
.filter(
351-
(type) => newTypes.includes(type) && type !== 'object' && type !== 'array'
355+
(type) => type !== 'object' && type !== 'array' && newTypes.includes(type)
352356
)
353357
.map((type) => ({ bsonType: type }));
354358
const newVariants = newTypes

0 commit comments

Comments
 (0)