feat(schema): add schema export modal, feature flag gated COMPASS-8703 #6668
Conversation
Anemy
added
feature flagged
|
This will probably become obvious once you format the json with spaces, but we'll need to add some styles so that scrolling is limited to the |
| schemaAccessor: SchemaAccessor; | ||
| signal: AbortSignal; | ||
| }): Promise<string> { | ||
| // TODO: Use the signal once we pull in the schema accessor type changes. |
There was a problem hiding this comment.
| | 'legacyJSON'; | ||
| export type ExportStatus = 'inprogress' | 'complete' | 'error'; | ||
| export type SchemaExportState = { | ||
| abortController?: AbortController; |
There was a problem hiding this comment.
Note: we have the abortController for schema analysis on the store level, this might get confusing
There was a problem hiding this comment.
Nice callout! Why do we have it on the store level? I'm thinking we update them to be aligned as both on the store or both in their reducer's states. Maybe not something that has to block this pr so we can unblock other things.
There was a problem hiding this comment.
I have a memory that this was asked by @gribnoysup and that it was how we dealt with abort controllers elsewhere, but a quick look supports neither of those, so I must've dreamt it 🙈
There was a problem hiding this comment.
You haven't dreamt it, this is not state, non-serializeable things shouldn't be in the store. In this store we pass abort controller for schema analysis through thunk already, in indexes we pass similarly non-state interval ids through thunk if you need some references in other plugins:
There was a problem hiding this comment.
Good callout, thanks y'all. Updated in #6717
| describe('with the enableExportSchema feature flag enabled', function () { | ||
| beforeEach(async function () { | ||
| // TODO(COMPASS-8819): remove web skip when defaulted true. | ||
| skipForWeb(this, "can't toggle features in compass-web"); |
There was a problem hiding this comment.
Hmmm, I think you added support for this in #6547 already so you should be able to toggle it
| ) => { | ||
| const queryBarProps = {}; | ||
| render( | ||
| <PreferencesProvider value={preferences}> |
There was a problem hiding this comment.
render method accepts default preferences and returns preferences with the rendering result, you don't need that extra setup here: https://github.com/mongodb-js/compass/blob/main/configs/testing-library-compass/README.md#render--renderwithconnections
| analysisState: AnalysisState; | ||
| errorMessage: string; | ||
| schema: Schema | null; | ||
| schemaAccessor: SchemaAccessor | null; |
There was a problem hiding this comment.
This is not state either, this is really way closer to service that should be passed through thunk argument. I linked one example for this in another place, for this one I guess it would be closer to PipelineBuilder in compass-aggregations plugin, it's just a service thats' relevant only for this plugin and we want to use in various actions
|
Are we going to follow up with store clean-up for this? |
COMPASS-8703
This needs some of the schema formatting work to be in first before we can open the pr. Keeping it in draft with a few TODOs until then. Once that's in I'll open it for reviews.
Screenshots