-
Notifications
You must be signed in to change notification settings - Fork 229
feat(data-modeling): add remove collection to diagram drawer COMPASS-9658 #7186
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds the functionality to remove collections from data modeling diagrams via the sidebar drawer. It also includes a defensive fix to handle cases where the selected item in the diagram no longer exists, preventing errors and gracefully closing the drawer instead.
- Implements collection deletion feature in the diagram editor sidebar
- Adds defensive handling for non-existent selected items to prevent throwing errors
- Updates e2e tests to cover collection management operations
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
packages/compass-e2e-tests/tests/data-modeling-tab.test.ts | Adds new test for collection management and updates existing test selectors |
packages/compass-e2e-tests/helpers/selectors.ts | Updates selectors for collection name input and adds delete button selectors |
packages/compass-data-modeling/src/store/diagram.ts | Implements deleteCollection action and RemoveCollection edit handling |
packages/compass-data-modeling/src/services/data-model-storage.ts | Adds RemoveCollection schema validation |
packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.tsx | Adds delete collection functionality and defensive handling for missing items |
packages/compass-data-modeling/src/components/drawer/diagram-editor-side-panel.spec.tsx | Adds unit test for collection deletion |
packages/compass-data-modeling/src/components/drawer/collection-drawer-content.tsx | Adds data-testid for the name input field |
Comments suppressed due to low confidence (1)
packages/compass-data-modeling/src/store/diagram.ts:272
- The RemoveRelationship case has been completely removed from updateSelectedItemsFromAppliedEdit, but this logic was likely needed to clear selection when a relationship is deleted. This could leave a dangling selection state.
case 'RenameCollection': {
}); | ||
|
||
userEvent.click( | ||
screen.getByTestId('data-modeling-drawer-actions-delete-action') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can keep this, but I would really suggest to get accustomed to use getByRole / getByLabelText / similar methods for that. This is just generally a test that closer to user experience and also a bit more stable as LG team sometimes messes up the html element prop propagation (and I think speifically for testid had some plans to replace them eventually with their own convention)
COMPASS-9658
Also a drive by updating how we handle when there's a selection on the diagram that doesn't exist. Previously we would throw, now we have it close the drawer by having no selection. Discussion on that in https://mongodb.slack.com/archives/C08P2P6ADJL/p1754762241343459
remove.collection.mp4