feat(application-system): New application - Work Accident Notification #16632
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
WalkthroughThe pull request introduces a comprehensive set of changes to the work accident notification application. Key additions include a new import for 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 ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16632 +/- ##
==========================================
- Coverage 36.77% 36.71% -0.06%
==========================================
Files 6858 6865 +7
Lines 142423 142832 +409
Branches 40623 40725 +102
==========================================
+ Hits 52378 52445 +67
- Misses 90045 90387 +342
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 50 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's limits when posting large numbers of comments.
🛑 Comments failed to post (150)
libs/application/templates/aosh/work-accident-notification/src/utils/formatPhonenumber.ts (1)
3-6: 🛠️ Refactor suggestion
Consider enhancing the function's robustness and reusability.
- The hardcoded 'IS' region code limits reusability across different regions
- Missing input validation for empty or invalid strings
- TypeScript types could be more specific
Consider this improved implementation:
-export const formatPhoneNumber = (phoneNumber: string): string => { +type CountryCode = 'IS' | 'US' | 'GB'; // Add more as needed + +export const formatPhoneNumber = ( + phoneNumber: string, + region: CountryCode = 'IS' +): string => { + if (!phoneNumber?.trim()) { + return phoneNumber; + } + - const phone = parsePhoneNumberFromString(phoneNumber, 'IS') + const phone = parsePhoneNumberFromString(phoneNumber, region) return phone?.formatNational() || phoneNumber }📝 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.type CountryCode = 'IS' | 'US' | 'GB'; // Add more as needed export const formatPhoneNumber = ( phoneNumber: string, region: CountryCode = 'IS' ): string => { if (!phoneNumber?.trim()) { return phoneNumber; } const phone = parsePhoneNumberFromString(phoneNumber, region) return phone?.formatNational() || phoneNumber }libs/application/templates/aosh/work-accident-notification/src/index.ts (1)
1-1: 🛠️ Refactor suggestion
Consider using named exports instead of default export.
Default exports can make tree-shaking less effective. Consider converting to named exports for better bundle optimization.
-import template from './lib/WorkAccidentNotificationTemplate' +import { WorkAccidentNotificationTemplate } from './lib/WorkAccidentNotificationTemplate' -export default template +export { WorkAccidentNotificationTemplate }Also applies to: 8-8
libs/application/templates/aosh/work-accident-notification/src/dataProviders/index.ts (1)
6-9: 🛠️ Refactor suggestion
Enhance type safety and add documentation.
The API definition could benefit from:
- TypeScript type annotations for better type safety
- JSDoc documentation explaining the API's purpose and return type
+/** + * Defines the API for retrieving AOSH (Administration of Occupational Safety and Health) input options. + * Used in the work accident notification process to fetch available options for form fields. + * + * @returns {Promise<AoshInputOptions>} The input options for the AOSH form + */ -export const getAoshInputOptionsApi = defineTemplateApi({ +export const getAoshInputOptionsApi = defineTemplateApi<AoshInputOptions>({ action: ApiActions.getInputOptions, externalDataId: 'aoshData' as const, }) +// Define the type at the top of the file +interface AoshInputOptions { + // Add the expected shape of your input options + // This ensures type safety when consuming the API +}Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/RepeatableSection/index.ts (1)
4-7: 🛠️ Refactor suggestion
Enhance type safety with explicit return type.
While the current implementation is functional, we can improve type safety and documentation.
Consider applying this enhancement:
-export const EmployeeAndAccidentInformationSection = (index: number) => [ +import { FormSection } from '@island.is/application/core' + +/** + * Combines employee and accident information sections for a given index + * @param index - The index of the repeatable section + * @returns Array of form sections for employee and accident information + */ +export const EmployeeAndAccidentInformationSection = (index: number): FormSection[] => [ employeeSection(index), causeAndConsequencesSection(index), ]Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/lib/constants.ts (1)
13-15:
⚠️ Potential issueExpand Roles enum to distinguish user types.
The PR objectives mention two distinct user types: individual users and company delegates. Consider expanding the Roles enum to explicitly differentiate between these types, as they have different permissions and flows.
Suggested implementation:
export enum Roles { - APPLICANT = 'applicant', + INDIVIDUAL_APPLICANT = 'individual_applicant', + COMPANY_DELEGATE = 'company_delegate', }📝 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.export enum Roles { INDIVIDUAL_APPLICANT = 'individual_applicant', COMPANY_DELEGATE = 'company_delegate', }libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.utils.ts (2)
17-27: 🛠️ Refactor suggestion
Improve type safety and add documentation.
The function needs better type safety and documentation:
- Missing type safety with generics
- Lacks null checks in the mapping chain
- No error handling for malformed data
- Function name could be more descriptive of its purpose
Consider this improved implementation:
+/** + * Extracts and flattens values from a nested form structure + * @param answers - The form answers object + * @param answer - The path to the target answer in the form + * @returns Array of extracted values + * @throws {Error} If the path doesn't exist or data is malformed + */ -export const getValueList = (answers: FormValue, answer: string) => { +export const extractFlattenedValues = <T = string>( + answers: FormValue, + answer: string +): T[] => { + if (!answers || !answer) { + throw new Error('Invalid input: answers and answer path are required') + } + const objectList = getValueViaPath(answers, answer, {}) as object + if (!objectList || typeof objectList !== 'object') { + return [] + } return Object.values(objectList) - .map((values: { label: string; value: string }[]) => { + .map((values: { label: string; value: T }[] | null) => { + if (!Array.isArray(values)) return [] return values?.map(({ value }) => { return value }) }) .flat() }📝 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./** * Extracts and flattens values from a nested form structure * @param answers - The form answers object * @param answer - The path to the target answer in the form * @returns Array of extracted values * @throws {Error} If the path doesn't exist or data is malformed */ export const extractFlattenedValues = <T = string>( answers: FormValue, answer: string ): T[] => { if (!answers || !answer) { throw new Error('Invalid input: answers and answer path are required') } const objectList = getValueViaPath(answers, answer, {}) as object if (!objectList || typeof objectList !== 'object') { return [] } return Object.values(objectList) .map((values: { label: string; value: T }[] | null) => { if (!Array.isArray(values)) return [] return values?.map(({ value }) => { return value }) }) .flat() }
4-15:
⚠️ Potential issueAdd input validation and improve type safety.
The function needs additional error handling and type safety improvements:
- Missing validation for date string format
- No bounds checking for hours (0-23) and minutes (0-59)
- No handling of invalid number strings
- Potential timezone issues
Consider this improved implementation:
export const getDateAndTime = ( date: string, - hours: string, - minutes: string, + hours: string | number, + minutes: string | number, ): Date => { + if (!date || isNaN(Date.parse(date))) { + throw new Error('Invalid date format') + } + const finalDate = new Date(date) + const parsedHours = typeof hours === 'string' ? parseInt(hours, 10) : hours + const parsedMinutes = typeof minutes === 'string' ? parseInt(minutes, 10) : minutes + + if (isNaN(parsedHours) || parsedHours < 0 || parsedHours > 23) { + throw new Error('Hours must be between 0 and 23') + } + + if (isNaN(parsedMinutes) || parsedMinutes < 0 || parsedMinutes > 59) { + throw new Error('Minutes must be between 0 and 59') + } + finalDate.setHours( - parseInt(hours, 10), // hours - parseInt(minutes, 10), // minutes + parsedHours, + parsedMinutes, ) return finalDate }📝 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.export const getDateAndTime = ( date: string, hours: string | number, minutes: string | number, ): Date => { if (!date || isNaN(Date.parse(date))) { throw new Error('Invalid date format') } const finalDate = new Date(date) const parsedHours = typeof hours === 'string' ? parseInt(hours, 10) : hours const parsedMinutes = typeof minutes === 'string' ? parseInt(minutes, 10) : minutes if (isNaN(parsedHours) || parsedHours < 0 || parsedHours > 23) { throw new Error('Hours must be between 0 and 23') } if (isNaN(parsedMinutes) || parsedMinutes < 0 || parsedMinutes > 59) { throw new Error('Minutes must be between 0 and 59') } finalDate.setHours( parsedHours, parsedMinutes, ) return finalDate }libs/application/templates/aosh/work-accident-notification/src/forms/Conclusion.ts (2)
7-11:
⚠️ Potential issueAdd a descriptive title for accessibility
The form's title is currently empty. Since this is a work accident notification form, it should have a meaningful title for accessibility and user experience purposes.
export const Conclusion: Form = buildForm({ id: 'ConclusionApplicationForm', - title: '', + title: 'Work Accident Notification Conclusion', logo: Logo, mode: FormModes.COMPLETED,📝 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.export const Conclusion: Form = buildForm({ id: 'ConclusionApplicationForm', title: 'Work Accident Notification Conclusion', logo: Logo, mode: FormModes.COMPLETED,
23-23: 💡 Codebase verification
Based on the search results, I can see that
buildFormConclusionSectionis consistently used across many templates with specific configuration options likealertTitle,alertMessage,expandableHeader,expandableDescription, etc. Looking at the same template (work-accident-notification), there's another usage with proper configuration:buildFormConclusionSection({ sectionTitle: conclusion.general.sectionTitle, multiFieldTitle: conclusion.general.title, alertTitle: conclusion.default.alertTitle, expandableHeader: conclusion.default.accordionTitle, expandableDescription: conclusion.default.accordionText, })Add standard conclusion section configuration
The empty configuration object should be replaced with standard conclusion section properties using the available message keys:
sectionTitlemultiFieldTitlealertTitleexpandableHeaderexpandableDescription🔗 Analysis chain
Consider adding configuration to the conclusion section
The
buildFormConclusionSectionis called with an empty configuration object. Consider if any specific configuration is needed for the work accident notification context.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other uses of buildFormConclusionSection to understand common patterns ast-grep --pattern 'buildFormConclusionSection({ $$$ })'Length of output: 77546
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/index.ts (2)
19-21:
⚠️ Potential issueImprove type safety and parameter usage.
Several improvements can be made to enhance type safety and code clarity:
- The type casting could be replaced with type guards
- Unused parameters should be removed
- Add validation for negative indices
- condition: (formValue: FormValue, externalData, user) => { - const answers = formValue as WorkAccidentNotification - return index < answers.employeeAmount || index === 0 + condition: (formValue: FormValue) => { + if (!isWorkAccidentNotification(formValue)) { + return false + } + return index >= 0 && (index < formValue.employeeAmount || index === 0) + },Add this type guard function to your utilities:
function isWorkAccidentNotification(value: FormValue): value is WorkAccidentNotification { return 'employeeAmount' in value }
9-9:
⚠️ Potential issueFix the section ID format.
The section ID contains a space which could cause issues in DOM manipulation. Consider using a hyphen or underscore instead.
- id: `employeeSection ${index}`, + id: `employeeSection-${index}`,📝 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.id: `employeeSection-${index}`,libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/deviation.ts (1)
9-36:
⚠️ Potential issueAdd TypeScript type annotations for better type safety.
The function lacks return type annotation and type-safe props for the custom component.
Apply these changes:
- export const deviationSection = (index: number) => + import { FormSection } from '@island.is/application/core' + + interface DeviationProps { + index: number + } + + export const deviationSection = (index: number): FormSection =>Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/typeOfInjury.ts (1)
9-36: 🛠️ Refactor suggestion
Enhance TypeScript type safety and documentation.
While the implementation is functionally correct, consider the following improvements to align with TypeScript best practices:
- Add return type annotation
- Create a type-safe reference to the 'TypeOfInjury' component
- Add JSDoc documentation
- Add type constraint for the index parameter
Here's the suggested implementation:
+ import { FormSection } from '@island.is/application/core' + import { TypeOfInjuryComponent } from '../components/TypeOfInjury' + /** + * Builds a form section for capturing type of injury details + * @param index - The zero-based index of the injury being reported + * @returns A form section configuration for type of injury + */ - export const typeOfInjurySection = (index: number) => + export const typeOfInjurySection = (index: number): FormSection => buildSubSection({ id: `typeOfInjury[${index}]`, title: sections.draft.typeOfInjury, children: [ buildMultiField({ title: causeAndConsequences.typeOfInjury.title, description: causeAndConsequences.typeOfInjury.description, id: `typeOfInjuryMultiField[${index}]`, children: [ buildHiddenInput({ id: `typeOfInjury[${index}].typeOfInjuryMostSerious`, doesNotRequireAnswer: true, }), buildCustomField( { id: `typeOfInjury[${index}].typeOfInjury`, title: '', - component: 'TypeOfInjury', + component: TypeOfInjuryComponent.name, }, { index: index, }, ), ], }), ], })Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/circumstances.ts (1)
9-36:
⚠️ Potential issueAdd input validation for the index parameter.
The index parameter should be validated to prevent potential issues with negative numbers.
export const circumstancesSection = (index: number) => + // Ensure index is non-negative + if (index < 0) { + throw new Error('Circumstance index must be non-negative'); + } buildSubSection({📝 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.export const circumstancesSection = (index: number) => { // Ensure index is non-negative if (index < 0) { throw new Error('Circumstance index must be non-negative'); } buildSubSection({ id: `circumstances[${index}]`, title: sections.draft.circumstances, children: [ buildMultiField({ title: causeAndConsequences.circumstances.title, description: causeAndConsequences.circumstances.description, id: `circumstancesMultiField[${index}]`, children: [ buildHiddenInput({ id: `circumstances[${index}].physicalActivitiesMostSerious`, doesNotRequireAnswer: true, }), buildCustomField( { id: `circumstances[${index}].physicalActivities`, title: '', component: 'Circumstance', }, { index: index, }, ), ], }), ], }) }libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/causeOfInjury.ts (1)
9-36: 🛠️ Refactor suggestion
Add TypeScript interface for custom component props.
The custom component 'CauseOfInjury' is used without type definitions. To improve type safety and maintainability, consider adding an interface.
interface CauseOfInjuryProps { index: number; // Add other props as needed }libs/application/templates/aosh/work-accident-notification/src/fields/TitleWithRemove/index.tsx (3)
8-14: 🛠️ Refactor suggestion
Improve type definitions for better type safety and reusability
The current props interface structure can be simplified and improved:
+export type TitleWithRemoveProps = { + index: number; +} -interface TitleWithRemoveProps { - field: { - props: { - index: number - } - } -}This change would:
- Make the props more straightforward and easier to use
- Allow better type inference
- Make the component more reusable across different contexts
- Export the type for use in other components
📝 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.export type TitleWithRemoveProps = { index: number; }
19-20: 🛠️ Refactor suggestion
Simplify prop access and add proper type guards
The nested optional chaining can be simplified by using the improved props structure suggested earlier.
-const idx = props.field?.props?.index +const { index } = propsCommittable suggestion was skipped due to low confidence.
22-33:
⚠️ Potential issueEnhance accessibility and responsiveness
The grid layout needs improvements for better accessibility and responsiveness:
- <GridRow marginBottom={1} marginTop={3}> + <GridRow marginBottom={1} marginTop={3} role="group" aria-label={formatMessage(employee.employee.employeeInformation)}> - <GridColumn span={['1/2']}> + <GridColumn span={['12/12', '6/12', '6/12']}> <Text fontWeight="semiBold" as="h5"> {formatMessage(employee.employee.employeeInformation)} </Text> </GridColumn> - <GridColumn span={['1/2']}> + <GridColumn span={['12/12', '6/12', '6/12']}> <Box display={'flex'} justifyContent={'flexEnd'}> - <DeleteEmployee {...props} index={idx} /> + <DeleteEmployee {...props} index={index} aria-label={formatMessage(employee.employee.removeEmployee)} /> </Box> </GridColumn> </GridRow>Changes include:
- Added proper ARIA attributes for better screen reader support
- Improved responsive grid spans for better mobile layout
- Added aria-label to the delete button (assuming the message exists)
Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/injuredBodyParts.ts (1)
9-36: 🛠️ Refactor suggestion
Consider enhancing type safety and reusability.
While the implementation is solid, there are opportunities for improvement:
- The
indexparameter could benefit from a more specific type to prevent negative values- The function could be more configurable to enhance reusability across different contexts
Consider applying these improvements:
-export const injuredBodyPartsSection = (index: number) => +type InjuredBodyPartsSectionConfig = { + index: number; + titleMessage?: string; + descriptionMessage?: string; +} + +export const injuredBodyPartsSection = ({ + index, + titleMessage = causeAndConsequences.injuredBodyParts.title, + descriptionMessage = causeAndConsequences.injuredBodyParts.description, +}: InjuredBodyPartsSectionConfig) =>This would:
- Ensure the index is non-negative through TypeScript's type system
- Allow customization of messages for different contexts
- Make the function more reusable across different parts of the application
Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/Deviation/index.tsx (2)
19-20:
⚠️ Potential issueImprove type safety in props handling
The type assertion on answers could be unsafe. Consider using type guards or runtime validation.
- const answers = application.answers as WorkAccidentNotification + const answers = application.answers + if (!isWorkAccidentNotification(answers)) { + throw new Error('Invalid answer type') + }Also, the optional chaining on
field?.props?.indexcould be simplified since the type definition ensures these properties exist.- const idx = props.field?.props?.index + const idx = props.field.props.indexCommittable suggestion was skipped due to low confidence.
7-13: 🛠️ Refactor suggestion
Consider enhancing type definitions for better reusability
The
DeviationPropsinterface could be more reusable across different components by:
- Exporting the interface for external consumption
- Making the index prop more generic to support different data structures
-interface DeviationProps { +export interface DeviationProps { field: { props: { - index: number + index: number + fieldId?: string } } }Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/TypeOfInjury/index.tsx (3)
22-36: 🛠️ Refactor suggestion
Consider adding error boundaries and improving props handling.
The component could benefit from:
- Error boundary implementation for runtime errors
- Explicit props instead of spreading
- Loading state handling
return ( + <ErrorBoundary fallback={<ErrorMessage />}> <CausesAndEffects majorGroupLength={2} externalDataKey={'aoshData.data.typeOfInjury'} heading={causeAndConsequences.typeOfInjury.heading} subHeading={causeAndConsequences.typeOfInjury.subHeading} answerId={`typeOfInjury[${idx}].typeOfInjury`} mostSeriousAnswerId={`typeOfInjury[${idx}].typeOfInjuryMostSerious`} screenId={'typeOfInjury'} mostSeriousAnswer={ (answers?.typeOfInjury?.[idx]?.typeOfInjuryMostSerious as string) || '' } - {...props} + // Pass only required props explicitly + application={application} + field={props.field} /> + </ErrorBoundary> )Committable suggestion was skipped due to low confidence.
7-13: 🛠️ Refactor suggestion
Consider improving type definitions for better reusability and type safety.
The
TypeOfInjuryPropsinterface could be improved:
- Consider flattening the nested structure
- Add explicit type for index
- Consider extracting to a separate types file for reuse
-interface TypeOfInjuryProps { - field: { - props: { - index: number - } - } -} +interface TypeOfInjuryProps { + fieldIndex: number; // Flattened structure +}Committable suggestion was skipped due to low confidence.
18-20:
⚠️ Potential issueImprove type safety in props handling.
The type casting and optional chaining could be replaced with proper type guards:
- Avoid using type assertion with
as- Consider adding validation for required fields
- const answers = application.answers as WorkAccidentNotification - const idx = props.field?.props?.index + const answers = validateAnswers(application.answers) + const idx = props.field.props.index // Since it's required + + function validateAnswers(answers: unknown): WorkAccidentNotification { + if (!answers || typeof answers !== 'object') { + throw new Error('Invalid answers format') + } + return answers as WorkAccidentNotification + }Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/Prerequisites.ts (2)
7-42: 🛠️ Refactor suggestion
Add TypeScript interface for form data structure.
To improve type safety and documentation, consider adding a dedicated interface for the form data structure.
interface WorkAccidentPrerequisites { information: unknown; // TODO: Define specific types accident: unknown; employee: unknown; overview: unknown; conclusion: unknown; } export const Prerequisites: Form<WorkAccidentPrerequisites> = buildForm({ // ... rest of the code });
7-42: 💡 Codebase verification
Prerequisites form needs validation implementation
The prerequisites form currently lacks necessary validation rules. The form uses external data providers for identity and user profile information but doesn't validate:
- User type (person vs company) through
identity.data.type- Company delegation permissions
- Required fields based on user type
- Add validation to
Prerequisites.tsusing thevalidationproperty inbuildForm:export const Prerequisites: Form = buildForm({ // ... existing properties validation: { type: { presence: true }, delegationPermissions: { presence: { if: (value, attributes) => attributes.type === 'company' } } } })🔗 Analysis chain
Consider adding validation rules for prerequisites.
Based on the PR objectives, the form needs to validate:
- User type (individual vs company delegation)
- Company delegation permissions
- Required fields based on user type
Let's check if validation rules exist elsewhere:
Consider implementing these validations using the form validation system from @island.is/application/core to maintain consistency with the platform's architecture.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for validation implementations rg -t typescript "validate|validation" libs/application/templates/aosh/work-accident-notification/src/Length of output: 139
Script:
#!/bin/bash # Search for validation implementations with ripgrep, without file type restriction rg "validate|validation" libs/application/templates/aosh/work-accident-notification/src/ # Also search for prerequisite-related code rg "prerequisite" libs/application/templates/aosh/work-accident-notification/src/ # Look for form validation patterns using ast-grep ast-grep --pattern 'buildForm({ $$$ validation: $_, $$$ })'Length of output: 3027
Script:
#!/bin/bash # Let's check the content of prerequisitesSection to see if it contains any validation cat libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/prerequisitesSection.ts # Let's also search for any validation patterns in the entire codebase ast-grep --pattern 'validate = ($$$) => { $$$ }' # And check for validation rules in other similar forms rg -A 5 "buildForm|buildSection" libs/application/templates/aosh/work-accident-notification/src/Length of output: 25752
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/InformationSection/workhealth.ts (4)
10-13: 🛠️ Refactor suggestion
Add TypeScript type for the exported section
To improve type safety and maintain consistency with TypeScript usage guidelines for library code, consider adding explicit type information.
+import { FormSection } from '@island.is/application/core' -export const workhealthSection = buildSubSection({ +export const workhealthSection: FormSection = buildSubSection({📝 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.import { FormSection } from '@island.is/application/core' export const workhealthSection: FormSection = buildSubSection({ id: 'workhealthSection', title: information.labels.workhealth.sectionTitle, children: [
24-30:
⚠️ Potential issueAdd a meaningful title to the alert field
Empty titles may impact accessibility. Consider adding a descriptive title that matches the alert's purpose.
buildAlertMessageField({ id: 'companyLaborProtection.alertField', alertType: 'info', - title: '', + title: information.labels.laborProtection.alertTitle, message: information.labels.laborProtection.alertMessageText, doesNotRequireAnswer: true, }),Committable suggestion was skipped due to low confidence.
31-35:
⚠️ Potential issueEnhance the checkbox field with validation and accessibility
The checkbox field is missing important properties for validation and accessibility.
buildCustomField({ id: 'companyLaborProtection.workhealthAndSafetyOccupation', - title: '', + title: information.labels.laborProtection.checkboxTitle, component: 'CheckboxFieldCustom', + required: true, + props: { + 'aria-label': information.labels.laborProtection.checkboxAriaLabel, + }, }),📝 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.buildCustomField({ id: 'companyLaborProtection.workhealthAndSafetyOccupation', title: information.labels.laborProtection.checkboxTitle, component: 'CheckboxFieldCustom', required: true, props: { 'aria-label': information.labels.laborProtection.checkboxAriaLabel, }, }),
1-9: 🛠️ Refactor suggestion
Consider importing specific messages for better tree-shaking
Instead of importing the entire
informationobject, consider importing only the required message keys to enable better tree-shaking.-import { information } from '../../../lib/messages' +import { + laborProtectionMessages, + workhealthMessages, + generalMessages +} from '../../../lib/messages'Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/Circumstance/index.tsx (2)
7-13: 🛠️ Refactor suggestion
Enhance type definitions and documentation for better reusability.
Consider the following improvements to make the component more maintainable and reusable:
- Export the
CircumstancePropsinterface for potential reuse.- Add JSDoc documentation to describe the purpose and usage of the interface.
- Consider making the interface more descriptive by renaming it to
CircumstanceFieldProps.+/** + * Props for the circumstance field component that handles physical activities in work accidents. + * @interface CircumstanceFieldProps + */ -interface CircumstanceProps { +export interface CircumstanceFieldProps { field: { props: { + /** Index of the circumstance in the array of circumstances */ index: number } } }📝 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./** * Props for the circumstance field component that handles physical activities in work accidents. * @interface CircumstanceFieldProps */ export interface CircumstanceFieldProps { field: { props: { /** Index of the circumstance in the array of circumstances */ index: number } } }
18-20:
⚠️ Potential issueAdd validation for required props and improve type safety.
The type casting and optional chaining suggest potential runtime issues:
- Add validation for required props
- Consider using type guards instead of direct casting
- const { application } = props - const answers = application.answers as WorkAccidentNotification - const idx = props.field?.props?.index + const { application, field } = props + if (!field?.props?.index) { + throw new Error('Index is required for Circumstance component') + } + const answers = application.answers + if (!isWorkAccidentNotification(answers)) { + throw new Error('Invalid answers type') + } + const idx = field.props.index +// Add type guard +function isWorkAccidentNotification( + answers: unknown +): answers is WorkAccidentNotification { + return (answers as WorkAccidentNotification)?.circumstances !== undefined +}📝 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.const { application, field } = props if (!field?.props?.index) { throw new Error('Index is required for Circumstance component') } const answers = application.answers if (!isWorkAccidentNotification(answers)) { throw new Error('Invalid answers type') } const idx = field.props.index // Add type guard function isWorkAccidentNotification( answers: unknown ): answers is WorkAccidentNotification { return (answers as WorkAccidentNotification)?.circumstances !== undefined }libs/application/templates/aosh/work-accident-notification/src/fields/InjuredBodyParts/index.tsx (3)
15-20:
⚠️ Potential issueImprove type safety in props handling.
The type casting of
answersusingasis unsafe. Consider using type guards or runtime validation.Consider this safer approach:
- const answers = application.answers as WorkAccidentNotification + const answers = application.answers + if (!isWorkAccidentNotification(answers)) { + throw new Error('Invalid answers type') + }Add this type guard:
function isWorkAccidentNotification(answers: unknown): answers is WorkAccidentNotification { return answers !== null && typeof answers === 'object'; }
1-13: 🛠️ Refactor suggestion
Consider improving type definitions and imports.
- The nested structure in
InjuredBodyPartsPropscould be flattened for better maintainability.- Add type safety for the index property.
- Import types explicitly from React.
Consider this refactoring:
-import { FC } from 'react' +import type { FC, PropsWithChildren } from 'react' interface InjuredBodyPartsProps { - field: { - props: { - index: number - } - } + index: number }This makes the component more maintainable and type-safe while following React best practices.
📝 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.import { FieldBaseProps } from '@island.is/application/types' import type { FC, PropsWithChildren } from 'react' import { causeAndConsequences } from '../../lib/messages' import { WorkAccidentNotification } from '../../lib/dataSchema' import { CausesAndEffects } from '../Components/CausesAndEffects' interface InjuredBodyPartsProps { index: number }
22-37: 💡 Codebase verification
🛠️ Refactor suggestion
Props spreading is safe, but error handling can be improved
The review comment is partially correct:
The prop spreading is actually safe in this case as
CausesAndEffectsPropshas a well-defined interface that extendsFieldBaseProps. The spread operator is only passing the expected props toMultiSelectDropdownController.However, the error handling suggestion is valid. While the component has some error handling for form validation, it lacks proper error boundaries for runtime errors and doesn't handle loading states for external data.
Specific improvements needed:
- Add error handling for
application.externalDataaccess- Add loading state while external data is being fetched
- Consider wrapping with ErrorBoundary for runtime error handling
🔗 Analysis chain
Review prop spreading and add error handling.
- The spread operator
{...props}could lead to prop pollution in theCausesAndEffectscomponent.- Missing error boundaries and loading states.
Let's verify the
CausesAndEffectscomponent's prop types:Consider these improvements:
return ( + <ErrorBoundary fallback={<ErrorMessage />}> <CausesAndEffects majorGroupLength={1} externalDataKey={'aoshData.data.partOfBodyInjured'} heading={causeAndConsequences.injuredBodyParts.heading} subHeading={causeAndConsequences.injuredBodyParts.subHeading} answerId={`injuredBodyParts[${idx}].partOfBodyInjured`} mostSeriousAnswerId={`injuredBodyParts[${idx}].partOfBodyInjuredMostSerious`} screenId={'injuredBodyParts'} mostSeriousAnswer={ - (answers?.injuredBodyParts?.[idx] - ?.partOfBodyInjuredMostSerious as string) || '' + answers.injuredBodyParts?.[idx]?.partOfBodyInjuredMostSerious ?? '' } - {...props} + // Only spread necessary props + application={application} + field={field} /> + </ErrorBoundary> )🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Find CausesAndEffects component definition ast-grep --pattern 'interface $name { $$$ }' | rg -A 10 'CausesAndEffects'Length of output: 76
Script:
#!/bin/bash # Let's try different approaches to find CausesAndEffects component # 1. Find the component definition ast-grep --pattern 'const CausesAndEffects = $$$' # 2. Find any imports of CausesAndEffects rg "import.*CausesAndEffects.*from" # 3. Find type/interface definitions related to CausesAndEffects rg "type.*CausesAndEffects|interface.*CausesAndEffects" # 4. Look for the actual component file fd "CausesAndEffects"Length of output: 1201
Script:
#!/bin/bash # Let's examine the CausesAndEffects component's props and implementation cat libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsxLength of output: 6254
libs/application/templates/aosh/work-accident-notification/src/lib/messages/conclusion.ts (2)
1-33: 🛠️ Refactor suggestion
Consider adding TypeScript type definitions.
While the code is functional, adding explicit TypeScript types would improve maintainability and align with the coding guidelines for libs.
Consider adding type definitions:
type MessageDescriptor = { id: string; defaultMessage: string; description: string; }; type ConclusionMessages = { general: { title: MessageDescriptor; sectionTitle: MessageDescriptor; }; default: { alertTitle: MessageDescriptor; accordionTitle: MessageDescriptor; accordionText: MessageDescriptor; }; }; export const conclusion: ConclusionMessages = { // ... existing code };
16-32:
⚠️ Potential issueReplace placeholder content and fix incorrect context references.
Several issues need attention:
- The accordionText contains Lorem Ipsum placeholder text instead of actual information about the next steps after submitting a work accident report
- The descriptions incorrectly reference "seller" instead of the work accident notification context
Consider applying these changes:
alertTitle: { id: 'aosh.wan.application:conclusion.default.alertTitle', defaultMessage: 'Tilkynning um vinnuslys móttekin!', - description: 'Conclusion seller alert title', + description: 'Alert title confirming work accident report submission', }, accordionTitle: { id: 'aosh.wan.application:conclusion.default.accordionTitle', defaultMessage: 'Hvað gerist næst?', - description: 'Conclusion seller accordion title', + description: 'Title for section explaining next steps after accident report submission', }, accordionText: { id: 'aosh.wan.application:conclusion.default.accordionText', - defaultMessage: `Eu velit scelerisque quam quisque sit vivamus sed ullamcorper. Dapibus tristique et nisi nec. Lobortis et sagittis eget at. Nulla morbi bibendum ipsum nullam donec feugiat lacus. Ultrices nunc tortor orci at. Non enim mi sagittis interdum volutpat.`, + defaultMessage: `After submitting the work accident notification, VER (Vinnueftirlitið) will review the report. You may be contacted if additional information is needed.`, - description: 'Conclusion seller accordion text', + description: 'Explanation of what happens after submitting a work accident report', },📝 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.default: defineMessages({ alertTitle: { id: 'aosh.wan.application:conclusion.default.alertTitle', defaultMessage: 'Tilkynning um vinnuslys móttekin!', description: 'Alert title confirming work accident report submission', }, accordionTitle: { id: 'aosh.wan.application:conclusion.default.accordionTitle', defaultMessage: 'Hvað gerist næst?', description: 'Title for section explaining next steps after accident report submission', }, accordionText: { id: 'aosh.wan.application:conclusion.default.accordionText', defaultMessage: `After submitting the work accident notification, VER (Vinnueftirlitið) will review the report. You may be contacted if additional information is needed.`, description: 'Explanation of what happens after submitting a work accident report', }, }),libs/application/templates/aosh/work-accident-notification/src/fields/CauseOfInjury/index.tsx (3)
8-11: 🛠️ Refactor suggestion
Consider moving OptionAndKey type to a shared types file.
For better reusability across the application, consider moving this type to
libs/application/templates/aosh/work-accident-notification/src/types/index.ts.
13-19: 🛠️ Refactor suggestion
Consider interface improvements for better maintainability.
- Export the interface for potential reuse
- Consider flattening the props structure
-interface CauseOfInjuryProps { +export interface CauseOfInjuryProps { - field: { - props: { - index: number - } - } + index: number }📝 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.export interface CauseOfInjuryProps { index: number }
24-26:
⚠️ Potential issueAdd proper type guards and error handling.
The type casting and property access could lead to runtime errors if the data is undefined.
- const answers = application.answers as WorkAccidentNotification - const idx = props.field?.props?.index + const answers = application.answers as WorkAccidentNotification | undefined + const idx = props.field?.props?.index + if (idx === undefined) { + throw new Error('Index is required for CauseOfInjury component') + }Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.ts (4)
1-11: 🛠️ Refactor suggestion
Consider exporting and importing section types explicitly.
For better type safety and documentation, consider explicitly importing the return types of your form sections (e.g.,
AnnouncementSection,InformationSection, etc.).import { buildForm } from '@island.is/application/core' import { Form, FormModes, Section } from '@island.is/application/types' +import { AnnouncementSection } from './AnnouncementSection/types' +import { InformationSection } from './InformationSection/types' +import { AccidentSection } from './AccidentSection/types' import { informationSection } from './InformationSection'Committable suggestion was skipped due to low confidence.
13-18: 🛠️ Refactor suggestion
Improve section generation implementation.
The current implementation has several potential issues:
- The magic number
20should be defined as a named constant- Creating 20 sections upfront might impact performance
- No validation for the maximum number of sections
Consider this implementation:
+const MAX_ACCIDENT_SECTIONS = 20 + const buildRepeatableSections = (): Section[] => { - const sections = [...Array(20)].map((_key, index) => { + const sections = [...Array(MAX_ACCIDENT_SECTIONS)].map((_key, index) => { + if (index >= MAX_ACCIDENT_SECTIONS) { + throw new Error(`Maximum number of sections (${MAX_ACCIDENT_SECTIONS}) exceeded`) + } return EmployeeAndAccidentInformationSection(index) }) return sections.flat() }Committable suggestion was skipped due to low confidence.
20-24:
⚠️ Potential issueAdd title for accessibility and documentation.
The form is missing a title, which is important for accessibility and documentation purposes. Consider adding a descriptive title that matches the application's purpose.
export const WorkAccidentNotificationForm: Form = buildForm({ id: 'WorkAccidentNotificationFormsDraft', - title: '', + title: 'Work Accident Notification Form', logo: Logo, mode: FormModes.DRAFT,📝 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.export const WorkAccidentNotificationForm: Form = buildForm({ id: 'WorkAccidentNotificationFormsDraft', title: 'Work Accident Notification Form', logo: Logo, mode: FormModes.DRAFT,
27-40: 💡 Codebase verification
🛠️ Refactor suggestion
Add error handling and documentation to improve form maintainability
The verification confirms that the form lacks both error handling mechanisms and proper documentation. Specifically:
- No error boundaries or try-catch blocks are implemented
- Missing JSDoc documentation for the form structure and its sections
- Form uses a mix of static and dynamic sections without clear documentation of their relationships
Action items:
- Add error handling for the dynamic
buildRepeatableSections()function- Document the form structure and section relationships using JSDoc
- Consider implementing error boundaries for the form sections
🔗 Analysis chain
Consider adding error boundaries and documentation.
The form combines static and dynamic sections without proper error handling or documentation about the structure and flow.
- Add JSDoc documentation:
/** * Work Accident Notification Form * * Form structure: * 1. Announcement Section - Initial information * 2. Information Section - Basic details * 3. Accident Section - Accident specific information * 4. Repeatable Sections - Multiple employee/accident entries * 5. Overview Section - Summary * 6. Conclusion Section - Final steps * * @returns {Form} The configured form instance */ export const WorkAccidentNotificationForm: Form = ...
- Let's verify the section implementations:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all imported sections are properly implemented # and exported with TypeScript types # Check for section type definitions echo "Checking for section type definitions..." rg -t typescript "export (type|interface)" ./libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/Length of output: 387
Script:
#!/bin/bash # Let's verify the form structure and error handling # Check for error boundary implementations echo "Checking for error boundaries..." rg -l "ErrorBoundary|error.*boundary|try.*catch" ./libs/application/templates/aosh/work-accident-notification/ # Check section implementations echo "Checking section implementations..." rg -l "Section|section" ./libs/application/templates/aosh/work-accident-notification/src/forms/ # Look for documentation echo "Checking for documentation..." rg -l "@returns|@param|\/\*\*" ./libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/Length of output: 3470
Script:
#!/bin/bash # Let's check for error handling and documentation with fixed regex # Check for error handling echo "Checking for error handling..." rg "try.*catch|throw|Error" ./libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/ # Look for documentation with fixed regex echo "Checking for documentation..." rg "(/\*\*|\* @|\*/)" ./libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/ # Check form structure and types echo "Checking form structure..." rg "type|interface|Form" ./libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/index.tsLength of output: 963
libs/application/templates/aosh/work-accident-notification/src/fields/Components/ReviewGroup.tsx (3)
50-50:
⚠️ Potential issueFix the flexbox justify-content value.
The value 'flexEnd' is incorrect and should be 'flex-end'.
- justifyContent={'flexEnd'} + justifyContent={'flex-end'}📝 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.justifyContent={'flex-end'}
52-54:
⚠️ Potential issueAdd null check for handleClick prop.
The onClick handler should be checked for existence since it's optional.
- <Button variant="utility" icon="pencil" onClick={handleClick}> + <Button + variant="utility" + icon="pencil" + onClick={handleClick ?? (() => {})} + >📝 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.<Button variant="utility" icon="pencil" onClick={handleClick ?? (() => {})} > {editMessage} </Button>
33-61: 🛠️ Refactor suggestion
Consider enhancing responsive design.
The grid layout could be more refined for different screen sizes. Consider adjusting the column spans for better responsive behavior.
- <GridColumn span={['1/2', '1/2', '1/2', '4/7', '2/3']}> + <GridColumn span={['12/12', '6/12', '6/12', '4/7', '2/3']}> - <GridColumn span={['1/2', '1/2', '1/2', '3/7', '1/3']}> + <GridColumn span={['12/12', '6/12', '6/12', '3/7', '1/3']}>📝 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.<GridRow> <GridColumn span={['12/12', '6/12', '6/12', '4/7', '2/3']}> <Box display={'flex'} alignItems={'center'} width="full" height="full" > <Text variant="h4">{title}</Text> </Box> </GridColumn> {editMessage && ( <GridColumn span={['12/12', '6/12', '6/12', '3/7', '1/3']}> <Box paddingBottom={2} paddingTop={2} display={'flex'} justifyContent={'flexEnd'} > <Button variant="utility" icon="pencil" onClick={handleClick}> {editMessage} </Button> </Box> </GridColumn> )} </GridRow> <GridRow> <GridColumn>{children}</GridColumn> </GridRow>libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/prerequisitesSection.ts (1)
1-16: 🛠️ Refactor suggestion
Consider exporting types for external data structure.
While the imports are well-organized, considering this is in the
libsdirectory, we should export TypeScript interfaces for the expected data structure from each provider to enhance reusability across different apps.Add type definitions:
export interface AoshInputOptions { // Define the expected structure from getAoshInputOptionsApi } export interface WorkAccidentPrerequisitesData { identity: Identity; userProfile: UserProfile; aoshOptions: AoshInputOptions; }libs/application/templates/aosh/work-accident-notification/src/lib/messages/shared.ts (3)
34-45: 🛠️ Refactor suggestion
Consider moving common options to a shared library.
The Yes/No options are universal and could be moved to a common shared messages library for reuse across different applications.
Consider creating a new shared messages file at
libs/shared/messages/common.tsfor these universal options.
1-3: 🛠️ Refactor suggestion
Consider adding TypeScript types for better type safety.
To improve type safety and maintainability, consider defining TypeScript interfaces for the message structure.
import { defineMessages } from 'react-intl' +interface ApplicationMessage { + id: string + defaultMessage: string + description: string +} + +interface SharedMessages { + application: Record<string, ApplicationMessage> + options: Record<string, ApplicationMessage> + buttons: Record<string, ApplicationMessage> +} + export const shared: SharedMessages = {📝 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.import { defineMessages } from 'react-intl' interface ApplicationMessage { id: string defaultMessage: string description: string } interface SharedMessages { application: Record<string, ApplicationMessage> options: Record<string, ApplicationMessage> buttons: Record<string, ApplicationMessage> } export const shared: SharedMessages = {
46-52: 🛠️ Refactor suggestion
Add confirmation message for destructive action.
The delete employee button represents a destructive action. Consider adding a confirmation message to prevent accidental deletions.
buttons: defineMessages({ deleteEmployee: { id: 'aosh.wan.application:shared.buttons.deleteEmployee', defaultMessage: 'Eyða starfsmanni', description: 'Delete employee', }, + deleteEmployeeConfirmation: { + id: 'aosh.wan.application:shared.buttons.deleteEmployeeConfirmation', + defaultMessage: 'Ertu viss um að þú viljir eyða starfsmanni?', + description: 'Confirmation message for deleting an employee', + }, }),📝 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.buttons: defineMessages({ deleteEmployee: { id: 'aosh.wan.application:shared.buttons.deleteEmployee', defaultMessage: 'Eyða starfsmanni', description: 'Delete employee', }, deleteEmployeeConfirmation: { id: 'aosh.wan.application:shared.buttons.deleteEmployeeConfirmation', defaultMessage: 'Ertu viss um að þú viljir eyða starfsmanni?', description: 'Confirmation message for deleting an employee', }, }),libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/CauseAndConsequencesSection/absence.ts (1)
29-45: 🛠️ Refactor suggestion
Enhance data transformation safety
While the code handles basic null checks, there are opportunities to improve type safety and error handling:
- The type assertion could be replaced with proper type narrowing
- Default values could be more explicit
- const absenceDueToAccident = getValueViaPath( - application.externalData, - 'aoshData.data.absenceDueToAccident', - [], - ) as AbsenceDueToAccidentDto[] + const absenceDueToAccident = getValueViaPath<AbsenceDueToAccidentDto[]>( + application.externalData, + 'aoshData.data.absenceDueToAccident', + [], + ) return absenceDueToAccident - .filter((absence) => absence?.code && absence?.name) + .filter((absence): absence is AbsenceDueToAccidentDto => + Boolean(absence?.code && absence?.name)) .map(({ name, code }) => ({ - label: name || '', - value: code || '', + label: name, + value: code, }))Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/AnnouncementSection/index.ts (3)
1-58: 🛠️ Refactor suggestion
Consider adding explicit TypeScript interfaces
To improve type safety and maintainability, consider defining interfaces for the section configuration and external data structure.
interface IdentityData { type: string; nationalId: string; name: string; } interface ExternalData { identity: { data: IdentityData; }; } interface AnnouncementSectionProps { formValue: FormValue; externalData: ExternalData; user: unknown; }
35-54:
⚠️ Potential issueAdd error handling for external data access
The dynamic title construction could fail if external data is undefined. Consider adding null checks and fallback values.
- const ssn = getValueViaPath( - application.externalData, - 'identity.data.nationalId', - ) - const name = getValueViaPath( - application.externalData, - 'identity.data.name', - ) + const ssn = getValueViaPath( + application.externalData, + 'identity.data.nationalId', + ) ?? 'N/A' + const name = getValueViaPath( + application.externalData, + 'identity.data.name', + ) ?? 'Unknown'📝 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.id: 'externalData.secondHeading', title: (application: Application) => { const ssn = getValueViaPath( application.externalData, 'identity.data.nationalId', ) ?? 'N/A' const name = getValueViaPath( application.externalData, 'identity.data.name', ) ?? 'Unknown' return { ...externalData.dataProvider.announcementHeadingSecond, values: { notandi: `${name} (${ssn})`, }, } }, titleVariant: 'h4', marginBottom: 3, }),
10-16: 🛠️ Refactor suggestion
Consider improving type safety in the condition
The type assertion
as stringcould be replaced with proper type checking to ensure runtime safety.- const type = getValueViaPath(externalData, 'identity.data.type') as string + const type = getValueViaPath(externalData, 'identity.data.type') + return typeof type === 'string' && type === 'person'📝 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.export const announcementSection = buildSection({ id: 'externalData', title: externalData.dataProvider.announcement, condition: (formValue: FormValue, externalData, user) => { const type = getValueViaPath(externalData, 'identity.data.type') return typeof type === 'string' && type === 'person' },libs/application/templates/aosh/work-accident-notification/src/utils/getAccidentInformationForOverview.ts (2)
24-26:
⚠️ Potential issueConsider handling missing municipality data.
The municipality lookup could return undefined, but there's no fallback handling.
Add a fallback for missing municipality data:
const chosenMunicipality = municipalities.find( ({ code }) => accident.municipality === code, ) + if (!chosenMunicipality) { + console.warn(`Municipality not found for code: ${accident.municipality}`); + }Committable suggestion was skipped due to low confidence.
28-52: 🛠️ Refactor suggestion
Improve code maintainability and type safety.
The overview array construction could be more maintainable and type-safe:
- The time formatting is brittle (slicing without validation)
- The template strings could benefit from being constants
- The filter at the end doesn't guarantee type safety
Consider these improvements:
+const formatTime = (time: string): string => { + if (!/^\d{4}$/.test(time)) { + return time; + } + return `${time.slice(0, 2)}:${time.slice(2, 4)}`; +} +interface OverviewField { + label: keyof typeof overview.labels; + getValue: (data: { accident: AccidentType; employees: EmployeeType[]; municipality?: MunicipalityDto }) => string; +} +const OVERVIEW_FIELDS: OverviewField[] = [ + { + label: 'dateAndTime', + getValue: ({ accident }) => `${formatDate(accident.date)}, ${formatTime(accident.time)}`, + }, + // ... other fields +]; return [ - `${formatMessage(overview.labels.dateAndTime)}: ${formatDate( - accident.date, - )}, ${accident.time.slice(0, 2)}:${accident.time.slice(2, 4)}`, + ...OVERVIEW_FIELDS.map(({ label, getValue }) => { + const value = getValue({ accident, employees, municipality: chosenMunicipality }); + return value ? `${formatMessage(overview.labels[label])}: ${value}` : null; + }), ].filter((n): n is string => Boolean(n))Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/AddEmployee/index.tsx (3)
50-64: 🛠️ Refactor suggestion
Enhance accessibility and user feedback.
The UI components need accessibility improvements and loading state feedback.
<Box> <Text variant="h5" marginBottom={2}> {formatMessage(overview.labels.addEmployeeDescription)} </Text> <Button variant="ghost" icon="add" fluid onClick={addEmployeeAndGoToScreen} + loading={isLoading} + disabled={isLoading} + aria-label={formatMessage(overview.labels.addEmployeeButton)} > {formatMessage(overview.labels.addEmployeeButton)} </Button> </Box>Committable suggestion was skipped due to low confidence.
11-21: 🛠️ Refactor suggestion
Consider adding type validation and constants.
The
employeeAmountvalue could benefit from:
- Type validation to ensure positive numbers
- A constant for the 'employeeAmount' path string
+const EMPLOYEE_AMOUNT_PATH = 'employeeAmount' as const; + +type EmployeeAmount = number & { readonly brand: unique symbol } + +function validateEmployeeAmount(value: number): EmployeeAmount { + if (value < 0) throw new Error('Employee amount must be non-negative'); + return value as EmployeeAmount; +} const employeeAmount = getValueViaPath( application.answers, - 'employeeAmount', + EMPLOYEE_AMOUNT_PATH, ) as number | undefinedCommittable suggestion was skipped due to low confidence.
23-48:
⚠️ Potential issueImplement proper error handling and loading state.
The function has several areas that need improvement:
- Empty error handling blocks
- No loading state during mutation
- Missing optional chaining (as flagged by static analysis)
+const [isLoading, setIsLoading] = useState(false); const addEmployeeAndGoToScreen = async () => { if (employeeAmount !== undefined) { + setIsLoading(true); setValue('employeeAmount', employeeAmount + 1) - const res = await updateApplication({ - variables: { - input: { - id: application.id, - answers: { - ...application.answers, - employeeAmount: employeeAmount + 1, - }, - }, - locale, - }, - }) + try { + const res = await updateApplication({ + variables: { + input: { + id: application.id, + answers: { + ...application.answers, + employeeAmount: employeeAmount + 1, + }, + }, + locale, + }, + }); + goToScreen?.(`employeeInformation[${employeeAmount}]`); + } catch (error) { + // Handle error appropriately + console.error('Failed to update employee:', error); + } finally { + setIsLoading(false); + } } else { - // Error because employee amount is undefined + console.error('Employee amount is undefined'); } }Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 41-41: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/aosh/work-accident-notification/src/utils/getCompanyInformationForOverview.ts (2)
40-61: 🛠️ Refactor suggestion
Refactor for improved maintainability and readability.
The return statement contains complex formatting logic that could be better organized.
Consider these improvements:
+ const formatWorkHealthInfo = ( + workplaceHealthAndSafety: WorkplaceHealthAndSafetyDto[], + companyLaborProtection: CompanyLaborProtectionType, + formatMessage: FormatMessage + ) => { + if (workplaceHealthAndSafety.length === 0) return '' + const occupations = companyLaborProtection.workhealthAndSafetyOccupation + ?.map((c) => workplaceHealthAndSafety.find((x) => c === x.code)?.name) + ?.filter(Boolean) + ?.join(', ') + return `${formatMessage(information.labels.workhealth.sectionTitle)}: ${occupations}` + } return [ company.name ?? undefined, company.nationalId ? formatKennitala(company.nationalId) : undefined, company.address && company.postnumber ? `${company.address}, ${company.postnumber}` : undefined, company.industryClassification ?? undefined, chosenSizeOfEnterprise?.name ?? undefined, - `${formatMessage(information.labels.workhealth.sectionTitle)}: ${ - workplaceHealthAndSafety.length > 0 - ? companyLaborProtection.workhealthAndSafetyOccupation - ?.map((c) => { - return workplaceHealthAndSafety.find((x) => c === x.code)?.name - }) - ?.join(', ') - : '' - }`, + formatWorkHealthInfo(workplaceHealthAndSafety, companyLaborProtection, formatMessage), company.email ? `${formatMessage(overview.labels.email)}: ${company.email}` : undefined, company.phonenumber ? `${formatMessage(overview.labels.phonenumber)}: ${formatPhoneNumber(company.phonenumber)}` : undefined, - ].filter((n) => n) + ].filter((item): item is string => Boolean(item))Committable suggestion was skipped due to low confidence.
21-38:
⚠️ Potential issueAdd type guards and null checks for better type safety.
The current implementation relies heavily on type assertions and doesn't handle potential undefined values properly.
Consider applying these improvements:
- const company = getValueViaPath(answers, 'companyInformation') as CompanyType + const company = getValueViaPath(answers, 'companyInformation') + if (!company || typeof company !== 'object') { + throw new Error('Invalid company information') + } + const typedCompany = company as CompanyType - const companyLaborProtection = getValueViaPath( + const maybeLaborProtection = getValueViaPath( answers, 'companyLaborProtection', - ) as CompanyLaborProtectionType + ) + if (!maybeLaborProtection || typeof maybeLaborProtection !== 'object') { + throw new Error('Invalid labor protection data') + } + const companyLaborProtection = maybeLaborProtection as CompanyLaborProtectionTypeCommittable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/InformationSection/projectPurchase.ts (1)
61-70: 🛠️ Refactor suggestion
Consider adding name field validation
The name field could benefit from input validation to ensure proper formatting and prevent special characters.
buildTextField({ id: 'projectPurchase.name', title: information.labels.projectPurchase.name, backgroundColor: 'blue', width: 'half', required: true, doesNotRequireAnswer: true, + validate: (value) => { + if (!/^[a-zA-ZÀ-ÿ\s'-]+$/.test(value)) { + return 'Name contains invalid characters'; + } + return undefined; + }, condition: (answer: FormValue) =>📝 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.buildTextField({ id: 'projectPurchase.name', title: information.labels.projectPurchase.name, backgroundColor: 'blue', width: 'half', required: true, doesNotRequireAnswer: true, validate: (value) => { if (!/^[a-zA-ZÀ-ÿ\s'-]+$/.test(value)) { return 'Name contains invalid characters'; } return undefined; }, condition: (answer: FormValue) => getValueViaPath(answer, 'projectPurchase.radio') === YES, }),libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdown.tsx (4)
21-25: 🛠️ Refactor suggestion
Optimize component declaration and performance.
- Remove unnecessary
React.PropsWithChildrenas this component doesn't render children.- Consider memoizing the component to prevent unnecessary re-renders.
-export const MultiSelectDropdown: FC< - React.PropsWithChildren<MultiSelectDropdownProps & FieldBaseProps> -> = (props) => { +export const MultiSelectDropdown: FC<MultiSelectDropdownProps & FieldBaseProps> = memo((props) => { const { group, options, onChange, values, majorGroupLength } = propsDon't forget to add the import:
-import { FC } from 'react' +import { FC, memo } from 'react'📝 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.export const MultiSelectDropdown: FC<MultiSelectDropdownProps & FieldBaseProps> = memo((props) => { const { group, options, onChange, values, majorGroupLength } = props ``` Note: For this suggestion to work properly, the import statement at the top of the file needs to be updated to include `memo`. Since the import statement isn't shown in the original code snippet, I'm only showing the modified component declaration part. The developer should ensure they add the import: ```typescript import { FC, memo } from 'react'
46-72: 🛠️ Refactor suggestion
Optimize onChange handler implementation.
The onChange handler is complex and could benefit from:
- Extraction into a separate memoized function
- Simplification of the action handling logic
+const useHandleChange = (onChange: MultiSelectDropdownProps['onChange'], group: Group, majorGroupLength: number) => { + return useCallback((ev: any, meta: any) => { + const code = group.code.substring(0, majorGroupLength); + const getOptions = (values: any) => + Array.isArray(values) + ? values.map(({ value, label }) => ({ value, label })) + : [values]; + + switch (meta.action) { + case 'select-option': + onChange(getOptions(ev) as Option[], code, true, meta.option?.value ?? ''); + break; + case 'remove-value': + case 'deselect-option': + const value = meta.action === 'remove-value' ? meta.removedValue : meta.option; + onChange(getOptions(value) as Option[], code, false, value?.value ?? ''); + break; + } + }, [onChange, group.code, majorGroupLength]); +}; // In component: +const handleChange = useHandleChange(onChange, group, majorGroupLength); -onChange={(ev, meta) => { - if (meta.action === 'select-option') { - // ... existing code - } else if (meta.action === 'remove-value') { - // ... existing code - } else if (meta.action === 'deselect-option') { - // ... existing code - } -}} +onChange={handleChange}Committable suggestion was skipped due to low confidence.
31-44:
⚠️ Potential issueImprove Select component configuration and accessibility.
- The
nameprop is hardcoded to "tag-select"- The
backgroundColoris hardcoded to "blue"- Missing accessibility attributes
<Select id={group.name} - backgroundColor="blue" + backgroundColor="white" - name="tag-select" + name={`group-${group.code}`} isMulti={true} label={group.name} + aria-label={`Select ${group.name}`} closeMenuOnSelect={false} hideSelectedOptions={false}📝 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.<Select id={group.name} backgroundColor="white" name={`group-${group.code}`} isMulti={true} label={group.name} aria-label={`Select ${group.name}`} closeMenuOnSelect={false} hideSelectedOptions={false} options={options.map((option) => ({ value: option.code, label: option.name, withCheckmark: true, isSelected: option.isSelected, }))}
8-19: 🛠️ Refactor suggestion
Enhance type safety with literal types and readonly modifiers.
The props interface could be improved for better type safety:
type MultiSelectDropdownProps = { - group: Group - options: Item[] + readonly group: Group + readonly options: readonly Item[] onChange: ( values: Option[], - code: string, + code: `${number}`, // Assuming codes are numeric checked: boolean, fullItemCode: string, ) => void - values: Option[] + readonly values: readonly Option[] majorGroupLength: number }Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/DeleteEmployee/index.tsx (5)
10-13: 🛠️ Refactor suggestion
Enhance type safety and documentation of the component.
The component's type definitions could be improved for better maintainability and type safety.
Consider these improvements:
+/** + * Component for handling employee deletion in work accident notifications. + * @param index - The index of the employee to delete + * @param allowDeleteFirst - Whether to allow deletion of the first employee + * @param application - The current application state + * @param refetch - Callback to refresh data after deletion + * @param goToScreen - Navigation callback + */ interface DeleteEmployeeProps { index: number allowDeleteFirst?: boolean + // Define required application props to avoid relying on FieldBaseProps + application: { + id: string + answers: WorkAccidentNotification + } + refetch?: () => void + goToScreen?: (screen: string) => void } export const DeleteEmployee: FC< - React.PropsWithChildren<DeleteEmployeeProps & FieldBaseProps> + DeleteEmployeeProps > = (props) => {Also applies to: 15-17
24-30:
⚠️ Potential issueImprove type safety and error handling in removeAtIndex function.
The helper function uses explicit
anytype and lacks proper error handling.Apply these improvements:
- // eslint-disable-next-line @typescript-eslint/no-explicit-any - const removeAtIndex = (array: Array<any>): Array<any> => { + const removeAtIndex = <T>(array: Array<T>): Array<T> => { + if (!array) { + return [] + } if (idx >= 0 && idx < array.length) { return [...array.slice(0, idx), ...array.slice(idx + 1)] } return array }📝 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.const removeAtIndex = <T>(array: Array<T>): Array<T> => { if (!array) { return [] } if (idx >= 0 && idx < array.length) { return [...array.slice(0, idx), ...array.slice(idx + 1)] } return array }
70-83: 🛠️ Refactor suggestion
Enhance user experience and accessibility.
The button component could benefit from additional UX improvements and accessibility features.
Consider these enhancements:
return ( <Button - disabled={answers.employeeAmount === 1 && !allowDeleteFirst} + disabled={isDeleting || (answers.employeeAmount === 1 && !allowDeleteFirst)} onClick={onDelete} colorScheme="default" iconType="outline" icon="trash" preTextIconType="filled" size="small" variant="text" + loading={isDeleting} + aria-label={formatMessage(shared.buttons.deleteEmployee)} + onClick={(e) => { + e.preventDefault() + if (window.confirm(formatMessage(shared.messages.confirmDelete))) { + onDelete() + } + }} > {formatMessage(shared.buttons.deleteEmployee)} </Button> )Committable suggestion was skipped due to low confidence.
32-68:
⚠️ Potential issueAdd error handling and loading state for delete operation.
The delete operation lacks proper error handling and loading state management, which could lead to a poor user experience if the mutation fails.
Consider these improvements:
+ const [isDeleting, setIsDeleting] = useState(false) + const onDelete = async () => { + try { + setIsDeleting(true) // Delete employee const updatedEmployees = removeAtIndex(answers.employee) - - const updatedAbsences = removeAtIndex(answers.absence) - const updatedDeviations = removeAtIndex(answers.deviations) - const updatedInjuredBodyParts = removeAtIndex(answers.injuredBodyParts) - const updatedTypeOfInjury = removeAtIndex(answers.typeOfInjury) - const updatedCircumstances = removeAtIndex(answers.circumstances) - const updatedCauseOfInjury = removeAtIndex(answers.causeOfInjury) + // Batch process all arrays + const updates = { + employee: updatedEmployees, + absence: removeAtIndex(answers.absence), + deviations: removeAtIndex(answers.deviations), + injuredBodyParts: removeAtIndex(answers.injuredBodyParts), + typeOfInjury: removeAtIndex(answers.typeOfInjury), + circumstances: removeAtIndex(answers.circumstances), + causeOfInjury: removeAtIndex(answers.causeOfInjury), + } const updateAmountOfEmployees = answers.employeeAmount === 1 ? 1 : answers.employeeAmount - 1 const res = await updateApplication({ variables: { input: { id: application.id, answers: { ...application.answers, - employee: updatedEmployees, - deviations: updatedDeviations, - absence: updatedAbsences, - causeOfInjury: updatedCauseOfInjury, - typeOfInjury: updatedTypeOfInjury, - circumstances: updatedCircumstances, - injuredBodyParts: updatedInjuredBodyParts, + ...updates, employeeAmount: updateAmountOfEmployees, }, }, locale, }, }) if (res.data && goToScreen && refetch) { refetch() } + } catch (error) { + // Handle error appropriately + console.error('Failed to delete employee:', error) + } finally { + setIsDeleting(false) + } }Committable suggestion was skipped due to low confidence.
1-84: 💡 Codebase verification
Component follows library guidelines but has limited reusability scope
The DeleteEmployee component, while well-structured with TypeScript and proper tree-shaking:
- Is tightly coupled to the work-accident-notification template's data schema
- Is only used within its own template (EmployeeAccordionItem and TitleWithRemove components)
- Contains business logic specific to employee management in work accident forms
While it meets the technical guidelines, it could be refactored into a more generic "DeleteItem" component to improve reusability across different templates.
🔗 Analysis chain
Verify compliance with coding guidelines.
The component follows the required guidelines for
libs/**/*files:
- ✅ Uses TypeScript for props and type definitions
- ✅ Exports as a named component for effective tree-shaking
- ✅ Designed for reuse across different NextJS apps
Let's verify the component's reusability:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if this component is imported by other applications rg -l "import.*DeleteEmployee.*work-accident-notification"Length of output: 60
Script:
#!/bin/bash # Let's try a broader search to find any imports of DeleteEmployee rg -l "import.*DeleteEmployee" # Also check if this component is used within the same application fd -e tsx -e ts | xargs rg -l "DeleteEmployee" # Check the structure of work-accident-notification to understand component usage fd -g "*work-accident-notification*"Length of output: 1250
Script:
#!/bin/bash # Let's check how DeleteEmployee is used in the files that import it rg -A 5 "DeleteEmployee" libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx rg -A 5 "DeleteEmployee" libs/application/templates/aosh/work-accident-notification/src/fields/TitleWithRemove/index.tsx # Check if this component pattern is used in other templates fd -e tsx -e ts | rg -l "DeleteEmployee" | grep -v "work-accident-notification"Length of output: 954
libs/application/templates/aosh/work-accident-notification/src/fields/Overview/EmployeeAccordionItem.tsx (4)
45-67: 🛠️ Refactor suggestion
Refactor duplicate ReviewGroup components for better maintainability.
The ReviewGroup components share similar props and structure. Consider extracting this into a reusable component to follow DRY principles.
Create a new component:
type InfoSectionProps = { title: string; getValue: () => Record<string, string>; onClick: () => void; formatMessage: (message: MessageDescriptor) => string; field: FieldBaseProps['field']; application: Application; }; const InfoSection: FC<InfoSectionProps> = ({ title, getValue, onClick, formatMessage, field, application, }) => ( <ReviewGroup handleClick={onClick} editMessage={formatMessage(overview.labels.editMessage)} title={title} isLast isFirst > <KeyValueFormField application={application} field={{ ...field, type: FieldTypes.KEY_VALUE, component: FieldComponents.KEY_VALUE, title: '', label: '', value: getValue(), }} /> </ReviewGroup> );Then use it in the main component:
<InfoSection title={formatMessage(overview.labels.employee)} getValue={() => getEmployeeInformationForOverview( application.externalData, employee, formatMessage, )} onClick={onClick} formatMessage={formatMessage} field={field} application={application} /> <InfoSection title={formatMessage(overview.labels.events)} getValue={() => getCauseAndConsequencesForOverview( application.externalData, application.answers, index, formatMessage, )} onClick={onClick} formatMessage={formatMessage} field={field} application={application} />Also applies to: 69-92
33-43:
⚠️ Potential issueFix accessibility and styling issues in the delete button container.
The Box component has styling issues that could affect accessibility and responsiveness.
<Box marginTop={1} width="full" display={'flex'} - justifyContent={'flexEnd'} + justifyContent={'flex-end'} + role="toolbar" + aria-label={formatMessage(overview.labels.employeeActions)} >Committable suggestion was skipped due to low confidence.
23-29: 🛠️ Refactor suggestion
Improve props typing and destructuring for better maintainability.
The component follows TypeScript best practices, but the props destructuring could be more explicit to improve code readability and type safety.
-export const EmployeeAccordionItem: FC< - React.PropsWithChildren<FieldBaseProps & EmployeeAccordionItemType> -> = ({ ...props }) => { - const { application, field, employee, onClick, index } = props +type Props = FieldBaseProps & EmployeeAccordionItemType + +export const EmployeeAccordionItem: FC<React.PropsWithChildren<Props>> = ({ + application, + field, + employee, + onClick, + index, + ...props +}) => {📝 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.type Props = FieldBaseProps & EmployeeAccordionItemType export const EmployeeAccordionItem: FC<React.PropsWithChildren<Props>> = ({ application, field, employee, onClick, index, ...props }) => { const { formatMessage } = useLocale() const answers = application.answers as WorkAccidentNotification
1-21: 🛠️ Refactor suggestion
Consider optimizing imports for better tree-shaking.
The imports from '@island.is/application/types' could be combined into a single import statement to improve tree-shaking capabilities.
-import { - FieldBaseProps, - FieldTypes, - FieldComponents, -} from '@island.is/application/types' +import type { FieldBaseProps, FieldTypes, FieldComponents } from '@island.is/application/types'📝 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.import type { FieldBaseProps, FieldTypes, FieldComponents } from '@island.is/application/types' import { FC } from 'react' import { Box, Text } from '@island.is/island-ui/core' import { useLocale } from '@island.is/localization' import { overview } from '../../lib/messages' import { ReviewGroup } from '../Components/ReviewGroup' import { KeyValueFormField } from '@island.is/application/ui-fields' import { EmployeeType, WorkAccidentNotification } from '../../lib/dataSchema' import { getEmployeeInformationForOverview } from '../../utils/getEmployeeInformationForOverview' import { getCauseAndConsequencesForOverview } from '../../utils/getCauseAndConsequencesForOverview' import { DeleteEmployee } from '../DeleteEmployee' type EmployeeAccordionItemType = { employee: EmployeeType onClick: () => void index: number }libs/application/templates/aosh/work-accident-notification/src/fields/Overview/index.tsx (2)
21-25: 🛠️ Refactor suggestion
Consider explicitly typing destructured props.
Instead of spreading props, consider explicitly typing the destructured properties for better type safety and code maintainability.
-export const Overview: FC<React.PropsWithChildren<FieldBaseProps>> = ({ - ...props -}) => { - const { application, goToScreen, field } = props +export const Overview: FC<React.PropsWithChildren<FieldBaseProps>> = ({ + application, + goToScreen, + field, + ...props +}) => {📝 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.export const Overview: FC<React.PropsWithChildren<FieldBaseProps>> = ({ application, goToScreen, field, ...props }) => { const { formatMessage } = useLocale()
86-99:
⚠️ Potential issueAdd key prop to AccordionItem and type EmployeeAccordionItem props.
Two issues need attention:
- Missing key prop in the mapped AccordionItem components
- Untyped prop spreading to EmployeeAccordionItem
{employees.map((employee, index) => ( <AccordionItem + key={`${field.id}-accordion-item-${index}`} id={`${field.id}-accordion-item-${index}`} label={`${formatMessage(overview.labels.employee)} ${ employees.length > 1 ? index + 1 : '' }`} > <EmployeeAccordionItem employee={employee} onClick={() => onClick(`employeeInformation[${index}]`)} index={index} - {...props} + field={field} + application={application} /> </AccordionItem> ))}Committable suggestion was skipped due to low confidence.
🧰 Tools
🪛 Biome
[error] 87-92: Missing key property for this element in iterable.
The order of the items may change, and having a key can help React identify which item was moved.
Check the React documentation.(lint/correctness/useJsxKeyInIterable)
libs/application/templates/aosh/work-accident-notification/src/utils/getCauseAndConsequencesForOverview.ts (1)
16-96: 🛠️ Refactor suggestion
Consider refactoring repeated data processing logic.
The code contains six similar blocks for processing different aspects of the accident data. This could be refactored into a reusable function to improve maintainability and reduce duplication.
type DataProcessor = { path: string; messageKey: keyof typeof overview.causeAndConsequences; }; const processData = ( answers: FormValue, index: number, config: DataProcessor ): string[] => { const data = getValueViaPath(answers, `${config.path}[${index}]`, {}) as object; return Object.values(data) .map((value: { label: string; value: string }[]) => value.map(({ label }) => label)) .flat(); }; // Usage example: const circumstances = processData(answers, index, { path: 'circumstances.physicalActivities', messageKey: 'circumstances' });libs/application/templates/aosh/work-accident-notification/src/utils/getEmployeeInformationForOverview.ts (1)
21-74: 🛠️ Refactor suggestion
Consider refactoring repetitive data processing patterns.
The code contains several similar blocks for processing different types of employee data. This could be refactored into a reusable utility function to reduce duplication and improve maintainability.
+type DataMapping<T> = { + path: string; + code: keyof EmployeeType; + type: new () => T; +}; + +const findMatchingData = <T>( + externalData: ExternalData, + employee: EmployeeType, + mapping: DataMapping<T>, +) => { + const data = getValueViaPath( + externalData, + mapping.path, + [], + ) as T[]; + return data.find( + ({ code }) => employee[mapping.code] === code, + ); +}; + export const getEmployeeInformationForOverview = ( externalData: ExternalData, employee: EmployeeType, formatMessage: FormatMessage, ) => { - // Post code - const postCode = getValueViaPath( - externalData, - 'aoshData.data.postCode', - [], - ) as MunicipalityDto[] - const chosenPostCode = postCode.find( - ({ code }) => employee.postnumberAndMunicipality === code, - ) + const chosenPostCode = findMatchingData(externalData, employee, { + path: 'aoshData.data.postCode', + code: 'postnumberAndMunicipality', + type: MunicipalityDto, + }); // ... apply similar pattern to other data processing blocksCommittable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdownController.tsx (5)
52-58: 🛠️ Refactor suggestion
Optimize state initialization with useMemo.
The stateItems initialization could be memoized to prevent unnecessary array mapping on every render.
+ const memoizedItems = useMemo( + () => items.map((x) => ({ ...x, isSelected: false })), + [items] + ) const [stateItems, setStateItems] = useState<Array<Item>>( - items.map((x) => ({ ...x, isSelected: false })) + memoizedItems )Committable suggestion was skipped due to low confidence.
39-51: 🛠️ Refactor suggestion
Consider memoizing the component for better performance.
Since this component handles complex state and receives array props, it could benefit from memoization to prevent unnecessary re-renders.
-export const MultiSelectDropdownController: FC< +export const MultiSelectDropdownController: React.memo< React.PropsWithChildren<MultiSelectControllerProps & FieldBaseProps> -> = (props) => { +> = React.memo((props) => { // ... component logic ... -} +})Committable suggestion was skipped due to low confidence.
124-142:
⚠️ Potential issueImprove render performance and prop handling.
- Using index as key could cause issues with component updates
- Spreading all props could pass unnecessary props to child components
return ( <> {groups.map((group, index) => ( <MultiSelectDropdown group={group} options={stateItems.filter( (item) => item.code.substring(0, majorGroupLength) === group.code.substring(0, majorGroupLength), )} values={answers[group.code.substring(0, majorGroupLength)]} - key={index} + key={group.code} onChange={onChange} - {...props} + disabled={props.disabled} + error={props.error} /> ))} </> )Committable suggestion was skipped due to low confidence.
102-122: 🛠️ Refactor suggestion
Optimize useEffect hooks and add cleanup.
The effects could be optimized to prevent unnecessary updates and memory leaks.
useEffect(() => { if (!pickedValue?.key || !pickedValue?.option) return setAnswers((currentValue) => { const newAnswers = { ...currentValue, [pickedValue.key]: [ ...(currentValue[pickedValue.key] || []), pickedValue.option, ], } setValue(answerId, newAnswers) return newAnswers }) -}, [pickedValue]) +}, [pickedValue?.key, pickedValue?.option]) useEffect(() => { + let mounted = true - onAnswerChange(answers) + if (mounted) { + onAnswerChange(answers) + } + return () => { + mounted = false + } }, [answers])📝 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.useEffect(() => { if (!pickedValue?.key || !pickedValue?.option) return setAnswers((currentValue) => { const newAnswers = { ...currentValue, [pickedValue.key]: [ ...(currentValue[pickedValue.key] || []), pickedValue.option, ], } setValue(answerId, newAnswers) return newAnswers }) }, [pickedValue?.key, pickedValue?.option]) useEffect(() => { let mounted = true if (mounted) { onAnswerChange(answers) } return () => { mounted = false } }, [answers])
60-100:
⚠️ Potential issueSimplify onChange handler and prevent potential race conditions.
The onChange handler duplicates logic and could have race conditions between state updates.
- Extract common update logic
- Use functional updates consistently
- Batch related state updates
const onChange = ( values: Option[], code: string, checked: boolean, fullItemCode: string, ) => { if (values === undefined) return + const updateState = (checked: boolean, values: Option[]) => { + setAnswers((currentValue) => { + const newAnswers = checked + ? { ...currentValue, [code]: values } + : { + ...currentValue, + [code]: (currentValue[code] || []).filter( + (value) => value.value !== values[0].value + ), + } + setValue(answerId, newAnswers) + return newAnswers + }) + + setStateItems((current) => + current.map((item) => + item.code === fullItemCode + ? { ...item, isSelected: checked } + : item + ) + ) + } + + updateState(checked, values) - if (checked) { - // ... existing code ... - } else { - // ... existing code ... - } }📝 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.const onChange = ( values: Option[], code: string, checked: boolean, fullItemCode: string, ) => { if (values === undefined) return const updateState = (checked: boolean, values: Option[]) => { setAnswers((currentValue) => { const newAnswers = checked ? { ...currentValue, [code]: values } : { ...currentValue, [code]: (currentValue[code] || []).filter( (value) => value.value !== values[0].value ), } setValue(answerId, newAnswers) return newAnswers }) setStateItems((current) => current.map((item) => item.code === fullItemCode ? { ...item, isSelected: checked } : item ) ) } updateState(checked, values) }libs/application/templates/aosh/work-accident-notification/src/lib/messages/externalData.ts (1)
1-3: 🛠️ Refactor suggestion
Add TypeScript type definitions for better type safety.
Consider adding type definitions for the messages object to improve maintainability and catch potential issues at compile time.
import { defineMessages } from 'react-intl' + import { MessageDescriptor } from 'react-intl' + interface ExternalDataMessages { + dataProvider: Record<string, MessageDescriptor> + nationalRegistry: Record<string, MessageDescriptor> + userProfile: Record<string, MessageDescriptor> + } - export const externalData = { + export const externalData: ExternalDataMessages = {📝 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.import { defineMessages } from 'react-intl' import { MessageDescriptor } from 'react-intl' interface ExternalDataMessages { dataProvider: Record<string, MessageDescriptor> nationalRegistry: Record<string, MessageDescriptor> userProfile: Record<string, MessageDescriptor> } export const externalData: ExternalDataMessages = {libs/application/templates/aosh/work-accident-notification/src/lib/messages/sections.ts (3)
23-23:
⚠️ Potential issueFix typos in message descriptions.
There are typos in several message descriptions where "setion" should be "section".
- description: 'Title of the employee setion (right side panel)', + description: 'Title of the employee section (right side panel)', - description: 'Title of the overview setion (right side panel)', + description: 'Title of the overview section (right side panel)', - description: 'Title of the conclusion setion (right side panel)', + description: 'Title of the conclusion section (right side panel)',Also applies to: 28-28, 33-33
101-103: 🛠️ Refactor suggestion
Add type safety for the employee number parameter.
The employee message uses a number parameter but lacks type definition. Consider using TypeScript's template literal type for better type safety.
+type EmployeeMessage = { + number: number +} employee: { id: 'aosh.wan.application:sections.draft.employee', - defaultMessage: 'Starfsmaður {number}', + defaultMessage: 'Starfsmaður {number}' as const, description: 'Title of employee section (right side panel)', + values: EmployeeMessage },Committable suggestion was skipped due to low confidence.
1-3: 🛠️ Refactor suggestion
Add TypeScript type definitions for better type safety.
Consider adding type definitions for the sections object to improve type safety and IDE support.
import { defineMessages } from 'react-intl' +type MessageSection = { + readonly [key: string]: { + readonly id: string + readonly defaultMessage: string + readonly description: string + } +} + +export const sections: { + prerequisites: MessageSection + draft: MessageSection +} = {📝 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.import { defineMessages } from 'react-intl' type MessageSection = { readonly [key: string]: { readonly id: string readonly defaultMessage: string readonly description: string } } export const sections: { prerequisites: MessageSection draft: MessageSection } = {libs/application/templates/aosh/work-accident-notification/src/fields/AccidentLocation/index.tsx (7)
17-20: 🛠️ Refactor suggestion
Consider moving the Option type to a shared types file.
The Option type is a common interface that could be reused across other select components in the application.
Consider moving it to a shared types file, such as
libs/application/templates/aosh/work-accident-notification/src/types/index.ts:// types/index.ts export interface SelectOption { value: string label: string }
88-89:
⚠️ Potential issueFix duplicate Controller names.
Both Controller components use the same name 'subMajorGroup', which could cause form validation issues.
- name={'subMajorGroup'} + name={'accident.accidentLocationParentGroup'}- name={'subMajorGroup'} + name={'accident.accidentLocation'}Also applies to: 121-122
64-64:
⚠️ Potential issueFix inconsistent Select names.
The name prop for the second Select component is incorrect (copied from the first one).
- name="subMajorGroupSelect" + name="majorGroupSelect"- name="subMajorGroupSelect" + name="minorGroupSelect"Also applies to: 100-100
59-85:
⚠️ Potential issueAdd error messages for required fields.
The Select components are marked as required but don't display error messages when validation fails.
<Select required + hasError={errors.accident?.accidentLocationParentGroup} + errorMessage={errors.accident?.accidentLocationParentGroup?.message} label={formatMessage(accident.about.locationOfAccidentMajorGroup)} // ... rest of the props /> <Select required + hasError={errors.accident?.accidentLocation} + errorMessage={errors.accident?.accidentLocation?.message} label={formatMessage(accident.about.locationOfAccidentMinorGroup)} // ... rest of the props />Don't forget to add the errors object from useFormContext:
- const { setValue } = useFormContext() + const { setValue, formState: { errors } } = useFormContext()Also applies to: 95-118
3-9:
⚠️ Potential issueRemove unused import
Checkbox.The Checkbox component is imported but never used in this file.
import { Box, - Checkbox, GridColumn, GridRow, Select, } from '@island.is/island-ui/core'📝 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.import { Box, GridColumn, GridRow, Select, } from '@island.is/island-ui/core'
29-41:
⚠️ Potential issueAdd error handling for external data.
The component assumes external data is always available and correctly formatted. Add proper error handling and loading states.
+ const workingEnvironment = getValueViaPath( + application.externalData, + 'aoshData.data.workingEnvironment', + ) as WorkingEnvironmentDto[] | undefined + if (!workingEnvironment) { + return ( + <Box paddingTop={2}> + <Text> + {formatMessage(accident.errors.workingEnvironmentDataNotAvailable)} + </Text> + </Box> + ) + } - const majorGroups = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workingEnvironment', - ) as WorkingEnvironmentDto[] - ).filter((group) => !group.validToSelect) + const majorGroups = workingEnvironment.filter((group) => !group.validToSelect) - const minorGroups = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workingEnvironment', - ) as WorkingEnvironmentDto[] - ).filter((group) => group.validToSelect) + const minorGroups = workingEnvironment.filter((group) => group.validToSelect)Committable suggestion was skipped due to low confidence.
36-51: 🛠️ Refactor suggestion
Optimize performance with useMemo.
The filtered arrays and options could be memoized to prevent unnecessary recalculations on re-renders.
- const majorGroups = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workingEnvironment', - ) as WorkingEnvironmentDto[] - ).filter((group) => !group.validToSelect) + const majorGroups = useMemo(() => ( + getValueViaPath( + application.externalData, + 'aoshData.data.workingEnvironment', + ) as WorkingEnvironmentDto[] + ).filter((group) => !group.validToSelect), [application.externalData]) - const minorGroups = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workingEnvironment', - ) as WorkingEnvironmentDto[] - ).filter((group) => group.validToSelect) + const minorGroups = useMemo(() => ( + getValueViaPath( + application.externalData, + 'aoshData.data.workingEnvironment', + ) as WorkingEnvironmentDto[] + ).filter((group) => group.validToSelect), [application.externalData])Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/CheckboxFieldCustom/index.tsx (5)
80-80:
⚠️ Potential issueApply optional chaining for safer null checks.
The static analysis tool correctly identified potential null reference issues. Use optional chaining for safer access to the choice array.
- checked={choice && choice.includes(option.code || '')} + checked={choice?.includes(option.code || '')}Also applies to: 111-111
🧰 Tools
🪛 Biome
[error] 80-80: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
129-131:
⚠️ Potential issueUse JSX children instead of children prop.
The ErrorMessage component should use JSX children syntax instead of the children prop for better readability and maintainability.
- <ErrorMessage - children={formatMessage(information.labels.workhealth.errorAlert)} - /> + <ErrorMessage> + {formatMessage(information.labels.workhealth.errorAlert)} + </ErrorMessage>📝 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.<ErrorMessage> {formatMessage(information.labels.workhealth.errorAlert)} </ErrorMessage>🧰 Tools
🪛 Biome
[error] 130-130: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
93-93:
⚠️ Potential issueFix the empty Controller name prop.
The Controller component requires a name prop for proper form integration. An empty name could cause issues with form state management.
- name={''} + name={'exceptionOption'}Committable suggestion was skipped due to low confidence.
29-42: 🛠️ Refactor suggestion
Optimize external data access and filtering.
The code duplicates the external data access for normal and exception options. Consider extracting this logic to improve maintainability and performance.
+ const workplaceHealthAndSafety = getValueViaPath( + application.externalData, + 'aoshData.data.workplaceHealthAndSafety', + ) as WorkplaceHealthAndSafetyDto[] + const normalOptions = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workplaceHealthAndSafety', - ) as WorkplaceHealthAndSafetyDto[] + workplaceHealthAndSafety ).filter((x) => x.code !== '1') const exceptionOption = ( - getValueViaPath( - application.externalData, - 'aoshData.data.workplaceHealthAndSafety', - ) as WorkplaceHealthAndSafetyDto[] + workplaceHealthAndSafety ).filter((x) => x.code === '1')📝 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.const workplaceHealthAndSafety = getValueViaPath( application.externalData, 'aoshData.data.workplaceHealthAndSafety', ) as WorkplaceHealthAndSafetyDto[] const normalOptions = ( workplaceHealthAndSafety ).filter((x) => x.code !== '1') const exceptionOption = ( workplaceHealthAndSafety ).filter((x) => x.code === '1')
43-54: 🛠️ Refactor suggestion
Consider memoizing filtered options for better performance.
The effect hook recalculates filtered options on every render. Consider using useMemo to optimize performance, especially if the options list is large.
+ const selectedOptions = useMemo(() => + options + .filter((option) => choice.includes(option.code || '')) + .map((option) => option.code), + [options, choice] + ) useEffect(() => { const options = getValueViaPath( application.externalData, 'aoshData.data.workplaceHealthAndSafety', ) as WorkplaceHealthAndSafetyDto[] setValue( 'companyLaborProtection.workhealthAndSafetyOccupation', - options - .filter((option) => choice.includes(option.code || '')) - .map((option) => option.code), + selectedOptions, ) - }, [application.externalData, choice, setValue]) + }, [application.externalData, selectedOptions, setValue])Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/AccidentSection/about.ts (4)
37-44:
⚠️ Potential issueAdd time format validation.
While the field uses a format mask '##:##', it doesn't validate that the input is a valid 24-hour time (00:00-23:59). Invalid times could be entered.
Consider adding validation or using a specialized time input component. Example implementation:
buildTextField({ id: 'accident.time', title: accident.about.time, placeholder: accident.about.timePlaceholder, required: true, width: 'half', format: '##:##', validate: (value) => { if (!/^([0-1][0-9]|2[0-3]):[0-5][0-9]$/.test(value)) { return 'Invalid time format. Please use HH:mm (00:00-23:59).' } return undefined }, })
77-98: 🛠️ Refactor suggestion
Improve type-safety in municipality data handling.
While the code handles null values, it could be more type-safe and handle edge cases better.
Consider this more robust implementation:
buildSelectField({ id: 'accident.municipality', title: accident.about.municipality, width: 'half', required: true, options: (application) => { const municipalities = getValueViaPath<MunicipalityDto[]>( application.externalData, 'aoshData.data.municipality', [], ) return municipalities .filter((municipality): municipality is Required<Pick<MunicipalityDto, 'code' | 'name'>> => Boolean(municipality?.code && municipality?.name) ) .map(({ code, name }) => ({ value: code, label: name, })) }, })This implementation:
- Uses generic type parameter with
getValueViaPath- Uses type predicate in filter for better type inference
- Removes redundant null checks in the mapping
124-150: 🛠️ Refactor suggestion
Consider extracting textarea configuration and adding minimum length.
The three textarea fields share similar configuration. Consider adding minimum length requirements and extracting shared configuration for better maintainability.
Example implementation:
const TEXTAREA_CONFIG = { variant: 'textarea' as const, rows: 7, maxLength: 500, minLength: 50, required: true, } as const buildTextField({ id: 'accident.wasDoing', title: accident.about.wasDoingTitle, placeholder: accident.about.wasDoingPlaceholder, ...TEXTAREA_CONFIG, }) // Apply to other textarea fields similarly
160-164:
⚠️ Potential issueAdd type safety for custom AccidentLocation component.
The custom component is used without type information, which could lead to runtime errors if the component's contract changes.
Consider defining and using an interface for the component's props:
interface AccidentLocationProps { id: string; value?: string; onChange: (value: string) => void; } buildCustomField<AccidentLocationProps>({ id: 'accident.accidentLocation', title: '', component: 'AccidentLocation', })libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.ts (4)
148-150:
⚠️ Potential issueFix spelling in defaultMessage for chooseFromListPlaceholder.
There appears to be a spelling error in the Icelandic text.
chooseFromListPlaceholder: { id: 'aosh.wan.application:employee.chooseFromListPlaceholder', - defaultMessage: 'Veldur úr lista', + defaultMessage: 'Veldu úr lista', description: 'Placeholder for occupation group select fields', },📝 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.id: 'aosh.wan.application:employee.chooseFromListPlaceholder', defaultMessage: 'Veldu úr lista', description: 'Placeholder for occupation group select fields',
71-75:
⚠️ Potential issueFix duplicate message ID for employmentRate.
There's a critical issue where the
employmentRatemessage uses the same ID asemploymentTime. This will cause conflicts in the internationalization system.employmentRate: { - id: 'aosh.wan.application:employee.employmentTime', + id: 'aosh.wan.application:employee.employmentRate', defaultMessage: 'Starfshlutfall', description: 'Employment rate %', },📝 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.employmentRate: { id: 'aosh.wan.application:employee.employmentRate', defaultMessage: 'Starfshlutfall', description: 'Employment rate %', },
81-90:
⚠️ Potential issueFix duplicate message ID for time-related fields.
The
startOfWorkdayDateandtimemessages are using the same ID, which will cause conflicts.startOfWorkdayDate: { - id: 'aosh.wan.application:employee.time', + id: 'aosh.wan.application:employee.startOfWorkdayDate', defaultMessage: 'Dagsetning byrjun starfsdags', description: 'Start of employees workday date', },📝 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.startOfWorkdayDate: { id: 'aosh.wan.application:employee.startOfWorkdayDate', defaultMessage: 'Dagsetning byrjun starfsdags', description: 'Start of employees workday date', }, time: { id: 'aosh.wan.application:employee.time', defaultMessage: 'Tími byrjun starfsdags', description: 'Start of employees workday time', },
1-3: 🛠️ Refactor suggestion
Add TypeScript types for better type safety and reusability.
Consider adding explicit TypeScript types for the messages object to improve type safety and make the messages more reusable across different NextJS apps.
import { defineMessages } from 'react-intl' +type EmployeeMessages = { + [K in keyof typeof employee.employee]: { + id: string + defaultMessage: string + description: string + } +} + -export const employee = { +export const employee: { employee: EmployeeMessages } = {Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/lib/messages/accident.ts (4)
1-3: 🛠️ Refactor suggestion
Add TypeScript type definitions for better type safety.
Consider adding TypeScript interfaces to define the message structure and improve type safety:
import { MessageDescriptor } from 'react-intl' interface AccidentMessages { about: { [key: string]: MessageDescriptor } } export const accident: AccidentMessages = {
47-50:
⚠️ Potential issueFix typo in municipality field message.
There's a spelling error in the municipality field message:
municipality: { id: 'aosh.wan.application:accident.about.municipality', - defaultMessage: 'Sveitarfélar slysstaðar', + defaultMessage: 'Sveitarfélag slysstaðar', description: 'Label of municipality input field', },📝 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.id: 'aosh.wan.application:accident.about.municipality', defaultMessage: 'Sveitarfélag slysstaðar', description: 'Label of municipality input field', },
11-14:
⚠️ Potential issueReplace Lorem Ipsum with actual description text.
The description message contains Lorem Ipsum placeholder text which should be replaced with the actual description before deployment:
description: { id: 'aosh.wan.application:accident.about.description', defaultMessage: - 'Et sed ut est aliquam proin elit sed. Nunc tellus lacus sed eu pulvinar. ', + // Add the actual description text hereCommittable suggestion was skipped due to low confidence.
84-111:
⚠️ Potential issueUpdate placeholder texts with relevant examples.
The placeholder texts for
wasDoingPlaceholder,wenWrongPlaceholder, andhowPlaceholderare identical. Each should have unique, context-specific examples:wasDoingPlaceholder: { defaultMessage: - 'Tilgreinið hvaða verkfæri eða vélar voru notaðar.\nDæmi:\n - Vann með handborvél\n - Vann með handborvél', + 'Tilgreinið hvaða verkfæri eða vélar voru notaðar.\nDæmi:\n - Vann við að bora göt í vegg\n - Var að skrúfa saman hillur', }, // ... similar updates needed for wenWrongPlaceholder and howPlaceholderCommittable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts (4)
11-14:
⚠️ Potential issueReplace Lorem ipsum placeholder text with actual description.
The description message contains placeholder text ("Et sed ut est aliquam...") which should be replaced with meaningful content before deployment.
75-79:
⚠️ Potential issueFix incorrect description for numberOfEmployees field.
The description "Company information industry classification label" appears to be copy-pasted and is incorrect for the
numberOfEmployeesfield.numberOfEmployees: { id: 'aosh.wan.application:information.company.numberOfEmployees', defaultMessage: 'Starfsmannafjöldi', - description: 'Company information industry classification label', + description: 'Company information number of employees label', },📝 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.numberOfEmployees: { id: 'aosh.wan.application:information.company.numberOfEmployees', defaultMessage: 'Starfsmannafjöldi', description: 'Company information number of employees label', },
148-154:
⚠️ Potential issueFix typo in Icelandic error message.
There's a typo in the error message: "veldur" should be "velur".
errorAlert: { id: 'aosh.wan.application:information.workhealth.errorAlert', defaultMessage: - "Vinsamlegast veldur a.m.k einn valmöguleika, ef ekkert á við veldu 'engin'", + "Vinsamlegast velur a.m.k einn valmöguleika, ef ekkert á við veldu 'engin'", description: 'error alert on workhealth information page when user attempt to navigate forwards without choosing an option', },📝 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.errorAlert: { id: 'aosh.wan.application:information.workhealth.errorAlert', defaultMessage: "Vinsamlegast velur a.m.k einn valmöguleika, ef ekkert á við veldu 'engin'", description: 'error alert on workhealth information page when user attempt to navigate forwards without choosing an option', },
1-3: 🛠️ Refactor suggestion
Add TypeScript types for better type safety and documentation.
Consider adding TypeScript interfaces to define the structure of your messages object. This will improve maintainability and provide better IDE support.
import { defineMessages } from 'react-intl' + +interface MessageDescriptor { + id: string + defaultMessage: string + description: string +} + +interface InformationMessages { + general: Record<string, MessageDescriptor> + labels: { + company: Record<string, MessageDescriptor> + laborProtection: Record<string, MessageDescriptor> + workhealth: Record<string, MessageDescriptor> + projectPurchase: Record<string, MessageDescriptor> + } +} -export const information = { +export const information: InformationMessages = {Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/lib/messages/overview.ts (5)
161-162:
⚠️ Potential issueFix incorrect description for workstation field.
The description for the
workstationfield incorrectly references "employment time" instead of "workstation".workstation: { id: 'aosh.wan.application:overview.employee.workstation', defaultMessage: 'Starfsstöð', - description: 'Employees employment time overview label', + description: 'Employees workstation overview label', },📝 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.description: 'Employees workstation overview label', },
27-31:
⚠️ Potential issueReplace Lorem Ipsum placeholder text.
The
moreInjuredDescriptioncontains Lorem Ipsum placeholder text. This should be replaced with actual content before deployment.moreInjuredDescription: { id: 'aosh.wan.application:overview.general.moreInjuredDescription', - defaultMessage: - 'Urna urna at habitasse viverra aliquam eget. Bibendum nunc eu vitae mattis lorem pharetra suscipit lectus sapien. Porta nec proin pretium magnis ac. Ac amet sed quisque in sed.', + defaultMessage: '[TODO: Add actual description for multiple injured parties]', description: 'More injured description', },Committable suggestion was skipped due to low confidence.
49-83:
⚠️ Potential issueFix duplicate message IDs in labels section.
Multiple labels are using the same ID 'aosh.wan.application:overview.labels.dateAndTime', which will cause conflicts in message resolution. Each message must have a unique ID.
didAoshCome: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.didAoshCome', defaultMessage: 'Kom Vinnueftirlitið?', - description: 'Date and time label', + description: 'Label for AOSH visit question', }, didPoliceCome: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.didPoliceCome', defaultMessage: 'Kom Lögreglan?', - description: 'Date and time label', + description: 'Label for police visit question', }, injuredAmount: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.injuredAmount', defaultMessage: 'Fjöldi slasaðra', - description: 'Date and time label', + description: 'Label for number of injured people', }, municipality: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.municipality', defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað', - description: 'Date and time label', + description: 'Label for accident municipality', }, exactLocation: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.exactLocation', defaultMessage: 'Nákvæm staðsetning slyssins', - description: 'Date and time label', + description: 'Label for exact accident location', }, accidentDescription: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.accidentDescription', defaultMessage: 'Tildrög slyssins', - description: 'Date and time label', + description: 'Label for accident description', }, locationOfAccident: { - id: 'aosh.wan.application:overview.labels.dateAndTime', + id: 'aosh.wan.application:overview.labels.locationOfAccident', defaultMessage: 'Vettvangur slyssins', - description: 'Date and time label', + description: 'Label for accident location type', },📝 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.didAoshCome: { id: 'aosh.wan.application:overview.labels.didAoshCome', defaultMessage: 'Kom Vinnueftirlitið?', description: 'Label for AOSH visit question', }, didPoliceCome: { id: 'aosh.wan.application:overview.labels.didPoliceCome', defaultMessage: 'Kom Lögreglan?', description: 'Label for police visit question', }, injuredAmount: { id: 'aosh.wan.application:overview.labels.injuredAmount', defaultMessage: 'Fjöldi slasaðra', description: 'Label for number of injured people', }, municipality: { id: 'aosh.wan.application:overview.labels.municipality', defaultMessage: 'Sveitarfélag þar sem slysið átti sér stað', description: 'Label for accident municipality', }, exactLocation: { id: 'aosh.wan.application:overview.labels.exactLocation', defaultMessage: 'Nákvæm staðsetning slyssins', description: 'Label for exact accident location', }, accidentDescription: { id: 'aosh.wan.application:overview.labels.accidentDescription', defaultMessage: 'Tildrög slyssins', description: 'Label for accident description', }, locationOfAccident: { id: 'aosh.wan.application:overview.labels.locationOfAccident', defaultMessage: 'Vettvangur slyssins', description: 'Label for accident location type', },
1-3: 🛠️ Refactor suggestion
Add TypeScript types for better type safety and documentation.
Consider adding TypeScript types to improve type safety and make the message structure more maintainable.
import { defineMessages } from 'react-intl' + +type MessageSection = Record<string, { id: string; defaultMessage: string; description: string }> + +interface OverviewMessages { + general: MessageSection + labels: MessageSection + employee: MessageSection + causeAndConsequences: MessageSection +} -export const overview = { +export const overview: OverviewMessages = {📝 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.import { defineMessages } from 'react-intl' type MessageSection = Record<string, { id: string; defaultMessage: string; description: string }> interface OverviewMessages { general: MessageSection labels: MessageSection employee: MessageSection causeAndConsequences: MessageSection } export const overview: OverviewMessages = {
89-94: 💡 Codebase verification
The employee description text is incorrectly placed and should be removed
The
employeeDescriptionfield contains text about medical certificates and submission methods that is out of context for an employee description. This text appears to be misplaced content that belongs in a different section about document submission requirements. The field is used inEmployeeAccordionItemcomponent to display information about an employee, but the current content does not match this purpose.🔗 Analysis chain
Review and update employee description content.
The
employeeDescriptioncontains text about medical certificates and submission methods that seems out of context for an employee description. Please verify if this content belongs here.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related medical certificate handling in the codebase rg -i "medical certificate|læknisvottorð" --type ts --type tsxLength of output: 92
Script:
#!/bin/bash # Search for related medical certificate handling in the codebase with correct file type rg -i "medical certificate|læknisvottorð" -t ts # Search for employeeDescription usage in the codebase rg -i "employeeDescription" -t ts -C 3 # Look for similar message patterns in the same directory fd . --full-path libs/application/templates/aosh/work-accident-notification -e ts -e tsx -x cat {}Length of output: 203012
libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (3)
1-3: 🛠️ Refactor suggestion
Consider adding TypeScript types for better maintainability.
To improve type safety and maintainability, consider defining TypeScript interfaces for the message structure.
interface MessageDescriptor { id: string; defaultMessage: string; description: string; } interface CauseAndConsequencesMessages { shared: Record<string, MessageDescriptor>; absence: Record<string, MessageDescriptor>; // ... other sections } export const causeAndConsequences: CauseAndConsequencesMessages = { // ... existing implementation }
158-159:
⚠️ Potential issueFix incorrect title for injured body parts section.
The title "Tegund áverka" (type of injury) appears to be incorrectly copied from the previous section. This should be updated to reflect that this section is about injured body parts.
121-122: 💡 Codebase verification
⚠️ Potential issueThe search results confirm multiple instances of placeholder text in the messages files, particularly concentrated in
causeAndConsequences.ts. The placeholders follow patterns like "?ennan texta vantar..." (this text is missing), "Lýstu því við hvað .... vantar þennan texta" (Describe what ... this text is missing), and similar variations. Other occurrences of "?" in the files appear to be legitimate question marks in actual content.Multiple placeholder texts found in message files - needs content replacement
The search confirms several instances of incomplete content in the messages:
causeAndConsequences.ts:
- "?ennan texta vantar..." (multiple occurrences)
- "Lýstu því við hvað .... vantar þennan texta" (multiple occurrences)
- "Þennan texta vantar..."
- "Lýstu því hvað .... vantar þennan texta"
These placeholder messages need to be replaced with proper content before deployment to ensure a complete user experience.
🔗 Analysis chain
Replace placeholder text with actual content.
Several messages contain placeholder or incomplete text:
- "?ennan texta vantar..." (Line 121)
- "Lýstu því við hvað .... vantar þennan texta" (Line 126)
These placeholders should be replaced with proper content before deployment.
Let's check if there are other instances of placeholder text:
Also applies to: 126-127
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for placeholder text patterns in message files rg -i "vantar|\.{3,}|\?" "libs/application/templates/aosh/work-accident-notification/src/lib/messages/"Length of output: 2315
libs/application/types/src/lib/InstitutionMapper.ts (1)
397-400:
⚠️ Potential issueAdd missing contentfulId property.
The new entry for
WORK_ACCIDENT_NOTIFICATIONis missing thecontentfulIdproperty, which is present in all other VER (Vinnueftirlitid) application entries. This might affect content management integration.Apply this diff to add the missing property:
[ApplicationTypes.WORK_ACCIDENT_NOTIFICATION]: { nationalId: InstitutionNationalIds.VINNUEFTIRLITID, slug: InstitutionTypes.VINNUEFTIRLITID, + contentfulId: InstitutionContentfulIds.VINNUEFTIRLITID, },📝 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.[ApplicationTypes.WORK_ACCIDENT_NOTIFICATION]: { nationalId: InstitutionNationalIds.VINNUEFTIRLITID, slug: InstitutionTypes.VINNUEFTIRLITID, contentfulId: InstitutionContentfulIds.VINNUEFTIRLITID, },libs/application/templates/aosh/work-accident-notification/src/lib/WorkAccidentNotificationTemplate.ts (2)
169-176: 🛠️ Refactor suggestion
Extend
mapUserToRoleto handle delegationsCurrently, the
mapUserToRolefunction only maps the applicant. Since the application allows for delegations (lines 39-46), consider updating this function to map delegated users to the appropriate role. This ensures that users with proper delegations can access and interact with the application as intended.Would you like assistance in updating the
mapUserToRolefunction to handle delegated users?
137-138:
⚠️ Potential issueEnsure consistent state names
The
nameproperty for theCOMPLETEDstate is set to'Completed', which is in English, while the names for thePREREQUISITESandDRAFTstates are in Icelandic ('Skilyrði'and'Tilkynning vinnuslyss'). For consistency and a better user experience, consider translating'Completed'to Icelandic.Suggested change:
- name: 'Completed', + name: 'Lokið',📝 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.name: 'Lokið', status: 'completed',libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (2)
162-165:
⚠️ Potential issueAvoid passing
childrenas a prop; use JSX children insteadSimilarly, in this instance, it is recommended to use JSX children for the
ErrorMessagecomponent.Apply this diff to fix the issue:
- <ErrorMessage - children={formatMessage( - causeAndConsequences.shared.mostSeriousAlert, - )} - /> + <ErrorMessage> + {formatMessage( + causeAndConsequences.shared.mostSeriousAlert, + )} +</ErrorMessage>📝 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.<ErrorMessage> {formatMessage( causeAndConsequences.shared.mostSeriousAlert, )} </ErrorMessage>🧰 Tools
🪛 Biome
[error] 162-162: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
144-147:
⚠️ Potential issueAvoid passing
childrenas a prop; use JSX children insteadPassing
childrendirectly as a prop is discouraged in React. Instead, place the content between the opening and closing tags of the component.Apply this diff to fix the issue:
- <ErrorMessage - children={formatMessage( - causeAndConsequences.shared.causeAndConsequencesNothingChosen, - )} - /> + <ErrorMessage> + {formatMessage( + causeAndConsequences.shared.causeAndConsequencesNothingChosen, + )} +</ErrorMessage>📝 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.<ErrorMessage> {formatMessage( causeAndConsequences.shared.causeAndConsequencesNothingChosen, )} </ErrorMessage>🧰 Tools
🪛 Biome
[error] 144-144: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (2)
17-17:
⚠️ Potential issueUse Optional Chaining for Null Checks
Consider using optional chaining to simplify the null check when validating the phone number. This improves readability and conciseness.
Apply this diff to implement the change:
return phone && phone.isValid()becomes
return phone?.isValid()📝 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.return phone?.isValid()🧰 Tools
🪛 Biome
[error] 17-17: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
36-45:
⚠️ Potential issueEnsure Valid Date Before Comparison
When validating the
datefield, ifdateStris invalid,new Date(dateStr)will result in anInvalid Date, and the comparison may not work as intended. It's important to verify thatdateStrrepresents a valid date before performing comparisons.Apply this diff to validate the date string:
date: z.string().refine( (dateStr) => { const date = new Date(dateStr) + if (isNaN(date.getTime())) { + return false + } const minDate = new Date('2020-01-01') return date >= minDate }, { - message: 'Dagsetning má ekki vera fyrir 1.1.2020', + message: 'Date must be valid and not before 1.1.2020', }, ),📝 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.date: z.string().refine( (dateStr) => { const date = new Date(dateStr) if (isNaN(date.getTime())) { return false } const minDate = new Date('2020-01-01') return date >= minDate }, { message: 'Date must be valid and not before 1.1.2020', }, ),libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/InformationSection/companySection.ts (8)
20-249: 🛠️ Refactor suggestion
Consider exporting types for improved TypeScript integration
To enhance type safety and reusability across different NextJS apps, consider defining and exporting types or interfaces for your form sections and fields.
90-91:
⚠️ Potential issueSafeguard against undefined address in defaultValue
The
defaultValueforcompanyInformation.addresscould be'undefined'ifstreetAddressis not available. It's important to ensure that the form does not display'undefined'to the user.Apply this diff:
- return streetAddress + return streetAddress ?? ''📝 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.return streetAddress ?? '' },
54-61:
⚠️ Potential issueHandle potential undefined 'nationalId' in defaultValue
In the
defaultValuefunction forcompanyInformation.nationalId, ifnationalIdis undefined, the form field may display'undefined'. Consider providing a fallback to ensure the field remains empty or displays a placeholder whennationalIdis unavailable.Apply this diff to handle undefined values:
- return nationalId + return nationalId ?? ''📝 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.const nationalId = getValueViaPath( application.externalData, 'identity.data.nationalId', undefined, ) as string | undefined return nationalId ?? '' },
68-76:
⚠️ Potential issueEnsure 'name' is defined in defaultValue
For
companyInformation.name, ifnameis undefined, the field may show'undefined'. To enhance user experience, provide a default value or handle the undefined case appropriately.Apply this diff:
- return name + return name ?? ''📝 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.defaultValue: (application: Application) => { const name = getValueViaPath( application.externalData, 'identity.data.name', undefined, ) as string | undefined return name ?? '' },
110-111:
⚠️ Potential issuePrevent 'undefined' from appearing in the postnumber field
When constructing the default value for
companyInformation.postnumber, ifpostalCodeorcityis undefined, the result may be'undefined - undefined'. This can be confusing for users.Apply this diff to handle undefined values:
- return `${postalCode} - ${city}` + const postcodeDisplay = postalCode ?? '' + const cityDisplay = city ?? '' + return `${postcodeDisplay} - ${cityDisplay}`Alternatively, check if both values are available:
+ if (postalCode && city) { + return `${postalCode} - ${city}` + } + return ''📝 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.const postcodeDisplay = postalCode ?? '' const cityDisplay = city ?? '' return `${postcodeDisplay} - ${cityDisplay}`
164-174:
⚠️ Potential issueHandle undefined 'mobilePhoneNumber' in defaultValue
Similarly, ensure that
companyInformation.phonenumberhas a valid default value even ifuserProfile.data.mobilePhoneNumberis undefined.Apply this diff:
) as string - }), + }) ?? '',Committable suggestion was skipped due to low confidence.
183-188:
⚠️ Potential issueAdd null check for 'type' in condition functions
In the
conditionfunctions,typemay be undefined if the external data does not includeidentity.data.type. This could lead to unexpected behavior.Modify the condition to handle undefined
type:const type = getValueViaPath( externalData, 'identity.data.type', ) as string - return type === 'company' + return type === 'company' && type !== undefinedOr ensure
typehas a safe fallback:+ const type = (getValueViaPath( + externalData, + 'identity.data.type', + ) as string) ?? ''Committable suggestion was skipped due to low confidence.
196-218: 🛠️ Refactor suggestion
Refactor repeated condition logic to improve maintainability
The
conditionfunctions checking fortype === 'company'are repeated across multiple fields. Consider abstracting this logic into a shared utility function or a variable to reduce code duplication.Example:
+ const isCompany = (externalData) => { + const type = getValueViaPath( + externalData, + 'identity.data.type', + ) as string + return type === 'company' + } buildTextField({ id: 'companyInformation.nameOfBranch', title: information.labels.company.nameOfBranch, backgroundColor: 'blue', width: 'half', defaultValue: (application: Application) => '', - condition: (formValue: FormValue, externalData, user) => { - const type = getValueViaPath( - externalData, - 'identity.data.type', - ) as string - return type === 'company' - }, + condition: (formValue, externalData) => isCompany(externalData), }),Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (3)
24-258: 🛠️ Refactor suggestion
Consider abstracting repetitive option-fetching logic
The code for fetching options in
buildSelectFieldis repeated multiple times. Abstracting this logic into a reusable helper function can enhance maintainability and reduce code duplication.Example of creating a helper function:
const getSelectFieldOptions = <T extends { code?: string; name?: string }>( application: Application, path: string, ): FieldOptions => { const data = getValueViaPath(application.externalData, path, []) as T[] return data .filter((item) => item.code && item.name) .map(({ code, name }) => ({ label: name || '', value: code || '', })) }And then use it in your select fields like:
buildSelectField({ id: `employee[${index}].employmentTime`, title: employee.employee.employmentTime, width: 'half', required: true, - options: (application) => { - const empLength = getValueViaPath( - application.externalData, - 'aoshData.data.lengthOfEmployment', - [], - ) as LengthOfEmploymentDto[] - return empLength - .filter((length) => length.code && length.name) - .map(({ name, code }) => ({ - label: name || '', - value: code || '', - })) - }, + options: (application) => + getSelectFieldOptions<LengthOfEmploymentDto>( + application, + 'aoshData.data.lengthOfEmployment', + ), }),
139-145:
⚠️ Potential issueEnsure valid employment rate input
The
employmentRatefield allows inputs up to999%. Consider adding validation to ensure the employment rate is between0%and100%to prevent invalid data entry.Apply this diff to add a validation rule:
buildTextField({ id: `employee[${index}].employmentRate`, title: employee.employee.employmentRate, width: 'half', required: true, format: '###%', + validation: (value) => { + const rate = parseInt(value.replace('%', ''), 10) + return rate >= 0 && rate <= 100 + ? undefined + : 'Employment rate must be between 0% and 100%' + }, }),📝 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.buildTextField({ id: `employee[${index}].employmentRate`, title: employee.employee.employmentRate, width: 'half', required: true, format: '###%', validation: (value) => { const rate = parseInt(value.replace('%', ''), 10) return rate >= 0 && rate <= 100 ? undefined : 'Employment rate must be between 0% and 100%' }, }),
214-226:
⚠️ Potential issueAvoid using magic numbers in conditions
In the condition at line 224, the value
'4'is hardcoded to check foremploymentStatus. It's better to use a constant or enumeration to represent this value for improved readability and maintainability.Apply this diff to define a constant and use it in the condition:
+const EMPLOYMENT_STATUS_TEMP = '4' // Code for 'starfsmannaleiga' ... buildTextField({ id: `employee[${index}].tempEmploymentSSN`, title: employee.employee.tempEmploymentSSN, width: 'half', variant: 'text', format: '######-####', condition: (formValue) => getValueViaPath( formValue, `employee[${index}].employmentStatus`, ) === EMPLOYMENT_STATUS_TEMP, required: true, }),Committable suggestion was skipped due to low confidence.
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (3)
43-154: 🛠️ Refactor suggestion
Refactor to eliminate code duplication
The
accidentForCreationDtoobject is constructed twice—once for logging and once for thecreateAccidentmethod. Define it once and reuse it to improve maintainability and reduce potential errors.Apply this refactoring:
+ const accidentForCreationDto = { companySSN: answers.companyInformation.nationalId, sizeOfEnterprise: parseInt( answers.companyInformation.numberOfEmployees, 10, ), nameOfBranchOrDepartment: answers.companyInformation.nameOfBranch, address: answers.companyInformation.address, postcode: answers.companyInformation.postnumber, workplaceHealthAndSafety: answers.companyLaborProtection.workhealthAndSafetyOccupation?.map( (code: string) => { return parseInt(code, 10) }, ), buyersSSN: answers.projectPurchase.nationalId, dateAndTimeOfAccident: getDateAndTime( answers.accident.date, answers.accident.time.split(':')[0], answers.accident.time.split(':')[1], ), aoshCame: answers.accident.didAoshCome === 'yes', policeCame: answers.accident.didPoliceCome === 'yes', numberOfVictims: answers.employee.length, municipalityWhereAccidentOccured: answers.accident.municipality, // Ensure code or name as required specificLocationOfAccident: answers.accident.exactLocation, detailedDescriptionOfAccident: answers.accident.wasDoing.concat( '\n', answers.accident.wentWrong, '\n', answers.accident.how, ), workingEnvironment: answers.accident.accidentLocation.value, victims: answers.employee.map((employee, index) => { return { victimsSSN: employee.nationalField.nationalId, employmentStatusOfVictim: employee.employmentStatus ? parseInt(employee.employmentStatus, 10) : 0, employmentAgencySSN: employee.tempEmploymentSSN ?? '', startedEmploymentForCompany: new Date(employee.startDate), lengthOfEmployment: employee.employmentTime ? parseInt(employee.employmentTime, 10) : 0, percentageOfFullWorkTime: employee.employmentRate ? parseInt(employee.employmentRate, 10) : 0, workhourArrangement: employee.workhourArrangement ? parseInt(employee.workhourArrangement, 10) : 0, startOfWorkingDay: getDateAndTime( employee.startOfWorkdayDate, employee.startTime.split(':')[0], employee.startTime.split(':')[1], ), workStation: employee.workstation ? parseInt(employee.workstation, 10) : 0, victimsOccupation: employee.victimsOccupation.value, absenceDueToAccident: answers.absence[index] ? parseInt(answers.absence[index], 10) : 0, specificPhysicalActivities: getValueList( application.answers, `circumstances[${index}].physicalActivities`, ), specificPhysicalActivityMostSevere: getValueViaPath( application.answers, `circumstances[${index}].physicalActivitiesMostSerious`, null, ) as string | null, workDeviations: getValueList( application.answers, `deviations[${index}].workDeviations`, ), workDeviationMostSevere: getValueViaPath( application.answers, `deviations[${index}].workDeviationsMostSerious`, null, ) as string | null, contactModeOfInjuries: getValueList( application.answers, `causeOfInjury[${index}].contactModeOfInjury`, ), contactModeOfInjuryMostSevere: getValueViaPath( application.answers, `causeOfInjury[${index}].contactModeOfInjuryMostSerious`, null, ) as string | null, partsOfBodyInjured: getValueList( application.answers, `injuredBodyParts[${index}].partOfBodyInjured`, ), partOfBodyInjuredMostSevere: getValueViaPath( application.answers, `injuredBodyParts[${index}].partOfBodyInjuredMostSerious`, null, ) as string | null, typesOfInjury: getValueList( application.answers, `typeOfInjury[${index}].typeOfInjury`, ), typeOfInjuryMostSevere: getValueViaPath( application.answers, `typeOfInjury[${index}].typeOfInjuryMostSerious`, null, ) as string | null, } }), userPhoneNumber: answers.companyInformation.phonenumber, userEmail: answers.companyInformation.email, } - console.log({ - accidentForCreationDto: { - // Object properties... - }, - }) + this.logger.log({ accidentForCreationDto }) await this.workAccidentClientService.createAccident(auth, { - accidentForCreationDto: { - // Object properties... - }, + accidentForCreationDto, })Also applies to: 157-268
42-44:
⚠️ Potential issueUse the injected
Loggerinstead ofconsole.logTo maintain consistency and adhere to best practices, use the injected
Loggerinstance for logging instead ofconsole.log.Apply this diff to fix the issue:
- console.log({ + this.logger.log({ accidentForCreationDto: {Committable suggestion was skipped due to low confidence.
156-270: 🛠️ Refactor suggestion
Add error handling for
createAccidentinvocationCurrently, there is no error handling for the
createAccidentcall. Implement error handling to manage potential failures and improve the application's robustness.Example of adding try-catch for error handling:
+ try { await this.workAccidentClientService.createAccident(auth, { accidentForCreationDto, }) + } catch (error) { + this.logger.error('Failed to create accident', error) + throw error + }Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/fields/Occupation/index.tsx (4)
309-481: 🛠️ Refactor suggestion
Extract repeated
Selectcomponents into separate sub-componentsThe rendering logic for the
Selectcomponents is repetitive and could be simplified by extracting each into its own sub-component. This enhances readability and makes the codebase easier to maintain.Example:
- Create a new sub-component
GroupSelect:const GroupSelect: FC<{ label: string name: string options: Options[] value: Options | null isDisabled: boolean onChange: (value?: EventOption) => void }> = ({ label, name, options, value, isDisabled, onChange }) => ( <Controller render={() => ( <Select label={label} name={name} options={options} value={value} isDisabled={isDisabled} backgroundColor="blue" placeholder={formatMessage(employee.employee.chooseFromListPlaceholder)} onChange={onChange} /> )} name={name} /> )
- Replace the existing
Selectcomponents:-<Select - label={formatMessage(employee.employee.majorGroupLabel)} - name={`employee[${idx}].victimsOccupationMajor`} - /* ... */ -/> +<GroupSelect + label={formatMessage(employee.employee.majorGroupLabel)} + name={`employee[${idx}].victimsOccupationMajor`} + options={majorGroupOptions.map(/* ... */)} + value={selectedMajorGroup} + isDisabled={false} + onChange={(v) => onChangeGroup('Major', v)} +/> <!-- Repeat for other group levels -->
184-202: 🛠️ Refactor suggestion
Refactor duplicated
onChangehandlers into a reusable functionThe functions
onChangeMajorGroup,onChangeSubMajorGroup, andonChangeMinorGroupshare similar logic. Consolidating them into a single reusable function will reduce code duplication and improve maintainability.Here's an example of how you might refactor these functions:
+const onChangeGroup = ( + groupLevel: 'Major' | 'SubMajor' | 'Minor', + value?: EventOption, +) => { + if (!value || !value.value || !value.label) return + + const selectedGroup: Options = { + value: value.value, + label: value.label, + } + + switch (groupLevel) { + case 'Major': + setSelectedMajorGroup(selectedGroup) + setSubMajorGroupOptions( + victimsOccupationSubMajorGroups.filter( + (group) => group.code?.startsWith(value.value), + ), + ) + setSelectedSubMajorGroup(null) + setSelectedMinorGroup(null) + setSelectedUnitGroup(null) + setMinorGroupOptions([]) + setUnitGroupOptions([]) + break + case 'SubMajor': + setSelectedSubMajorGroup(selectedGroup) + setMinorGroupOptions( + victimOccupationMinorGroups.filter( + (group) => group.code?.startsWith(value.value), + ), + ) + setSelectedMinorGroup(null) + setSelectedUnitGroup(null) + setUnitGroupOptions([]) + break + case 'Minor': + setSelectedMinorGroup(selectedGroup) + setUnitGroupOptions( + victimsOccupationUnitGroups.filter( + (group) => group.code?.startsWith(value.value), + ), + ) + setSelectedUnitGroup(null) + break + } + + setSelectedSearchGroup(null) +} -const onChangeMajorGroup = (value?: EventOption) => { /* ... */ } -const onChangeSubMajorGroup = (value?: EventOption) => { /* ... */ } -const onChangeMinorGroup = (value?: EventOption) => { /* ... */ }Then, update your
Selectcomponents'onChangehandlers:<Select /* ... */ - onChange={(v) => { - setSelectedSearchGroup(null) - if (!v) { - setSelectedMajorGroup(null) - return - } - onChangeMajorGroup(v) - }} + onChange={(v) => onChangeGroup('Major', v)} />Committable suggestion was skipped due to low confidence.
124-141:
⚠️ Potential issueAdd missing dependencies to
useEffectto prevent stale stateThe
useEffecthook on lines 124-141 utilizesselectedMajorGroup,selectedSubMajorGroup, andselectedMinorGroupwithin its scope but has an empty dependency array. This may lead to stale data or unexpected behavior when the selected groups change.Update the dependency array to include the necessary dependencies:
-useEffect(() => { +useEffect(() => { // Effect logic... -}, []) +}, [ + selectedMajorGroup, + selectedSubMajorGroup, + selectedMinorGroup, + selectedUnitGroup, +])📝 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.useEffect(() => { setMajorGroupOptions(victimsOccupationMajorGroups) setSubMajorGroupOptions( victimsOccupationSubMajorGroups.filter( (group) => group.code?.substring(0, 1) === selectedMajorGroup?.value, ), ) setMinorGroupOptions( victimOccupationMinorGroups.filter( (group) => group.code?.substring(0, 2) === selectedSubMajorGroup?.value, ), ) setUnitGroupOptions( victimsOccupationUnitGroups.filter( (group) => group.code?.substring(0, 3) === selectedMinorGroup?.value, ), ) }, [ selectedMajorGroup, selectedSubMajorGroup, selectedMinorGroup, selectedUnitGroup, ])
101-119: 🛠️ Refactor suggestion
Optimize data processing with
useMemoThe functions
getValidOptionsandgetAllValidSelectsprocess occupation groups and may be called multiple times, potentially impacting performance. UtilizinguseMemocan cache the results and enhance efficiency.Example implementation:
+const memoizedMajorGroupOptions = useMemo(() => getValidOptions(victimsOccupationMajorGroups), [victimsOccupationMajorGroups]) +const memoizedSubMajorGroupOptions = useMemo(() => getValidOptions(victimsOccupationSubMajorGroups), [victimsOccupationSubMajorGroups]) +const memoizedMinorGroupOptions = useMemo(() => getValidOptions(victimOccupationMinorGroups), [victimOccupationMinorGroups]) +const memoizedUnitGroupOptions = useMemo(() => getValidOptions(victimsOccupationUnitGroups), [victimsOccupationUnitGroups]) -const allValidSelects = getAllValidSelects() +const allValidSelects = useMemo(() => { + return [ + ...memoizedMajorGroupOptions, + ...memoizedSubMajorGroupOptions, + ...memoizedMinorGroupOptions, + ...memoizedUnitGroupOptions, + ] +}, [ + memoizedMajorGroupOptions, + memoizedSubMajorGroupOptions, + memoizedMinorGroupOptions, + memoizedUnitGroupOptions, +])📝 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.const getValidOptions = (groups: VictimsOccupationDto[]) => { // Helper function that filters and maps valid options return groups .filter((x) => x.validToSelect) .map((item) => ({ value: item.code, label: item.name || '', })) } const memoizedMajorGroupOptions = useMemo(() => getValidOptions(victimsOccupationMajorGroups), [victimsOccupationMajorGroups]) const memoizedSubMajorGroupOptions = useMemo(() => getValidOptions(victimsOccupationSubMajorGroups), [victimsOccupationSubMajorGroups]) const memoizedMinorGroupOptions = useMemo(() => getValidOptions(victimOccupationMinorGroups), [victimOccupationMinorGroups]) const memoizedUnitGroupOptions = useMemo(() => getValidOptions(victimsOccupationUnitGroups), [victimsOccupationUnitGroups]) const allValidSelects = useMemo(() => { return [ ...memoizedMajorGroupOptions, ...memoizedSubMajorGroupOptions, ...memoizedMinorGroupOptions, ...memoizedUnitGroupOptions, ] }, [ memoizedMajorGroupOptions, memoizedSubMajorGroupOptions, memoizedMinorGroupOptions, memoizedUnitGroupOptions, ])
…island-is/island.is into feature/aosh-work-accident-notification
Datadog ReportAll test runs ✅ 76 Total Test Services: 0 Failed, 73 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (7)
|
There was a problem hiding this comment.
Actionable comments posted: 19
🧹 Outside diff range and nitpick comments (11)
libs/application/templates/aosh/work-accident-notification/src/utils/index.ts (1)
1-2: LGTM! Consider adding type exports.
The exports for company and accident information utilities look good. However, consider also exporting their TypeScript types to improve type safety across the application.
Add type exports like this:
+export type { CompanyInformation } from './getCompanyInformationForOverview'
export { getCompanyInformationForOverview } from './getCompanyInformationForOverview'
+export type { AccidentInformation } from './getAccidentInformationForOverview'
export { getAccidentInformationForOverview } from './getAccidentInformationForOverview'libs/application/templates/aosh/work-accident-notification/src/fields/index.ts (1)
1-13: LGTM! Consider adding documentation.
The barrel file structure with individual named exports is good for tree-shaking and reusability. Consider adding JSDoc comments to group related exports and document their purposes.
+/**
+ * Form Fields
+ */
export { Occupation } from './Occupation'
export { Circumstance } from './Circumstance'
export { Deviation } from './Deviation'
export { CauseOfInjury } from './CauseOfInjury'
export { TypeOfInjury } from './TypeOfInjury'
export { InjuredBodyParts } from './InjuredBodyParts'
export { AccidentLocation } from './AccidentLocation'
+/**
+ * UI Components
+ */
export { CheckboxFieldCustom } from './CheckboxFieldCustom'
export { Overview } from './Overview'
export { TitleWithRemove } from './TitleWithRemove'
+/**
+ * Employee Management
+ */
export { AddEmployee } from './AddEmployee'
export { DeleteEmployee } from './DeleteEmployee'
export { EmployeeStartTimeError } from './EmployeeStartTimeError'libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx (2)
18-20: Remove unnecessary PropsWithChildren.
The component doesn't render any children, so PropsWithChildren can be removed for better type clarity.
-export const EmployeeStartTimeError: FC<
- React.PropsWithChildren<FieldBaseProps & EmployeeStartTimeErrorProps>
-> = (props) => {
+export const EmployeeStartTimeError: FC<FieldBaseProps & EmployeeStartTimeErrorProps> = (props) => {1-56: Consider making the component more reusable.
According to the coding guidelines for libs/**/*, components should be reusable across different NextJS apps. Consider:
- Making the time validation period configurable
- Abstracting the error message handling
- Creating a generic time validation hook
Would you like me to provide an example of how to refactor this into a more reusable component?
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts (1)
1-75: Consider using a robust date-time library.
The current implementation relies heavily on the native Date object, which can lead to timezone-related issues and inconsistent behavior across different browsers. Consider using a robust date-time library like date-fns or luxon for more reliable date manipulation.
These libraries provide:
- Immutable date objects
- Timezone handling
- Consistent parsing and formatting
- Built-in validation
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (2)
25-35: Add TypeScript type annotations for better type safety.
Consider adding explicit type annotations to improve type safety and documentation:
-export const employeeSubSection = (index: number) =>
+export const employeeSubSection = (index: number): FormSection =>36-270: Consider breaking down the form into smaller components.
The form section is quite large with many fields. Consider breaking it down into logical groupings (e.g., personal info, employment details, work arrangement) for better maintainability and reusability.
libs/clients/work-accident-ver/src/clientConfig.json (3)
1-21: Consider enhancing API metadata for better documentation.
The API metadata is well-structured but could benefit from additional OpenAPI enhancements:
- Add
termsOfServiceURL - Include API license information
- Add more detailed server configurations for different environments
{
"openapi": "3.0.1",
"info": {
"title": "Tilkynna vinnuslys",
"description": "Vefþjónusta til þess að senda inn tilkynningar um vinnuslys.",
"contact": {
"name": "Ut teymi vinnueftirlits",
"email": "[email protected]"
},
"version": "v1",
+ "termsOfService": "https://www.vinnueftirlit.is/terms",
+ "license": {
+ "name": "MIT",
+ "url": "https://opensource.org/licenses/MIT"
+ },
"x-pricing": "free",
"x-category": "official",
"x-links": {
"responsibleParty ": "https://www.vinnueftirlit.is"
}
},
"servers": [
{
- "url": "/slysaskraning"
+ "url": "/slysaskraning",
+ "description": "Production server"
+ },
+ {
+ "url": "/slysaskraning-dev",
+ "description": "Development server"
}
],23-95: Enhance POST /api/Accidents endpoint documentation.
The endpoint documentation is thorough but could be improved:
- The description mixes English and Icelandic - consider using separate fields
- Request body description references wrong model (
AccidentForCreationResponseDtoinstead ofAccidentForCreationDto) - Missing examples for request/response bodies
"post": {
"tags": ["Accidents"],
"summary": "Endpoint for reporting a occupational accident.",
- "description": "This endpoint consumes a data model `AccidentForCreationDto` for occupational accident reporting.\r\nÞessi endapunktúr tekur á móti gagnamódeli fyrir tilkynningar um vinnuslys.\r\n\r\nThis endpoint returns `AccidentForCreationResponseDto` Data model for confirmation of the submission of a notification of an occupational accident. Gagnalíkan fyrir staðfestingu á innsendingu tilkynningar um vinnuslys.",
+ "description": "This endpoint consumes a data model `AccidentForCreationDto` for occupational accident reporting and returns `AccidentForCreationResponseDto` for confirmation.",
+ "x-description-is": "Þessi endapunktúr tekur á móti gagnamódeli fyrir tilkynningar um vinnuslys og skilar staðfestingu á innsendingu tilkynningar um vinnuslys.",
"operationId": "CreateAccident",
"requestBody": {
- "description": "`AccidentForCreationResponseDto`",
+ "description": "`AccidentForCreationDto` - Accident report data",
+ "required": true,
"content": {
"application/json": {
"schema": {
"$ref": "#/components/schemas/AccidentForCreationDto"
- }
+ },
+ "example": {
+ "companySSN": "1234567890",
+ "dateAndTimeOfAccident": "2024-10-15T14:30:00Z",
+ "numberOfVictims": 1
+ }
}
}
},1111-1125: Enhance API security configuration.
The security configuration could be improved:
- Consider adding OAuth2 for better authorization
- Add rate limiting security requirements
- Document token format and requirements
"components": {
"securitySchemes": {
"Bearer": {
"type": "apiKey",
- "description": "JWT Token",
+ "description": "JWT Token. Must be in format: Bearer {token}. Token must be obtained from the authentication service.",
"name": "Authorization",
"in": "header"
- }
+ },
+ "OAuth2": {
+ "type": "oauth2",
+ "flows": {
+ "authorizationCode": {
+ "authorizationUrl": "https://auth.vinnueftirlit.is/oauth/authorize",
+ "tokenUrl": "https://auth.vinnueftirlit.is/oauth/token",
+ "scopes": {
+ "accidents:read": "Read accident reports",
+ "accidents:write": "Create accident reports"
+ }
+ }
+ }
+ }
}
},
"security": [
{
- "Bearer": []
+ "Bearer": [],
+ "OAuth2": ["accidents:read", "accidents:write"]
}
]libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
181-181: Clarify the expected value for 'municipalityWhereAccidentOccured'
The comment "// Vilja þau code eða name til baka?" indicates uncertainty about whether a code or name is expected.
Confirm the API requirements for this field and adjust the value accordingly to ensure correct data is sent.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
- libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (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/index.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/utils/index.ts (1 hunks)
- libs/clients/work-accident-ver/src/clientConfig.json (1 hunks)
- libs/clients/work-accident-ver/src/lib/workAccident.service.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/templates/aosh/work-accident-notification/src/lib/messages/employee.ts
🧰 Additional context used
📓 Path-based instructions (8)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/application/templates/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/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/EmployeeSection/employee.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/utils/dateManipulation.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/utils/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/clients/work-accident-ver/src/clientConfig.json (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/clients/work-accident-ver/src/lib/workAccident.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (6)
libs/application/templates/aosh/work-accident-notification/src/fields/index.ts (1)
1-13: 🛠️ Refactor suggestion
Consider exporting shared types for better reusability.
To improve type safety and reusability across the application, consider exporting shared TypeScript interfaces and types used by these components.
Let's check if there are shared types that should be exported:
libs/clients/work-accident-ver/src/lib/workAccident.service.ts (1)
1-41: Verify service integration and error handling.
Let's verify how this service is integrated with other components and ensure proper error handling throughout the system.
✅ Verification successful
Service integration is properly handled with built-in error management
The service is well-integrated within the system with proper error handling:
- Used by
work-accident-notification.service.tsin the application templates module - The system has a comprehensive error handling infrastructure:
- Global HTTP exception filters are in place via
HttpExceptionFilter - Problem-based error handling through
ProblemFilterandErrorFilter - Errors are automatically transformed into proper HTTP responses
- Global HTTP exception filters are in place via
The service leverages the built-in error handling mechanisms of the NestJS framework and the application's custom error filters, making additional error handling at the service level unnecessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for proper error handling in components using this service
echo "Checking error handling in components using WorkAccidentClientService..."
# Look for service usage without try-catch blocks
ast-grep --pattern 'new WorkAccidentClientService($$$)'
# Check for proper error handling in components using the service
rg -l "WorkAccidentClientService" | xargs rg "try.*catch" -A 5
# Look for potential memory leaks in error scenarios
ast-grep --pattern 'catch (error) {
$$$
}'
Length of output: 323
Script:
#!/bin/bash
# Let's try a different approach to find service usage and error handling
# Find all files that import or use WorkAccidentClientService
rg -l "WorkAccidentClientService"
# Find service method invocations
rg "workAccidentClient\." -A 2
# Look for error handling patterns around the service methods
rg "getOptionsData|createAccident" -B 2 -A 2
# Check for HTTP exception handling
rg "HttpException|@Catch|ExceptionFilter" -A 2
Length of output: 17356
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (2)
1-24: LGTM! Well-organized imports following best practices.
The imports are properly organized by source and utilize named imports for better tree-shaking.
166-173: Verify start time is within workday date.
Consider adding cross-field validation to ensure the start time is within the workday date.
Also applies to: 175-181
libs/clients/work-accident-ver/src/clientConfig.json (1)
96-160: GET /api/Accidents endpoint needs implementation.
The endpoint is marked as not implemented in the description. Consider:
- Adding proper pagination metadata in the response schema
- Documenting sorting and filtering capabilities
- Adding rate limiting headers
"get": {
"tags": ["Accidents"],
"summary": "This endpoint returns `AccidentDto` Data model for the submission of a notification of a work accident.",
"description": "This endpoint is not implemented and returns NotImplementedException.",
"operationId": "GetAccidents",
"responses": {
"200": {
- "description": "OK"
+ "description": "OK",
+ "headers": {
+ "X-Rate-Limit-Limit": {
+ "description": "The number of allowed requests in the current period",
+ "schema": { "type": "integer" }
+ }
+ },
+ "content": {
+ "application/json": {
+ "schema": {
+ "type": "object",
+ "properties": {
+ "data": {
+ "type": "array",
+ "items": {
+ "$ref": "#/components/schemas/AccidentDto"
+ }
+ },
+ "pagination": {
+ "type": "object",
+ "properties": {
+ "total": { "type": "integer" },
+ "pages": { "type": "integer" },
+ "current": { "type": "integer" },
+ "perPage": { "type": "integer" }
+ }
+ }
+ }
+ }
+ }
+ }
}
}libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
42-295: Add validation for critical fields before submission
The application constructs a complex data object with many fields that may come from user input or optional sources.
Implement validation to ensure all required fields are present and correctly formatted before submitting to the createAccident service. This can prevent runtime errors and ensure data integrity.
libs/application/templates/aosh/work-accident-notification/src/utils/index.ts
Outdated
Show resolved
Hide resolved
...cation/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx
Show resolved
Hide resolved
...cation/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx
Show resolved
Hide resolved
| setBeforeSubmitCallback?.(async () => { | ||
| const values = getValues('employee') | ||
| if ( | ||
| values[index].startOfWorkdayDate && | ||
| values[index].startTime && | ||
| dateIsWithing36Hours( | ||
| application, | ||
| values[index].startOfWorkdayDate, | ||
| values[index].startTime, | ||
| ) | ||
| ) { | ||
| setInputError(false) | ||
| return [true, null] | ||
| } | ||
| setInputError(true) | ||
| return [false, ''] | ||
| }) |
There was a problem hiding this comment.
Add cleanup and improve error handling in the validation callback.
The current implementation has several potential issues:
- The beforeSubmit callback should be cleaned up on component unmount
- setState in async callbacks could lead to race conditions
- Missing validation for invalid date formats
+ useEffect(() => {
setBeforeSubmitCallback?.(async () => {
const values = getValues('employee')
+ if (!values?.[index]) {
+ return [false, '']
+ }
if (
values[index].startOfWorkdayDate &&
values[index].startTime &&
dateIsWithing36Hours(
application,
values[index].startOfWorkdayDate,
values[index].startTime,
)
) {
- setInputError(false)
+ // Use functional update to avoid race conditions
+ setInputError(() => false)
return [true, null]
}
- setInputError(true)
+ setInputError(() => true)
return [false, '']
})
+
+ return () => {
+ // Cleanup callback on unmount
+ setBeforeSubmitCallback?.(undefined)
+ }
+ }, [setBeforeSubmitCallback, application, index])📝 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.
| setBeforeSubmitCallback?.(async () => { | |
| const values = getValues('employee') | |
| if ( | |
| values[index].startOfWorkdayDate && | |
| values[index].startTime && | |
| dateIsWithing36Hours( | |
| application, | |
| values[index].startOfWorkdayDate, | |
| values[index].startTime, | |
| ) | |
| ) { | |
| setInputError(false) | |
| return [true, null] | |
| } | |
| setInputError(true) | |
| return [false, ''] | |
| }) | |
| useEffect(() => { | |
| setBeforeSubmitCallback?.(async () => { | |
| const values = getValues('employee') | |
| if (!values?.[index]) { | |
| return [false, ''] | |
| } | |
| if ( | |
| values[index].startOfWorkdayDate && | |
| values[index].startTime && | |
| dateIsWithing36Hours( | |
| application, | |
| values[index].startOfWorkdayDate, | |
| values[index].startTime, | |
| ) | |
| ) { | |
| // Use functional update to avoid race conditions | |
| setInputError(() => false) | |
| return [true, null] | |
| } | |
| setInputError(() => true) | |
| return [false, ''] | |
| }) | |
| return () => { | |
| // Cleanup callback on unmount | |
| setBeforeSubmitCallback?.(undefined) | |
| } | |
| }, [setBeforeSubmitCallback, application, index]) |
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
70-75: Consider using template literals for string concatenation.
Using concat with multiple \n operations is less readable than template literals:
-detailedDescriptionOfAccident: answers.accident.wasDoing.concat(
- '\n',
- answers.accident.wentWrong,
- '\n',
- answers.accident.how,
-),
+detailedDescriptionOfAccident: `${answers.accident.wasDoing}
+${answers.accident.wentWrong}
+${answers.accident.how}`,📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (2)
1-26: LGTM! Clean and well-structured class setup.
The class follows dependency injection best practices and properly extends the base template service.
49-51: Address TODO comment and improve default value handling.
The TODO comment indicates incomplete implementation for the default branch number.
Would you like me to help create a configuration constant for the default branch number?
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
.../lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts
Outdated
Show resolved
Hide resolved
| const answers = application.answers as unknown as WorkAccidentNotification | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider safer type handling for application answers.
The type casting using unknown could be unsafe. Consider using type guards or validation:
-const answers = application.answers as unknown as WorkAccidentNotification
+const answers = this.validateAnswers(application.answers)
+private validateAnswers(answers: unknown): WorkAccidentNotification {
+ // Add type validation logic here
+ if (!this.isWorkAccidentNotification(answers)) {
+ throw new Error('Invalid answers format')
+ }
+ return answers
+}Committable suggestion was skipped due to low confidence.
| // Go to screen | ||
| goToScreen && goToScreen(`employeeInformation[${employeeAmount}]`) | ||
| } else { | ||
| // Error message |
There was a problem hiding this comment.
@sigruntg Díla við þessi else cases
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Outside diff range and nitpick comments (4)
libs/clients/work-accident-ver/src/lib/workAccident.service.ts (1)
29-43: Add JSDoc documentation for public methods.
Since this service is in the libs directory and meant to be reusable, public methods should be well-documented.
Add documentation for both methods:
/**
* Retrieves options data for work accident forms.
* @param auth - User authentication object
* @param locale - Language locale for the returned data
* @returns Promise resolving to form options data
*/
async getOptionsData(...)
/**
* Creates a new work accident report.
* @param auth - User authentication object
* @param requestParameters - Work accident details
* @returns Promise resolving to the created accident report
* @throws {UnauthorizedException} If user lacks required permissions
*/
async createAccident(...)libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (3)
29-50: Enhance logging for better observability.
While error handling is present, consider adding debug logs for successful operations and including more context in error logs.
Apply this diff to improve logging:
async getInputOptions({
currentUserLocale,
auth,
}: TemplateApiModuleActionProps): Promise<DataDto> {
+ this.logger.debug('[work-accident-notification-service]: Fetching options data', {
+ locale: currentUserLocale
+ })
const data = await this.workAccidentClientService
.getOptionsData(auth, currentUserLocale)
.catch(() => {
this.logger.warn(
- '[work-accident-notification-service]: Error fetching data from AOSH',
+ '[work-accident-notification-service]: Error fetching data from AOSH', {
+ locale: currentUserLocale
+ }
)
throw new TemplateApiError(
{
summary:
'Ekki tókst að sækja gögn til VER, vinsamlegast reynið síðar',
title: 'Villa í umsókn',
},
400,
)
})
+ this.logger.debug('[work-accident-notification-service]: Successfully fetched options data')
return data
}67-67: Remove TODO comment with implementation.
The TODO comment indicates a hardcoded default value ('1') that needs to be confirmed by Vinnueftirlit.
Would you like me to create a GitHub issue to track this pending decision from Vinnueftirlit?
87-93: Improve string concatenation for accident description.
Using string concatenation with \n could lead to formatting issues. Consider using an array join or template literal.
Apply this diff:
-detailedDescriptionOfAccident: answers.accident.wasDoing.concat(
- '\n',
- answers.accident.wentWrong,
- '\n',
- answers.accident.how,
-),
+detailedDescriptionOfAccident: [
+ answers.accident.wasDoing,
+ answers.accident.wentWrong,
+ answers.accident.how,
+].filter(Boolean).join('\n'),📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1 hunks)
- libs/clients/work-accident-ver/src/lib/workAccident.service.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/work-accident-ver/src/lib/workAccident.service.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (1)
libs/application/template-api-modules/src/lib/modules/templates/aosh/work-accident-notification/work-accident-notification.service.ts (1)
1-27: LGTM! Well-structured class with proper dependency injection.
The class follows best practices with:
- Proper inheritance from BaseTemplateApiService
- Dependency injection for logger and workAccidentClientService
- Clear type imports and module organization
| nameOfBranchOrDepartment: | ||
| answers.companyInformation.nameOfBranch ?? '1', // TODO: Vinnueftirlit will decide what number is default | ||
| address: answers.companyInformation.address, | ||
| postcode: answers.companyInformation.postnumber.slice(0, 3), |
There was a problem hiding this comment.
Validate postcode slicing.
The postcode slicing assumes a fixed format but lacks validation.
Apply this diff to add validation:
-postcode: answers.companyInformation.postnumber.slice(0, 3),
+postcode: answers.companyInformation.postnumber?.slice(0, 3) ?? '',📝 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.
| postcode: answers.companyInformation.postnumber.slice(0, 3), | |
| postcode: answers.companyInformation.postnumber?.slice(0, 3) ?? '', |
| .catch(() => { | ||
| this.logger.warn( | ||
| '[work-accident-notification-service]: Error submitting application to AOSH', | ||
| ) | ||
| return { | ||
| success: false, | ||
| message: 'Villa í umsókn, ekki tókst að skila umsókn til VER.', | ||
| } | ||
| }) |
There was a problem hiding this comment.
Improve error handling in catch block.
The catch block returns an object instead of throwing an error, which could lead to silent failures.
Apply this diff to maintain consistent error handling:
.catch(() => {
this.logger.warn(
'[work-accident-notification-service]: Error submitting application to AOSH',
)
- return {
- success: false,
- message: 'Villa í umsókn, ekki tókst að skila umsókn til VER.',
- }
+ throw new TemplateApiError(
+ {
+ title: 'Villa í umsókn',
+ summary: 'Ekki tókst að skila umsókn til VER.',
+ },
+ 400,
+ )
})📝 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.
| .catch(() => { | |
| this.logger.warn( | |
| '[work-accident-notification-service]: Error submitting application to AOSH', | |
| ) | |
| return { | |
| success: false, | |
| message: 'Villa í umsókn, ekki tókst að skila umsókn til VER.', | |
| } | |
| }) | |
| .catch(() => { | |
| this.logger.warn( | |
| '[work-accident-notification-service]: Error submitting application to AOSH', | |
| ) | |
| throw new TemplateApiError( | |
| { | |
| title: 'Villa í umsókn', | |
| summary: 'Ekki tókst að skila umsókn til VER.', | |
| }, | |
| 400, | |
| ) | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Outside diff range and nitpick comments (9)
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts (1)
13-29: Document business rules and add return type.
The function implements important business logic for minimum date calculation but lacks documentation explaining the rules.
+/**
+ * Calculates the minimum allowed date for accident reporting based on business rules:
+ * - If accident occurred before noon (12:00), minimum date is 2 days prior
+ * - If accident occurred after noon, minimum date is 1 day prior
+ *
+ * @param application - The application containing accident details
+ * @returns Date - The calculated minimum allowed date
+ */
-export const getMinDate = (application: Application) => {
+export const getMinDate = (application: Application): Date => {libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (2)
21-28: Add error message to TimeWithRefine schema
The time validation refinement should include a meaningful error message for better user experience.
const TimeWithRefine = z
.string()
- .refine((x) => (x ? isValid24HFormatTime(x) : false), {})
+ .refine((x) => (x ? isValid24HFormatTime(x) : false), {
+ message: 'Time must be in 24-hour format (HHMM)',
+ })1-242: Consider splitting schemas for better modularity
Since this file is in the libs directory and should be reusable across different NextJS apps, consider:
- Splitting the schemas into separate files based on their domain (e.g.,
employee.schema.ts,accident.schema.ts) - Creating a barrel file (
index.ts) to re-export all schemas - Moving utility functions (
isValid24HFormatTime,isValidPhoneNumber) to a separate utilities file
This would improve maintainability and make it easier to reuse specific schemas across different applications.
🧰 Tools
🪛 Biome
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts (3)
101-105: Fix formatting in email field description.
Remove the trailing space in the description.
- description: 'Email of company ',
+ description: 'Email of company',144-146: Maintain consistent quote usage in messages.
Use consistent quotes throughout the messages. The current message mixes single and double quotes.
- "Vinsamlegast veldur a.m.k einn valmöguleika, ef ekkert á við veldu 'engin'",
+ 'Vinsamlegast veldur a.m.k einn valmöguleika, ef ekkert á við veldu "engin"',1-182: Consider splitting messages into separate files for better maintainability.
As the application grows, maintaining all messages in a single file might become challenging. Consider splitting messages into separate files by domain (e.g., company.messages.ts, workhealth.messages.ts, etc.) and creating an index file to export them all.
This would improve:
- Code organization and maintainability
- Module bundling and tree-shaking
- Team collaboration when working on different features
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (3)
26-36: Consider simplifying the title implementation.
The title function could be simplified by using object spread operator directly with the values.
title: () => {
- return {
- ...sections.draft.employee,
- values: {
- number: index + 1,
- },
- }
+ return { ...sections.draft.employee, values: { number: index + 1 } }
},109-115: Extract margin values as constants.
The margin values used in description fields should be extracted as constants for consistency and maintainability.
+const DESCRIPTION_MARGINS = {
+ SECTION_TOP: 4,
+ SECTION_BOTTOM: 1,
+ SUBSECTION_TOP: 3,
+} as const;
buildDescriptionField({
id: `employee[${index}].workArrangement`,
title: employee.employee.workArrangement,
titleVariant: 'h5',
- marginTop: 4,
- marginBottom: 1,
+ marginTop: DESCRIPTION_MARGINS.SECTION_TOP,
+ marginBottom: DESCRIPTION_MARGINS.SECTION_BOTTOM,
}),Also applies to: 232-236
61-73: Enhance accessibility for address fields.
Consider adding aria-labels and helper text to improve accessibility for address input fields.
buildNationalIdWithNameField({
id: `employee[${index}].nationalField`,
required: true,
title: '',
+ ariaLabel: employee.employee.nationalIdAriaLabel,
}),
buildTextField({
id: `employee[${index}].address`,
maxLength: 255,
title: employee.employee.address,
width: 'half',
variant: 'text',
required: true,
+ ariaLabel: employee.employee.addressAriaLabel,
+ helperText: employee.employee.addressHelperText,
}),📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (7)
- libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/utils/index.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- libs/application/templates/aosh/work-accident-notification/src/fields/EmployeeStartTimeError/index.tsx
- libs/application/templates/aosh/work-accident-notification/src/utils/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.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/lib/messages/information.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."
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.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
libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts
[error] 18-18: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (5)
libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts (1)
6-8: 🛠️ Refactor suggestion
Enhance type safety and documentation for employment status.
Consider adding a type definition and improving documentation:
+/** Employment status codes used in work accident notifications */
+export type EmploymentStatusCode = '4'; // Add more status codes as needed
+
export const EMPLOYMENT_STATUS = {
+ /** Temporary employment agency (starfsmannaleiga) */
TEMP_AGENCY: '4', // starfsmannaleiga
} as const;
+
+export type EmploymentStatus = typeof EMPLOYMENT_STATUS[keyof typeof EMPLOYMENT_STATUS];Also, verify if more employment status codes will be added later, as having only one status seems incomplete.
libs/application/templates/aosh/work-accident-notification/src/lib/dataSchema.ts (1)
234-242: LGTM! Well-organized type exports
The type exports are well-structured and follow TypeScript best practices.
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts (2)
118-131: LGTM! Labor protection messages are well structured.
The messages are properly organized with clear descriptions and meaningful IDs.
151-182: LGTM! Project purchase messages are well implemented.
The messages provide clear guidance for optional project purchase information with proper descriptions and IDs.
libs/application/templates/aosh/work-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts (1)
1-25: LGTM! Well-organized imports following best practices.
The imports are properly structured and support reusability across different NextJS apps.
libs/application/templates/aosh/work-accident-notification/src/shared/constants.ts
Show resolved
Hide resolved
| export const getMaxDate = (application: Application) => { | ||
| const date = getValueViaPath( | ||
| application.answers, | ||
| 'accident.date', | ||
| new Date().toString(), | ||
| ) as string | ||
| return new Date(date) | ||
| } |
There was a problem hiding this comment.
Add timezone handling for accurate accident date recording.
The function should explicitly handle timezones to ensure accurate accident date recording across different regions.
export const getMaxDate = (application: Application) => {
+ // Use explicit timezone for consistency
const date = getValueViaPath(
application.answers,
'accident.date',
- new Date().toString(),
+ new Date().toLocaleDateString('is-IS', { timeZone: 'Atlantic/Reykjavik' }),
) as string
- return new Date(date)
+ // Ensure date is interpreted in Iceland's timezone
+ return new Date(date + 'T00:00:00.000Z')
}Committable suggestion was skipped due to low confidence.
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts
Show resolved
Hide resolved
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts
Show resolved
Hide resolved
libs/application/templates/aosh/work-accident-notification/src/utils/dateManipulation.ts
Show resolved
Hide resolved
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts
Show resolved
Hide resolved
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts
Show resolved
Hide resolved
libs/application/templates/aosh/work-accident-notification/src/lib/messages/information.ts
Show resolved
Hide resolved
...ork-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts
Show resolved
Hide resolved
...ork-accident-notification/src/forms/WorkAccidentNotificationForm/EmployeeSection/employee.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (2)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (1)
1-185: Well-structured localization messages with some missing content.
The file is well-organized with clear message categorization and follows best practices for i18n. The structure is reusable and maintainable. However, several sections have placeholder or missing texts that need to be completed before the application can be properly localized.
Consider adding JSDoc comments at the top of the file to document the purpose and usage of these messages.
import { defineMessages } from 'react-intl'
+/**
+ * Localization messages for the Work Accident Notification application's
+ * cause and consequences sections. These messages are used across various
+ * components to provide consistent, localized text for labels, descriptions,
+ * and form fields.
+ */
export const causeAndConsequences = {libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (1)
65-67: Correct variable name activites to activities
The variable activites appears to be misspelled. It should be activities for clarity and consistency.
Apply this diff to correct the variable name throughout the code:
-const activites = (
+const activities = (Update all usages accordingly:
-const activites = (
+const activities = (
...
-options={activites.map((activity) => ({
+options={activities.map((activity) => ({
...
-items={activites}
+items={activities}Also applies to: 98-102, 136-136
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdown.tsx (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (2)
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/lib/messages/causeAndConsequences.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
libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx
[error] 144-144: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
[error] 162-162: Avoid passing children using a prop
The canonical way to pass children in React is to use JSX elements
(lint/correctness/noChildrenProp)
🔇 Additional comments (1)
libs/application/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx (1)
24-33: Proper use of TypeScript for defining props and exporting types
The CausesAndEffectsProps and OptionAndKey types are well-defined, enhancing type safety and reusability across different NextJS apps.
...plication/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts
Show resolved
Hide resolved
...plication/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts
Outdated
Show resolved
Hide resolved
...plication/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts
Show resolved
Hide resolved
...plication/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts
Show resolved
Hide resolved
...plication/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts
Outdated
Show resolved
Hide resolved
...ication/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx
Show resolved
Hide resolved
...ication/templates/aosh/work-accident-notification/src/fields/Components/CausesAndEffects.tsx
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (1)
1-185: Maintain consistent spacing in message strings.
There are inconsistent spaces in some defaultMessage strings (e.g., double spaces after periods). Consider normalizing spaces across all messages for consistent formatting.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdown.tsx (1 hunks)
- libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- libs/application/templates/aosh/work-accident-notification/src/fields/Components/MultiSelectDropdown.tsx
🧰 Additional context used
📓 Path-based instructions (1)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (2)
libs/application/templates/aosh/work-accident-notification/src/lib/messages/causeAndConsequences.ts (2)
1-3: LGTM! Well-structured internationalization setup.
The file follows best practices for internationalization:
- Clean export pattern enabling tree-shaking
- Proper use of react-intl's defineMessages
- Located in libs directory for reusability
64-88: LGTM! Well-defined circumstances section.
The section has complete and descriptive messages with clear guidance for users.
| causeAndConsequencesNothingChosen: { | ||
| id: 'aosh.wan.application:causeAndConsequences.shared.causeAndConsequencesNothingChosen', | ||
| defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika', | ||
| description: | ||
| 'error alerting users to choose at least on option (cause and consequences pages)', | ||
| }, | ||
| selectPlaceholder: { | ||
| id: 'aosh.wan.application:causeAndConsequences.shared.selectPlaceholder', | ||
| defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika', | ||
| description: | ||
| 'error alerting users to choose at least on option (cause and consequences pages)', | ||
| }, |
There was a problem hiding this comment.
Remove duplicate error message.
The selectPlaceholder message duplicates causeAndConsequencesNothingChosen. Both have the same default message and description. Consider consolidating these into a single message for better maintainability.
- selectPlaceholder: {
- id: 'aosh.wan.application:causeAndConsequences.shared.selectPlaceholder',
- defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika',
- description:
- 'error alerting users to choose at least on option (cause and consequences pages)',
- },📝 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.
| causeAndConsequencesNothingChosen: { | |
| id: 'aosh.wan.application:causeAndConsequences.shared.causeAndConsequencesNothingChosen', | |
| defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika', | |
| description: | |
| 'error alerting users to choose at least on option (cause and consequences pages)', | |
| }, | |
| selectPlaceholder: { | |
| id: 'aosh.wan.application:causeAndConsequences.shared.selectPlaceholder', | |
| defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika', | |
| description: | |
| 'error alerting users to choose at least on option (cause and consequences pages)', | |
| }, | |
| causeAndConsequencesNothingChosen: { | |
| id: 'aosh.wan.application:causeAndConsequences.shared.causeAndConsequencesNothingChosen', | |
| defaultMessage: 'Vinsamlegast veldu a.m.k einn valmöguleika', | |
| description: | |
| 'error alerting users to choose at least on option (cause and consequences pages)', | |
| }, |
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.description', | ||
| defaultMessage: | ||
| 'Lýstu hvers kyns áverki eða áverkar hlaust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | ||
| description: 'description of the injured body parts page', |
There was a problem hiding this comment.
Fix copy-pasted description for injured body parts.
The description appears to be copied from the typeOfInjury section and doesn't accurately describe the injured body parts selection process.
- defaultMessage:
- 'Lýstu hvers kyns áverki eða áverkar hlaust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.',
+ defaultMessage:
+ 'Veldu þá líkamshluta sem slösuðust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.',📝 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.
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.description', | |
| defaultMessage: | |
| 'Lýstu hvers kyns áverki eða áverkar hlaust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | |
| description: 'description of the injured body parts page', | |
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.description', | |
| defaultMessage: | |
| 'Veldu þá líkamshluta sem slösuðust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | |
| description: 'description of the injured body parts page', |
| 'Text below heading before choosing circumstance of accident', | ||
| }, | ||
| }), | ||
| deviations: defineMessages({ | ||
| title: { | ||
| id: 'aosh.wan.application:causeAndConsequences.deviations.title', | ||
| defaultMessage: 'Frávik í vinnuferli', | ||
| description: 'Title of the deviation page', | ||
| }, | ||
| description: { | ||
| id: 'aosh.wan.application:causeAndConsequences.deviations.description', | ||
| defaultMessage: | ||
| 'Lýstu því við hvaða aðstæður slysið varð með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina.', | ||
| description: 'description of the deviation page', | ||
| }, | ||
| heading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.deviations.heading', | ||
| defaultMessage: 'Hvað fór úrskeiðis þegar slysið varð?', | ||
| description: 'Heading before choosing deviation of accident', | ||
| }, | ||
| subHeading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.deviations.subHeading', | ||
| defaultMessage: | ||
| 'Lýstu því við hvað það var sem gerðist með því að velja úr listanum hér fyrir neðan. Merktu við alla möguleika sem eiga við.', | ||
| description: 'Text below heading before choosing deviation of accident', | ||
| }, | ||
| }), | ||
| causeOfInjury: defineMessages({ | ||
| title: { | ||
| id: 'aosh.wan.application:causeAndConsequences.causeOfInjury.title', | ||
| defaultMessage: 'Orsök áverka', | ||
| description: 'Title of the cause of injury page', | ||
| }, | ||
| description: { | ||
| id: 'aosh.wan.application:causeAndConsequences.causeOfInjury.description', | ||
| defaultMessage: | ||
| 'Lýstu því við hvaða aðstæður áverkinn myndaðist með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | ||
| description: 'description of the cause of injury page', | ||
| }, | ||
| heading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.causeOfInjury.heading', | ||
| defaultMessage: '?ennan texta vantar...', | ||
| description: 'Heading before choosing cause of injury of accident', | ||
| }, | ||
| subHeading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.causeOfInjury.subHeading', | ||
| defaultMessage: 'Lýstu því við hvað .... vantar þennan texta', | ||
| description: | ||
| 'Text below heading before choosing cause of injury of accident', | ||
| }, | ||
| }), | ||
| typeOfInjury: defineMessages({ | ||
| title: { | ||
| id: 'aosh.wan.application:causeAndConsequences.typeOfInjury.title', | ||
| defaultMessage: 'Tegund áverka', | ||
| description: 'Title of the type of injury page', | ||
| }, | ||
| description: { | ||
| id: 'aosh.wan.application:causeAndConsequences.typeOfInjury.description', | ||
| defaultMessage: | ||
| 'Lýstu hvers kyns áverki eða áverkar hlaust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | ||
| description: 'description of the type of injury page', | ||
| }, | ||
| heading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.typeOfInjury.heading', | ||
| defaultMessage: 'Þennan texta vantar...', | ||
| description: 'Heading before choosing type of injury of accident', | ||
| }, | ||
| subHeading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.typeOfInjury.subHeading', | ||
| defaultMessage: 'Lýstu því við hvað .... vantar þennan texta', | ||
| description: | ||
| 'Text below heading before choosing type of injury of accident', | ||
| }, | ||
| }), | ||
| injuredBodyParts: defineMessages({ | ||
| title: { | ||
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.title', | ||
| defaultMessage: 'Skaddaðir líkamshlutar', | ||
| description: 'Title of the injured body parts page', | ||
| }, | ||
| description: { | ||
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.description', | ||
| defaultMessage: | ||
| 'Lýstu hvers kyns áverki eða áverkar hlaust með því að velja úr listanum hér fyrir neðan. Þú getur líka slegið inn í leitina. Marktu við alla möguleika sem eiga við.', | ||
| description: 'description of the injured body parts page', | ||
| }, | ||
| heading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.heading', | ||
| defaultMessage: 'Þennan texta vantar...', | ||
| description: 'Heading before choosing injured body parts of accident', | ||
| }, | ||
| subHeading: { | ||
| id: 'aosh.wan.application:causeAndConsequences.injuredBodyParts.subHeading', | ||
| defaultMessage: 'Lýstu því hvað .... vantar þennan texta', | ||
| description: | ||
| 'Text below heading before choosing injured body parts of accident', | ||
| }, | ||
| }), | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Missing content in message definitions needs to be updated
The verification revealed several placeholder messages that need proper content:
- Missing or incomplete headings and subheadings in:
causeOfInjury.heading: Currently shows "?ennan texta vantar..."typeOfInjury.heading: Currently shows "Þennan texta vantar..."injuredBodyParts.heading: Currently shows "Þennan texta vantar..."- Related subHeading messages also contain incomplete text with "...vantar þennan texta"
These placeholder messages are actively used in their respective components (TypeOfInjury, InjuredBodyParts, CauseOfInjury) and should be updated with proper content to maintain a consistent user experience.
🔗 Analysis chain
Verify message key usage across components.
Let's verify that all message keys are properly used in the components and that there are no unused messages.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for usage of message keys in components
echo "Searching for message usage in components..."
rg -l "causeAndConsequences\." --type ts --type tsx
# Search for potential typos in message keys
echo "Checking for potential typos in message keys..."
rg "causeAndConsequence[^s]" --type ts --type tsx
Length of output: 365
Script:
#!/bin/bash
# Search for usage of message keys in components with correct file type flags
echo "Searching for message usage in components..."
rg -l "causeAndConsequences\." -t ts
# Search for potential typos in message keys
echo "Checking for potential typos in message keys..."
rg "causeAndConsequence[^s]" -t ts
# Search for imports of this message file
echo "Searching for imports of the messages file..."
rg -l "causeAndConsequences.*messages" -t ts
Length of output: 3770
Script:
#!/bin/bash
# Let's check the actual usage of specific message keys that have placeholder content
echo "Checking usage of messages with placeholder content..."
rg "causeAndConsequences\.(typeOfInjury|causeOfInjury|injuredBodyParts)\.(heading|subHeading)" -A 2 -t ts
# Also check if there are any hardcoded strings that should use these messages
echo "Checking for potential hardcoded strings..."
ast-grep --pattern 'defaultMessage: "Þennan texta vantar..."'
Length of output: 5244
...
Attach a link to issue if relevant
What
This PR sets up the initial deployment of the "Work Accident Notification" application to the development environment and includes a feature flag for production to control its visibility.
The Work Accident Notification application enables both individual users and users with company delegations to register an accident. Individual users can only submit reports for their own sole proprietorships, while submitting on behalf of a company requires an assigned delegation. The application flow remains largely the same for both user types, with only a few differences in required input fields based on user role
Why
Deploying to the dev environment allows VER (Vinnueftirlitið) to perform functional testing on the application to ensure it meets requirements and identify any potential issues/edges cases etc.
Screenshots / Gifs
Gagnaöflun
Fetch information from VER, National registry and user profile
Announcement
Announcement for individuals to alert them that they can only submit accident report for their own sole proprietorship. Note that the text in the screenshot is outdated and will be changed.
Company
Base information
Basic information
Base information 1: Base information as an individual user.
Base information 2: Base information as a company delegation user
Occupational Health and Safety
Project buyer
If "No" is selected the fields below are not visible.
Accident
Employee
Causes and Consequences
Absence
Circumstances, Deviation, Cause of injury, Type of injury and Injured body part
All of these steps have the same format/setup
Select field with added checkbox functionality (Island UI addition)
If more than one option is chose, the most severe must be chosen
Overview
Add another employee
Overview screen
Misc
Multiple employees, something we want to look into condensing so multiple employees don't extend the screen endlessly.
Checklist:
Summary by CodeRabbit
Release Notes
New Features
WorkAccidentNotificationTemplateModuleto manage work accident notifications.WorkAccidentNotificationTemplateServicefor handling operations related to work accident notifications, including input options and application submissions.AccidentLocation,AddEmployee,CauseOfInjury,EmployeeStartTimeError, and others.Documentation
Style