refactor: Clean up option list typing #14429
Conversation
📝 WalkthroughWalkthroughThis pull request introduces a comprehensive refactoring of type definitions and naming conventions related to option lists across multiple frontend packages. The changes involve renaming types, updating import statements, and modifying function signatures to improve type consistency and clarity. The refactoring focuses on standardizing how option lists are handled, including their data structures, references, and utility functions, while also eliminating outdated definitions. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
github-actions
bot
added
area/ui-editor
TomasEng
added
skip-releasenotes
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
TomasEng
added
the
quality/code
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14429 +/- ##
==========================================
- Coverage 95.65% 95.65% -0.01%
==========================================
Files 1889 1889
Lines 24554 24545 -9
Branches 2817 2817
==========================================
- Hits 23488 23479 -9
Misses 805 805
Partials 261 261 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
frontend/packages/shared/src/hooks/queries/useOptionListsQuery.ts (1)
5-15: Consider adding error type parameter for better type safety.The type updates are consistent with the new type system. Consider enhancing type safety by explicitly defining the error type parameter in
UseQueryResultanduseQuery.-): UseQueryResult<OptionListsResponse> => { +): UseQueryResult<OptionListsResponse, Error> => { const { getOptionLists } = useServicesContext(); - return useQuery<OptionListsResponse>({ + return useQuery<OptionListsResponse, Error>({frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.ts (1)
1-1: Type renaming implemented consistently.The changes from
OptionListsReferencestoOptionListReferencesare applied consistently in both import and type usage.Consider renaming the function from
mapToCodeListsUsagetomapToCodeListUsageto maintain consistency with the singular form convention being established in the types.Also applies to: 5-5
frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.ts (2)
1-2: Type renaming implemented consistently.The changes from
OptionsListDatatoOptionListDataandOptionsListsResponsetoOptionListsResponseare applied consistently across imports and parameter types.To maintain complete consistency with the new singular form convention in types, consider:
-export const convertOptionsListsDataToCodeListsData = (optionListsData: OptionListsResponse) => { - const codeListsData = []; +export const convertOptionListDataToCodeListData = (optionListsData: OptionListsResponse) => { + const codeListData = []; optionListsData.map((optionListData) => { - const codeListData = convertOptionsListDataToCodeListData(optionListData); - codeListsData.push(codeListData); + const convertedData = convertOptionListItemToCodeListItem(optionListData); + codeListData.push(convertedData); }); - return codeListsData; + return codeListData; }; -const convertOptionsListDataToCodeListData = (optionListData: OptionListData) => { +const convertOptionListItemToCodeListItem = (optionListData: OptionListData) => {Also applies to: 5-5, 14-14
Line range hint
1-24: Overall type refactoring looks good with minor suggestions for complete consistency.The type renaming from plural to singular form has been implemented consistently across all files. To fully achieve the PR's objective of consistent naming conventions, consider:
- Aligning function names with the singular form convention established in types
- Updating variable names to match the new naming pattern
These changes would make the codebase more uniform and easier to maintain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (24)
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx(2 hunks)frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts(3 hunks)frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.ts(2 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.test.ts(2 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.ts(1 hunks)frontend/packages/shared/src/api/queries.ts(2 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListIdMutation.test.ts(4 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListIdMutation.ts(3 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.ts(5 hunks)frontend/packages/shared/src/hooks/queries/useOptionListQuery.test.ts(2 hunks)frontend/packages/shared/src/hooks/queries/useOptionListQuery.ts(1 hunks)frontend/packages/shared/src/hooks/queries/useOptionListsQuery.ts(1 hunks)frontend/packages/shared/src/hooks/queries/useOptionListsReferencesQuery.ts(1 hunks)frontend/packages/shared/src/mocks/queriesMock.ts(2 hunks)frontend/packages/shared/src/types/OptionList.ts(1 hunks)frontend/packages/shared/src/types/OptionListReferences.ts(1 hunks)frontend/packages/shared/src/types/api/OptionListsResponse.ts(1 hunks)frontend/packages/shared/src/types/api/OptionsLists.ts(0 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.test.tsx(4 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListLabels/OptionListLabels.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.test.ts(6 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.ts(7 hunks)frontend/packages/ux-editor/src/utils/exportUtils.test.ts(2 hunks)frontend/packages/ux-editor/src/utils/exportUtils.ts(2 hunks)
💤 Files with no reviewable changes (1)
- frontend/packages/shared/src/types/api/OptionsLists.ts
🔇 Additional comments (21)
frontend/packages/shared/src/types/api/OptionListsResponse.ts (1)
1-3: LGTM! Well-structured type definition.The placement of
OptionListsResponsein thetypes/apifolder and its naming convention align perfectly with the PR objectives. The type is focused and properly imports its dependency.frontend/packages/shared/src/types/OptionList.ts (1)
1-9: LGTM! Well-organized type definitions.The type definitions are clear, properly structured, and follow TypeScript best practices:
- Good separation of concerns between
OptionListandOptionListData- Appropriate use of optional properties
- Clear and consistent naming
frontend/packages/shared/src/types/OptionListReferences.ts (1)
1-8: LGTM! Clear and well-structured reference types.The type definitions follow good practices:
- Clear distinction between single reference and collection
- Descriptive property names
- Proper external dependency management
frontend/packages/shared/src/hooks/queries/useOptionListQuery.ts (1)
5-5: Type renaming implemented consistently.The changes from
OptionsListtoOptionListare applied consistently across import, return type, and generic type parameter. This aligns with the PR's objective of maintaining consistent naming conventions.Also applies to: 11-11, 13-13
frontend/packages/shared/src/hooks/queries/useOptionListsReferencesQuery.ts (1)
5-5: Type renaming implemented consistently.The changes from
OptionListsReferencestoOptionListReferencesare applied consistently across import, return type, and generic type parameter. This maintains the singular form convention for type names.Also applies to: 10-10, 12-12
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListLabels/OptionListLabels.tsx (1)
5-5: Type refactoring looks good!The changes align well with the PR objectives:
- Moving type definition out of the api folder
- Using consistent singular form naming convention
- Maintaining the same component functionality
Also applies to: 8-8
frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.test.ts (1)
3-3: Type refactoring looks good!The changes maintain test integrity while improving type organization:
- Moved type out of api folder as intended
- Improved type naming consistency
- Test functionality preserved
Also applies to: 13-13
frontend/packages/shared/src/hooks/queries/useOptionListQuery.test.ts (1)
7-7: Type refactoring looks good!The changes successfully:
- Relocated type definition out of api folder
- Maintained consistent singular form naming
- Preserved test functionality
Also applies to: 19-19
frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts (1)
3-3: Type refactoring looks good!The changes demonstrate proper type organization:
- Kept Response type in api folder as intended
- Improved type naming consistency
- Maintained test functionality
Also applies to: 8-8, 33-33, 45-45
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListIdMutation.ts (1)
2-2: LGTM! Type renaming is consistent throughout the file.The changes correctly implement the type renaming from
OptionsListsResponsetoOptionListsResponseacross imports and type annotations, aligning with the PR's objective of cleaning up option list typing.Also applies to: 24-24, 41-41, 42-42, 44-44
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.ts (2)
4-5: LGTM! Import statements correctly updated.The import statements have been properly updated to reflect the new type structure, maintaining clear separation between API types and domain types.
24-24: LGTM! Type annotations consistently updated across all functions.The type annotations have been systematically updated throughout the file, maintaining type safety and consistency in the mutation logic.
Also applies to: 40-40, 44-46, 56-58, 66-68, 81-83
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.ts (1)
2-2: LGTM! Consistent type updates across utility functions.The type changes from
OptionsListtoOptionListhave been applied consistently across all utility functions, maintaining the intended functionality while improving type clarity.Also applies to: 21-21, 37-37, 54-54, 90-90, 123-123, 133-134
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListIdMutation.test.ts (1)
9-9: LGTM! Test file correctly updated with new types.The test file has been properly updated to use the new
OptionListsResponsetype, ensuring test coverage remains valid with the refactored types.Also applies to: 41-41, 55-55, 68-68
frontend/packages/ux-editor/src/utils/exportUtils.ts (1)
13-13: LGTM! Type changes are consistent and well-implemented.The refactoring from
OptionsListsResponsetoOptionListData[]improves type clarity while maintaining functionality. The changes are consistently applied across the import statement, class property, and constructor parameter.Also applies to: 21-21, 31-31
frontend/app-development/features/appContentLibrary/AppContentLibrary.test.tsx (1)
14-14: LGTM! Test file updates maintain consistency with type changes.The changes correctly update the test file to use
OptionListDatatype, ensuring type consistency across the codebase. The mock data structure and test functionality remain intact.Also applies to: 22-22, 150-150
frontend/packages/ux-editor/src/utils/exportUtils.test.ts (1)
8-8: LGTM! Test type definitions updated correctly.The test file has been properly updated to use
OptionListDatatype, maintaining consistency with the implementation changes. All test cases remain functionally equivalent.Also applies to: 105-105
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.test.tsx (1)
3-3: LGTM! Consistent type updates in test implementation.The changes correctly update all occurrences of
OptionsListtoOptionList, maintaining type consistency throughout the test file. Mock implementations and test cases remain functionally equivalent.Also applies to: 20-21, 27-27, 211-212, 218-218
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/utils/optionsUtils.test.ts (1)
2-2: Type updates are consistent and complete.The renaming from
OptionsListtoOptionListhas been properly applied across all test cases while maintaining the original test functionality.Also applies to: 29-29, 70-70, 138-139, 144-145, 182-182, 215-215, 222-222, 229-229, 236-236, 244-244, 250-250
frontend/packages/shared/src/api/queries.ts (1)
94-96: Type organization improvements are well-structured.The refactoring of option list related types follows a clear pattern:
- Types are properly segregated into separate imports
- API-specific types are correctly placed in the
types/apifolder- Function signatures are consistently updated to use the new types
Also applies to: 125-127
frontend/packages/shared/src/mocks/queriesMock.ts (1)
72-73: Mock implementations correctly reflect type changes.The mock implementations have been properly updated to use the new type system:
- Import statements mirror the reorganization in queries.ts
- Mock return types are consistent with their corresponding query functions
- Empty array initializers maintain type safety
Also applies to: 76-76, 114-115, 117-118
There was a problem hiding this comment.
Fin opprydning!
Jeg ser at Option[] brukes flere steder hvor OptionList kunne vært brukt. Nå brukes de litt om hverandre. Burde vi kanskje forholde oss til OptionList der vi kan, for å holde koden konsistent?
Et annet punkt er at flere kodeelementer som ikke er typer fortsatt heter optionsList med s i midten. Kanskje det også kan fikses i denne PR-en?
...d/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.ts
Outdated
Show resolved
Hide resolved
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.ts
Outdated
Show resolved
Hide resolved
...tModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListLabels/OptionListLabels.tsx
Outdated
Show resolved
Hide resolved
|
@ErlingHauan, nå har jeg tatt en runde og oppdatert variablenavnene. Det er sikkert mer som kan gjøres, men det tenker jeg blir for mye for én PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (3)
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx (2)
Line range hint
39-52: Maintain consistent naming throughout the component.While the prop was renamed to
optionListIdin theLibraryOptionsEditor, the type definition and parameter still useoptionsId. For consistency, these should be updated as well.type OptionsListResolverProps = { handleDelete: () => void; - optionsId: string; + optionListId: string; }; function OptionListResolver({ handleDelete, - optionsId, + optionListId, }: OptionsListResolverProps): React.ReactNode {
Line range hint
33-33: Update the parent component's prop reference.For completeness, this line should also use the new prop name.
- return <OptionListResolver optionsId={component.optionsId} handleDelete={handleDelete} />; + return <OptionListResolver optionListId={component.optionsId} handleDelete={handleDelete} />;frontend/packages/ux-editor/src/utils/exportUtils.ts (1)
Line range hint
144-148: Add null check to prevent runtime errors.The code accesses
optionListData.datawithout checking if a matching option list was found. This could cause runtime errors ifcomponent.optionsIddoesn't match any option list title.const optionListData = this.optionListDataList.find( (optionListData) => optionListData.title === component.optionsId, ); + if (!optionListData) { + return undefined; + } return optionListData.data;
🧹 Nitpick comments (7)
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts (2)
16-16: Maintain consistent singular naming convention.For consistency with the new naming convention established in this PR, rename
updatedOptionsListtoupdatedOptionList.-const updatedOptionsList: Option[] = [{ ...option1, description: 'description' }]; +const updatedOptionList: Option[] = [{ ...option1, description: 'description' }];
51-51: Update hyphenated ID to follow singular naming convention.For consistency with the new naming convention, update the test ID from 'some-other-options-list-id' to 'some-other-option-list-id'.
-const existingOptionsList = { title: 'some-other-options-list-id', data: optionList }; +const existingOptionsList = { title: 'some-other-option-list-id', data: optionList };frontend/packages/ux-editor/src/utils/exportUtils.ts (3)
21-21: Consider renaming to avoid redundancy.The property name
optionListDataListis redundant since the typeOptionListData[]already indicates it's a list. Consider renaming tooptionListDatafor better clarity.- private readonly optionListDataList: OptionListData[]; + private readonly optionListData: OptionListData[];
31-31: Maintain consistent naming between parameter and property.The parameter name
optionListsDatadiffers from the property name, which could be confusing. Consider using the same name for both.- optionListsData: OptionListData[], + optionListData: OptionListData[],
40-40: Update property initialization for consistency.Update the initialization to match the suggested property and parameter naming.
- this.optionListDataList = optionListsData; + this.optionListData = optionListData;frontend/app-development/features/appContentLibrary/utils/convertOptionListDataListToCodeListDataList.ts (1)
4-6: Consider a more concise function name.While descriptive, the function name is quite long. Consider shortening to
convertToCodeListDataListsince the context is already clear from the types.frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx (1)
33-36: Consider adding error handling for the update operation.While the change handler is well-implemented, it might benefit from error handling for the update operation.
const handleOptionsListChange = (newOptionList: OptionList) => { if (hasOptionListChanged(optionList, newOptionList)) { - updateOptionList({ optionListId, optionList: newOptionList }); + updateOptionList( + { optionListId, optionList: newOptionList }, + { + onError: (error) => { + console.error('Failed to update option list:', error); + // Consider showing a user-friendly error message + }, + } + ); doReloadPreview(); } };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
frontend/app-development/features/appContentLibrary/AppContentLibrary.tsx(3 hunks)frontend/app-development/features/appContentLibrary/utils/convertOptionListDataListToCodeListDataList.ts(1 hunks)frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts(3 hunks)frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.ts(0 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.test.ts(4 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.ts(1 hunks)frontend/packages/shared/src/api/queries.ts(2 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts(5 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.ts(2 hunks)frontend/packages/shared/src/mocks/queriesMock.ts(2 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx(3 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx(1 hunks)frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListLabels/OptionListLabels.tsx(1 hunks)frontend/packages/ux-editor/src/utils/exportUtils.ts(4 hunks)
💤 Files with no reviewable changes (1)
- frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.ts
✅ Files skipped from review due to trivial changes (1)
- frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/ManualOptionsEditor/ManualOptionsEditor.tsx
🚧 Files skipped from review as they are similar to previous changes (6)
- frontend/app-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts
- frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.test.ts
- frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListLabels/OptionListLabels.tsx
- frontend/packages/shared/src/mocks/queriesMock.ts
- frontend/packages/shared/src/api/queries.ts
- frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (6)
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/OptionListEditor.tsx (1)
64-64: LGTM! Prop renaming aligns with the refactoring objectives.The change from
optionsIdtooptionListIdfollows the PR's goal of making option list related naming more consistent.frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts (1)
Line range hint
21-77: Well-structured test cases with good coverage!The test cases effectively cover various scenarios including parameter validation, cache updates for both existing and new lists, and single list updates. The implementation aligns well with the PR's objective of standardizing option list related types and naming conventions.
frontend/packages/ux-editor/src/utils/exportUtils.ts (1)
13-13: LGTM! Import changes align with refactoring goals.The new import of
OptionListDatatype follows the established naming conventions and modular organization principles outlined in the PR objectives.frontend/app-development/features/appContentLibrary/utils/convertOptionListDataListToCodeListDataList.ts (1)
8-12: LGTM! Clean and efficient implementation.The helper function is well-structured with clear property mapping.
frontend/packages/ux-editor/src/components/config/editModal/EditOptions/OptionTabs/EditTab/OptionListEditor/LibraryOptionsEditor/LibraryOptionsEditor.tsx (1)
14-14: LGTM! Consistent type usage and prop naming.The renaming from
optionsIdtooptionListIdaligns well with the PR's objective of consistent naming conventions.Also applies to: 18-18, 23-23
frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.ts (1)
4-11: LGTM! Clean and efficient implementation.The function is well-structured with:
- Proper null checking
- Clear type definitions
- Concise mapping logic
frontend/app-development/features/appContentLibrary/AppContentLibrary.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
@ErlingHauan, nå har jeg tatt en runde og oppdatert variablenavnene. Det er sikkert mer som kan gjøres, men det tenker jeg blir for mye for én PR.
Flott! Og jeg er helt enig 😊
...-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts
Outdated
Show resolved
Hide resolved
...-development/features/appContentLibrary/utils/convertOptionsListsDataToCodeListsData.test.ts
Outdated
Show resolved
Hide resolved
frontend/app-development/features/appContentLibrary/utils/mapToCodeListsUsage.ts
Outdated
Show resolved
Hide resolved
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts
Outdated
Show resolved
Hide resolved
frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts
Outdated
Show resolved
Hide resolved
…stMutation.test.ts Co-authored-by: Erling Hauan <[email protected]>
…stMutation.test.ts Co-authored-by: Erling Hauan <[email protected]>
…n/altinn-studio into cleanup-option-list-typing
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
frontend/app-development/features/appContentLibrary/utils/mapToCodeListUsages.test.ts (1)
20-35: Add more test cases for edge cases.Consider adding tests for:
- Null input
- Empty array input
- Invalid/malformed input objects (missing required properties)
it('maps null optionListUsages to empty array', () => { const codeListUsages = mapToCodeListUsages(null); expect(codeListUsages).toEqual([]); }); it('maps empty optionListUsages array to empty array', () => { const codeListUsages = mapToCodeListUsages([]); expect(codeListUsages).toEqual([]); }); it('handles malformed optionListUsages objects', () => { const malformedUsages = [{ optionListId: 'id' }] as OptionListReferences; const codeListUsages = mapToCodeListUsages(malformedUsages); expect(codeListUsages).toEqual([{ codeListId: 'id', codeListIdSources: undefined }]); });frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.test.ts (1)
Line range hint
6-49: Add test cases for null checks.Add test cases to verify the handling of:
- Null/undefined input
- Null/undefined properties in input objects
it('handles null input by returning empty array', () => { const result = mapToCodeListDataList(null); expect(result).toEqual([]); }); it('handles undefined properties in option list data', () => { const optionListDataList = [{ title: undefined, data: undefined, hasError: undefined }] as OptionListData[]; const result = mapToCodeListDataList(optionListDataList); expect(result).toEqual([{ title: '', data: null, hasError: false }]); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/app-development/features/appContentLibrary/AppContentLibrary.tsx(4 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.test.ts(3 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.ts(1 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListUsages.test.ts(1 hunks)frontend/app-development/features/appContentLibrary/utils/mapToCodeListUsages.ts(1 hunks)frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/app-development/features/appContentLibrary/AppContentLibrary.tsx
- frontend/packages/shared/src/hooks/mutations/useUpdateOptionListMutation.test.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build environment and run e2e test
- GitHub Check: Testing
🔇 Additional comments (1)
frontend/app-development/features/appContentLibrary/utils/mapToCodeListUsages.ts (1)
4-12: Clean implementation with proper null checks!The function correctly handles falsy input and maintains a clear mapping between option list and code list properties.
frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.ts
Show resolved
Hide resolved
frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.ts
Show resolved
Hide resolved
frontend/app-development/features/appContentLibrary/utils/mapToCodeListDataList.test.ts
Outdated
Show resolved
Hide resolved
Description
Renamed and moved option list related types in order to follow general guidelines and our own conventions:
types/apifolder should only containResponse,ParamsandPayloadtypestypes/apifolder should only be used by functions directly related to network requestsRelated Issue(s)
Verification
Summary by CodeRabbit
Based on the comprehensive summary of changes, here are the high-level release notes:
Type System Refinement
Code Structure Improvements
Key Changes
OptionsListwithOptionList.app-shared/types/api/OptionsListsto more specific type modules.mapToCodeListDataListandmapToCodeListUsages.Testing
These changes primarily focus on improving type safety, code readability, and maintaining consistent naming conventions throughout the application.