-
Notifications
You must be signed in to change notification settings - Fork 74
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: replace filename validation messages #14434
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThis pull request focuses on standardizing and consolidating error messages across multiple frontend components and utility functions. The changes primarily involve updating error message keys and removing specific validation checks for names and file names. The modifications aim to simplify error handling by using a more generic Changes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/packages/ux-editor-v3/src/utils/designViewUtils/designViewUtils.ts (1)
29-29
: Consider consolidating duplicate implementations.This file appears to be a duplicate of
frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts
. While the change to usevalidation_errors.name_invalid
is correct, having duplicate implementations could lead to maintenance issues.Consider:
- Moving this shared logic to a common utility package
- Having both packages import from the common utility
- Deprecating one of the duplicate implementations if possible
frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.test.ts (1)
51-51
: Consider updating test descriptions to match new error handling.While the changes to use
validation_errors.name_invalid
are correct, the test descriptions still reference specific error types (e.g., "returns length error key", "returns format error"). Consider updating these descriptions to reflect the consolidated error handling approach.- it('returns length error key when name is too long', () => { + it('returns invalid name error when name is too long', () => { - it('returns formate error when name contains period (.)', () => { + it('returns invalid name error when name contains period (.)', () => { - it('returns format error key when name contains illegal characters', () => { + it('returns invalid name error when name contains illegal characters', () => {Also applies to: 60-60, 69-69
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
frontend/language/src/nb.json
(2 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CreateNewCodeListModal/CreateNewCodeListModal.test.tsx
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useCommonCodeListNameErrorMessages.ts
(0 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useInputCodeListNameErrorMessage.ts
(1 hunks)frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useUploadCodeListNameErrorMessage.ts
(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CreateCustomReceiptForm/CreateCustomReceiptForm.test.tsx
(1 hunks)frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceipt/CustomReceipt.test.tsx
(1 hunks)frontend/packages/shared/src/utils/layoutSetsUtils.ts
(1 hunks)frontend/packages/ux-editor-v3/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx
(2 hunks)frontend/packages/ux-editor-v3/src/utils/designViewUtils/designViewUtils.test.ts
(2 hunks)frontend/packages/ux-editor-v3/src/utils/designViewUtils/designViewUtils.ts
(1 hunks)frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.test.ts
(3 hunks)frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useCommonCodeListNameErrorMessages.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (9)
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useInputCodeListNameErrorMessage.ts (1)
7-7
: LGTM! Error message key updated correctly.The change from filename-specific validation to generic name validation aligns with the PR objectives. The new error message key is consistent with other files in the codebase.
frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.ts (1)
29-29
: LGTM! Error message key updated correctly.The change to use
validation_errors.name_invalid
aligns with the PR objectives and maintains consistency across the codebase. The validation logic appropriately relies on the shared utility function.frontend/packages/ux-editor-v3/src/utils/designViewUtils/designViewUtils.test.ts (1)
50-50
: LGTM! Changes align with PR objectives.The consolidation of error messages under
validation_errors.name_invalid
improves consistency in the UI by using a single, generic error key for name validation issues.Also applies to: 59-59
frontend/libs/studio-content-library/src/ContentLibrary/LibraryBody/pages/CodeListPage/CodeListsActionsBar/CreateNewCodeListModal/CreateNewCodeListModal.test.tsx (1)
86-86
: LGTM! Consistent error message handling.The update to use
validation_errors.name_invalid
maintains consistency with the new error handling approach across the application.frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CustomReceipt/CustomReceipt.test.tsx (1)
32-32
: LGTM! Error message standardization applied correctly.The update to use
validation_errors.name_invalid
in thelayoutSetIdTextKeys
object maintains consistency with the new error handling approach.frontend/packages/ux-editor-v3/src/containers/DesignView/PageAccordion/NavigationMenu/InputPopover/InputPopover.test.tsx (1)
158-165
: LGTM! Test assertions updated correctly.The test assertions have been properly updated to use the new generic validation message key 'validation_errors.name_invalid' instead of filename-specific validation messages, aligning with the PR's objective to standardize validation messages.
Also applies to: 176-183
frontend/packages/process-editor/src/components/ConfigPanel/ConfigEndEvent/CustomReceiptContent/CreateCustomReceiptForm/CreateCustomReceiptForm.test.tsx (1)
197-197
: LGTM! Test assertion updated correctly.The test assertion has been properly updated to use the new generic validation message key 'validation_errors.name_invalid', consistent with the PR's objective to standardize validation messages.
frontend/packages/shared/src/utils/layoutSetsUtils.ts (1)
21-21
: LGTM! Validation error key updated correctly.The validation function has been properly updated to return the new generic validation message key 'validation_errors.name_invalid', consistent with the PR's objective to standardize validation messages.
frontend/language/src/nb.json (1)
1809-1809
: LGTM! Error messages updated with clear and consistent wording.The Norwegian translations have been properly updated:
- Page uniqueness error message is now more specific and user-friendly
- Name validation error message is now generic and reusable across different contexts
The changes align well with the PR's objective to standardize validation messages.
Also applies to: 1908-1908
...src/ContentLibrary/LibraryBody/pages/CodeListPage/hooks/useUploadCodeListNameErrorMessage.ts
Show resolved
Hide resolved
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14434 +/- ##
==========================================
- Coverage 95.65% 95.65% -0.01%
==========================================
Files 1885 1884 -1
Lines 24495 24479 -16
Branches 2812 2809 -3
==========================================
- Hits 23431 23415 -16
Misses 804 804
Partials 260 260 ☔ View full report in Codecov by Sentry. |
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.
Lurt å endre fra filnavn til navn. Ett spørsmål om et begrep i koden.
frontend/packages/ux-editor-v3/src/utils/designViewUtils/designViewUtils.test.ts
Outdated
Show resolved
Hide resolved
frontend/packages/ux-editor/src/utils/designViewUtils/designViewUtils.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.
Looks good! ⭐
I have one question if we should say something about length? The regex says between 2 and 28 characters. Either add it to the general format error message or keep the length error message, but adapt it?
Description
In some validation messages, we used the term "Filnavn" when we were not dealing with files in the UI.
Update for layout
Update for layout set
Update for code list library
Related Issue(s)
Verification
Summary by CodeRabbit
Localization
Error Handling
validation_errors.name_invalid
message.User Experience