fix(application-system): Fixing export error and input error in custom component #17046
Conversation
…validation if user chooses most serious and then removes that option from multiselect
…g, validaiton etc..
…ident-notification
…hub.com/island-is/island.is into feature/aosh-work-accident-notification
… for when a user removes options from list that was previously chosen as the most serious, minor comments added and removed
…island-is/island.is into feature/aosh-work-accident-notification
…island-is/island.is into feature/aosh-work-accident-notification
…pi/infra and generating values
WalkthroughThis pull request introduces several modifications across multiple components related to the work accident notification system. Key changes include streamlining import paths for 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
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #17046 +/- ##
==========================================
- Coverage 35.70% 35.67% -0.03%
==========================================
Files 6938 6937 -1
Lines 147097 146953 -144
Branches 41815 41784 -31
==========================================
- Hits 52516 52432 -84
+ Misses 94581 94521 -60
Flags with carried forward coverage won't be shown. Click here to find out more. see 49 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 35 Total Test Services: 0 Failed, 32 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (21)
libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts (1)
10-10: Add type annotation and documentation for MAX_EMPLOYEES constantThe constant would benefit from explicit typing and documentation explaining its purpose and any business rules behind the value of 20.
Consider applying this improvement:
-export const MAX_EMPLOYEES = 20 +/** Maximum number of employees that can be added to a work accident notification. + * This limit is enforced in the AddEmployee component and WorkAccidentNotificationForm. + */ +export const MAX_EMPLOYEES: number = 20libs/application/templates/aosh/work-accident-notification/src/index.ts (1)
6-6: Add JSDoc documentation for the exported typeTo improve reusability and maintainability across different NextJS apps, consider adding JSDoc documentation for the exported type.
+/** + * Represents the complete structure of a work accident notification form. + * This type is used across different components to ensure consistent data handling. + */ export type WorkAccidentNotification = WorkAccidentNotificationAnswerslibs/application/templates/aosh/work-accident-notification/src/fields/TypeOfInjury/index.tsx (1)
Line range hint
1-35: Component structure looks good but consider enhancing reusabilityThe component follows TypeScript best practices with proper type definitions. However, since this is in the
libs/directory, consider making the following aspects more reusable:
- The
CausesAndEffectscomponent could potentially be generalized for other form scenarios- The message imports could be made configurable through props
libs/application/templates/aosh/work-accident-notification/src/fields/CauseOfInjury/index.tsx (1)
Line range hint
25-46: LGTM: Reusable component implementationThe component follows best practices for reusability:
- Uses generic FieldBaseProps for consistency across forms
- Implements proper type safety with optional chaining
- Maintains clear separation of concerns with CausesAndEffects subcomponent
Consider extracting the answer type casting to a custom hook for reuse across similar components:
function useWorkAccidentAnswers(application: Application) { return application.answers as WorkAccidentNotification }libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx (1)
Line range hint
1-65: Well-structured component with good TypeScript practices!The component demonstrates several good practices:
- Strong TypeScript typing with proper interfaces
- Effective use of React hooks (useState, useFormContext)
- Clear error handling and validation logic
- Modular and reusable design
Consider extracting the date validation logic into a custom hook for better reusability across other form components.
libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx (1)
Line range hint
31-46: Consider enhancing error handling for the GraphQL mutation.The current implementation only shows a generic error message when the mutation fails. Consider capturing and displaying more specific error information to help users understand and resolve issues.
Here's a suggested improvement:
if (res.data) { setUndefinedEmployeeAmount(false) setCouldNotAddEmployee(false) // Go to screen goToScreen && goToScreen(`employeeInformation[${employeeAmount}]`) } else { + console.error('Failed to add employee:', res.errors) setCouldNotAddEmployee(true) }libs/application/templates/aosh/work-accident-notification/src/fields/DeleteEmployee/index.tsx (2)
Line range hint
29-65: Consider extracting array manipulation logic to a shared utility.The
removeAtIndexfunction and the array update logic could be moved to a shared utility to:
- Promote code reuse across components
- Reduce the risk of inconsistent array manipulations
- Simplify testing and maintenance
Consider creating a utility function like:
// utils/arrayManipulation.ts export const removeItemAtIndex = <T>(array: T[], index: number): T[] => { if (index >= 0 && index < array.length) { return [...array.slice(0, index), ...array.slice(index + 1)] } return array } export const updateRelatedArrays = (answers: WorkAccidentNotification, index: number) => { return { employee: removeItemAtIndex(answers.employee, index), absence: removeItemAtIndex(answers.absence, index), // ... other arrays } }
Line range hint
1-93: Consider generalizing the deletion pattern for reuse across applications.The component implements a common deletion pattern that could be beneficial across different NextJS apps. Consider:
- Creating a base deletion component that handles the common logic
- Using composition to add specific business logic
- Implementing a hook for the deletion mutation logic
Example approach:
// hooks/useDeleteItem.ts export const useDeleteItem = <T extends Record<string, any>>( updateMutation: MutationFunction, options: { onSuccess?: () => void; buildUpdatePayload: (item: T, index: number) => Partial<T>; } ) => { // ... implementation } // components/base/DeleteButton.tsx export const DeleteButton: FC<DeleteButtonProps> = ({ onDelete, disabled, children, }) => { // ... implementation }libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx (1)
27-27: Consider maintaining consistency in props usageWhile the code works correctly, the component still uses
props.fieldin KeyValueFormField components but excludes it from the destructuring. Consider destructuring the field prop for consistency and clarity.- const { application, employee, onClick, index } = props + const { application, employee, onClick, index, field } = propslibs/application/templates/aosh/work-accident-notification/src/fields/AccidentLocation/index.tsx (3)
Line range hint
11-14: Enhance type safety for Option interfaceConsider making the Option interface more strict and reusable across the application.
-type Option = { +export interface SelectOption { value: string label: string + disabled?: boolean }
Line range hint
35-47: Extract filtering logic into a custom hookThe filtering logic for major and minor groups could be extracted into a reusable hook to improve maintainability and reusability across different components.
// useGroupFiltering.ts export function useGroupFiltering( workingEnvironment: WorkingEnvironmentDto[], selectedMajorGroup: SelectOption | null ) { const majorGroups = useMemo( () => workingEnvironment.filter((group) => !group.validToSelect), [workingEnvironment] ); const minorGroupOptions = useMemo( () => selectedMajorGroup ? workingEnvironment.filter( (group) => group.validToSelect && group.code?.substring(0, 2) === selectedMajorGroup.value?.substring(0, 2) ) : [], [workingEnvironment, selectedMajorGroup] ); return { majorGroups, minorGroupOptions }; }
Line range hint
89-106: Improve form handling type safety and error statesConsider adding explicit error handling and improving type safety in the form context usage.
<Controller + control={control} render={({ field: { onChange }, fieldState: { error } }) => ( <Select required label={formatMessage(accident.about.locationOfAccidentMinorGroup)} name="accident.accidentLocation" options={minorGroupOptions.map((group) => ({ label: group.name || '', value: group.code, }))} isDisabled={!selectedMajorGroup} value={selectedMinorGroup} backgroundColor="blue" + error={error?.message} onChange={(v) => { onChange(v); setSelectedMinorGroup({ label: v?.label || '', value: v?.value || '', }); setValue('accident.accidentLocation', v); setValue( 'accident.accidentLocationParentGroup', selectedMajorGroup, ); }} /> )} name={'accident.accidentLocation'} />libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (3)
Line range hint
89-108: Enhance validation error messages and number parsingThe employment rate validation could be improved for better user experience and robustness:
Consider applying these improvements:
.refine( (data) => { - const rateNum = parseInt(data.employmentRate, 10) + const rateNum = Number(data.employmentRate) + if (isNaN(rateNum)) return false return rateNum > 0 && rateNum <= 100 }, { - message: '1%-100%', + message: 'Employment rate must be between 1% and 100%', path: ['employmentRate'], }, )
Line range hint
110-146: Improve type safety in the cause and effect schema generatorThe schema generator uses type assertion which could be made more type-safe.
Consider applying these improvements:
const createCauseAndEffectSchema = ( fieldKey: string, mostSeriousKey: string, ) => { + type OptionType = { value: string; label: string } + type SchemaType = { + [key: string]: OptionType[] + } return z .object({ [fieldKey]: z - .record(z.array(option)) + .record(z.array(option)) as unknown as z.ZodType<SchemaType> [mostSeriousKey]: z.string().optional().nullable(), }) .refine( (data) => { - // Narrow the type of data[fieldKey] - const fieldData = data[fieldKey] as - | Record<string, { value: string; label: string }[]> - | undefined + const fieldData = data[fieldKey] if (!fieldData) return false const totalOptions = Object.values(fieldData).reduce( (sum, arr) => sum + arr.length, 0, ) return totalOptions < 2 || (totalOptions >= 2 && data[mostSeriousKey]) }, { path: [mostSeriousKey], + message: 'Must select the most serious option when multiple are chosen' }, ) }
Line range hint
1-222: Consider enhancing schema documentation and error handlingWhile the schema implementation is solid, consider these architectural improvements:
- Add JSDoc comments to document the purpose and constraints of each schema
- Implement custom error maps for localization
- Consider extracting common validation patterns (like kennitala validation) into reusable helpers
Example implementation:
// Helper for kennitala validation const createKennitalaSchema = (errorMessage?: string) => z.string().refine( (value) => value && value.length !== 0 && kennitala.isValid(value), { message: errorMessage ?? 'Invalid national ID' } ); // Example usage in schemas const basicCompanySchema = z.object({ nationalId: createKennitalaSchema('Invalid company ID'), // ... rest of the schema });libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (2)
Line range hint
1-54: LGTM! Consider documenting the new prop.The type definitions are well-structured and follow TypeScript best practices. The component is properly exported for reuse across different NextJS apps.
Add JSDoc comment for the
majorGroupLengthprop to explain its purpose:type CausesAndEffectsProps = { externalDataKey: string // Example aoshData.data.physicalActivities heading: MessageDescriptor // Contentful translation subHeading: MessageDescriptor // Contentful translation answerId: string // Example circumstances.physicalActivities mostSeriousAnswerId: string // Example circumstances.physicialActivitesMostSerious screenId: string // Example circumstances ... used to find answers to autofill mostSeriousAnswer?: string // Example physicialActivitiesMostSerious ... used to find answers to autofill + /** Length of the major group code used for hierarchical categorization */ majorGroupLength: number }
Line range hint
120-129: Consider making the validation more explicit.While the implementation is correct, the early return condition could be more explicit about what constitutes an invalid state.
- if (!code) return + if (!code || !value?.value || !value?.label) { + setPickedValue(undefined) + return + }libs/application/templates/aosh/work-accident-notification/src/fields/Occupation/index.tsx (4)
Line range hint
11-24: Consider extracting a generic hierarchical select componentThe current implementation is tightly coupled to the work accident notification domain. Consider extracting the hierarchical selection logic into a reusable component that could be used across different forms.
Example approach:
interface HierarchicalOption { code: string name: string level: number validToSelect: boolean } interface HierarchicalSelectProps<T extends HierarchicalOption> { options: T[] value: T | null onChange: (value: T | null) => void levels: Array<{ label: string filterFn: (option: T, parentValue: T | null) => boolean }> }
Line range hint
39-63: Consider using useReducer for complex state managementThe component manages multiple related state variables for different selection levels. Consider using
useReducerto centralize state management and make state transitions more predictable.Example approach:
type State = { majorGroup: Options | null subMajorGroup: Options | null minorGroup: Options | null unitGroup: Options | null searchGroup: Options | null options: { major: VictimsOccupationDto[] subMajor: VictimsOccupationDto[] minor: VictimsOccupationDto[] unit: VictimsOccupationDto[] } } type Action = | { type: 'SET_MAJOR_GROUP', payload: Options | null } | { type: 'SET_OPTIONS', level: keyof State['options'], payload: VictimsOccupationDto[] } // ... other actions
Line range hint
273-401: Add form validation for required fieldsThe form implementation lacks validation rules. Consider adding validation to ensure required fields are filled before form submission.
Example approach:
<Controller rules={{ required: formatMessage(employee.employee.requiredField), validate: (value) => { if (!value?.value) { return formatMessage(employee.employee.invalidSelection) } return true } }} render={...} name={`employee[${idx}].victimsOccupationMajor`} />
Line range hint
402-430: Enhance accessibility with ARIA attributesThe Select components could benefit from additional ARIA attributes to improve accessibility for screen readers.
Example improvements:
<Select label={formatMessage(employee.employee.unitGroupLabel)} + aria-label={formatMessage(employee.employee.unitGroupLabel)} + aria-describedby={`occupation-unit-help-${idx}`} name={`employee[${idx}].victimsOccupationUnit`} // ... other props />
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (20)
libs/application/templates/aosh/work-accident-notification/src/fields/AccidentLocation/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx(2 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/CauseOfInjury/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/CheckboxFieldCustom/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Circumstance/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx(3 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/DeleteEmployee/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Deviation/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/InjuredBodyParts/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Occupation/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx(2 hunks)libs/application/templates/aosh/work-accident-notification/src/fields/TypeOfInjury/index.tsx(1 hunks)libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/index.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/index.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/index.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts(1 hunks)libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- libs/application/templates/aosh/work-accident-notification/src/fields/CheckboxFieldCustom/index.tsx
- libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/index.ts
🧰 Additional context used
📓 Path-based instructions (18)
libs/application/templates/aosh/work-accident-notification/src/fields/AccidentLocation/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/aosh/work-accident-notification/src/fields/AddEmployee/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/aosh/work-accident-notification/src/fields/CauseOfInjury/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/aosh/work-accident-notification/src/fields/Circumstance/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/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.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/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.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/aosh/work-accident-notification/src/fields/DeleteEmployee/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/aosh/work-accident-notification/src/fields/Deviation/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/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/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/aosh/work-accident-notification/src/fields/InjuredBodyParts/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/aosh/work-accident-notification/src/fields/Occupation/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/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.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/aosh/work-accident-notification/src/fields/TypeOfInjury/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/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/index.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/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.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/aosh/work-accident-notification/src/index.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/aosh/work-accident-notification/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/aosh/work-accident-notification/src/shared/constants.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/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx
[error] 63-63: Unnecessary use of boolean literals in conditional expression.
Simplify your code by directly assigning the result without using a ternary operator.
If your goal is negation, you may use the logical NOT (!) or double NOT (!!) operator for clearer and concise code.
Check for more details about NOT operator.
Unsafe fix: Remove the conditional expression with
(lint/complexity/noUselessTernary)
🔇 Additional comments (31)
libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts (1)
10-10: Verify consistent usage of MAX_EMPLOYEES across components
Let's ensure the constant is properly imported and consistently used across the codebase.
✅ Verification successful
Let me verify the usage of MAX_EMPLOYEES with more context by checking the actual implementation in the files where it's imported.
MAX_EMPLOYEES constant is properly integrated and used consistently
The constant is correctly imported and used in both components:
- In AddEmployee component: Used to validate and limit the number of employees that can be added
- In WorkAccidentNotificationForm: Used to build the correct number of repeatable sections for employee information
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify MAX_EMPLOYEES usage in related components
# Check import statements and usage
echo "Checking imports and usage of MAX_EMPLOYEES:"
rg -l "MAX_EMPLOYEES" libs/application/templates/aosh/work-accident-notification/src/
# Check for any remaining hardcoded value of 20 that should use the constant
echo "Checking for hardcoded values that should use MAX_EMPLOYEES:"
rg "20" --type ts libs/application/templates/aosh/work-accident-notification/src/
Length of output: 21666
Script:
#!/bin/bash
# Get implementation details from files using MAX_EMPLOYEES
echo "Checking AddEmployee component implementation:"
rg -A 10 "MAX_EMPLOYEES" libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx
echo -e "\nChecking WorkAccidentNotificationForm implementation:"
rg -A 10 "MAX_EMPLOYEES" libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.ts
Length of output: 1711
libs/application/templates/aosh/work-accident-notification/src/index.ts (2)
1-2: LGTM! Clean import structure
The import statements follow TypeScript best practices with clear, explicit type imports.
Line range hint 1-8: Verify type usage across components
The renaming of the type and streamlining of import paths requires verification across all consuming components.
✅ Verification successful
Let me verify the imports further to ensure all usages are consistent.
Type rename verification is complete and consistent
The type rename appears to be correctly implemented across the codebase:
- The type
WorkAccidentNotificationis correctly exported from the index file as an alias forWorkAccidentNotificationAnswers - All component imports are using the correct relative path (
../..or../../..) to import the type from the index file - The type definition in
dataSchema.tsis properly defined using Zod schema - No legacy imports or inconsistent type usage patterns were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all components are using the updated type import correctly
# Search for any old-style imports that might have been missed
rg -l "import.*WorkAccidentNotification.*dataSchema" libs/application/templates/aosh/work-accident-notification/
# Check for any potential type usage that might need updating
ast-grep --pattern 'WorkAccidentNotification'
Length of output: 2809
Script:
#!/bin/bash
# Check the actual type usage in files to verify consistency
ast-grep --pattern 'type $_ = WorkAccidentNotification'
# Check for any potential interface extensions or type aliases
ast-grep --pattern 'extends WorkAccidentNotification'
# Look at the dataSchema file to understand the type definition
cat libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts
Length of output: 6460
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/index.ts (2)
Line range hint 12-28: Implementation looks good!
The section builder implementation is well-structured:
- Properly typed with TypeScript
- Clear composition of sub-sections
- Effective conditional rendering based on employee count
10-10: Verify the new import path resolution
While the import path has been simplified, we should ensure it correctly resolves to the intended type definition and doesn't impact tree-shaking.
Let's verify the type resolution and identify the actual source file:
✅ Verification successful
Import path change is correct and well-structured
The simplified import path ../../.. correctly resolves to the root src/index.ts, which properly re-exports the WorkAccidentNotification type. The type is ultimately sourced from lib/dataSchema.ts through a clean export chain:
lib/dataSchema.tsdefinesWorkAccidentNotificationAnswersindex.tsre-exports it asWorkAccidentNotification
This change maintains proper type resolution while following good practices for module organization.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the WorkAccidentNotification type resolution
# Expected: Find the source file and verify it exports the type
# Find potential source files
echo "Searching for WorkAccidentNotification type definition..."
ast-grep --pattern 'export type WorkAccidentNotification = {
$$$
}'
# Verify the import path resolution
echo "Verifying import path resolution..."
fd -e ts -e tsx . libs/application/templates/aosh/work-accident-notification/src | xargs rg "export.*WorkAccidentNotification"
Length of output: 1000
Script:
#!/bin/bash
# Let's verify the import chain to ensure the type is correctly exported and imported
# Check the content of index.ts to verify the re-export
echo "Content of src/index.ts:"
cat libs/application/templates/aosh/work-accident-notification/src/index.ts
# Check the actual type definition in dataSchema.ts
echo -e "\nContent of src/lib/dataSchema.ts (relevant parts):"
rg -A 5 "WorkAccidentNotification" libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts
Length of output: 1245
libs/application/templates/aosh/work-accident-notification/src/fields/Deviation/index.tsx (2)
Line range hint 1-38: LGTM! Component follows TypeScript and reusability best practices
The component demonstrates good practices:
- Strong TypeScript typing with proper interface definitions
- Clean prop composition using
PropsWithChildren - Named imports for better tree-shaking
- Reusable component structure with clear prop interfaces
5-5: Verify the new import path for WorkAccidentNotification
The import path has been simplified to ../... Let's verify that this path correctly resolves to the module containing the WorkAccidentNotification type.
✅ Verification successful
Import path change is correct and consistent
The import path ../.. correctly resolves to the root src directory where WorkAccidentNotification type is exported. This is verified by:
- The type is properly exported in
src/index.tsasexport type WorkAccidentNotification = WorkAccidentNotificationAnswers - Multiple sibling components in the
fieldsdirectory consistently use the same import path - The type definition originates from
dataSchema.tsand is re-exported through the index file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the existence and export of WorkAccidentNotification type
# at the parent directory level
# Check for index files or dataSchema files that might export WorkAccidentNotification
fd -t f -e ts -e tsx "index|dataSchema" "libs/application/templates/aosh/work-accident-notification/src" \
--exec grep -l "export.*WorkAccidentNotification"
# Verify the import is used consistently across sibling components
fd -t f -e tsx . "libs/application/templates/aosh/work-accident-notification/src/fields" \
--exec grep -l "import.*WorkAccidentNotification.*from '\.\./\.\.'"
Length of output: 1635
Script:
#!/bin/bash
# Let's verify the actual export of WorkAccidentNotification in the identified files
echo "=== Checking src/index.ts ==="
cat libs/application/templates/aosh/work-accident-notification/src/index.ts
echo -e "\n=== Checking src/lib/dataSchema.ts ==="
cat libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts
Length of output: 6863
libs/application/templates/aosh/work-accident-notification/src/fields/TypeOfInjury/index.tsx (1)
5-5: Verify the new import path structure
The import path has been simplified, but we should verify that this change maintains proper type exports and doesn't affect tree-shaking.
Let's verify the exports and imports:
✅ Verification successful
Let me gather more information about the exports to ensure the type is properly exported from the barrel file.
Let me check the actual type definition of WorkAccidentNotificationAnswers to complete the verification.
Import path change is safe and correctly structured
The import path change from '../../lib/dataSchema' to '../..' is valid and well-structured because:
- The type
WorkAccidentNotificationis properly exported from the barrel file (index.ts) - The type is correctly aliased from
WorkAccidentNotificationAnswers - Multiple components in the same directory structure are using this import pattern consistently
- The type definition is complete and properly maintained in the dataSchema file
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the WorkAccidentNotification type export and its usage
# Check the barrel file exports
ast-grep --pattern 'export { $_ as WorkAccidentNotification }'
# Check if there are other components using the same import pattern
rg "import.*WorkAccidentNotification.*from '\.\./\.\.'" libs/
Length of output: 1731
Script:
#!/bin/bash
# Check the index.ts/index.tsx files in the work-accident-notification directory for exports
fd -e ts -e tsx "^index\." libs/application/templates/aosh/work-accident-notification/src/ -x cat {}
# Check for the actual type definition
ast-grep --pattern 'type WorkAccidentNotification = $_'
Length of output: 54668
Script:
#!/bin/bash
# Check the dataSchema file for the type definition
fd -e ts "dataSchema" libs/application/templates/aosh/work-accident-notification/src/ -x cat {}
Length of output: 6357
libs/application/templates/aosh/work-accident-notification/src/fields/Circumstance/index.tsx (2)
Line range hint 1-38: Component implementation follows best practices.
The component:
- Uses TypeScript effectively for props definition
- Implements proper type safety with FieldBaseProps
- Maintains reusability through clear props interface
- Follows functional component patterns
5-5: Verify the impact of the import path change on tree-shaking.
The import path has been made less specific, which could affect tree-shaking optimization. Consider keeping the explicit import path to ensure better bundling optimization.
Let's verify the export location and potential impact:
Consider maintaining explicit import paths for better code maintainability and optimal tree-shaking. This aligns with the coding guidelines for libs/**/* regarding effective bundling practices.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.ts (3)
11-11: LGTM! Good practice using centralized constants.
The addition of the MAX_EMPLOYEES import from a shared constants file improves maintainability and follows the DRY principle.
Line range hint 19-35: LGTM! Form structure follows good practices.
The form structure is well-organized with modular sections and clear separation of concerns. It adheres to the reusability guidelines for libs directory by:
- Using shared components and types from @island.is
- Maintaining TypeScript type safety
- Following a modular structure that can be reused across different NextJS apps
13-17: Consider optimizing the repeatable sections generation.
While the logic is correct, the current implementation using Array spread with map could be memory intensive for large MAX_EMPLOYEES values. Consider a more efficient approach.
Here's a more memory-efficient implementation:
-const buildRepeatableSections = (): Section[] => {
- const sections = [...Array(MAX_EMPLOYEES)].map((_key, index) => {
- return EmployeeAndAccidentInformationSection(index)
- })
- return sections.flat()
-}
+const buildRepeatableSections = (): Section[] => {
+ const sections: Section[] = [];
+ for (let i = 0; i < MAX_EMPLOYEES; i++) {
+ sections.push(...EmployeeAndAccidentInformationSection(i));
+ }
+ return sections;
+}Let's verify the value of MAX_EMPLOYEES to ensure it's reasonable:
libs/application/templates/aosh/work-accident-notification/src/fields/CauseOfInjury/index.tsx (2)
Line range hint 8-24: LGTM: Well-structured TypeScript interfaces
The component demonstrates good TypeScript practices:
- Clear interface definition for props
- Proper type composition with FieldBaseProps
- Explicit type for the OptionAndKey structure
6-6: Verify the new import path's impact on tree-shaking
The import path has been changed to a more general parent directory import. While this simplifies the import structure, we should verify that this doesn't impact tree-shaking negatively.
✅ Verification successful
Import path change is safe for tree-shaking
The import path change from '../../lib/dataSchema' to '../..' is actually more efficient. The type WorkAccidentNotification is properly re-exported from the root index.ts, where it's a direct type alias of WorkAccidentNotificationAnswers. This maintains type safety and doesn't impact tree-shaking as it's just a type import that gets removed during compilation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check what's being exported from the parent directory and verify the import path
# Look for the main export file and its contents
fd -e ts -e tsx index --full-path libs/application/templates/aosh/work-accident-notification/src --exec cat {}
# Check if WorkAccidentNotification is re-exported elsewhere
rg -l "export.*WorkAccidentNotification" --type ts --type tsx
Length of output: 54567
Script:
#!/bin/bash
# Check for WorkAccidentNotification exports and type definitions
rg -l "export.*WorkAccidentNotification" --type typescript
# Check the content of the parent directory's index file and dataSchema
fd -e ts -e tsx "dataSchema|index" --full-path libs/application/templates/aosh/work-accident-notification/src/lib --exec cat {}
Length of output: 6867
Script:
#!/bin/bash
# Check the content of the root index file to see how WorkAccidentNotification is exported
cat libs/application/templates/aosh/work-accident-notification/src/index.ts
# Check for any other potential exports of WorkAccidentNotification
rg "export.*type.*WorkAccidentNotification" --type ts
Length of output: 690
libs/application/templates/aosh/work-accident-notification/src/fields/InjuredBodyParts/index.tsx (2)
Line range hint 1-42: LGTM! Component follows coding guidelines
The component adheres to the coding guidelines for library code:
- Uses TypeScript for props definition (FieldBaseProps, InjuredBodyPartsProps)
- Exports named components for effective tree-shaking
- Implements reusable form field logic that can be used across NextJS apps
6-6: Verify the new import path for WorkAccidentNotification type
While the import path change aligns with the broader refactoring effort, let's verify that the type is properly exported from the root index.
✅ Verification successful
Let me gather more information to verify if the type is exported from the root index file.
Based on the gathered information, I can now generate the final response:
Import path change is correct and consistent
The import path ../.. for WorkAccidentNotification type is correct and consistent with the codebase for the following reasons:
- The type is properly re-exported from the root index as
export type WorkAccidentNotification = WorkAccidentNotificationAnswers - The same import pattern is consistently used across 11 different files in the same directory structure
- The type is properly defined in the dataSchema file and contains all the necessary type information
- The import path follows the relative path resolution from the component's location to the root index
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the WorkAccidentNotification type export and its usage
# Check if the type is exported from the root index
ast-grep --pattern 'export type WorkAccidentNotification'
# Check for other similar import patterns to ensure consistency
rg -l "import.*WorkAccidentNotification.*from '\.\./\.\.'" libs/application/templates/aosh/work-accident-notification/src/
Length of output: 1248
Script:
#!/bin/bash
# Check the root index file content
fd -e ts -e tsx index libs/application/templates/aosh/work-accident-notification/src/ -x cat {}
# Check if WorkAccidentNotification is re-exported from lib/dataSchema
fd -e ts dataSchema libs/application/templates/aosh/work-accident-notification/src/ -x cat {}
Length of output: 60818
libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx (1)
7-7: Verify the new import path consistency across the codebase.
The import path change for WorkAccidentNotification aligns with the broader refactoring mentioned in the PR summary.
Let's verify the consistency of this import pattern across other components:
✅ Verification successful
Import path change is consistent with the codebase patterns
The verification shows that all components within the work-accident-notification template consistently use the relative import path ../.. for importing WorkAccidentNotification, including:
- TypeOfInjury
- Overview/EmployeeAccordionItem
- EmployeeStartTimeError
- Deviation
- DeleteEmployee
- Circumstance
- CheckboxFieldCustom
- CauseOfInjury
- Occupation
- InjuredBodyParts
- AccidentLocation
The change aligns with the established import pattern across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if other components use the same import pattern for WorkAccidentNotification
# Search for WorkAccidentNotification imports
rg "import.*WorkAccidentNotification.*from" --type typescript
# Check specifically for any remaining imports from the old path
rg "import.*WorkAccidentNotification.*from.*lib/dataSchema" --type typescript
Length of output: 213
Script:
#!/bin/bash
# Description: Check if other components use the same import pattern for WorkAccidentNotification
# Search for WorkAccidentNotification imports using ripgrep without file type filter
rg "import.*WorkAccidentNotification.*from"
# Check for files that might be importing from lib/dataSchema
rg "import.*from.*lib/dataSchema"
Length of output: 24693
libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx (1)
10-10: LGTM! Good practice using shared constants.
The import follows proper tree-shaking practices and promotes code reusability by utilizing a shared constant.
libs/application/templates/aosh/work-accident-notification/src/fields/DeleteEmployee/index.tsx (2)
Line range hint 10-19: Well-structured TypeScript implementation!
The component follows TypeScript best practices with:
- Clear props interface definition
- Proper use of FC type with PropsWithChildren
- Explicit typing for optional props
8-8: Verify tree-shaking effectiveness with the new import path.
While the simplified import path aligns with the broader refactoring effort, let's ensure it maintains effective tree-shaking.
✅ Verification successful
Import path change is safe for tree-shaking
The simplified import path ../.. correctly resolves to the root src directory where the WorkAccidentNotification type is properly exported. The type is a direct re-export from dataSchema.ts without any side effects, maintaining effective tree-shaking capabilities.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if WorkAccidentNotification type is properly exported and tree-shakeable
# Look for barrel files that might affect tree-shaking
# Check for index files that might affect tree-shaking
fd -e ts -e tsx index libs/application/templates/aosh/work-accident-notification/src --exec rg -l "export.*WorkAccidentNotification"
# Check the actual type definition location
ast-grep --pattern 'type WorkAccidentNotification = {
$$$
}'
Length of output: 378
Script:
#!/bin/bash
# Let's check the actual type definition and its export
rg -A 5 "type WorkAccidentNotification" libs/application/templates/aosh/work-accident-notification/src
# Check how the type is exported in the index files
cat libs/application/templates/aosh/work-accident-notification/src/index.ts
cat libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.ts
Length of output: 2766
libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx (2)
Line range hint 18-91: Implementation follows best practices
The component implementation:
- Uses proper TypeScript types for props and data
- Follows reusability patterns with generic props interface
- Properly integrates with the island-ui component system
12-12: Verify the new import structure across the application
The WorkAccidentNotification type import has been moved from the local schema to the parent directory. While this change appears to be part of a broader refactor, we should ensure it follows best practices for tree-shaking and module organization.
Also applies to: 16-16
✅ Verification successful
Import path is correctly structured and follows best practices
The import of EmployeeType from ../../lib/dataSchema is correct and follows the established pattern in the codebase. The analysis shows that:
- The type is properly defined in the dataSchema.ts file as
export type EmployeeType = z.TypeOf<typeof employeeSchema> - The import is used correctly in the EmployeeAccordionItem component for type safety
- This follows the same pattern as other imports from dataSchema.ts across the codebase
- There are no circular dependencies introduced
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the consistency of WorkAccidentNotification type imports
# and ensure no circular dependencies are introduced
# Check for all WorkAccidentNotification imports
rg --type typescript "import.*WorkAccidentNotification.*from" -A 1
# Check for potential circular dependencies
ast-grep --pattern 'import { $$$ } from "../.."'
Length of output: 154
Script:
#!/bin/bash
# Let's try different approaches to verify the import structure
# Search for any imports from dataSchema
rg "import.*from.*dataSchema"
# Search for EmployeeType imports specifically
rg "import.*EmployeeType"
# Look for files containing both EmployeeType and WorkAccidentNotification
rg "EmployeeType|WorkAccidentNotification" --type ts --type tsx
# Find all TypeScript/React files in the work-accident-notification directory
fd "\.(ts|tsx)$" libs/application/templates/aosh/work-accident-notification
Length of output: 44652
Script:
#!/bin/bash
# Let's check the content of the dataSchema file and its usage
# Check the dataSchema file content
cat libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts
# Check how EmployeeType is used in the component
cat libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx
Length of output: 9459
libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.tsx (1)
Line range hint 1-144: Verify component reusability across NextJS apps
The component follows good practices for reusability but relies on external dependencies.
Let's verify the dependencies and their usage across the codebase:
✅ Verification successful
Based on the search results, I can see that the MultiSelectDropdownController component is only used within the work accident notification template (libs/application/templates/aosh/work-accident-notification/) and is not shared across other NextJS applications. The component's dependencies (@island.is/application/types and @island.is/application/core) are widely used across the codebase, but they are part of the application's core infrastructure.
Component is scoped to a single template and follows good practices
The component is properly scoped to its specific use case within the work accident notification template and follows good practices by:
- Using standard form context from react-hook-form
- Properly typed with TypeScript
- Using shared core utilities and types from the application framework
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check component dependencies and usage
# Check for direct imports of this component
rg -l "MultiSelectDropdownController" --type ts --type tsx
# Check for usage of @island.is/application types
ast-grep --pattern 'import { $_ } from "@island.is/application/types"'
# Check for usage of @island.is/application/core
ast-grep --pattern 'import { $_ } from "@island.is/application/core"'
Length of output: 231
Script:
#!/bin/bash
# Let's try a different approach to check component usage and dependencies
# Check for MultiSelectDropdownController usage with proper file extension
rg -l "MultiSelectDropdownController"
# Check imports from @island.is packages using ripgrep
rg -l "@island.is/application/types"
rg -l "@island.is/application/core"
# Look for similar dropdown components
rg -l "MultiSelectDropdown"
# Check the package.json for dependencies
fd "package.json" --exec cat {}
Length of output: 100219
libs/application/templates/aosh/work-accident-notification/src/fields/AccidentLocation/index.tsx (2)
2-3: LGTM: Import organization improvements
The import changes improve code organization and follow effective tree-shaking practices by using specific imports.
Also applies to: 9-9
1-1: Verify component usage across NextJS apps
Let's verify that this component is consistently used across different NextJS applications within the workspace.
libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (1)
Line range hint 212-222: LGTM! Well-structured type exports
The type exports are well-organized and provide comprehensive type coverage for all schema components. The use of z.TypeOf ensures type safety and maintainability.
libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (3)
Line range hint 54-73: LGTM! Clean state management implementation.
The component follows React best practices with proper use of hooks and form context. The new prop is correctly integrated into the existing structure.
72-81: LGTM! Efficient uniqueness handling with Set.
The use of Set for handling unique options is a good performance optimization, especially when dealing with large datasets. The implementation maintains correct state management while preventing duplicates.
Line range hint 1-1: Verify tree-shaking compatibility.
Since this component is in the libs directory, let's ensure it's properly tree-shakeable.
✅ Verification successful
The component is properly tree-shakeable
The component follows best practices for tree-shaking compatibility:
- Uses named export instead of default export
- No side effects before the export
- Imports are properly scoped
- Component is a pure functional component with TypeScript types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for side effects and verify exports
ast-grep --pattern 'export { $_ }' # Check named exports
ast-grep --pattern 'export default $_' # Check default exports
Length of output: 100683
Script:
#!/bin/bash
# Check for side effects in the component file
rg -l "^import.*@island.is/application/types" --type ts
rg -A 5 "^export.*CausesAndEffects" --type ts
Length of output: 100327
Script:
#!/bin/bash
# Check the specific file for exports and side effects
rg -A 10 "CausesAndEffects" "libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx"
Length of output: 982
libs/application/templates/aosh/work-accident-notification/src/fields/Occupation/index.tsx (1)
9-9: LGTM: Import path change aligns with TypeScript best practices
The simplified relative import path for WorkAccidentNotification improves code maintainability while maintaining proper type safety.
libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx
Show resolved
Hide resolved
...ates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.tsx
Show resolved
Hide resolved
...ates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.tsx
Show resolved
Hide resolved
...
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
New Features
MAX_EMPLOYEESto limit the maximum number of employees.Improvements
WorkAccidentNotificationtype across multiple components.Bug Fixes
Documentation