fix(citizenship): error indexing child #17106
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17106 +/- ##
==========================================
- Coverage 35.72% 35.71% -0.01%
==========================================
Files 6920 6920
Lines 147485 147498 +13
Branches 41987 42001 +14
==========================================
Hits 52686 52686
- Misses 94799 94812 +13
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
WalkthroughThe changes involve modifications across several components related to the handling of child information within the citizenship application process. Key updates include enhancements to the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (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
Documentation and Community
|
Datadog ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (1)
Line range hint
53-65: Consider enhancing type safety and reusability.While the function handles both lookup by nationalId and index, it could benefit from:
- A more explicit return type instead of using undefined
- A custom type guard for better type narrowing
Consider this enhancement:
type CustodyChildResult = { child: ApplicantChildCustodyInformation | null; isSelected: boolean; }; function getSelectedCustodyChild( externalData: ExternalData, answers: FormValue, index: number, childNationalId?: string, ): CustodyChildResult { const custodyChildren = getValueViaPath( externalData, 'childrenCustodyInformation.data', [], ) as ApplicantChildCustodyInformation[]; const childInfo = childNationalId ? custodyChildren.find((x) => x.nationalId === childNationalId) : (index < custodyChildren.length ? custodyChildren[index] : null); const selectedChildren = (answers as Citizenship).selectedChildren; const isSelected = selectedChildren?.includes(childNationalId ?? childInfo?.nationalId ?? ''); return { child: childInfo ?? null, isSelected, }; }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (2)
53-54: LGTM! Improved array initialization.
The explicit empty array initialization ensures type safety and prevents undefined access issues.
Line range hint 53-65: Verify function usage across the codebase.
Let's ensure all consumers of this function handle both the index and nationalId cases correctly.
✅ Verification successful
Function usage is consistent with both index and nationalId parameters
Based on the verification results, all consumers of getSelectedCustodyChild are correctly handling both the index and nationalId cases:
- In
selectedIndividual.ts, the function is called withsectionIndexparameter, which aligns with the index-based lookup - In
SelectedChildrenExtra/index.tsx, it's called with bothindexandchildparameters, supporting both lookup methods - No test files were found that need updates
- The function is used consistently across all its implementations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for all usages of getSelectedCustodyChild
rg "getSelectedCustodyChild" --type typescript -B 2 -A 2
# Look for potential test files
fd "childrenInfo.spec.ts|childrenInfo.test.ts"
Length of output: 140
Script:
#!/bin/bash
# Search for all usages of getSelectedCustodyChild using ripgrep without file type filter
rg "getSelectedCustodyChild" -B 2 -A 2
# Look for test files using a more generic pattern
fd "childrenInfo" | grep -E "\.(spec|test)\.(ts|tsx|js|jsx)$"
# Find all TypeScript files that might reference this function
fd "\.(ts|tsx)$" | xargs rg "getSelectedCustodyChild"
Length of output: 11796
Script:
#!/bin/bash
# Let's examine the implementation of the functions that use getSelectedCustodyChild
ast-grep --pattern 'const selectedChild = getSelectedCustodyChild($$$)'
# Let's also check the implementation of the component that uses it
ast-grep --pattern 'const childCustodyData = getSelectedCustodyChild($$$)'
# And check how the index parameter is passed in these calls
rg "getSelectedCustodyChild.*index" -B 2 -A 2
Length of output: 2824
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (6)
libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/SupportingDocumentsSection/PassportSubSection.ts (1)
Line range hint
1-190: Consider enhancing type safetyWhile the code follows TypeScript patterns, there are opportunities to improve type safety:
- The form field builder functions could benefit from stricter typing for their configuration objects
- The external data paths could be typed to prevent runtime errors
Consider creating a type-safe accessor for external data paths:
type ExternalDataPaths = { 'individual.data': NationalRegistryIndividual; 'applicantInformation.data.currentPassportItem': TravelDocumentViewModel; 'travelDocumentTypes.data': OptionSetItem[]; 'countries.data': OptionSetItem[]; }; function getTypedValueViaPath<T extends keyof ExternalDataPaths>( data: unknown, path: T, defaultValue: ExternalDataPaths[T] ): ExternalDataPaths[T] { return getValueViaPath(data, path, defaultValue) as ExternalDataPaths[T]; }libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (1)
63-64: Improve code readability with optional chainingThe current implementation can be simplified using optional chaining.
Apply this diff to improve readability:
- selectedChildren && - selectedChildren.find((sc) => sc === childInfo?.nationalId) + selectedChildren?.find((sc) => sc === childInfo?.nationalId)🧰 Tools
🪛 Biome (1.9.4)
[error] 63-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenPassportSubSection.ts (1)
51-61: LGTM! Consider adding error handling documentationThe switch to
buildHiddenInputand use ofgetSelectedCustodyChildimproves the robustness of nationalId handling. The implementation correctly handles potential undefined values through the optional chaining operator.Consider adding a code comment explaining the error handling behavior when no custody child is found, to help future maintainers understand the expected
undefinedreturn value.libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/DocumentReview.tsx (1)
Line range hint
1-150: Consider refactoring document rendering logicThe component has multiple nested conditional renderings and repeated patterns for handling different document types. This could be simplified to improve maintainability.
Consider:
- Creating a reusable document rendering component
- Extracting document type mapping logic
- Using a configuration object for document types
Example approach:
interface DocumentConfig { key: keyof Citizenship['supportingDocuments']; getFiles: (doc: any) => Array<{name: string}>; } const documentConfigs: DocumentConfig[] = [ { key: 'birthCertificate', getFiles: (doc) => doc?.birthCertificate || [], }, // ... other document types ]; // Reusable component const DocumentList: FC<{documents: DocumentConfig[], data: any}> = ({ documents, data, }) => ( <> {documents.map(({key, getFiles}) => getFiles(data).map(file => <Text>{file.name}</Text>) )} </> );🧰 Tools
🪛 Biome (1.9.4)
[error] 113-113: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
libs/application/template-api-modules/src/lib/modules/templates/directorate-of-immigration/citizenship/citizenship.service.ts (2)
286-286: Consider using Array.prototype.filter with type predicate.The current filtering can be improved for better type inference.
- const nonNullPassports = answers.childrenPassport?.filter((x) => !!x) + const nonNullPassports = answers.childrenPassport?.filter((x): x is NonNullable<typeof x> => !!x)
396-403: Improve null handling in passport mapping.While the null coalescing operators provide safety, the code can be more concise using object destructuring with defaults.
- nonNullPassports?.map(async (p) => ({ - nationalId: p?.nationalId ?? '', - dateOfIssue: new Date(p?.publishDate ?? ''), - dateOfExpiry: new Date(p?.expirationDate ?? ''), - passportNumber: p?.passportNumber ?? '', - passportTypeId: parseInt(p?.passportTypeId ?? ''), - countryIdOfIssuer: p?.countryOfIssuerId ?? '', - file: await this.getUrlForAttachment(application, p?.attachment), + nonNullPassports?.map(async ({ + nationalId = '', + publishDate = '', + expirationDate = '', + passportNumber = '', + passportTypeId = '', + countryOfIssuerId = '', + attachment + }) => ({ + nationalId, + dateOfIssue: new Date(publishDate), + dateOfExpiry: new Date(expirationDate), + passportNumber, + passportTypeId: parseInt(passportTypeId), + countryIdOfIssuer: countryOfIssuerId, + file: await this.getUrlForAttachment(application, attachment), })) || [],
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
libs/application/template-api-modules/src/lib/modules/templates/directorate-of-immigration/citizenship/citizenship.service.ts(2 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/ChildrenPassportReview.tsx(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/DocumentReview.tsx(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/fields/SelectChildren/index.tsx(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenOtherDocumentsSubSection.ts(2 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenPassportSubSection.ts(2 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/SupportingDocumentsSection/PassportSubSection.ts(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/lib/dataSchema.ts(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts(1 hunks)libs/application/templates/directorate-of-immigration/citizenship/src/utils/selectedIndividual.ts(1 hunks)
👮 Files not reviewed due to content moderation or server errors (1)
- libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenOtherDocumentsSubSection.ts
🧰 Additional context used
📓 Path-based instructions (10)
libs/application/templates/directorate-of-immigration/citizenship/src/lib/dataSchema.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/SupportingDocumentsSection/PassportSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/ChildrenPassportReview.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/DocumentReview.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenOtherDocumentsSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/fields/SelectChildren/index.tsx (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/utils/selectedIndividual.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/template-api-modules/src/lib/modules/templates/directorate-of-immigration/citizenship/citizenship.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/forms/CitizenshipForm/ChildrenSupportingDocuments/ChildrenPassportSubSection.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🪛 Biome (1.9.4)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts
[error] 63-64: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
libs/application/templates/directorate-of-immigration/citizenship/src/utils/childrenInfo.ts (2)
53-54: LGTM! Improved array initialization
The default empty array initialization prevents undefined errors and improves code robustness.
58-60: LGTM! Added array bounds checking
Proper array bounds checking prevents potential undefined behavior when accessing array elements.
libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/ChildrenPassportReview.tsx (1)
43-43: LGTM! Added safe property access
The addition of optional chaining for accessing nationalId prevents potential runtime errors and aligns with best practices for TypeScript/JavaScript property access.
libs/application/templates/directorate-of-immigration/citizenship/src/fields/Review/DocumentReview.tsx (1)
112-112: LGTM! Null safety improvement
The addition of optional chaining operator prevents potential runtime errors when handling passport attachments.
libs/application/templates/directorate-of-immigration/citizenship/src/lib/dataSchema.ts (1)
350-350: LGTM: Schema change aligns with service implementation.
The modification to allow nullable elements in the childrenPassport array improves type safety and matches the filtering logic in the service.
...ation/citizenship/src/forms/CitizenshipForm/SupportingDocumentsSection/PassportSubSection.ts
Show resolved
Hide resolved
...application/templates/directorate-of-immigration/citizenship/src/utils/selectedIndividual.ts
Outdated
Show resolved
Hide resolved
...ication/templates/directorate-of-immigration/citizenship/src/fields/SelectChildren/index.tsx
Outdated
Show resolved
Hide resolved
…sland-is/island.is into fix/citizenship-child-indexing
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit