-
Notifications
You must be signed in to change notification settings - Fork 75
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
fix: Validation when creating a new data model #14049
fix: Validation when creating a new data model #14049
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14049 +/- ##
==========================================
+ Coverage 95.46% 95.50% +0.04%
==========================================
Files 1806 1811 +5
Lines 23553 23579 +26
Branches 2731 2733 +2
==========================================
+ Hits 22485 22520 +35
+ Misses 813 805 -8
+ Partials 255 254 -1 ☔ View full report in Codecov by Sentry. |
...p-development/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.tsx
Outdated
Show resolved
Hide resolved
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.
Nice work! ⭐
Some feedback only, mainly regarding the tests! 🤗
I also see that we have som inconsistency regarding the text key schema_editor
. Looks like it is used a bit all over the solution 🤔 But that will be a different issue to solve!
...elopment/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.test.tsx
Outdated
Show resolved
Hide resolved
...elopment/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.test.tsx
Outdated
Show resolved
Hide resolved
...elopment/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.test.tsx
Outdated
Show resolved
Hide resolved
...p-development/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.tsx
Outdated
Show resolved
Hide resolved
...p-development/features/dataModelling/SchemaEditorWithToolbar/TopToolbar/CreateNewWrapper.tsx
Outdated
Show resolved
Hide resolved
frontend/app-development/features/dataModelling/hooks/useValidateSchemaName.test.ts
Outdated
Show resolved
Hide resolved
frontend/app-development/features/dataModelling/hooks/useValidateSchemaName.test.ts
Outdated
Show resolved
Hide resolved
frontend/app-development/features/dataModelling/hooks/useValidateSchemaName.ts
Outdated
Show resolved
Hide resolved
...yout/PageHeader/SubHeader/SettingsModalButton/SettingsModal/mocks/applicationMetadataMock.ts
Outdated
Show resolved
Hide resolved
frontend/app-development/features/dataModelling/utils/validationUtils.ts
Outdated
Show resolved
Hide resolved
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.
Will approve this now! Great improvements here! ⭐
I only have one small comment in the file structure. I see that you moved XSDUpload to the same level as TopToolBar, but actually it makes more sense having it in its own folder since the ideal structure would be if all the components used in TopToolBar
was placed in its own folders CreateNewWrapper
, XSDUpload
, SchemaSelect
, DeleteWrapper
and GenerateModelsButton
😄
Thanks 😄 I see you suggested putting |
Go for it 💯 |
The backend does not support Norwegian characters
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.
Nice ⭐ Bare et lite forslag til tekst og test 🤗
frontend/packages/shared/src/hooks/useValidateSchemaName.test.ts
Outdated
Show resolved
Hide resolved
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.
Perfect 🥳
Description
.
as valid characters, as the backend does not allow themScreenshot
Solves issues
Verification