feat(data-modeling): new diagram creation flow COMPASS-10050 #7666
Conversation
| @@ -0,0 +1,410 @@ | |||
| import React from 'react'; | |||
There was a problem hiding this comment.
Most of the code in this file is from new-diagram-form.spec.tsx, moved it to this folder
| // 1. Import the components we use from leafygreen. | ||
| import { Copyable } from '@leafygreen-ui/copyable'; | ||
| import { Badge } from '@leafygreen-ui/badge'; | ||
| import { default as Banner } from '@leafygreen-ui/banner'; |
There was a problem hiding this comment.
LG showed deprecated for default import and I changed it to named one.
| if (isFetchingCollections) { | ||
| return ( | ||
| <div className={loadingStyles}> | ||
| <SpinLoaderWithLabel progressText=""> | ||
| Fetching collections ... | ||
| </SpinLoaderWithLabel> | ||
| </div> | ||
| ); | ||
| } | ||
|
|
||
| if (error) { | ||
| return ( | ||
| <div className={errorStyles}> | ||
| <WarningSummary warnings={[error.message]} /> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
All of the code in this file was already existent in new-diagram-form except for these two views.
…PASS-10050-new-diagram-creation-flow
There was a problem hiding this comment.
Pull request overview
This PR refactors the diagram creation flow in the data modeling feature by consolidating multi-step wizard interactions into a more streamlined two-step process. The changes simplify state management by introducing a form-fields-based approach and remove redundant intermediate "loading" states.
Key changes:
- Replaced the old multi-step wizard (name → connection → database → collections) with a consolidated two-step flow (setup diagram → select collections)
- Refactored state management to use a
formFieldsstructure with inline error/loading states instead of separate top-level step states - Split the monolithic
new-diagram-form.tsxcomponent into separate modular components for better maintainability
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
packages/compass-e2e-tests/tests/data-modeling-tab.test.ts |
Removed redundant confirm button clicks from the e2e test setup |
packages/compass-data-modeling/test/setup-store.tsx |
Updated test storage service from noop to in-memory implementation |
packages/compass-data-modeling/src/store/generate-diagram-wizard.ts |
Refactored wizard state machine with consolidated steps and form-field-based state management |
packages/compass-data-modeling/src/components/new-diagram/setup-diagram-step.tsx |
New component for the consolidated setup step (connection, database, diagram name) |
packages/compass-data-modeling/src/components/new-diagram/select-collections-step.tsx |
Extracted collections selection into a separate component |
packages/compass-data-modeling/src/components/new-diagram/new-diagram-modal.tsx |
New modal wrapper managing the two-step wizard flow |
packages/compass-data-modeling/src/components/new-diagram/new-diagram-modal.spec.tsx |
Comprehensive tests for the new modal component |
packages/compass-data-modeling/src/components/new-diagram-form.tsx |
Removed legacy monolithic component |
packages/compass-data-modeling/src/components/new-diagram-form.spec.tsx |
Removed legacy test file |
packages/compass-data-modeling/src/components/data-modeling.tsx |
Updated import path for the new modal component |
packages/compass-components/src/components/leafygreen.tsx |
Changed Banner import from default to named export |
| const newDiagramName = [ | ||
| database, | ||
| date.getDate(), | ||
| date.getMonth() + 1, | ||
| date.getFullYear(), | ||
| ].join('_'); |
There was a problem hiding this comment.
The date components (day and month) are not zero-padded, which could result in inconsistent diagram names like database_1_5_2024 vs database_15_12_2024. Consider padding to ensure consistent naming: date.getDate().toString().padStart(2, '0') and (date.getMonth() + 1).toString().padStart(2, '0').
| const newDiagramName = [ | |
| database, | |
| date.getDate(), | |
| date.getMonth() + 1, | |
| date.getFullYear(), | |
| ].join('_'); | |
| const day = date.getDate().toString().padStart(2, '0'); | |
| const month = (date.getMonth() + 1).toString().padStart(2, '0'); | |
| const year = date.getFullYear(); | |
| const newDiagramName = [database, day, month, year].join('_'); |
| database, | ||
| }); | ||
|
|
||
| // If the current diagram name is empty, we want to auto-generate the it |
There was a problem hiding this comment.
Corrected spelling of 'the it' to 'it'
| // If the current diagram name is empty, we want to auto-generate the it | |
| // If the current diagram name is empty, we want to auto-generate it |
In draft while working on e2e tests.
Preview
New.DM.Diagram.Creation.mov
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes