Skip to content

Commit 04daaa9

Browse files
paula-stachopaula-stacho
authored
feat(compass-data-modeling): update field type COMPASS-9659 (#7248)
1 parent 01231f9 commit 04daaa9

File tree

7 files changed

+766
-75
lines changed

7 files changed

+766
-75
lines changed

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

Lines changed: 148 additions & 8 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;
@@ -166,15 +180,15 @@ describe('DiagramEditorSidePanel', function () {
166180
it('should render a nested field context drawer', async function () {
167181
const result = renderDrawer();
168182
result.plugin.store.dispatch(
169-
selectField('flights.routes', ['airline', 'id'])
183+
selectField('flights.routes', ['airline', '_id'])
170184
);
171185

172186
await waitForDrawerToOpen();
173-
expect(screen.getByTitle('routes.airline.id')).to.be.visible;
187+
expect(screen.getByTitle('routes.airline._id')).to.be.visible;
174188

175189
const nameInput = screen.getByLabelText('Field name');
176190
expect(nameInput).to.be.visible;
177-
expect(nameInput).to.have.value('id');
191+
expect(nameInput).to.have.value('_id');
178192

179193
const selectedTypes = getMultiComboboxValues('lg-combobox-datatype');
180194
expect(selectedTypes).to.have.lengthOf(1);
@@ -184,16 +198,16 @@ describe('DiagramEditorSidePanel', function () {
184198
it('should delete a field', async function () {
185199
const result = renderDrawer();
186200
result.plugin.store.dispatch(
187-
selectField('flights.routes', ['airline', 'id'])
201+
selectField('flights.routes', ['airline', '_id'])
188202
);
189203

190204
await waitForDrawerToOpen();
191-
expect(screen.getByTitle('routes.airline.id')).to.be.visible;
205+
expect(screen.getByTitle('routes.airline._id')).to.be.visible;
192206

193207
userEvent.click(screen.getByLabelText(/delete field/i));
194208

195209
await waitFor(() => {
196-
expect(screen.queryByText('routes.airline.id')).not.to.exist;
210+
expect(screen.queryByText('routes.airline._id')).not.to.exist;
197211
});
198212
expect(screen.queryByLabelText('Name')).to.not.exist;
199213

@@ -205,7 +219,7 @@ describe('DiagramEditorSidePanel', function () {
205219

206220
expect(
207221
modifiedCollection?.jsonSchema.properties?.airline.properties
208-
).to.not.have.property('id'); // deleted field
222+
).to.not.have.property('_id'); // deleted field
209223
expect(
210224
modifiedCollection?.jsonSchema.properties?.airline.properties
211225
).to.have.property('name'); // sibling field remains
@@ -254,13 +268,139 @@ describe('DiagramEditorSidePanel', function () {
254268
await waitForDrawerToOpen();
255269
expect(screen.getByTitle('routes.airline.name')).to.be.visible;
256270

257-
updateInputWithBlur('Field name', 'id');
271+
updateInputWithBlur('Field name', '_id');
258272

259273
await waitFor(() => {
260274
expect(screen.queryByText('Field already exists.')).to.exist;
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+
});
362+
363+
it('top level _id field is treated as readonly', async function () {
364+
const result = renderDrawer();
365+
result.plugin.store.dispatch(selectField('flights.routes', ['_id']));
366+
367+
await waitForDrawerToOpen();
368+
expect(screen.getByTitle('routes._id')).to.be.visible;
369+
370+
expect(screen.queryByLabelText(/delete field/i)).not.to.exist;
371+
expect(screen.getByLabelText('Field name')).to.have.attribute(
372+
'aria-disabled',
373+
'true'
374+
);
375+
expect(screen.getByLabelText('Datatype')).to.have.attribute(
376+
'aria-disabled',
377+
'true'
378+
);
379+
});
380+
381+
it('nested _id field is not treated as readonly', async function () {
382+
const result = renderDrawer();
383+
result.plugin.store.dispatch(
384+
selectField('flights.routes', ['airline', '_id'])
385+
);
386+
387+
await waitForDrawerToOpen();
388+
expect(screen.getByTitle('routes.airline._id')).to.be.visible;
389+
390+
expect(screen.queryByLabelText(/delete field/i)).to.exist;
391+
expect(screen.queryByLabelText(/delete field/i)).to.have.attribute(
392+
'aria-disabled',
393+
'false'
394+
);
395+
expect(screen.getByLabelText('Field name')).to.have.attribute(
396+
'aria-disabled',
397+
'false'
398+
);
399+
expect(screen.getByLabelText('Datatype')).to.have.attribute(
400+
'aria-disabled',
401+
'false'
402+
);
403+
});
264404
});
265405

266406
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: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import React, { useMemo } from 'react';
1+
import React, { useMemo, useState } from 'react';
22
import { connect } from 'react-redux';
33
import type {
44
FieldPath,
@@ -11,6 +11,7 @@ import {
1111
} from '@mongodb-js/compass-components';
1212
import { BSONType } from 'mongodb';
1313
import {
14+
changeFieldType,
1415
createNewRelationship,
1516
deleteRelationship,
1617
getCurrentDiagramFromState,
@@ -56,8 +57,7 @@ type FieldDrawerContentProps = {
5657
onChangeFieldType: (
5758
namespace: string,
5859
fieldPath: FieldPath,
59-
fromBsonType: string | string[],
60-
toBsonType: string | string[]
60+
newTypes: string[]
6161
) => void;
6262
};
6363

@@ -117,6 +117,11 @@ 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+
120125
const { value: fieldName, ...nameInputProps } = useChangeOnBlur(
121126
fieldPath[fieldPath.length - 1],
122127
(fieldName) => {
@@ -137,17 +142,25 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
137142
[fieldPath, fieldPaths, fieldName]
138143
);
139144

140-
const handleTypeChange = (newTypes: string | string[]) => {
141-
onChangeFieldType(namespace, fieldPath, types, newTypes);
145+
const handleTypeChange = (newTypes: string[]) => {
146+
setFieldTypes(newTypes);
147+
if (newTypes.length === 0) {
148+
setFieldTypeEditErrorMessage('Field must have a type.');
149+
return;
150+
}
151+
setFieldTypeEditErrorMessage(undefined);
152+
onChangeFieldType(namespace, fieldPath, newTypes);
142153
};
143154

155+
const isReadOnly = useMemo(() => isIdField(fieldPath), [fieldPath]);
156+
144157
return (
145158
<>
146159
<DMDrawerSection label="Field properties">
147160
<DMFormFieldContainer>
148161
<TextInput
149162
label="Field name"
150-
disabled={isIdField(fieldPath)}
163+
disabled={isReadOnly}
151164
data-testid="data-model-collection-drawer-name-input"
152165
sizeVariant="small"
153166
value={fieldName}
@@ -162,12 +175,14 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
162175
data-testid="lg-combobox-datatype"
163176
label="Datatype"
164177
aria-label="Datatype"
165-
disabled={true} // TODO(COMPASS-9659): enable when field type change is implemented
166-
value={types}
178+
disabled={isReadOnly}
179+
value={fieldTypes}
167180
size="small"
168181
multiselect={true}
169182
clearable={false}
170183
onChange={handleTypeChange}
184+
state={fieldTypeEditErrorMessage ? 'error' : undefined}
185+
errorMessage={fieldTypeEditErrorMessage}
171186
>
172187
{BSON_TYPES.map((type) => (
173188
<ComboboxOption key={type} value={type} />
@@ -228,6 +243,6 @@ export default connect(
228243
onEditRelationshipClick: selectRelationship,
229244
onDeleteRelationshipClick: deleteRelationship,
230245
onRenameField: renameField,
231-
onChangeFieldType: () => {}, // TODO(COMPASS-9659): updateFieldSchema,
246+
onChangeFieldType: changeFieldType,
232247
}
233248
)(FieldDrawerContent);

‎packages/compass-data-modeling/src/store/apply-edit.ts‎

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel {
186186
jsonSchema: updateSchema({
187187
jsonSchema: collection.jsonSchema,
188188
fieldPath: edit.field,
189-
update: 'removeField',
189+
updateParameters: { update: 'removeField' },
190190
}),
191191
};
192192
}),
@@ -217,8 +217,29 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel {
217217
jsonSchema: updateSchema({
218218
jsonSchema: collection.jsonSchema,
219219
fieldPath: edit.field,
220-
update: 'renameField',
221-
newFieldName: edit.newName,
220+
updateParameters: {
221+
update: 'renameField',
222+
newFieldName: edit.newName,
223+
},
224+
}),
225+
};
226+
}),
227+
};
228+
}
229+
case 'ChangeFieldType': {
230+
return {
231+
...model,
232+
collections: model.collections.map((collection) => {
233+
if (collection.ns !== edit.ns) return collection;
234+
return {
235+
...collection,
236+
jsonSchema: updateSchema({
237+
jsonSchema: collection.jsonSchema,
238+
fieldPath: edit.field,
239+
updateParameters: {
240+
update: 'changeFieldSchema',
241+
newFieldSchema: edit.to,
242+
},
222243
}),
223244
};
224245
}),

‎packages/compass-data-modeling/src/store/diagram.ts‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,11 @@ import type { MongoDBJSONSchema } from 'mongodb-schema';
2929
import { getCoordinatesForNewNode } from '@mongodb-js/diagramming';
3030
import { collectionToBaseNodeForLayout } from '../utils/nodes-and-edges';
3131
import toNS from 'mongodb-ns';
32-
import { traverseSchema } from '../utils/schema-traversal';
32+
import {
33+
getFieldFromSchema,
34+
getSchemaWithNewTypes,
35+
traverseSchema,
36+
} from '../utils/schema-traversal';
3337
import { applyEdit as _applyEdit } from './apply-edit';
3438
import { getNewUnusedFieldName } from '../utils/schema';
3539

@@ -718,6 +722,34 @@ export function renameField(
718722
return applyEdit({ type: 'RenameField', ns, field, newName });
719723
}
720724

725+
export function changeFieldType(
726+
ns: string,
727+
fieldPath: FieldPath,
728+
newTypes: string[]
729+
): DataModelingThunkAction<void, ApplyEditAction | ApplyEditFailedAction> {
730+
return (dispatch, getState) => {
731+
const collectionSchema = selectCurrentModelFromState(
732+
getState()
733+
).collections.find((collection) => collection.ns === ns)?.jsonSchema;
734+
if (!collectionSchema) throw new Error('Collection not found in model');
735+
const field = getFieldFromSchema({
736+
jsonSchema: collectionSchema,
737+
fieldPath: fieldPath,
738+
});
739+
if (!field) throw new Error('Field not found in schema');
740+
const to = getSchemaWithNewTypes(field.jsonSchema, newTypes);
741+
dispatch(
742+
applyEdit({
743+
type: 'ChangeFieldType',
744+
ns,
745+
field: fieldPath,
746+
from: field.jsonSchema,
747+
to,
748+
})
749+
);
750+
};
751+
}
752+
721753
function getPositionForNewCollection(
722754
existingCollections: DataModelCollection[],
723755
newCollection: Omit<DataModelCollection, 'displayPosition'>

0 commit comments

Comments
 (0)