fix(j-s): Date of Birth Validation #17083
Conversation
WalkthroughThe pull request introduces several changes across various files in the judicial system application. Key modifications include updates to the handling of nullable values for properties such as Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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
|
![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
🧹 Outside diff range and nitpick comments (17)
apps/judicial-system/backend/src/app/transformers/nationalId.transformer.ts (1)
7-13: LGTM! Consider adding JSDoc commentsThe implementation correctly handles null values while maintaining the existing functionality.
Consider adding JSDoc comments to document the behavior:
+/** + * Transforms a national ID by removing hyphens. Preserves null/undefined values. + * @param {Object} params - The input parameters + * @param {string | null} [params.value] - The national ID value to transform + * @returns {string | undefined | null} The transformed national ID, or null/undefined if no value provided + */ export const nationalIdTransformer: NationalIdTransformer = ({ value }) => {apps/judicial-system/web/src/components/Inputs/InputNationalId.tsx (3)
57-61: Convert empty string to null before calling onBlurWhile the validation logic is correct, consider converting empty strings to null before calling onBlur to maintain consistency with the PR's objective of setting empty values to null in the database.
- onBlur(inputValue) + onBlur(inputValue || null)
Line range hint
13-21: Enhance component documentation for null value handlingThe component's JSDoc comment should be updated to explicitly document how null values are handled, especially for date of birth validation, to align with the PR's objectives.
/** * A reusable input component for national ids. A national id can eiter be a SSN * or a date of birth. This component handles input validation for national ids, - * setting and removing the validation's error message. + * setting and removing the validation's error message. When the field is not required, + * empty values are treated as null. For date of birth validation, both null values + * and properly formatted dates are accepted. */
Line range hint
9-12: Strengthen TypeScript types for callback functionsConsider making the types more explicit about null value handling to improve type safety and documentation.
interface Props extends InputProps { isDateOfBirth: boolean + onBlur: (value: string | null) => void + onChange?: (value: string) => void }apps/judicial-system/backend/src/app/formatters/indictmentPdf.ts (2)
116-122: Consider improving locale-specific string handlingThe current implementation has hardcoded string replacements for prosecutor office names. This approach might be problematic for maintenance and internationalization.
Consider extracting these transformations into a configuration file or using a more robust i18n solution:
// prosecutorOfficeTransforms.ts export const PROSECUTOR_OFFICE_TRANSFORMS = { 'is-IS': { 'lögreglustjórinn': 'lögreglustjórans', 'saksóknari': 'saksóknara' } };
116-122: Add error handling for undefined prosecutor officeThe nullish coalescing operator might hide potential data validation issues. Consider adding explicit error handling.
formatMessage(indictment.signature, { prosecutorsOfficeName: - lowercase(theCase.prosecutorsOffice?.name) + theCase.prosecutorsOffice?.name + ? lowercase(theCase.prosecutorsOffice.name) .replace('lögreglustjórinn', 'lögreglustjórans') .replace('saksóknari', 'saksóknara') - ?? '', + : undefined, // Let formatMessage handle the fallback date: formatDate(nowFactory(), 'PPP'), }),apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Defendant/Defendant.tsx (1)
Line range hint
42-52: Consider adding error handling for updateDefendant operation.The
handleUpdateDefendantfunction could benefit from proper error handling to manage potential API failures and provide feedback to users.Consider updating the implementation like this:
const handleUpdateDefendant = useCallback( async (updatedDefendant: UpdateDefendantInput) => { updateDefendantState(updatedDefendant) if (updatedDefendant.defendantId) { - updateDefendant(updatedDefendant) + try { + await updateDefendant(updatedDefendant) + } catch (error) { + // Handle error appropriately (e.g., show error message to user) + console.error('Failed to update defendant:', error) + } } }, [updateDefendantState, updateDefendant], )apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx (1)
193-193: Consider enhancing type safety for nationalId handling.While the current implementation works, we could improve type safety by:
- Using a custom type for the nationalId field
- Adding runtime validation for the null case
Consider this type-safe approach:
type NationalId = string | null; interface NationalIdProps { value: NationalId; onChange: (value: NationalId) => void; onBlur: (value: NationalId) => void; } const validateNationalId = (value: string): NationalId => { return value || null; };Also applies to: 201-201
apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx (1)
Line range hint
97-107: Consider centralizing national ID null handlingTo improve maintainability and reduce code duplication, consider extracting the national ID null handling logic into a utility function.
+ const normalizeNationalId = (nationalId?: string) => nationalId || null; + // Then use it in both places: - nationalId: defendant.nationalId || null, + nationalId: normalizeNationalId(defendant.nationalId),apps/judicial-system/web/src/components/Inputs/InputAdvocate.tsx (3)
54-67: LGTM! Consider adding JSDoc comments.The new interfaces correctly implement nullable fields, aligning with the PR's objective. The structure is clean and well-typed.
Consider adding JSDoc comments to document the purpose of these interfaces:
+/** + * Interface for updating lawyer information in the system. + * All fields are nullable to handle cases where no information is provided. + */ interface LawyerUpdate { defenderName: string | null defenderNationalId: string | null defenderEmail: string | null defenderPhoneNumber: string | null } +/** + * Interface for updating spokesperson information in the system. + * All fields are nullable to handle cases where no information is provided. + */ interface SpokespersonUpdate { spokespersonName: string | null spokespersonNationalId: string | null spokespersonEmail: string | null spokespersonPhoneNumber: string | null }
Line range hint
140-173: Consider adding null checks for lawyer object propertiesWhile the implementation correctly handles the case when no lawyer is selected, there might be edge cases where the lawyer object exists but contains null values.
Consider adding null checks for the lawyer object properties:
- defenderName: lawyer ? lawyer.name : label, - defenderNationalId: lawyer ? lawyer.nationalId : null, - defenderEmail: lawyer ? lawyer.email : null, - defenderPhoneNumber: lawyer ? lawyer.phoneNr : null, + defenderName: lawyer?.name ?? label, + defenderNationalId: lawyer?.nationalId ?? null, + defenderEmail: lawyer?.email ?? null, + defenderPhoneNumber: lawyer?.phoneNr ?? null,
243-253: Consider refactoring for better maintainabilityThe current implementation correctly handles null values, but the switch statement could be simplified for better maintainability.
Consider using an object map approach:
- const formatUpdate = useCallback((property: InputType, value: string) => { - switch (property) { - case 'defenderEmail': { - return { - defenderEmail: value || null, - } - } - case 'defenderPhoneNumber': { - return { defenderPhoneNumber: value || null } - } - case 'spokespersonEmail': { - return { spokespersonEmail: value || null } - } - case 'spokespersonPhoneNumber': { - return { spokespersonPhoneNumber: value || null } - } - } - }, []) + const formatUpdate = useCallback((property: InputType, value: string) => { + const updateMap: Record<InputType, string> = { + defenderEmail: 'defenderEmail', + defenderPhoneNumber: 'defenderPhoneNumber', + spokespersonEmail: 'spokespersonEmail', + spokespersonPhoneNumber: 'spokespersonPhoneNumber' + } + return { [updateMap[property]]: value || null } + }, [])apps/judicial-system/web/src/utils/validate.ts (2)
265-267: Consider simplifying the validation logic.The current implementation can be made more concise while maintaining readability.
- const defendantsAreValid = workingCase.defendants?.every( - (defendant) => validate([[defendant.defendantPlea, ['empty']]]).isValid, - ) + const defendantsAreValid = workingCase.defendants?.every((defendant) => + validate([[defendant.defendantPlea, ['empty']]]).isValid + )
Line range hint
1-1: Address the TODO comment regarding missing tests.Given the critical nature of validation logic in a judicial system, comprehensive test coverage is essential. Would you like me to help generate test cases for these validation functions?
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx (1)
190-197: Consider adding validation for date of birth format.Since this PR focuses on date of birth validation, consider adding explicit format validation when
noNationalIdis true, as the field would then represent a date of birth.if (noNationalId) { + // Validate date format when field represents date of birth + const isValidDateFormat = /^\d{2}\.\d{2}\.\d{4}$/.test(nationalId); + if (nationalId && !isValidDateFormat) { + return; + } handleUpdateCivilClaimant({ caseId: workingCase.id, civilClaimantId, nationalId: nationalId || null, }) }apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
189-192: Consider adding JSDoc documentation and validation constraints.While the type definitions are correct, consider the following improvements:
- Add JSDoc documentation to describe the purpose and expected format of each field
- Consider adding validation constraints for
postponedIndefinitelyExplanationandcivilDemandsfieldsExample documentation:
+ /** Date and location for arraignment */ arraignmentDate?: UpdateDateLog + /** Date and location for court hearing */ courtDate?: UpdateDateLog + /** Explanation for indefinite postponement of the case */ postponedIndefinitelyExplanation?: string + /** Civil demands associated with the case */ civilDemands?: string
Line range hint
516-607: Consider refactoring to reduce code duplication between date and comment update handlers.The
handleDateUpdatesandhandleCommentUpdatesmethods share similar patterns. Consider extracting the common logic into a generic handler to improve maintainability.Example refactor:
interface UpdateHandler<T, K extends keyof UpdateCase> { model: any; types: Record<K, T>; getWhere: (caseId: string, type: T) => object; getValue: (update: any) => any; } private async handleUpdates<T, K extends keyof UpdateCase>( theCase: Case, update: UpdateCase, handler: UpdateHandler<T, K>, transaction: Transaction ) { for (const key in handler.types) { const updateValue = update[key as K]; if (updateValue !== undefined) { const type = handler.types[key as K]; const where = handler.getWhere(theCase.id, type); const existing = await handler.model.findOne({ where, transaction }); if (existing) { if (updateValue === null) { await handler.model.destroy({ where, transaction }); } else { await handler.model.update( handler.getValue(updateValue), { where, transaction } ); } } else if (updateValue !== null) { await handler.model.create( { caseId: theCase.id, ...handler.getValue(updateValue), type }, { transaction } ); } delete update[key as K]; } } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
apps/judicial-system/backend/src/app/formatters/indictmentPdf.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/case.service.ts(1 hunks)apps/judicial-system/backend/src/app/transformers/nationalId.transformer.ts(1 hunks)apps/judicial-system/web/src/components/Inputs/InputAdvocate.tsx(4 hunks)apps/judicial-system/web/src/components/Inputs/InputNationalId.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx(4 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx(2 hunks)apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx(2 hunks)apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Defendant/Defendant.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx(1 hunks)apps/judicial-system/web/src/utils/validate.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
apps/judicial-system/backend/src/app/formatters/indictmentPdf.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/transformers/nationalId.transformer.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/components/Inputs/InputAdvocate.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/components/Inputs/InputNationalId.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Defendant/Defendant.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/utils/validate.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
📓 Learnings (3)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (2)
Learnt from: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx:194-200
Timestamp: 2024-11-27T14:34:38.028Z
Learning: In `Overview.tsx`, the `defendant.subpoenas` array never contains null or undefined `subpoena` objects, so additional null checks are not needed.
Learnt from: oddsson
PR: island-is/island.is#16731
File: apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx:172-186
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `IndictmentOverview.tsx`, since the defendants data does not change, using `useMemo` to memoize the filtering logic is unnecessary.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx (1)
Learnt from: gudjong
PR: island-is/island.is#15421
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx:55-61
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The `updateCase` method in the `apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx` file has its own error handling, and additional error handling in the `initialize` function is not necessary.
apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx (1)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts:0-0
Timestamp: 2024-11-19T09:04:44.520Z
Learning: Additional documentation is not required when the code is clear and self-explanatory.
🔇 Additional comments (12)
apps/judicial-system/backend/src/app/transformers/nationalId.transformer.ts (1)
1-5: Verify the impact of breaking type changes
The type changes correctly implement null handling as per requirements. However, since this is a breaking change, we should verify all consumers of this transformer.
✅ Verification successful
Breaking type changes are safe to proceed
The type changes in the nationalId.transformer are safe because:
- All usages are in DTO classes where the nationalId fields are already marked as optional (
?) - The transformer is consistently used with
@Transform()decorator in combination with@ApiPropertyOptional - All consuming DTOs properly handle the nullable/optional nature of the fields
The changes align with the existing codebase patterns and won't cause runtime issues.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all usages of nationalIdTransformer to verify they handle null values correctly
# Search for imports and usages
rg -A 3 "nationalIdTransformer" --type typescript
Length of output: 84
Script:
#!/bin/bash
# Search for nationalIdTransformer using ripgrep without file type restriction
rg -l "nationalIdTransformer"
# Get content of files containing nationalIdTransformer
rg -A 3 "nationalIdTransformer"
Length of output: 13339
apps/judicial-system/backend/src/app/formatters/indictmentPdf.ts (1)
116-122: Verify string replacement logic
Let's verify that these string replacements cover all possible prosecutor office name formats.
✅ Verification successful
Let me analyze the search results to verify the string replacement logic for prosecutor office names.
From the search results, I can see that there are several variations of prosecutor office names in the codebase:
-
Names starting with "Lögreglustjórinn":
- Lögreglustjórinn á höfuðborgarsvæðinu
- Lögreglustjórinn á Suðurnesjum
- Lögreglustjórinn á Norðurl vest
-
Names ending with "saksóknari":
- Héraðssaksóknari
- Ríkissaksóknari
- Aðalsaksóknari
Looking at the code in indictmentPdf.ts:
.replace('lögreglustjórinn', 'lögreglustjórans')
.replace('saksóknari', 'saksóknara')The replacements are correct and handle both patterns:
- Converting "lögreglustjórinn" to "lögreglustjórans" for police offices
- Converting "saksóknari" to "saksóknara" for prosecutor offices
This is also consistent with the transformation logic used in other parts of the codebase, as seen in apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/PoliceDemands/PoliceDemands.tsx.
String replacement logic is correctly implemented
The current implementation handles all prosecutor office name formats found in the codebase. The transformations are consistent with Icelandic grammar rules for the genitive case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for prosecutor office names in the codebase to ensure all formats are handled
# Search for prosecutor office related strings
rg -i "lögreglustjóri|saksóknar" --type ts
# Search for test cases related to prosecutor office name formatting
rg -i "test.*prosecutor.*office|test.*lögreglustjóri" --type ts
Length of output: 18794
apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Defendant/Defendant.tsx (1)
110-110: LGTM! Proper handling of null nationalId.
The change correctly implements the requirement to set nationalId to null when no value is provided, aligning with the PR's objective to fix national ID handling.
Let's verify the consistent handling of nationalId across related components:
✅ Verification successful
Consistent handling of null nationalId confirmed
The code correctly handles the case where a defendant's nationalId is undefined by explicitly setting it to null, which aligns with the PR's objective. The context shows that:
- The nationalId assignment is part of a larger defendant data structure
- The code uses the nullish coalescing operator (
||) to ensure null is set when nationalId is undefined - This handling is consistent with the component's structure where other defendant properties are managed similarly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent nationalId handling in related components
# Expected: All components should handle nationalId similarly by allowing null values
# Search for nationalId assignments in defendant-related components
ast-grep --pattern 'nationalId: $_'
# Search for potential inconsistencies in nationalId handling
rg -A 3 'nationalId(?!.*null)'
Length of output: 3769
Script:
#!/bin/bash
# Let's try a different approach to find nationalId handling patterns
# Search for nationalId assignments in TypeScript/JavaScript files
rg -t ts -t js "nationalId.*=" -A 2
# Search for type definitions related to nationalId
ast-grep --pattern "nationalId?: $_"
# Search for nationalId in interfaces and types
ast-grep --pattern "interface $_ {
$$$
nationalId: $_
$$$
}"
Length of output: 101008
Script:
#!/bin/bash
# Let's search specifically in the judicial system app for nationalId handling
rg -t ts -t tsx "nationalId" apps/judicial-system/
# Search for type definitions of defendants in the judicial system
ast-grep --pattern "interface $_ {
$$$
defendants: $_
$$$
}"
# Search for defendant-related interfaces
ast-grep --pattern "interface Defendant {
$$$
nationalId: $_
$$$
}"
Length of output: 360
Script:
#!/bin/bash
# Fix the file type and path issues in the previous search
rg -t typescript -t tsx "nationalId" apps/judicial-system/web/src/
# Look for defendant type definitions
ast-grep --pattern "type Defendant = {
$$$
nationalId: $_
$$$
}"
# Search for defendant-related code in the specific file
rg -A 3 "defendants\[0\]" apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Defendant/Defendant.tsx
Length of output: 1013
apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx (1)
193-193: LGTM! Verify null handling consistency.
The change to use null instead of empty strings for missing national IDs is correct and aligns with the PR objectives. This ensures proper database state when no national ID is provided.
Let's verify the null handling consistency across the codebase:
Also applies to: 201-201
✅ Verification successful
Null handling for nationalId is consistent with the codebase
Based on the search results, the change to handle empty nationalId values as null is consistent with the codebase's patterns. The evidence shows:
- The Defendant interface in the judicial system allows for optional nationalId (marked with
?) - The codebase consistently handles cases where nationalId might be null or undefined through:
- Null checks before using the value
- Conditional rendering based on nationalId presence
- Format utilities that handle null/undefined cases
- The change aligns with the existing validation logic that checks for both empty and null cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent null handling in national ID fields
# Look for other instances where nationalId is handled to ensure consistent null handling
# Search for nationalId assignments
rg -A 2 "nationalId:?\s*[^,}]*" --type typescript
# Search for database schema definitions related to nationalId
rg -A 5 "nationalId.*:.*" --type graphql
Length of output: 161
Script:
#!/bin/bash
# Let's try with ripgrep without file type filters
# Search for nationalId assignments and related patterns
rg -i "nationalId.*[:=]" -A 2
# Search for schema or type definitions
rg -i "nationalId.*:" --glob "*.{ts,tsx,graphql,gql}"
# Search for specific handling of national ID in forms or components
rg -i "nationalId" --glob "*.tsx" -A 3
Length of output: 131140
Script:
#!/bin/bash
# Let's search specifically for nationalId handling in the DefendantInfo component and related files
rg -A 3 "nationalId.*=.*null" --glob "apps/judicial-system/web/src/**/*.{ts,tsx}"
# Look for type definitions and interfaces related to defendant info
rg -A 3 "interface.*Defendant" --glob "apps/judicial-system/**/*.{ts,tsx}"
# Search for other instances of null handling in defendant-related code
rg -A 3 "defendant.*nationalId" --glob "apps/judicial-system/**/*.{ts,tsx}"
Length of output: 44040
apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx (2)
97-97: LGTM: Proper handling of null national IDs
The change correctly implements the nullish coalescing operator to ensure that empty or undefined national IDs are properly converted to null, preventing the issue where previous defenders could retain access.
107-107: LGTM: Consistent null handling for new defendants
The change maintains consistency with the updateDefendant implementation, ensuring that new defendants are also created with proper null handling for national IDs.
apps/judicial-system/web/src/utils/validate.ts (2)
277-284: LGTM! Validation logic aligns with requirements.
The implementation correctly handles both national ID and date of birth validation, allowing for null values when noNationalId is true. This aligns with the PR objective to fix date of birth validation.
293-293: LGTM! Clean and readable code.
The simplified reference to defendantsAreValid improves code readability.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (2)
Line range hint 354-366: LGTM! Consistent handling of null national IDs during navigation.
The changes ensure that empty national IDs are properly set to null instead of empty strings, which aligns with the database requirements and prevents issues with previous defenders retaining access.
Line range hint 424-447: LGTM! Consistent handling of null national IDs during defendant creation.
The changes maintain consistency by initializing national IDs as null in both the temporary and persisted states of new defendants.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Processing/Processing.tsx (2)
78-78: LGTM! Type definition properly handles nullable national IDs.
The updated type definition aligns with the PR objectives by explicitly handling cases where national IDs may be null. This change improves type safety and better represents the actual data model.
190-190: Verify the impact of null national IDs on the database.
The changes correctly implement null handling for national IDs, but let's verify that this doesn't affect any existing queries or validations.
Also applies to: 195-195
✅ Verification successful
Null national IDs are safely handled in the codebase
Based on the validation code found in the judicial system, the nationalId field is properly validated with specific rules that account for both null and non-null cases. The validation logic explicitly handles cases where noNationalId is true by switching to date-of-birth validation instead of national ID validation. This confirms that the database impact of null national IDs has been considered in the system's validation layer.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential issues with null national ID handling in queries and validations
# Search for GraphQL queries/mutations handling nationalId
rg -l "nationalId.*null" --type ts
# Look for validation logic related to national IDs
ast-grep --pattern 'nationalId.*validate'
# Check for database schema definitions
fd "schema|model" --type ts --exec rg -l "nationalId"
Length of output: 4421
Script:
#!/bin/bash
# Let's check for GraphQL schema definitions and database models specifically
rg -l "nationalId.*type.*" --type typescript
# Check for validation logic in the judicial system
rg -l "nationalId.*validate" --type typescript apps/judicial-system/
# Look for GraphQL mutations in the judicial system
ast-grep --pattern 'mutation {
$$$
nationalId
$$$
}'
# Check database models and types in the judicial system
rg -A 5 "nationalId" apps/judicial-system/backend/src/app/modules/models/
Length of output: 415
Script:
#!/bin/bash
# Let's try with correct file type flags and paths
# Check for GraphQL schema definitions and database models
rg -l "nationalId.*type" --type ts
# Check for validation logic in the judicial system
rg -A 5 "nationalId" apps/judicial-system/web/src/utils/validate.ts
# Look for GraphQL mutations and types in the judicial system
rg -A 5 "nationalId" apps/judicial-system/web/src/types/
rg -A 5 "nationalId" apps/judicial-system/web/src/utils/mutations.ts
# Check for database models in the judicial system
rg -A 5 "nationalId" apps/judicial-system/shared/src/models/
Length of output: 2431
Datadog ReportAll test runs ✅ 43 Total Test Services: 0 Failed, 39 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
Date of Birth Validation
Hætta að geyma kennitölur með bandstriki í gagnagrunni
What
Why
Screenshots / Gifs
Screen.Recording.2024-11-28.at.19.38.53.mov
Checklist:
Summary by CodeRabbit
Release Notes
New Features
nationalIdacross various components, improving data integrity.Bug Fixes
Refactor
Documentation