Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
151 changes: 99 additions & 52 deletions packages/compass-data-modeling/src/components/new-diagram-form.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
selectCollections,
selectConnection,
selectDatabase,
toggleInferRelationships,
} from '../store/generate-diagram-wizard';
import {
Banner,
Expand All @@ -35,7 +36,9 @@ import {
SearchInput,
Combobox,
ComboboxOption,
Checkbox,
} from '@mongodb-js/compass-components';
import { usePreference } from 'compass-preferences-model/provider';

const footerStyles = css({
flexDirection: 'row',
Expand All @@ -46,12 +49,6 @@ const footerTextStyles = css({ marginRight: 'auto' });

const footerActionsStyles = css({ display: 'flex', gap: spacing[200] });

const formContainerStyles = css({
display: 'flex',
flexDirection: 'column',
gap: spacing[400],
});

const FormStepContainer: React.FunctionComponent<{
title: string;
description?: string;
Expand Down Expand Up @@ -112,20 +109,27 @@ const FormStepContainer: React.FunctionComponent<{
);
};

const SelectListStyles = css({
height: 300,
const selectListStyles = css({
maxHeight: 200,
overflow: 'scroll',
});

function SelectCollectionsStep({
collections,
selectedCollections,
automaticallyInferRelationships,
onCollectionsSelect,
onAutomaticallyInferRelationshipsToggle,
}: {
collections: string[];
selectedCollections: string[];
automaticallyInferRelationships: boolean;
onCollectionsSelect: (colls: string[]) => void;
onAutomaticallyInferRelationshipsToggle: (newVal: boolean) => void;
}) {
const showAutoInferOption = usePreference(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basic Compass FF question, can this be removed now that the FF is enabled?

Copy link
Collaborator

@addaleax addaleax Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes – but see Sergey's answer below for why we may want to keep it for a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this one really need to stay for that long, I really wanted it only for the initial algo merging while there was no form checkbox, I'll clean this up in a week or so.

'enableAutomaticRelationshipInference'
);
const [searchTerm, setSearchTerm] = useState('');
const filteredCollections = useMemo(() => {
try {
Expand All @@ -136,50 +140,80 @@ function SelectCollectionsStep({
}
}, [collections, searchTerm]);
return (
<FormFieldContainer className={formContainerStyles}>
<SearchInput
aria-label="Search collections"
value={searchTerm}
data-testid="new-diagram-search-collections"
onChange={(e) => {
setSearchTerm(e.target.value);
}}
/>
<SelectList
className={SelectListStyles}
items={filteredCollections.map((collName) => {
return {
id: collName,
selected: selectedCollections.includes(collName),
'data-testid': `new-diagram-collection-checkbox-${collName}`,
};
})}
label={{ displayLabelKey: 'id', name: 'Collection Name' }}
onChange={(items: { id: string; selected: boolean }[]) => {
// When a user is searching, less collections are shown to the user
// and we need to keep existing selected collections selected.
const currentSelectedItems = selectedCollections.filter(
(collName) => {
const item = items.find((x) => x.id === collName);
// The already selected item was not shown to the user (using search),
// and we have to keep it selected.
return item ? item.selected : true;
}
);
<>
<FormFieldContainer>
<SearchInput
aria-label="Search collections"
value={searchTerm}
data-testid="new-diagram-search-collections"
onChange={(e) => {
setSearchTerm(e.target.value);
}}
/>
</FormFieldContainer>
<FormFieldContainer>
<SelectList
className={selectListStyles}
items={filteredCollections.map((collName) => {
return {
id: collName,
selected: selectedCollections.includes(collName),
'data-testid': `new-diagram-collection-checkbox-${collName}`,
};
})}
label={{ displayLabelKey: 'id', name: 'Collection Name' }}
onChange={(items) => {
// When a user is searching, less collections are shown to the user
// and we need to keep existing selected collections selected.
const currentSelectedItems = selectedCollections.filter(
(collName) => {
const item = items.find((x) => x.id === collName);
// The already selected item was not shown to the user (using search),
// and we have to keep it selected.
return item ? item.selected : true;
}
);

const newSelectedItems = items
.filter((item) => {
return item.selected;
})
.map((item) => {
return item.id;
});
onCollectionsSelect(
Array.from(new Set([...newSelectedItems, ...currentSelectedItems]))
);
}}
></SelectList>
</FormFieldContainer>
const newSelectedItems = items
.filter((item) => {
return item.selected;
})
.map((item) => {
return item.id;
});
onCollectionsSelect(
Array.from(
new Set([...newSelectedItems, ...currentSelectedItems])
)
);
}}
></SelectList>
</FormFieldContainer>
{showAutoInferOption && (
<FormFieldContainer>
<Checkbox
checked={automaticallyInferRelationships}
onChange={(evt) => {
onAutomaticallyInferRelationshipsToggle(
evt.currentTarget.checked
);
}}
label="Automatically infer relationships"
// @ts-expect-error Element is accepted, but not typed correctly
description={
<>
Compass will try to automatically discover relationships in
selected collections. This operation will run multiple find
requests against indexed fields of the collections and{' '}
<strong>
will take additional time per collection being analyzed.
</strong>
</>
}
></Checkbox>
</FormFieldContainer>
)}
</>
);
}

Expand All @@ -199,6 +233,7 @@ type NewDiagramFormProps = {
selectedCollections: string[];
error: Error | null;
analysisInProgress: boolean;
automaticallyInferRelationships: boolean;

onCancel: () => void;
onNameChange: (name: string) => void;
Expand All @@ -212,6 +247,7 @@ type NewDiagramFormProps = {
onDatabaseConfirmSelection: () => void;
onCollectionsSelect: (collections: string[]) => void;
onCollectionsSelectionConfirm: () => void;
onAutomaticallyInferRelationshipsToggle: (newVal: boolean) => void;
};

const NewDiagramForm: React.FunctionComponent<NewDiagramFormProps> = ({
Expand All @@ -226,6 +262,7 @@ const NewDiagramForm: React.FunctionComponent<NewDiagramFormProps> = ({
selectedCollections,
error,
analysisInProgress,
automaticallyInferRelationships,
onCancel,
onNameChange,
onNameConfirm,
Expand All @@ -238,6 +275,7 @@ const NewDiagramForm: React.FunctionComponent<NewDiagramFormProps> = ({
onDatabaseConfirmSelection,
onCollectionsSelect,
onCollectionsSelectionConfirm,
onAutomaticallyInferRelationshipsToggle,
}) => {
const connections = useConnectionsList();
const [activeConnections, otherConnections] = useMemo(() => {
Expand Down Expand Up @@ -349,7 +387,7 @@ const NewDiagramForm: React.FunctionComponent<NewDiagramFormProps> = ({
);
case 'select-connection':
return (
<FormFieldContainer className={formContainerStyles}>
<FormFieldContainer>
<Combobox
label=""
aria-label="Select connection"
Expand Down Expand Up @@ -419,16 +457,22 @@ const NewDiagramForm: React.FunctionComponent<NewDiagramFormProps> = ({
collections={collections}
onCollectionsSelect={onCollectionsSelect}
selectedCollections={selectedCollections}
automaticallyInferRelationships={automaticallyInferRelationships}
onAutomaticallyInferRelationshipsToggle={
onAutomaticallyInferRelationshipsToggle
}
/>
);
}
}, [
activeConnections,
automaticallyInferRelationships,
collections,
connections.length,
currentStep,
databases,
diagramName,
onAutomaticallyInferRelationshipsToggle,
onCollectionsSelect,
onConnectionSelect,
onDatabaseSelect,
Expand Down Expand Up @@ -516,6 +560,8 @@ export default connect(
error,
analysisInProgress:
state.analysisProgress.analysisProcessStatus === 'in-progress',
automaticallyInferRelationships:
state.generateDiagramWizard.automaticallyInferRelations,
};
},
{
Expand All @@ -531,5 +577,6 @@ export default connect(
onDatabaseConfirmSelection: confirmSelectDatabase,
onCollectionsSelect: selectCollections,
onCollectionsSelectionConfirm: confirmSelectedCollections,
onAutomaticallyInferRelationshipsToggle: toggleInferRelationships,
}
)(NewDiagramForm);
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ export type SelectCollectionsAction = {

export type ToggleInferRelationsAction = {
type: GenerateDiagramWizardActionTypes.TOGGLE_INFER_RELATIONS;
newVal: boolean;
};

export type ConfirmSelectedCollectionsAction = {
Expand Down Expand Up @@ -330,6 +331,14 @@ export const generateDiagramWizardReducer: Reducer<
step: 'LOADING_COLLECTIONS',
};
}
if (
isAction(action, GenerateDiagramWizardActionTypes.TOGGLE_INFER_RELATIONS)
) {
return {
...state,
automaticallyInferRelations: action.newVal,
};
}
return state;
};

Expand Down Expand Up @@ -546,3 +555,12 @@ export function cancelSelectedConnection(): CancelSelectedConnectionAction {
export function cancelSelectedDatabase(): CancelSelectedDatabaseAction {
return { type: GenerateDiagramWizardActionTypes.CANCEL_SELECTED_DATABASE };
}

export function toggleInferRelationships(
newVal: boolean
): ToggleInferRelationsAction {
return {
type: GenerateDiagramWizardActionTypes.TOGGLE_INFER_RELATIONS,
newVal,
};
}
2 changes: 1 addition & 1 deletion packages/compass-preferences-model/src/feature-flags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export const featureFlags: Required<{
},

enableAutomaticRelationshipInference: {
stage: 'development',
stage: 'released',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we remove FF's once they've been fully released?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally keep them around as no-ops since there's no harm in doing so and Compass would otherwise reject usage of the feature flag. Not a huge deal either way, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sooo... we should be cleaning them up over time, but practically we always forget to schedule this work, so a lot of very old flags are hanging out in the codebase (tell me if this sounds familiar @johnjackweir 😄) and as Anna correctly noticed there's not really much harm in that and so very little pressure for cleaning up

The main reason we have a dedicated released state is to be able to easy roll back in the first few weeks of the feature being available by just flipping the value instead of, for example, reverting the patches that would remove the flag from the code completely. But after this "trial" period we should be cleaning those up, in theory

description: {
short:
'Enable automatic relationship inference during data model generation',
Expand Down
Loading