fix: store diagram with database COMPASS-9718 #7489
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds mandatory database storage to diagram data, making the database field required instead of optional. Since this is a pre-release change, fallback logic for handling diagrams without database information is removed.
- Adds
databasefield to diagram storage schema and all diagram operations - Updates diagram creation, export, and import flows to include database information
- Refactors collection naming to use stored database field instead of deriving it from namespace
- Updates tests and fixtures to include the new required database field
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-data-modeling/src/services/data-model-storage.ts | Adds required database field to MongoDBDataModelDescription schema |
| packages/compass-data-modeling/src/store/diagram.ts | Updates diagram state management to include database field and refactors collection naming logic |
| packages/compass-data-modeling/src/store/analysis-process.ts | Adds database field to analysis completion action |
| packages/compass-data-modeling/src/store/export-diagram.ts | Updates diagram export to include database parameter |
| packages/compass-data-modeling/src/services/open-and-download-diagram.ts | Updates diagram file format to include database field with validation |
| packages/compass-data-modeling/src/components/saved-diagrams-list.tsx | Simplifies database display by using stored database field instead of deriving from collections |
| packages/compass-data-modeling/src/components/diagram-card.tsx | Updates component type to remove decorated databases field |
| packages/compass-e2e-tests/tests/data-modeling-tab.test.ts | Adds test assertion for database field in exported diagram |
| Multiple test files | Updates test fixtures and assertions to include database field |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
packages/compass-data-modeling/src/components/saved-diagrams-list.tsx
Outdated
Show resolved
Hide resolved
paula-stacho
added
the
feature flagged
| * anything that would require re-fetching data associated with the diagram | ||
| */ | ||
| connectionId: z.string().nullable(), | ||
| database: z.string(), |
There was a problem hiding this comment.
Can we please add a processor here that would try to extract the value from existing model instead of breaking all existing diagrams? It's just a good practice to try to follow even if we know that we can break it right now
There was a problem hiding this comment.
Okay, I thought we could avoid the extra code as we don't have any public users yet, but you're right, sooner or later these things will come anyway.
There was a problem hiding this comment.
Added z.preprocess
…ist.tsx Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
paula-stacho
force-pushed
the
COMPASS-9718
branch
from
4a917eb to
5b76efb
Compare
Description
Storing database with diagram. I figured since we're before release, we can make this non-optional - so you'll loose your pre-existing diagrams, but we don't have to maintain any fallbacks going forward.
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes