Skip to content

Revert "feat(compass-data-modeling): update field type COMPASS-9659 (#7248) #7271

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Revert "feat(compass-data-modeling): update field type COMPASS-9659 (#7248) #7271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
156 changes: 8 additions & 148 deletions packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand All @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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 () {
Expand Down
33 changes: 9 additions & 24 deletions packages/compass-data-modeling/src/components/drawer/field-drawer-content.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useMemo, useState } from 'react';
import React, { useMemo } from 'react';
import { connect } from 'react-redux';
import type {
FieldPath,
Expand All @@ -11,7 +11,6 @@ import {
} from '@mongodb-js/compass-components';
import { BSONType } from 'mongodb';
import {
changeFieldType,
createNewRelationship,
deleteRelationship,
getCurrentDiagramFromState,
Expand Down Expand Up @@ -57,7 +56,8 @@ type FieldDrawerContentProps = {
onChangeFieldType: (
namespace: string,
fieldPath: FieldPath,
newTypes: string[]
fromBsonType: string | string[],
toBsonType: string | string[]
) => void;
};

Expand Down Expand Up @@ -117,11 +117,6 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
onRenameField,
onChangeFieldType,
}) => {
const [fieldTypeEditErrorMessage, setFieldTypeEditErrorMessage] = useState<
string | undefined
>();
const [fieldTypes, setFieldTypes] = useState<string[]>(types);

const { value: fieldName, ...nameInputProps } = useChangeOnBlur(
fieldPath[fieldPath.length - 1],
(fieldName) => {
Expand All @@ -142,25 +137,17 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
[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 (
<>
<DMDrawerSection label="Field properties">
<DMFormFieldContainer>
<TextInput
label="Field name"
disabled={isReadOnly}
disabled={isIdField(fieldPath)}
data-testid="data-model-collection-drawer-name-input"
sizeVariant="small"
value={fieldName}
Expand All @@ -175,14 +162,12 @@ const FieldDrawerContent: React.FunctionComponent<FieldDrawerContentProps> = ({
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) => (
<ComboboxOption key={type} value={type} />
Expand Down Expand Up @@ -243,6 +228,6 @@ export default connect(
onEditRelationshipClick: selectRelationship,
onDeleteRelationshipClick: deleteRelationship,
onRenameField: renameField,
onChangeFieldType: changeFieldType,
onChangeFieldType: () => {}, // TODO(COMPASS-9659): updateFieldSchema,
}
)(FieldDrawerContent);
27 changes: 3 additions & 24 deletions packages/compass-data-modeling/src/store/apply-edit.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}),
};
}),
Expand Down Expand Up @@ -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,
}),
};
}),
Expand Down
34 changes: 1 addition & 33 deletions packages/compass-data-modeling/src/store/diagram.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

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

export function changeFieldType(
ns: string,
fieldPath: FieldPath,
newTypes: string[]
): DataModelingThunkAction<void, ApplyEditAction | ApplyEditFailedAction> {
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<DataModelCollection, 'displayPosition'>
Expand Down
Loading
Loading