diff --git a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx index 13d6fa16228..7b8a39e9745 100644 --- a/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx +++ b/packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx @@ -63,20 +63,6 @@ async function comboboxSelectItem( }); } -async function multiComboboxToggleItem( - label: string, - value: string, - visibleLabel = value -) { - userEvent.click(screen.getByRole('textbox', { name: label })); - await waitFor(() => { - const listbox = screen.getByRole('listbox'); - expect(listbox).to.be.visible; - const option = within(listbox).getByRole('option', { name: visibleLabel }); - userEvent.click(option); - }); -} - function getMultiComboboxValues(testId: string) { const combobox = screen.getByTestId(testId); expect(combobox).to.be.visible; @@ -180,15 +166,15 @@ describe('DiagramEditorSidePanel', function () { it('should render a nested field context drawer', async function () { const result = renderDrawer(); result.plugin.store.dispatch( - selectField('flights.routes', ['airline', '_id']) + selectField('flights.routes', ['airline', 'id']) ); await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline._id')).to.be.visible; + expect(screen.getByTitle('routes.airline.id')).to.be.visible; const nameInput = screen.getByLabelText('Field name'); expect(nameInput).to.be.visible; - expect(nameInput).to.have.value('_id'); + expect(nameInput).to.have.value('id'); const selectedTypes = getMultiComboboxValues('lg-combobox-datatype'); expect(selectedTypes).to.have.lengthOf(1); @@ -198,16 +184,16 @@ describe('DiagramEditorSidePanel', function () { it('should delete a field', async function () { const result = renderDrawer(); result.plugin.store.dispatch( - selectField('flights.routes', ['airline', '_id']) + selectField('flights.routes', ['airline', 'id']) ); await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline._id')).to.be.visible; + expect(screen.getByTitle('routes.airline.id')).to.be.visible; userEvent.click(screen.getByLabelText(/delete field/i)); await waitFor(() => { - expect(screen.queryByText('routes.airline._id')).not.to.exist; + expect(screen.queryByText('routes.airline.id')).not.to.exist; }); expect(screen.queryByLabelText('Name')).to.not.exist; @@ -219,7 +205,7 @@ describe('DiagramEditorSidePanel', function () { expect( modifiedCollection?.jsonSchema.properties?.airline.properties - ).to.not.have.property('_id'); // deleted field + ).to.not.have.property('id'); // deleted field expect( modifiedCollection?.jsonSchema.properties?.airline.properties ).to.have.property('name'); // sibling field remains @@ -268,139 +254,13 @@ describe('DiagramEditorSidePanel', function () { await waitForDrawerToOpen(); expect(screen.getByTitle('routes.airline.name')).to.be.visible; - updateInputWithBlur('Field name', '_id'); + updateInputWithBlur('Field name', 'id'); await waitFor(() => { expect(screen.queryByText('Field already exists.')).to.exist; expect(screen.queryByText('routes.airline.name')).to.exist; }); }); - - it('should change the field type', async function () { - const result = renderDrawer(); - result.plugin.store.dispatch( - selectField('flights.routes', ['airline', 'name']) - ); - - await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline.name')).to.be.visible; - - // before - string - const selectedTypesBefore = getMultiComboboxValues( - 'lg-combobox-datatype' - ); - expect(selectedTypesBefore).to.have.members(['string']); - - // add int and bool and remove string - await multiComboboxToggleItem('Datatype', 'int'); - await multiComboboxToggleItem('Datatype', 'bool'); - await multiComboboxToggleItem('Datatype', 'string'); - - const modifiedCollection = selectCurrentModelFromState( - result.plugin.store.getState() - ).collections.find((coll) => { - return coll.ns === 'flights.routes'; - }); - expect( - modifiedCollection?.jsonSchema.properties?.airline?.properties?.name - .bsonType - ).to.have.members(['int', 'bool']); - }); - - it('should not completely remove the type', async function () { - const result = renderDrawer(); - result.plugin.store.dispatch( - selectField('flights.routes', ['airline', 'name']) - ); - - await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline.name')).to.be.visible; - - // before - string - const selectedTypesBefore = getMultiComboboxValues( - 'lg-combobox-datatype' - ); - expect(selectedTypesBefore).to.have.members(['string']); - - // remove string without adding anything else - await multiComboboxToggleItem('Datatype', 'string'); - - await waitFor(() => { - // error message shown - expect(screen.queryByText('Field must have a type.')).to.exist; - const modifiedCollection = selectCurrentModelFromState( - result.plugin.store.getState() - ).collections.find((coll) => { - return coll.ns === 'flights.routes'; - }); - // type remains unchanged - expect( - modifiedCollection?.jsonSchema.properties?.airline?.properties?.name - .bsonType - ).to.equal('string'); - }); - - // finally, add some types - await multiComboboxToggleItem('Datatype', 'bool'); - await multiComboboxToggleItem('Datatype', 'int'); - - await waitFor(() => { - // error goes away - expect(screen.queryByText('Field must have a type.')).not.to.exist; - const modifiedCollection = selectCurrentModelFromState( - result.plugin.store.getState() - ).collections.find((coll) => { - return coll.ns === 'flights.routes'; - }); - // new type applied - expect( - modifiedCollection?.jsonSchema.properties?.airline?.properties?.name - .bsonType - ).to.have.members(['bool', 'int']); - }); - }); - - it('top level _id field is treated as readonly', async function () { - const result = renderDrawer(); - result.plugin.store.dispatch(selectField('flights.routes', ['_id'])); - - await waitForDrawerToOpen(); - expect(screen.getByTitle('routes._id')).to.be.visible; - - expect(screen.queryByLabelText(/delete field/i)).not.to.exist; - expect(screen.getByLabelText('Field name')).to.have.attribute( - 'aria-disabled', - 'true' - ); - expect(screen.getByLabelText('Datatype')).to.have.attribute( - 'aria-disabled', - 'true' - ); - }); - - it('nested _id field is not treated as readonly', async function () { - const result = renderDrawer(); - result.plugin.store.dispatch( - selectField('flights.routes', ['airline', '_id']) - ); - - await waitForDrawerToOpen(); - expect(screen.getByTitle('routes.airline._id')).to.be.visible; - - expect(screen.queryByLabelText(/delete field/i)).to.exist; - expect(screen.queryByLabelText(/delete field/i)).to.have.attribute( - 'aria-disabled', - 'false' - ); - expect(screen.getByLabelText('Field name')).to.have.attribute( - 'aria-disabled', - 'false' - ); - expect(screen.getByLabelText('Datatype')).to.have.attribute( - 'aria-disabled', - 'false' - ); - }); }); it('should change the content of the drawer when selecting different items', async function () { diff --git a/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx b/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx index 226e6937d2b..7adea901509 100644 --- a/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx +++ b/packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx @@ -1,4 +1,4 @@ -import React, { useMemo, useState } from 'react'; +import React, { useMemo } from 'react'; import { connect } from 'react-redux'; import type { FieldPath, @@ -11,7 +11,6 @@ import { } from '@mongodb-js/compass-components'; import { BSONType } from 'mongodb'; import { - changeFieldType, createNewRelationship, deleteRelationship, getCurrentDiagramFromState, @@ -57,7 +56,8 @@ type FieldDrawerContentProps = { onChangeFieldType: ( namespace: string, fieldPath: FieldPath, - newTypes: string[] + fromBsonType: string | string[], + toBsonType: string | string[] ) => void; }; @@ -117,11 +117,6 @@ const FieldDrawerContent: React.FunctionComponent = ({ onRenameField, onChangeFieldType, }) => { - const [fieldTypeEditErrorMessage, setFieldTypeEditErrorMessage] = useState< - string | undefined - >(); - const [fieldTypes, setFieldTypes] = useState(types); - const { value: fieldName, ...nameInputProps } = useChangeOnBlur( fieldPath[fieldPath.length - 1], (fieldName) => { @@ -142,25 +137,17 @@ const FieldDrawerContent: React.FunctionComponent = ({ [fieldPath, fieldPaths, fieldName] ); - const handleTypeChange = (newTypes: string[]) => { - setFieldTypes(newTypes); - if (newTypes.length === 0) { - setFieldTypeEditErrorMessage('Field must have a type.'); - return; - } - setFieldTypeEditErrorMessage(undefined); - onChangeFieldType(namespace, fieldPath, newTypes); + const handleTypeChange = (newTypes: string | string[]) => { + onChangeFieldType(namespace, fieldPath, types, newTypes); }; - const isReadOnly = useMemo(() => isIdField(fieldPath), [fieldPath]); - return ( <> = ({ data-testid="lg-combobox-datatype" label="Datatype" aria-label="Datatype" - disabled={isReadOnly} - value={fieldTypes} + disabled={true} // TODO(COMPASS-9659): enable when field type change is implemented + value={types} size="small" multiselect={true} clearable={false} onChange={handleTypeChange} - state={fieldTypeEditErrorMessage ? 'error' : undefined} - errorMessage={fieldTypeEditErrorMessage} > {BSON_TYPES.map((type) => ( @@ -243,6 +228,6 @@ export default connect( onEditRelationshipClick: selectRelationship, onDeleteRelationshipClick: deleteRelationship, onRenameField: renameField, - onChangeFieldType: changeFieldType, + onChangeFieldType: () => {}, // TODO(COMPASS-9659): updateFieldSchema, } )(FieldDrawerContent); diff --git a/packages/compass-data-modeling/src/store/apply-edit.ts b/packages/compass-data-modeling/src/store/apply-edit.ts index 2e34d273a39..908efd81f4b 100644 --- a/packages/compass-data-modeling/src/store/apply-edit.ts +++ b/packages/compass-data-modeling/src/store/apply-edit.ts @@ -186,7 +186,7 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel { jsonSchema: updateSchema({ jsonSchema: collection.jsonSchema, fieldPath: edit.field, - updateParameters: { update: 'removeField' }, + update: 'removeField', }), }; }), @@ -217,29 +217,8 @@ export function applyEdit(edit: Edit, model?: StaticModel): StaticModel { jsonSchema: updateSchema({ jsonSchema: collection.jsonSchema, fieldPath: edit.field, - updateParameters: { - update: 'renameField', - newFieldName: edit.newName, - }, - }), - }; - }), - }; - } - case 'ChangeFieldType': { - return { - ...model, - collections: model.collections.map((collection) => { - if (collection.ns !== edit.ns) return collection; - return { - ...collection, - jsonSchema: updateSchema({ - jsonSchema: collection.jsonSchema, - fieldPath: edit.field, - updateParameters: { - update: 'changeFieldSchema', - newFieldSchema: edit.to, - }, + update: 'renameField', + newFieldName: edit.newName, }), }; }), diff --git a/packages/compass-data-modeling/src/store/diagram.ts b/packages/compass-data-modeling/src/store/diagram.ts index 95c5ac4b63a..f66d44ce5c9 100644 --- a/packages/compass-data-modeling/src/store/diagram.ts +++ b/packages/compass-data-modeling/src/store/diagram.ts @@ -29,11 +29,7 @@ import type { MongoDBJSONSchema } from 'mongodb-schema'; import { getCoordinatesForNewNode } from '@mongodb-js/diagramming'; import { collectionToBaseNodeForLayout } from '../utils/nodes-and-edges'; import toNS from 'mongodb-ns'; -import { - getFieldFromSchema, - getSchemaWithNewTypes, - traverseSchema, -} from '../utils/schema-traversal'; +import { traverseSchema } from '../utils/schema-traversal'; import { applyEdit as _applyEdit } from './apply-edit'; import { getNewUnusedFieldName } from '../utils/schema'; @@ -722,34 +718,6 @@ export function renameField( return applyEdit({ type: 'RenameField', ns, field, newName }); } -export function changeFieldType( - ns: string, - fieldPath: FieldPath, - newTypes: string[] -): DataModelingThunkAction { - return (dispatch, getState) => { - const collectionSchema = selectCurrentModelFromState( - getState() - ).collections.find((collection) => collection.ns === ns)?.jsonSchema; - if (!collectionSchema) throw new Error('Collection not found in model'); - const field = getFieldFromSchema({ - jsonSchema: collectionSchema, - fieldPath: fieldPath, - }); - if (!field) throw new Error('Field not found in schema'); - const to = getSchemaWithNewTypes(field.jsonSchema, newTypes); - dispatch( - applyEdit({ - type: 'ChangeFieldType', - ns, - field: fieldPath, - from: field.jsonSchema, - to, - }) - ); - }; -} - function getPositionForNewCollection( existingCollections: DataModelCollection[], newCollection: Omit diff --git a/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx b/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx index 0275df4c38a..9b7a0e59a8e 100644 --- a/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx +++ b/packages/compass-data-modeling/src/utils/schema-traversal.spec.tsx @@ -3,7 +3,6 @@ import { traverseSchema, getFieldFromSchema, updateSchema, - getSchemaWithNewTypes, } from './schema-traversal'; import Sinon from 'sinon'; @@ -464,9 +463,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: {}, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({}); }); @@ -494,9 +491,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['address', 'age'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal(schema); }); @@ -514,9 +509,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -567,9 +560,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['person', 'address'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -606,9 +597,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['person', 'address', 'city'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -652,9 +641,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['names', 'first'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -694,9 +681,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['addresses', 'streetNumber'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -739,9 +724,7 @@ describe('removeField', function () { const result = updateSchema({ fieldPath: ['todos', 'completed'], jsonSchema: schema, - updateParameters: { - update: 'removeField', - }, + update: 'removeField', }); expect(result).to.deep.equal({ ...schema, @@ -772,10 +755,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: {}, - updateParameters: { - update: 'renameField', - newFieldName: 'newName', - }, + update: 'renameField', + newFieldName: 'newName', }); expect(result).to.deep.equal({}); }); @@ -803,10 +784,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['address', 'age'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'newName', - }, + update: 'renameField', + newFieldName: 'newName', }); expect(result).to.deep.equal(schema); }); @@ -824,10 +803,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['name'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'newName', - }, + update: 'renameField', + newFieldName: 'newName', }); expect(result).to.deep.equal({ ...schema, @@ -880,10 +857,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['person', 'address'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'location', - }, + update: 'renameField', + newFieldName: 'location', }); expect(result).to.deep.equal({ ...schema, @@ -921,10 +896,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['person', 'address', 'city'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'town', - }, + update: 'renameField', + newFieldName: 'town', }); expect(result).to.deep.equal({ ...schema, @@ -970,10 +943,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['names', 'first'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'given', - }, + update: 'renameField', + newFieldName: 'given', }); expect(result).to.deep.equal({ ...schema, @@ -1014,10 +985,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['addresses', 'streetNumber'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'street_num', - }, + update: 'renameField', + newFieldName: 'street_num', }); expect(result).to.deep.equal({ ...schema, @@ -1061,10 +1030,8 @@ describe('renameField', function () { const result = updateSchema({ fieldPath: ['todos', 'completed'], jsonSchema: schema, - updateParameters: { - update: 'renameField', - newFieldName: 'done', - }, + update: 'renameField', + newFieldName: 'done', }); expect(result).to.deep.equal({ ...schema, @@ -1089,336 +1056,3 @@ describe('renameField', function () { }); }); }); - -describe('getSchemaWithNewTypes', function () { - describe('basic types', function () { - it('updates a single type', function () { - const newTypes = ['string', 'int']; - const result = getSchemaWithNewTypes({ bsonType: 'string' }, newTypes); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - - it('updates an array of types', function () { - const newTypes = ['bool', 'int']; - const result = getSchemaWithNewTypes( - { bsonType: ['string', 'bool'] }, - newTypes - ); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - }); - - describe('complex types', function () { - describe('cleans up the root schema', function () { - it('changes an object to a string', function () { - const newTypes = ['string']; - const result = getSchemaWithNewTypes( - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - newTypes - ); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - - it('changes an array to a string', function () { - const newTypes = ['string']; - const result = getSchemaWithNewTypes( - { - bsonType: 'array', - items: { - bsonType: 'int', - }, - }, - newTypes - ); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - }); - - describe('cleans up parts of anyOf', function () { - it('removes object but keeps array', function () { - const newTypes = ['array']; - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'array', - items: { - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - // array is no longer part of anyOf, now it is the only type and so the root schema - expect(result).to.deep.equal(oldSchema.anyOf[1]); - }); - - it('removes array but keeps object', function () { - const newTypes = ['object']; - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'array', - items: { - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - // object is no longer part of anyOf, now it is the only type and so the root schema - expect(result).to.deep.equal(oldSchema.anyOf[0]); - }); - - it('removes one of many types', function () { - const newTypes = ['object', 'array', 'string', 'bool']; // removes int - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'array', - items: { - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - }, - { - bsonType: 'string', - }, - { - bsonType: 'int', - }, - { - bsonType: 'bool', - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).to.not.to.have.property('bsonType'); - expect(result.anyOf).to.have.lengthOf(4); - expect(result.anyOf).to.have.deep.members([ - oldSchema.anyOf[0], - oldSchema.anyOf[1], - oldSchema.anyOf[2], - // int - is missing - oldSchema.anyOf[4], - ]); - expect(result.anyOf).to.not.have.deep.members([ - oldSchema.anyOf[3], // int - is missing - ]); - }); - }); - - describe('uses anyOf for a mixture of simple and complex types', function () { - it('adds another type on top of object and array', function () { - const newTypes = ['object', 'array', 'bool']; - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'array', - items: { - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('bsonType'); - expect(result.anyOf).to.have.lengthOf(3); - expect(result.anyOf).to.have.deep.members([ - oldSchema.anyOf[0], - oldSchema.anyOf[1], - { - bsonType: 'bool', - }, - ]); - }); - - it('adds object alongside a string', function () { - const newTypes = ['string', 'object']; - const oldSchema = { - bsonType: 'string', - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('bsonType'); - expect(result.anyOf).to.have.lengthOf(2); - expect(result.anyOf).to.deep.include({ - bsonType: 'string', - }); - expect(result.anyOf).to.deep.include({ - bsonType: 'object', - properties: {}, - required: [], - }); - }); - - it('adds array alongside a string', function () { - const newTypes = ['string', 'array']; - const oldSchema = { - bsonType: 'string', - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('bsonType'); - expect(result.anyOf).to.have.lengthOf(2); - expect(result.anyOf).to.deep.include({ - bsonType: 'string', - }); - expect(result.anyOf).to.deep.include({ - bsonType: 'array', - items: {}, - }); - }); - - it('adds string alongside an object', function () { - const newTypes = ['string', 'object']; - const oldSchema = { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('bsonType'); - expect(result).not.to.have.property('properties'); - expect(result).not.to.have.property('required'); - expect(result.anyOf).to.have.lengthOf(2); - expect(result.anyOf).to.have.deep.members([ - { - bsonType: 'string', - }, - oldSchema, - ]); - }); - - it('adds string alongside an array', function () { - const newTypes = ['string', 'array']; - const oldSchema = { - bsonType: 'array', - items: { bsonType: 'int' }, - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('bsonType'); - expect(result).not.to.have.property('items'); - expect(result.anyOf).to.have.lengthOf(2); - expect(result.anyOf).to.have.deep.members([ - { - bsonType: 'string', - }, - oldSchema, - ]); - }); - }); - - describe('cleans up anyOf when it is no longer needed', function () { - it('removes array from a mixed type', function () { - const newTypes = ['int', 'double']; - const oldSchema = { - anyOf: [ - { - bsonType: 'array', - items: [{ bsonType: 'string' }], - }, - { - bsonType: 'int', - }, - { - bsonType: 'double', - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('anyOf'); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - - it('removes object from a mixed type', function () { - const newTypes = ['int', 'bool']; - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'int', - }, - { - bsonType: 'bool', - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('anyOf'); - expect(result).to.deep.equal({ bsonType: newTypes }); - }); - - it('removes string from a mixed type, leaving object', function () { - const newTypes = ['object']; - const oldSchema = { - anyOf: [ - { - bsonType: 'object', - properties: { - name: { bsonType: 'string' }, - }, - required: ['name'], - }, - { - bsonType: 'string', - }, - ], - }; - const result = getSchemaWithNewTypes(oldSchema, newTypes); - expect(result).not.to.have.property('anyOf'); - expect(result).to.deep.equal(oldSchema.anyOf[0]); - }); - }); - }); -}); diff --git a/packages/compass-data-modeling/src/utils/schema-traversal.tsx b/packages/compass-data-modeling/src/utils/schema-traversal.tsx index ad1ea01f84b..8a8afb793eb 100644 --- a/packages/compass-data-modeling/src/utils/schema-traversal.tsx +++ b/packages/compass-data-modeling/src/utils/schema-traversal.tsx @@ -1,4 +1,4 @@ -import type { JSONSchema, MongoDBJSONSchema } from 'mongodb-schema'; +import type { MongoDBJSONSchema } from 'mongodb-schema'; import type { FieldPath } from '../services/data-model-storage'; /** @@ -96,23 +96,6 @@ function searchItemsForChild( return undefined; } -function getFieldTypes(jsonSchema: MongoDBJSONSchema): string[] { - const types: string[] = []; - if (jsonSchema.bsonType) { - if (Array.isArray(jsonSchema.bsonType)) { - types.push(...jsonSchema.bsonType); - } else { - types.push(jsonSchema.bsonType); - } - } - if (jsonSchema.anyOf) { - types.push( - ...jsonSchema.anyOf.flatMap((variant) => variant.bsonType || []) - ); - } - return types; -} - /** * Finds a single field in a MongoDB JSON schema. */ @@ -157,8 +140,21 @@ export const getFieldFromSchema = ({ // Reached the end of path, return the field information if (fieldPath.length === 1) { + const types: string[] = []; + if (nextStep.bsonType) { + if (Array.isArray(nextStep.bsonType)) { + types.push(...nextStep.bsonType); + } else { + types.push(nextStep.bsonType); + } + } + if (nextStep.anyOf) { + types.push( + ...nextStep.anyOf.flatMap((variant) => variant.bsonType || []) + ); + } return { - fieldTypes: getFieldTypes(nextStep), + fieldTypes: types, jsonSchema: nextStep, }; } @@ -171,16 +167,14 @@ export const getFieldFromSchema = ({ }; type UpdateOperationParameters = { - update: 'removeField' | 'renameField' | 'changeFieldSchema'; + update: 'removeField' | 'renameField'; newFieldName?: string; - newFieldSchema?: MongoDBJSONSchema; }; const applySchemaUpdate = ({ schema, fieldName, newFieldName, - newFieldSchema, update, }: { schema: MongoDBJSONSchema; @@ -225,21 +219,6 @@ const applySchemaUpdate = ({ } return newSchema; } - case 'changeFieldSchema': { - if (!schema.properties || !schema.properties[fieldName]) - throw new Error('Field to change type does not exist'); - if (!newFieldSchema) - throw new Error( - 'New field schema is required for the change operation' - ); - return { - ...schema, - properties: { - ...schema.properties, - [fieldName]: newFieldSchema, - }, - }; - } default: return schema; } @@ -251,12 +230,12 @@ const applySchemaUpdate = ({ export const updateSchema = ({ jsonSchema, fieldPath, - updateParameters, + update, + newFieldName, }: { jsonSchema: MongoDBJSONSchema; fieldPath: FieldPath; - updateParameters: UpdateOperationParameters; -}): MongoDBJSONSchema => { +} & UpdateOperationParameters): MongoDBJSONSchema => { const newSchema = { ...jsonSchema, }; @@ -269,7 +248,8 @@ export const updateSchema = ({ return applySchemaUpdate({ schema: newSchema, fieldName: nextInPath, - ...updateParameters, + update, + newFieldName, }); } newSchema.properties = { @@ -277,7 +257,8 @@ export const updateSchema = ({ [nextInPath]: updateSchema({ jsonSchema: newSchema.properties[nextInPath], fieldPath: remainingFieldPath, - updateParameters, + update, + newFieldName, }), }; } @@ -286,7 +267,8 @@ export const updateSchema = ({ updateSchema({ jsonSchema: variant, fieldPath: fieldPath, - updateParameters, + update, + newFieldName, }) ); } @@ -295,14 +277,16 @@ export const updateSchema = ({ newSchema.items = updateSchema({ jsonSchema: newSchema.items, fieldPath: fieldPath, - updateParameters, + update, + newFieldName, }); } else { newSchema.items = newSchema.items.map((item) => updateSchema({ jsonSchema: item, fieldPath: fieldPath, - updateParameters, + update, + newFieldName, }) ); } @@ -310,104 +294,3 @@ export const updateSchema = ({ return newSchema; }; - -const getMin1ArrayVariants = (oldSchema: JSONSchema) => { - const arrayVariants = oldSchema.anyOf?.filter( - (variant) => variant.bsonType === 'array' - ); - if (arrayVariants && arrayVariants.length > 0) { - return arrayVariants as [MongoDBJSONSchema, ...MongoDBJSONSchema[]]; - } - return [ - { - bsonType: 'array', - items: oldSchema.items || {}, - }, - ]; -}; - -const getMin1ObjectVariants = ( - oldSchema: JSONSchema -): [MongoDBJSONSchema, ...MongoDBJSONSchema[]] => { - const objectVariants = oldSchema.anyOf?.filter( - (variant) => variant.bsonType === 'object' - ); - if (objectVariants && objectVariants.length > 0) { - return objectVariants as [MongoDBJSONSchema, ...MongoDBJSONSchema[]]; - } - return [ - { - bsonType: 'object', - properties: oldSchema.properties || {}, - required: oldSchema.required || [], - }, - ]; -}; - -const getOtherVariants = ( - oldSchema: MongoDBJSONSchema, - newTypes: string[] -): MongoDBJSONSchema[] => { - const existingAnyOfVariants = - oldSchema.anyOf?.filter( - (variant) => - typeof variant.bsonType === 'string' && - variant.bsonType !== 'object' && - variant.bsonType !== 'array' && - newTypes.includes(variant.bsonType) - ) || []; - const existingAnyOfTypes = existingAnyOfVariants - .map((v) => v.bsonType) - .flat(); - const existingBasicTypes = oldSchema.bsonType - ? Array.isArray(oldSchema.bsonType) - ? oldSchema.bsonType - : [oldSchema.bsonType] - : []; - const existingBasicVariants = existingBasicTypes - .filter( - (type) => type !== 'object' && type !== 'array' && newTypes.includes(type) - ) - .map((type) => ({ bsonType: type })); - const newVariants = newTypes - .filter( - (type) => - type !== 'object' && - type !== 'array' && - !existingAnyOfTypes.includes(type) && - !existingBasicTypes.includes(type) - ) - .map((type) => ({ bsonType: type })); - return [...existingAnyOfVariants, ...existingBasicVariants, ...newVariants]; -}; - -export function getSchemaWithNewTypes( - oldSchema: MongoDBJSONSchema, - newTypes: string[] -): MongoDBJSONSchema { - const oldTypes = getFieldTypes(oldSchema); - if (oldTypes.join(',') === newTypes.join(',')) return oldSchema; - - // Simple schema - new type does includes neither object nor array - if (!newTypes.some((t) => t === 'object' || t === 'array')) { - return { bsonType: newTypes }; - } - - // Complex schema - const arrayVariants: MongoDBJSONSchema[] = newTypes.includes('array') - ? getMin1ArrayVariants(oldSchema) - : []; - const objectVariants: MongoDBJSONSchema[] = newTypes.includes('object') - ? getMin1ObjectVariants(oldSchema) - : []; - const otherVariants: MongoDBJSONSchema[] = getOtherVariants( - oldSchema, - newTypes - ); - - const newVariants = [...arrayVariants, ...objectVariants, ...otherVariants]; - if (newVariants.length === 1) { - return newVariants[0]; - } - return { anyOf: newVariants }; -} diff --git a/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json b/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json index 23a672f1c8c..b205fec74ae 100644 --- a/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json +++ b/packages/compass-data-modeling/test/fixtures/data-model-with-relationships.json @@ -204,7 +204,7 @@ "name": { "bsonType": "string" }, - "_id": { + "id": { "bsonType": "string" } }