feat(j-s): Allow multiple subtypes in indictments #17192
Conversation
…-indictment-subtype
…island.is into j-s/select-indictment-subtype
…-indictment-subtype
…-indictment-subtype
…-indictment-subtype
…island.is into j-s/select-indictment-subtype
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThis pull request includes updates to various components and services within the judicial system application. Key changes involve modifying import statements for utility functions, correcting an Icelandic term in the 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
|
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
|
oddsson
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (20)
apps/judicial-system/web/src/routes/CourtOfAppeal/AppealCase/AppealCase.tsx (1)
Line range hint
1-250: Component implementation follows NextJS best practices.The component demonstrates strong adherence to NextJS and TypeScript best practices:
- Proper type safety with TypeScript interfaces and type guards
- Efficient state management using React hooks
- Clean separation of concerns with modular components
- GraphQL integration with proper error handling
Consider implementing the following improvements:
- Extract the judge selection logic into a custom hook for better reusability
- Add error boundaries to handle GraphQL query failures gracefully
- Consider implementing optimistic updates for better UX during judge assignments
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.strings.ts (2)
4-10: Consider adding TypeScript type safety for message parametersThe implementation looks good, but we could improve type safety by defining an enum or union type for the
substanceTypeparameter.Add this type definition:
export type SubstanceType = 'ILLEGAL_DRUGS_DRIVING' | 'PRESCRIPTION_DRUGS_DRIVING' | string;Then update the message definition:
substanceTitle: { id: 'judicial.system.core:substances.title', defaultMessage: '{substanceType, select, ILLEGAL_DRUGS_DRIVING {Fíkniefni} PRESCRIPTION_DRUGS_DRIVING {Lyf} other {Efni}}', description: 'Notaður sem titill á "veldu" lista á ákæruliða skrefi í ákærum.', } as MessageDescriptor<{ substanceType: SubstanceType }>,
Line range hint
4-24: Consider reducing duplication between message definitionsThe
substanceTitleandsubstanceLabelmessages have identicaldefaultMessagevalues and similar purposes. Consider consolidating them to maintain DRY principles and ease future maintenance.You could either:
- Remove one of them if they serve the same purpose, or
- Extract the common select pattern into a shared constant:
const SUBSTANCE_TYPE_SELECT = '{substanceType, select, ILLEGAL_DRUGS_DRIVING {Fíkniefni} PRESCRIPTION_DRUGS_DRIVING {Lyf} other {Efni}}'; export const substances = defineMessages({ substanceTitle: { id: 'judicial.system.core:substances.title', defaultMessage: SUBSTANCE_TYPE_SELECT, description: 'Notaður sem titill á "veldu" lista á ákæruliða skrefi í ákærum.', }, substanceLabel: { id: 'judicial.system.core:substances.label', defaultMessage: SUBSTANCE_TYPE_SELECT, description: 'Notaður sem titill á "veldu" lista á ákæruliða skrefi í ákærum.', }, // ... });apps/judicial-system/web/src/utils/utils.spec.tsx (2)
334-396: Enhance test descriptions and reduce duplicationThe test implementation is solid with good coverage of edge cases. Consider these improvements:
- Make test descriptions more specific:
- "should return false if no notification exists of the specified type"
- "should return true if the most recent notification was sent successfully"
- Extract common test data setup into a helper function to reduce duplication.
Example helper function:
const createTestNotification = (type: NotificationType, success: boolean, created: string) => ({ id: faker.datatype.uuid(), created, caseId: faker.datatype.uuid(), type, recipients: [{ success, address: faker.internet.email() }] })
486-486: Fix grammatical error in test description- test('should return false if any subarray is the given object has no items', () => { + test('should return false if any subarray in the given object has no items', () => {apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (2)
517-521: Consider memoizing the indictment count lookupThe find operation within the render cycle could be inefficient if the component re-renders frequently. Consider memoizing this lookup using useMemo.
+const memoizedIndictmentCount = useMemo( + () => + workingCase.indictmentCounts?.find( + (indictmentCount) => + indictmentCount.policeCaseNumber === workingCase.policeCaseNumbers?.[index], + ), + [workingCase.indictmentCounts, workingCase.policeCaseNumbers, index] +); <PoliceCaseInfo // ... other props ... - indictmentCount={workingCase.indictmentCounts?.find( - (indictmentCount) => - indictmentCount.policeCaseNumber === workingCase.policeCaseNumbers?.[index], - )} + indictmentCount={memoizedIndictmentCount} />
Line range hint
1-624: Consider splitting the component for better maintainabilityThe Defendant component is handling multiple complex responsibilities including:
- Police case management
- Defendant management
- Indictment count updates
- Navigation logic
Consider splitting this into smaller, focused components to improve maintainability and testability.
Suggested structure:
- Create separate containers for police cases and defendants
- Move the form submission logic to a custom hook
- Extract the navigation logic to a higher-order component
This would make the code more modular and easier to test.
apps/judicial-system/web/src/utils/validate.ts (1)
Line range hint
1-1: Address the TODO: Add comprehensive test coverage.This validation utility contains critical business logic for the judicial system. Comprehensive test coverage is essential to ensure reliability and catch potential issues early.
Would you like me to help create a test suite for these validation functions? I can generate test cases covering various scenarios, including:
- Valid/invalid traffic violation indictments
- Edge cases for multiple subtypes
- Civil claims validation
- Error cases
apps/judicial-system/web/src/utils/hooks/useNationalRegistry/index.ts (2)
Line range hint
14-71: Consider optimizing data fetching and error handling patternsWhile the hook implementation is generally sound, there are some opportunities for improvement in line with NextJS best practices:
- Consider using Next.js's built-in API route helpers for type-safe API calls
- The error handling could be more granular to distinguish between different types of errors
- The mounting logic could be simplified using
useEffect's cleanup functionHere's a suggested improvement:
import { useEffect, useState } from 'react' import { useIntl } from 'react-intl' import useSWR from 'swr' import type { ApiError } from '@island.is/judicial-system-web/types' // Type-safe API response handling type ApiResponse<T> = { data?: T error?: ApiError } const useNationalRegistry = (nationalId?: string | null) => { const { formatMessage } = useIntl() const [shouldFetch, setShouldFetch] = useState<boolean>(false) const isValidNationalId = validate([[nationalId, ['national-id']]]).isValid // Type-safe fetcher const fetcher = async <T,>(url: string): Promise<ApiResponse<T>> => { try { const res = await fetch(url) const data = await res.json() return { data } } catch (error) { return { error: error as ApiError } } } // Rest of the implementation... }Additionally, consider implementing retry logic for failed requests and proper request cancellation on unmount.
Line range hint
23-48: Enhance type safety for API responsesThe current implementation could benefit from stronger TypeScript types for the API responses to ensure type safety throughout the data flow.
Consider adding explicit type guards:
type NationalRegistryError = { error: string status: number } function isNationalRegistryError( response: unknown, ): response is NationalRegistryError { return ( typeof response === 'object' && response !== null && 'error' in response && 'status' in response ) } // Usage in the hook: if (personData && isNationalRegistryError(personData)) { toast.error(formatMessage(errors.nationalRegistry)) }apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts (4)
44-49: Consider consolidating duplicate incident messagesThe new
incidentTitlemessage appears to duplicate the functionality of the existingincidentLabelmessage, as they have identical default messages and similar descriptions.Consider removing one of these messages to maintain DRY principles:
- incidentTitle: { - id: 'judicial.system.core:indictments_indictment.indictment_count.incident_title', - defaultMessage: 'Brot', - description: - 'Notaður sem titill á "brot" lista á ákæruliða skrefi í ákærum.', - },
62-67: Consider consolidating duplicate incident description messagesSimilar to the incident messages, there appears to be duplication between
incidentDescriptionTitleandincidentDescriptionLabel.Consider removing one of these messages to maintain DRY principles:
- incidentDescriptionTitle: { - id: 'judicial.system.core:indictments_indictment.indictment_count.incident_description_title', - defaultMessage: 'Atvikalýsing', - description: - 'Notaður sem titill á "atvikalýsing" svæði á ákæruliða skrefi í ákærum.', - },
80-85: Consider consolidating duplicate blood alcohol content messagesThere appears to be duplication between
bloodAlcoholContentTitleandbloodAlcoholContentLabel.Consider removing one of these messages to maintain DRY principles:
- bloodAlcoholContentTitle: { - id: 'judicial.system.core:indictments_indictment.indictment_count.blood_alcohol_content_title', - defaultMessage: 'Vínandamagn', - description: - 'Notaður sem titill á "vínandamagn" svæði á ákæruliða skrefi í ákærum.', - },
98-103: Consider consolidating duplicate laws broken messagesThere appears to be duplication between
lawsBrokenTitleandlawsBrokenLabel.Consider removing one of these messages to maintain DRY principles:
- lawsBrokenTitle: { - id: 'judicial.system.core:indictments_indictment.indictment_count.laws_broken_title', - defaultMessage: 'Lagaákvæði', - description: - 'Notaður sem titill á "lagaákvæði" leitarboxi á ákæruliða skrefi í ákærum.', - },apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx (3)
243-257: Simplify subtype removal logicThe current implementation has complex nested conditions that could be simplified for better maintainability.
+const validSubtypes = indictmentCount?.indictmentCountSubtypes || [] +const remainingSubtypes = subtypes.filter((s) => s !== subtype) +const validRemainingSubtypes = remainingSubtypes.filter((s) => + validSubtypes.includes(s) +) updatePoliceCase(index, { policeCaseNumber: policeCaseNumbers[index], - subtypes: subtypes.filter((s) => s !== subtype), + subtypes: remainingSubtypes, }) updateIndictmentCount( policeCaseNumbers[index], crimeScene || {}, { - [policeCaseNumbers[index]]: subtypes.filter( - (s) => - s !== subtype && - indictmentCount?.indictmentCountSubtypes?.includes(s), - ), + [policeCaseNumbers[index]]: validRemainingSubtypes, }, )
286-290: Extract common update logic to reduce duplicationThe place and date update logic share similar patterns. Consider extracting this into a helper function.
+const handleCrimeSceneUpdate = (updatedCrimeScene: CrimeScene) => { + updateIndictmentCount( + policeCaseNumbers[index], + updatedCrimeScene, + { [policeCaseNumbers[index]]: subtypesArray } + ) +} // In place update updatePoliceCase() -updateIndictmentCount( - policeCaseNumbers[index], - { ...crimeScene, place: event.target.value }, - { [policeCaseNumbers[index]]: subtypesArray }, -) +handleCrimeSceneUpdate({ ...crimeScene, place: event.target.value }) // In date update -updateIndictmentCount( - policeCaseNumbers[index], - { ...crimeScene, date: date }, - { [policeCaseNumbers[index]]: subtypesArray }, -) +handleCrimeSceneUpdate({ ...crimeScene, date: date })Also applies to: 306-310
Line range hint
1-316: Consider performance optimizationsWhile the component follows NextJS best practices, consider these optimizations:
- Memoize callback functions using useCallback for updatePoliceCase and setPoliceCase
- Consider using React.memo for the component if it re-renders frequently with the same props
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.tsx (1)
Line range hint
53-59: Consider edge case in substance deletion logic.The deletion logic might cause unnecessary re-renders when
indictmentCount.substancesis empty. Consider refactoring to maintain a consistent object structure.const handleDeleteSubstance = (substanceId: SubstanceEnum) => { if (indictmentCount.substances) { delete indictmentCount.substances[substanceId] + // If substances object is empty after deletion, set it to an empty object + // to maintain consistent structure + const substances = Object.keys(indictmentCount.substances).length === 0 + ? {} + : indictmentCount.substances + onChange({ substances }) + return } - onChange({ substances: indictmentCount.substances }) + onChange({ substances: {} }) }apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (1)
292-362: Consider simplifyinggetIncidentDescriptionfor better readabilityThe complex conditional logic within
getIncidentDescriptioncould be refactored to improve readability and maintainability. Breaking the function into smaller, focused functions may enhance code clarity.apps/judicial-system/web/src/utils/utils.ts (1)
151-165: Consider enhancing the JSDoc with validation behavior.While the documentation is clear and includes a good example, it would be helpful to document the validation behavior for edge cases (empty object, undefined input).
Add the following to the JSDoc:
/** * Returns true if all subarrays in the given object have only one item. * * Example: * const indictmentSubtypes = { * '123-123-123': ['item1'], * '234-234-234': ['item2'] * } * * const result = hasOnlyOneItemInSubArrays(indictmentSubtypes) * console.log(result) // true * + * Edge cases: + * - Returns false if input is undefined + * - Returns false if input is an empty object + * - Returns false if any subarray has zero or multiple items * * @param indictmentSubtypes - The object containing the subarrays to check. * @returns {boolean} - True if all subarrays have only one item, false otherwise. */
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (29)
apps/judicial-system/backend/src/app/modules/event/event.service.ts(1 hunks)apps/judicial-system/web/src/components/CaseFile/CaseFile.tsx(1 hunks)apps/judicial-system/web/src/components/CourtArrangements/CourtArrangements.tsx(1 hunks)apps/judicial-system/web/src/components/RestrictionTags/RestrictionTags.tsx(1 hunks)apps/judicial-system/web/src/routes/Court/InvestigationCase/HearingArrangements/HearingArrangements.tsx(1 hunks)apps/judicial-system/web/src/routes/Court/RestrictionCase/HearingArrangements/HearingArrangements.tsx(1 hunks)apps/judicial-system/web/src/routes/Court/components/ReceptionAndAssignment/ReceptionAndAssignment.tsx(1 hunks)apps/judicial-system/web/src/routes/CourtOfAppeal/AppealCase/AppealCase.tsx(1 hunks)apps/judicial-system/web/src/routes/CourtOfAppeal/Overview/Overview.tsx(1 hunks)apps/judicial-system/web/src/routes/CourtOfAppeal/Summary/Summary.spec.tsx(1 hunks)apps/judicial-system/web/src/routes/CourtOfAppeal/Summary/Summary.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx(2 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx(8 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.spec.tsx(2 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts(7 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx(8 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.strings.ts(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.tsx(2 hunks)apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Overview/Overview.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Overview/Overview.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx(1 hunks)apps/judicial-system/web/src/utils/hooks/useAppealAlertBanner/index.tsx(1 hunks)apps/judicial-system/web/src/utils/hooks/useIndictmentCounts/index.ts(1 hunks)apps/judicial-system/web/src/utils/hooks/useNationalRegistry/index.ts(1 hunks)apps/judicial-system/web/src/utils/hooks/useSections/index.ts(1 hunks)apps/judicial-system/web/src/utils/utils.spec.tsx(4 hunks)apps/judicial-system/web/src/utils/utils.ts(1 hunks)apps/judicial-system/web/src/utils/validate.ts(2 hunks)
✅ Files skipped from review due to trivial changes (12)
- apps/judicial-system/web/src/components/CaseFile/CaseFile.tsx
- apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Overview/Overview.tsx
- apps/judicial-system/backend/src/app/modules/event/event.service.ts
- apps/judicial-system/web/src/routes/CourtOfAppeal/Overview/Overview.tsx
- apps/judicial-system/web/src/components/RestrictionTags/RestrictionTags.tsx
- apps/judicial-system/web/src/utils/hooks/useAppealAlertBanner/index.tsx
- apps/judicial-system/web/src/routes/Prosecutor/components/DefendantInfo/DefendantInfo.tsx
- apps/judicial-system/web/src/components/CourtArrangements/CourtArrangements.tsx
- apps/judicial-system/web/src/routes/Court/components/ReceptionAndAssignment/ReceptionAndAssignment.tsx
- apps/judicial-system/web/src/utils/hooks/useSections/index.ts
- apps/judicial-system/web/src/routes/Court/InvestigationCase/HearingArrangements/HearingArrangements.tsx
- apps/judicial-system/web/src/routes/Prosecutor/InvestigationCase/Defendant/Defendant.tsx
🧰 Additional context used
📓 Path-based instructions (17)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.strings.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/routes/Prosecutor/Indictments/Indictment/Substances/Substances.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/CourtOfAppeal/AppealCase/AppealCase.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/Overview/Overview.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/hooks/useIndictmentCounts/index.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/utils/utils.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/routes/CourtOfAppeal/Summary/Summary.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."
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.spec.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/hooks/useNationalRegistry/index.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/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/utils/utils.spec.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/CourtOfAppeal/Summary/Summary.spec.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/Court/RestrictionCase/HearingArrangements/HearingArrangements.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/Indictment/IndictmentCount.strings.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/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.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/Indictment/IndictmentCount.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."
📓 Learnings (3)
apps/judicial-system/web/src/routes/CourtOfAppeal/Summary/Summary.tsx (2)
Learnt from: unakb
PR: island-is/island.is#17112
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:166-178
Timestamp: 2024-12-04T10:10:49.089Z
Learning: In the Judicial System application, error handling for failed case transitions in the `Summary` component (`apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx`) is managed externally via a toaster notification. Therefore, additional error handling within the modal is not necessary.
Learnt from: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-11-12T15:15:11.835Z
Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (3)
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.
Learnt from: thorhildurt
PR: island-is/island.is#17198
File: apps/judicial-system/web/src/routes/Prison/IndictmentOverview/IndictmentOverview.tsx:42-50
Timestamp: 2024-12-11T14:25:44.741Z
Learning: In `IndictmentOverview.tsx`, when updating the punishment type, update the UI state before making the API call to immediately reflect the change.
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.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (3)
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.
Learnt from: thorhildurt
PR: island-is/island.is#17198
File: apps/judicial-system/web/src/routes/Prison/IndictmentOverview/IndictmentOverview.tsx:42-50
Timestamp: 2024-12-11T14:25:44.741Z
Learning: In `IndictmentOverview.tsx`, when updating the punishment type, update the UI state before making the API call to immediately reflect the change.
Learnt from: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-11-12T15:15:11.835Z
Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
🔇 Additional comments (23)
apps/judicial-system/web/src/routes/CourtOfAppeal/AppealCase/AppealCase.tsx (1)
31-31: LGTM: Import path update aligns with codebase restructuring.
The change to import from utils/utils instead of the previous location aligns with the PR's objective of renaming and consolidating utility functions.
apps/judicial-system/web/src/utils/utils.spec.tsx (2)
17-17: LGTM: Import statement follows best practices
The new utility import is correctly placed alongside related utility imports.
446-498: LGTM: Comprehensive test coverage for the new utility function
The test cases thoroughly cover all scenarios including edge cases, with clear and consistent test patterns.
apps/judicial-system/web/src/utils/hooks/useIndictmentCounts/index.ts (1)
83-83: LGTM! Clean use of object property shorthand
The change simplifies the code while maintaining the same functionality.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (1)
282-303: LGTM! Clean implementation of multiple subtypes support
The function has been elegantly extended to handle multiple subtypes while maintaining type safety and clean conditional property assignment.
apps/judicial-system/web/src/routes/Court/RestrictionCase/HearingArrangements/HearingArrangements.tsx (1)
31-31: LGTM! Verify the import path update across the codebase.
The import path change from 'stepHelper' to 'utils' aligns with the PR objectives for better clarity.
Let's verify that this refactoring has been consistently applied across the codebase:
✅ Verification successful
Import path refactoring has been consistently applied
The verification shows that:
- No remaining imports from the old 'stepHelper' path exist (except in a test file which is expected)
- The new 'utils/utils' path is consistently used across the codebase
- The change aligns with the broader codebase structure
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining imports from the old 'stepHelper' path
# and verify the new 'utils' path is used consistently
# Check for any remaining imports from stepHelper
echo "Checking for remaining stepHelper imports..."
rg "from.*stepHelper'"
# Check the usage pattern of the new utils path
echo "Verifying new utils path usage..."
rg "from.*utils/utils'"
Length of output: 3874
apps/judicial-system/web/src/utils/validate.ts (1)
20-20: LGTM! Import statement aligns with PR objectives.
The addition of hasOnlyOneItemInSubArrays utility function supports the new feature of handling multiple subtypes in indictments.
apps/judicial-system/web/src/utils/hooks/useNationalRegistry/index.ts (1)
12-12: LGTM: Import path update aligns with PR objectives
The change from stepHelper to utils matches the PR's goal of renaming the utility file for better clarity.
Let's verify that all imports from stepHelper have been consistently updated:
✅ Verification successful
Import path update is correct and consistent
The import change from stepHelper to utils is valid and all references are consistent. The isBusiness function is:
- Correctly implemented in
apps/judicial-system/web/src/utils/utils.ts - Properly imported in all dependent files from the new location
- Used consistently across the codebase for validating national IDs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining imports from stepHelper
# Expect: No results, indicating all imports have been updated
rg "from.*stepHelper"
Length of output: 23
Script:
#!/bin/bash
# Let's verify the import path exists and contains the expected function
fd -t f "utils.ts" -x echo "File: {}"
# Check the content of utils.ts to verify the function exists
fd -t f "utils.ts" -x cat "{}"
# Check if there are any references to stepHelper in git history
git log -S "stepHelper" --oneline
Length of output: 89321
Script:
#!/bin/bash
# Let's verify the import path and function existence
rg -g "*.ts" -g "*.tsx" "export.*isBusiness.*function" -A 5
# Check if there are any other files importing isBusiness
rg -g "*.ts" -g "*.tsx" "import.*\{.*isBusiness.*\}.*from"
Length of output: 675
Script:
#!/bin/bash
# Let's check the implementation of isBusiness in the utils file
rg -g "*.ts" -g "*.tsx" "export const isBusiness" -B 2 -A 5 apps/judicial-system/web/src/utils/utils.ts
# Let's also check the usage context in the files that import it
rg -g "*.ts" -g "*.tsx" "isBusiness\(" -B 2 -A 5
Length of output: 4762
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts (3)
9-13: LGTM: Police case number title message is well-defined
The message definition follows the established pattern and provides clear context for localization.
26-31: LGTM: Vehicle registration number title message is well-defined
The message definition maintains consistency with the existing pattern and provides clear context.
161-166: LGTM: New message supports multiple subtypes feature
The indictmentDescriptionSubtypesAutofill message is well-defined and directly supports the PR objective of allowing multiple subtypes in indictments.
Let's verify the usage of this message:
✅ Verification successful
Let me check the usage context of this message by examining the component code.
Message is correctly implemented and used
The message is properly used in the component with both single and multiple subtype scenarios. The implementation correctly:
- Handles single subtype case by directly using the subtype value
- Handles multiple subtypes by joining them with commas
- Provides the required placeholders (subtypes and date) in both cases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components using this message
rg "indictmentDescriptionSubtypesAutofill" --type ts
Length of output: 518
Script:
#!/bin/bash
# Get the component code with context
rg "indictmentDescriptionSubtypesAutofill" -B 5 -A 5 apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx
Length of output: 634
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx (2)
23-23: LGTM: Type definitions and Props interface updates are well-structured
The changes to the Props interface and type imports appropriately support the new multiple subtypes feature.
Also applies to: 57-57, 60-60
89-90: LGTM: Safe array initialization
The spread operator usage is appropriate here since IndictmentSubtype elements are primitive values.
apps/judicial-system/web/src/routes/CourtOfAppeal/Summary/Summary.tsx (1)
34-34: LGTM! Import refactoring aligns with codebase reorganization.
The change is part of the broader utility function consolidation, moving from stepHelper to utils.
apps/judicial-system/web/src/routes/Prosecutor/RestrictionCase/Overview/Overview.tsx (2)
51-51: LGTM! Import refactoring maintains consistency.
The change aligns with the codebase-wide consolidation of utility functions into the utils module.
51-51: Verify completeness of PR changes for multiple subtypes feature.
The reviewed changes only show utility function reorganization. However, the PR objective mentions allowing multiple subtypes in indictments, which isn't reflected in these changes.
✅ Verification successful
Let me try another search strategy to verify the changes.
PR changes align with multiple subtypes feature implementation
The codebase search reveals extensive changes supporting multiple subtypes in indictments:
- Multiple files contain code for handling arrays of subtypes (
IndictmentSubtype[]) - Components like
PoliceCaseInfoandIndictmentCounthave been modified to handle multiple subtypes per case - Type definitions and interfaces show support for multiple subtype storage and manipulation
- The utility function
createCaseResentExplanationis part of a larger set of changes that support this feature
The changes are consistent with the PR objective of allowing multiple subtypes in indictments.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for files containing indictment-related changes
echo "Searching for indictment-related files..."
rg -l "indictment|subtype" --type ts --type tsx
# Search for specific component or type definitions related to indictments
echo "Searching for indictment type definitions..."
ast-grep --pattern 'interface $name {
$$$
subtype
$$$
}'
Length of output: 334
Script:
#!/bin/bash
# Search for files containing indictment-related changes with correct file extensions
echo "Searching for indictment-related files..."
rg -l "indictment|subtype" --type typescript --type tsx
# Look for any type definitions or interfaces with multiple or array of subtypes
echo "Searching for array/multiple subtype patterns..."
rg -A 5 "subtype.*\[\]|subtypes?" --type typescript --type tsx
# Check for recent changes in the utils directory
echo "Checking utils directory for related changes..."
rg -l "createCaseResentExplanation" --type typescript --type tsx
Length of output: 597
Script:
#!/bin/bash
# Search without file type restrictions since typescript/tsx extensions aren't recognized
echo "Searching for indictment-related files..."
rg -l "indictment|subtype"
# Look for any type definitions or interfaces with multiple or array of subtypes
echo "Searching for array/multiple subtype patterns..."
rg -A 5 "subtype.*\[\]|subtypes?"
# Check for recent changes in the utils directory
echo "Checking utils directory for related changes..."
fd -e ts -e tsx . | xargs rg -l "createCaseResentExplanation"
Length of output: 93732
Script:
#!/bin/bash
# Search for createCaseResentExplanation function definition and usage
echo "Searching for createCaseResentExplanation function..."
rg -p "createCaseResentExplanation" --type ts --type tsx
# Search for files containing indictment-related changes with .ts or .tsx extension
echo "Searching for indictment-related files..."
fd -e ts -e tsx . | xargs rg -l "indictment|subtype"
# Look for type definitions or interfaces with subtype arrays
echo "Searching for subtype array patterns..."
fd -e ts -e tsx . | xargs rg -A 5 "subtype.*\[\]|subtypes?"
Length of output: 86657
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/Substances/Substances.tsx (3)
9-9: LGTM! Import statement follows project conventions.
The import statement for SectionHeading follows the established absolute import pattern and is appropriately grouped with other UI component imports.
Line range hint 1-120: Well-structured component with proper TypeScript usage.
The component follows React and TypeScript best practices:
- Proper use of TypeScript for type safety
- Efficient state management with useMemo
- Clear separation of concerns
- Proper prop typing and interfaces
69-75: Verify heading hierarchy in the document outline.
While the SectionHeading implementation looks good, please ensure that using "h4" maintains a proper document outline hierarchy. There should be h1, h2, and h3 elements preceding this h4 in the DOM.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (1)
466-468: Confirm clearing offenses when TRAFFIC_VIOLATION is deselected
Resetting the offenses field when the TRAFFIC_VIOLATION subtype is deselected may lead to unintended data loss. Please verify if this behavior aligns with the expected user experience.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.spec.tsx (1)
199-302: Good coverage of incident description scenarios in tests
The added tests for getIncidentDescription effectively cover various scenarios, enhancing the reliability of the component.
apps/judicial-system/web/src/utils/utils.ts (2)
166-176: LGTM! Clean and type-safe implementation.
The function implementation is concise, handles edge cases appropriately, and uses TypeScript effectively.
166-176: Verify the usage of this utility function.
Let's verify how this function is being used in the codebase to ensure it aligns with the PR's objective of allowing multiple subtypes in indictments.
✅ Verification successful
Let me try another search strategy to find the usages of this function.
Let me try one more search to find where this function is actually used in the application code.
Let me try one final search with simpler flags.
Let me check the actual usage in the IndictmentCount component.
Based on the code analysis, I can now provide a final response about the utility function's usage.
The hasOnlyOneItemInSubArrays function is correctly implemented for handling multiple subtypes in indictments
The function is used in two key validation scenarios:
- In
isTrafficViolationStepValidIndictments: It's used as part of a validation check where either all subarrays must have one item OR the indictment counts must have specific subtypes selected. - In
IndictmentCount.tsx: It's used to determine if a case has single subtypes to control the UI flow and form behavior.
The implementation aligns with the PR's objective as it:
- Returns
falsewhen arrays have multiple items, allowing multiple subtypes - Is used in validation logic that supports both single and multiple subtype scenarios
- Helps control UI rendering based on whether subtypes are single or multiple
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find usages of hasOnlyOneItemInSubArrays to understand integration
# Search for direct function calls
echo "Direct function calls:"
rg "hasOnlyOneItemInSubArrays" --type ts --type tsx -A 3
# Search for related indictment subtype handling
echo "\nRelated indictment subtype handling:"
rg "indictmentSubtypes.*Record<string, string\[\]>" --type ts
Length of output: 495
Script:
#!/bin/bash
# Search for usages with correct file type flags
echo "Direct function calls:"
rg "hasOnlyOneItemInSubArrays" -t typescript -t tsx -A 3
# Search for test file content
echo "\nTest file content:"
cat "apps/judicial-system/web/src/utils/utils.spec.tsx"
Length of output: 12693
Script:
#!/bin/bash
# Search with ripgrep using different flags
echo "Searching for function usage:"
rg -l "hasOnlyOneItemInSubArrays" --type=ts --type=js --type=jsx --type=tsx
# Search for files containing indictmentSubtypes
echo "\nSearching for files with indictmentSubtypes:"
rg -l "indictmentSubtypes" --type=ts --type=js --type=jsx --type=tsx
Length of output: 372
Script:
#!/bin/bash
# Search for all files containing the function name
echo "Files containing hasOnlyOneItemInSubArrays:"
find . -type f -exec grep -l "hasOnlyOneItemInSubArrays" {} \;
# Search for all files containing indictmentSubtypes
echo "\nFiles containing indictmentSubtypes:"
find . -type f -exec grep -l "indictmentSubtypes" {} \;
Length of output: 401
Script:
#!/bin/bash
# Check the content of IndictmentCount.tsx
echo "Content of IndictmentCount.tsx:"
cat "./apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx"
# Check validate.ts as well
echo "\nContent of validate.ts:"
cat "./apps/judicial-system/web/src/utils/validate.ts"
Length of output: 46808
| import { | ||
| FormContextWrapper, | ||
| IntlProviderWrapper, | ||
| } from '@island.is/judicial-system-web/src/utils/testHelpers' | ||
| import * as stepHelper from '@island.is/judicial-system-web/src/utils/utils' |
There was a problem hiding this comment.
💡 Codebase verification
Import path change needs to be applied consistently across the codebase
The search results show that while the file has been renamed to utils.ts, there are still multiple files using different import paths. Some files are using the new path @island.is/judicial-system-web/src/utils/utils while others still reference stepHelper. This inconsistency should be addressed.
Locations that need attention:
apps/judicial-system/web/src/routes/CourtOfAppeal/Summary/Summary.spec.tsxapps/judicial-system/web/src/routes/Court/components/ReceptionAndAssignment/ReceptionAndAssignment.tsxapps/judicial-system/web/src/routes/Court/RestrictionCase/HearingArrangements/HearingArrangements.tsx- And several other files in the judicial-system web app that still reference the old naming
🔗 Analysis chain
LGTM! Import path change aligns with codebase restructuring.
The change from stepHelper to utils matches the PR's objective of improving clarity in utility function organization.
Let's verify the consistency of this change across the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to the old stepHelper path
# to ensure the refactoring was complete
# Search for any remaining references to stepHelper
rg "stepHelper" --type ts --type tsx
# Search for the new utils path to confirm consistent usage
rg "@island\.is/judicial-system-web/src/utils/utils" --type ts --type tsx
Length of output: 168
Script:
#!/bin/bash
# Let's try a different approach using more basic search commands
# Search for any remaining references to stepHelper
rg -t typescript -t tsx "stepHelper"
# Search for the new utils path to confirm consistent usage
rg -t typescript -t tsx "@island\.is/judicial-system-web/src/utils/utils"
# Also search using fd to find any files with stepHelper in their name
fd stepHelper
# And search for utils files to verify the new structure
fd "utils\.ts"
Length of output: 12428
Script:
#!/bin/bash
# Let's try one more search focusing on the specific directory and file patterns
rg -g "*.ts*" "stepHelper" apps/judicial-system/web/
# Also search for the specific import pattern in the judicial system web directory
rg -g "*.ts*" "@island\.is/judicial-system-web/src/utils/utils" apps/judicial-system/web/
# Check if the utils.ts file exists in the expected location
ls -l apps/judicial-system/web/src/utils/utils.ts
Length of output: 2908
| hasOnlyOneItemInSubArrays(workingCase.indictmentSubtypes) || | ||
| (workingCase.indictmentCounts?.every( | ||
| (indictmentCount) => | ||
| indictmentCount.indictmentCountSubtypes && | ||
| indictmentCount.indictmentCountSubtypes?.length > 0, | ||
| ) && | ||
| workingCase.demands && | ||
| (!workingCase.hasCivilClaims || workingCase.civilDemands)), |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve code readability and add error handling.
The validation logic is correct but could benefit from the following improvements:
- Break down the complex boolean expression for better readability
- Add null/undefined checks for
workingCase.indictmentCounts - Add JSDoc comments explaining the validation rules
Consider refactoring like this:
+/**
+ * Validates traffic violation indictments based on the following rules:
+ * 1. All subtypes arrays must contain only one item, OR
+ * 2. All indictment counts must have at least one subtype AND
+ * 3. Must have demands AND
+ * 4. If civil claims exist, must have civil demands
+ */
export const isTrafficViolationStepValidIndictments = (
workingCase: Case,
): boolean => {
+ // Early return if basic validation fails
+ if (!workingCase.indictmentSubtypes) {
+ return false;
+ }
+
+ // Check if all subtypes arrays have exactly one item
+ if (hasOnlyOneItemInSubArrays(workingCase.indictmentSubtypes)) {
+ return true;
+ }
+
+ // Validate indictment counts and demands
+ const hasValidIndictmentCounts = workingCase.indictmentCounts?.every(
+ (indictmentCount) =>
+ indictmentCount.indictmentCountSubtypes &&
+ indictmentCount.indictmentCountSubtypes.length > 0
+ ) ?? false;
+
+ const hasValidDemands = Boolean(
+ workingCase.demands &&
+ (!workingCase.hasCivilClaims || workingCase.civilDemands)
+ );
+
+ return hasValidIndictmentCounts && hasValidDemands;
- return Boolean(
- hasOnlyOneItemInSubArrays(workingCase.indictmentSubtypes) ||
- (workingCase.indictmentCounts?.every(
- (indictmentCount) =>
- indictmentCount.indictmentCountSubtypes &&
- indictmentCount.indictmentCountSubtypes?.length > 0,
- ) &&
- workingCase.demands &&
- (!workingCase.hasCivilClaims || workingCase.civilDemands)),
- )
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| hasOnlyOneItemInSubArrays(workingCase.indictmentSubtypes) || | |
| (workingCase.indictmentCounts?.every( | |
| (indictmentCount) => | |
| indictmentCount.indictmentCountSubtypes && | |
| indictmentCount.indictmentCountSubtypes?.length > 0, | |
| ) && | |
| workingCase.demands && | |
| (!workingCase.hasCivilClaims || workingCase.civilDemands)), | |
| /** | |
| * Validates traffic violation indictments based on the following rules: | |
| * 1. All subtypes arrays must contain only one item, OR | |
| * 2. All indictment counts must have at least one subtype AND | |
| * 3. Must have demands AND | |
| * 4. If civil claims exist, must have civil demands | |
| */ | |
| export const isTrafficViolationStepValidIndictments = ( | |
| workingCase: Case, | |
| ): boolean => { | |
| // Early return if basic validation fails | |
| if (!workingCase.indictmentSubtypes) { | |
| return false; | |
| } | |
| // Check if all subtypes arrays have exactly one item | |
| if (hasOnlyOneItemInSubArrays(workingCase.indictmentSubtypes)) { | |
| return true; | |
| } | |
| // Validate indictment counts and demands | |
| const hasValidIndictmentCounts = workingCase.indictmentCounts?.every( | |
| (indictmentCount) => | |
| indictmentCount.indictmentCountSubtypes && | |
| indictmentCount.indictmentCountSubtypes.length > 0 | |
| ) ?? false; | |
| const hasValidDemands = Boolean( | |
| workingCase.demands && | |
| (!workingCase.hasCivilClaims || workingCase.civilDemands) | |
| ); | |
| return hasValidIndictmentCounts && hasValidDemands; | |
| } |
| legalArgumentsTitle: { | ||
| id: 'judicial.system.core:indictments_indictment.indictment_count.legal_arguments_title', | ||
| defaultMessage: 'Heimfærsla', | ||
| description: | ||
| 'Notaður sem titill á "heimfærslu" svæði á ákæruliða skrefi í ákærum.', | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider a broader refactoring of title/label messages
There appears to be a pattern of duplication between "Title" and "Label" messages throughout the file, including these legal arguments messages.
Consider a broader refactoring to:
- Establish a clear distinction between when to use "Title" vs "Label" messages, or
- Consolidate them into a single message type across all fields.
This would affect multiple message pairs including:
legalArgumentsTitle/legalArgumentsLabelincidentTitle/incidentLabelincidentDescriptionTitle/incidentDescriptionLabelbloodAlcoholContentTitle/bloodAlcoholContentLabellawsBrokenTitle/lawsBrokenLabel
If there's no functional difference between titles and labels in the UI, consider keeping only the "Label" versions for consistency.
|
|
||
| updatePoliceCase(index, { | ||
| subtypes: [...(subtypes || []), indictmentSubtype], | ||
| subtypes: [...subtypesArray, indictmentSubtype], | ||
| }) | ||
|
|
||
| updateIndictmentCount(policeCaseNumbers[index], crimeScene || {}) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider enhancing subtype selection handling
The current implementation could be improved in two ways:
- Include the subtypes parameter in the updateIndictmentCount call for consistency
- Consider combining the updates to avoid potential race conditions
updatePoliceCase(index, {
subtypes: [...subtypesArray, indictmentSubtype],
})
-updateIndictmentCount(policeCaseNumbers[index], crimeScene || {})
+updateIndictmentCount(
+ policeCaseNumbers[index],
+ crimeScene || {},
+ { [policeCaseNumbers[index]]: [...subtypesArray, indictmentSubtype] }
+)Committable suggestion skipped: line range outside the PR's diff.
| const hasSingleSubtype = hasOnlyOneItemInSubArrays(subtypes) | ||
|
|
||
| const singleSubType = | ||
| (policeCaseNumber && subtypes?.[policeCaseNumber]?.[0]) || undefined | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure subtypes is safely accessed
Consider adding default values or null checks to prevent potential undefined errors when accessing subtypes. This will enhance robustness, especially if subtypes might be undefined or empty.
| <Box marginBottom={2}> | ||
| {subtypes.length > 1 && ( |
There was a problem hiding this comment.
Handle potential undefined subtypes in rendering
Ensure that subtypes is defined before accessing subtypes.length to prevent runtime errors. Consider initializing subtypes with a default value or adding a null check.
| {(isTrafficViolationCase(workingCase) || | ||
| (indictmentCount?.indictmentCountSubtypes?.includes( | ||
| IndictmentSubtype.TRAFFIC_VIOLATION, | ||
| ) ?? | ||
| false)) && ( |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Reevaluate the condition for rendering traffic violation fields
Combining isTrafficViolationCase(workingCase) with checks on indictmentCountSubtypes might cause inconsistent rendering of traffic violation fields. Consider refining this condition to ensure accurate display based on the selected subtypes.
Allow multiple subtypes in indictments
Asana
What
The changes in this PR enable users to create indictments with multiple subtypes. The main complexity with this change is showing correct fields and autofilling correct values in correct fields based on what subtypes are selected. Please refer to the Asana task to see what exactly should happen when certain subtypes are selected.
There are also other, smaller changes implemented
stepHelperfile inutilstoutils.stepHelperis a bit outdated andutilsis a bit clearer IMO.Why
This is crucial for us to be able to offer combo indictments.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Defendantcomponent to manage indictment counts and subtypes.Bug Fixes
Documentation
Chores